Skip to content

Commit 049b60c

Browse files
authored
[NFC][Clang] Avoid potential null pointer dereferences in Sema::AddInitializerToDecl(). (#106235)
Control flow analysis performed by a static analysis tool revealed the potential for null pointer dereferences to occur in conjunction with the `Init` parameter in `Sema::AddInitializerToDecl()`. On entry to the function, `Init` is required to be non-null as there are multiple potential branches that unconditionally dereference it. However, there were two places where `Init` is compared to null thus implying that `Init` is expected to be null in some cases. These checks appear to be purely defensive checks and thus unnecessary. Further, there were several cases where code checked `Result`, a variable of type `ExprResult`, for an invalid value, but did not check for a valid but null value and then proceeded to unconditionally dereference the potential null result. This change elides the unnecessary defensive checks and changes some checks for an invalid result to instead branch on an unusable result (either an invalid result or a valid but null result).
1 parent 1f0d545 commit 049b60c

File tree

1 file changed

+7
-8
lines changed

1 file changed

+7
-8
lines changed

clang/lib/Sema/SemaDecl.cpp

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13319,8 +13319,7 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
1331913319
}
1332013320

1332113321
// WebAssembly tables can't be used to initialise a variable.
13322-
if (Init && !Init->getType().isNull() &&
13323-
Init->getType()->isWebAssemblyTableType()) {
13322+
if (!Init->getType().isNull() && Init->getType()->isWebAssemblyTableType()) {
1332413323
Diag(Init->getExprLoc(), diag::err_wasm_table_art) << 0;
1332513324
VDecl->setInvalidDecl();
1332613325
return;
@@ -13463,7 +13462,7 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
1346313462
if (getLangOpts().DebuggerCastResultToId && DclT->isObjCObjectPointerType() &&
1346413463
Init->getType() == Context.UnknownAnyTy) {
1346513464
ExprResult Result = forceUnknownAnyToType(Init, Context.getObjCIdType());
13466-
if (Result.isInvalid()) {
13465+
if (!Result.isUsable()) {
1346713466
VDecl->setInvalidDecl();
1346813467
return;
1346913468
}
@@ -13491,7 +13490,7 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
1349113490
InitializationSequence Init(*this, Entity, Kind, MultiExprArg(E));
1349213491
return Init.Failed() ? ExprError() : E;
1349313492
});
13494-
if (Res.isInvalid()) {
13493+
if (!Res.isUsable()) {
1349513494
VDecl->setInvalidDecl();
1349613495
} else if (Res.get() != Args[Idx]) {
1349713496
Args[Idx] = Res.get();
@@ -13504,7 +13503,7 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
1350413503
/*TopLevelOfInitList=*/false,
1350513504
/*TreatUnavailableAsInvalid=*/false);
1350613505
ExprResult Result = InitSeq.Perform(*this, Entity, Kind, Args, &DclT);
13507-
if (Result.isInvalid()) {
13506+
if (!Result.isUsable()) {
1350813507
// If the provided initializer fails to initialize the var decl,
1350913508
// we attach a recovery expr for better recovery.
1351013509
auto RecoveryExpr =
@@ -13528,8 +13527,8 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
1352813527
InitSeq.step_begin()->Kind ==
1352913528
InitializationSequence::SK_ParenthesizedListInit;
1353013529
QualType VDeclType = VDecl->getType();
13531-
if (Init && !Init->getType().isNull() &&
13532-
!Init->getType()->isDependentType() && !VDeclType->isDependentType() &&
13530+
if (!Init->getType().isNull() && !Init->getType()->isDependentType() &&
13531+
!VDeclType->isDependentType() &&
1353313532
Context.getAsIncompleteArrayType(VDeclType) &&
1353413533
Context.getAsIncompleteArrayType(Init->getType())) {
1353513534
// Bail out if it is not possible to deduce array size from the
@@ -13592,7 +13591,7 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
1359213591
ExprResult Result =
1359313592
ActOnFinishFullExpr(Init, VDecl->getLocation(),
1359413593
/*DiscardedValue*/ false, VDecl->isConstexpr());
13595-
if (Result.isInvalid()) {
13594+
if (!Result.isUsable()) {
1359613595
VDecl->setInvalidDecl();
1359713596
return;
1359813597
}

0 commit comments

Comments
 (0)