[clang] 049b60c - [NFC][Clang] Avoid potential null pointer dereferences in Sema::AddInitializerToDecl(). (#106235)

[via cfe-commits]

Author: Tom Honermann
Date: 2024-08-29T17:00:19-04:00
New Revision: 049b60c5bb7e774b74772c6b89c72593f73a89b0

URL: https://github.com/llvm/llvm-project/commit/049b60c5bb7e774b74772c6b89c72593f73a89b0
DIFF: https://github.com/llvm/llvm-project/commit/049b60c5bb7e774b74772c6b89c72593f73a89b0.diff

LOG: [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).

Added: 
    

Modified: 
    clang/lib/Sema/SemaDecl.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 4efa80778e71b9..6327ae9b99aa4c 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13319,8 +13319,7 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
   }
 
   // WebAssembly tables can't be used to initialise a variable.
-  if (Init && !Init->getType().isNull() &&
-      Init->getType()->isWebAssemblyTableType()) {
+  if (!Init->getType().isNull() && Init->getType()->isWebAssemblyTableType()) {
     Diag(Init->getExprLoc(), diag::err_wasm_table_art) << 0;
     VDecl->setInvalidDecl();
     return;
@@ -13463,7 +13462,7 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
   if (getLangOpts().DebuggerCastResultToId && DclT->isObjCObjectPointerType() &&
       Init->getType() == Context.UnknownAnyTy) {
     ExprResult Result = forceUnknownAnyToType(Init, Context.getObjCIdType());
-    if (Result.isInvalid()) {
+    if (!Result.isUsable()) {
       VDecl->setInvalidDecl();
       return;
     }
@@ -13491,7 +13490,7 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
             InitializationSequence Init(*this, Entity, Kind, MultiExprArg(E));
             return Init.Failed() ? ExprError() : E;
           });
-      if (Res.isInvalid()) {
+      if (!Res.isUsable()) {
         VDecl->setInvalidDecl();
       } else if (Res.get() != Args[Idx]) {
         Args[Idx] = Res.get();
@@ -13504,7 +13503,7 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
                                    /*TopLevelOfInitList=*/false,
                                    /*TreatUnavailableAsInvalid=*/false);
     ExprResult Result = InitSeq.Perform(*this, Entity, Kind, Args, &DclT);
-    if (Result.isInvalid()) {
+    if (!Result.isUsable()) {
       // If the provided initializer fails to initialize the var decl,
       // we attach a recovery expr for better recovery.
       auto RecoveryExpr =
@@ -13528,8 +13527,8 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
                       InitSeq.step_begin()->Kind ==
                           InitializationSequence::SK_ParenthesizedListInit;
     QualType VDeclType = VDecl->getType();
-    if (Init && !Init->getType().isNull() &&
-        !Init->getType()->isDependentType() && !VDeclType->isDependentType() &&
+    if (!Init->getType().isNull() && !Init->getType()->isDependentType() &&
+        !VDeclType->isDependentType() &&
         Context.getAsIncompleteArrayType(VDeclType) &&
         Context.getAsIncompleteArrayType(Init->getType())) {
       // 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) {
   ExprResult Result =
       ActOnFinishFullExpr(Init, VDecl->getLocation(),
                           /*DiscardedValue*/ false, VDecl->isConstexpr());
-  if (Result.isInvalid()) {
+  if (!Result.isUsable()) {
     VDecl->setInvalidDecl();
     return;
   }