[clang][Analyzer] Fix error path of builtin overflow

Author: pskrgag

clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp

@@ -96,10 +96,9 @@ class BuiltinFunctionChecker : public Checker<eval::Call> {
   void handleOverflowBuiltin(const CallEvent &Call, CheckerContext &C,
                              BinaryOperator::Opcode Op,
                              QualType ResultType) const;
-  const NoteTag *createBuiltinNoOverflowNoteTag(CheckerContext &C,
-                                                bool BothFeasible, SVal Arg1,
-                                                SVal Arg2, SVal Result) const;
-  const NoteTag *createBuiltinOverflowNoteTag(CheckerContext &C) const;
+  const NoteTag *createBuiltinOverflowNoteTag(CheckerContext &C,
+                                              bool BothFeasible, SVal Arg1,
+                                              SVal Arg2, SVal Result) const;
   std::pair<bool, bool> checkOverflow(CheckerContext &C, SVal RetVal,
                                       QualType Res) const;
 
@@ -121,30 +120,25 @@ class BuiltinFunctionChecker : public Checker<eval::Call> {
 
 } // namespace
 
-const NoteTag *BuiltinFunctionChecker::createBuiltinNoOverflowNoteTag(
-    CheckerContext &C, bool BothFeasible, SVal Arg1, SVal Arg2,
+const NoteTag *BuiltinFunctionChecker::createBuiltinOverflowNoteTag(
+    CheckerContext &C, bool overflow, SVal Arg1, SVal Arg2,
     SVal Result) const {
-  return C.getNoteTag([Result, Arg1, Arg2, BothFeasible](
+  return C.getNoteTag([Result, Arg1, Arg2, overflow](
                           PathSensitiveBugReport &BR, llvm::raw_ostream &OS) {
     if (!BR.isInteresting(Result))
       return;
 
-    // Propagate interestingness to input argumets if result is interesting.
+    // Propagate interestingness to input arguments if result is interesting.
     BR.markInteresting(Arg1);
     BR.markInteresting(Arg2);
 
-    if (BothFeasible)
+    if (overflow)
+      OS << "Assuming overflow";
+    else
       OS << "Assuming no overflow";
   });
 }
 
-const NoteTag *
-BuiltinFunctionChecker::createBuiltinOverflowNoteTag(CheckerContext &C) const {
-  return C.getNoteTag([](PathSensitiveBugReport &BR,
-                         llvm::raw_ostream &OS) { OS << "Assuming overflow"; },
-                      /*isPrunable=*/true);
-}
-
 std::pair<bool, bool>
 BuiltinFunctionChecker::checkOverflow(CheckerContext &C, SVal RetVal,
                                       QualType Res) const {
@@ -194,30 +188,29 @@ void BuiltinFunctionChecker::handleOverflowBuiltin(const CallEvent &Call,
   SVal RetVal = SVB.evalBinOp(State, Op, Arg1, Arg2, ResultType);
 
   auto [Overflow, NotOverflow] = checkOverflow(C, RetValMax, ResultType);
-  if (NotOverflow) {
-    ProgramStateRef StateNoOverflow = State->BindExpr(
-        CE, C.getLocationContext(), SVB.makeTruthVal(false, BoolTy));
+  auto initializeState = [&](bool isOverflow) {
+    ProgramStateRef NewState = State->BindExpr(
+        CE, C.getLocationContext(), SVB.makeTruthVal(isOverflow, BoolTy));
 
     if (auto L = Call.getArgSVal(2).getAs<Loc>()) {
-      StateNoOverflow =
-          StateNoOverflow->bindLoc(*L, RetVal, C.getLocationContext());
+      NewState = NewState->bindLoc(*L, RetVal, C.getLocationContext());
 
-      // Propagate taint if any of the argumets were tainted
+      // Propagate taint if any of the arguments were tainted
       if (isTainted(State, Arg1) || isTainted(State, Arg2))
-        StateNoOverflow = addTaint(StateNoOverflow, *L);
+        NewState = addTaint(NewState, *L);
     }
 
     C.addTransition(
-        StateNoOverflow,
-        createBuiltinNoOverflowNoteTag(
-            C, /*BothFeasible=*/NotOverflow && Overflow, Arg1, Arg2, RetVal));
-  }
+        NewState,
+        createBuiltinOverflowNoteTag(
+            C, /*overflow=*/isOverflow, Arg1, Arg2, RetVal));
+  };
 
-  if (Overflow) {
-    C.addTransition(State->BindExpr(CE, C.getLocationContext(),
-                                    SVB.makeTruthVal(true, BoolTy)),
-                    createBuiltinOverflowNoteTag(C));
-  }
+  if (NotOverflow)
+    initializeState(false);
+
+  if (Overflow)
+    initializeState(true);
 }
 
 bool BuiltinFunctionChecker::isBuiltinLikeFunction(

clang/test/Analysis/builtin_overflow.c

@@ -26,7 +26,7 @@ void test_add_overflow(void)
    int res;
 
    if (__builtin_add_overflow(__INT_MAX__, 1, &res)) {
-     clang_analyzer_dump_int(res); //expected-warning{{1st function call argument is an uninitialized value}}
+     clang_analyzer_dump_int(res); //expected-warning{{-2147483648 S32b}}
      return;
    }
 
@@ -38,7 +38,7 @@ void test_add_underoverflow(void)
    int res;
 
    if (__builtin_add_overflow(__INT_MIN__, -1, &res)) {
-     clang_analyzer_dump_int(res); //expected-warning{{1st function call argument is an uninitialized value}}
+     clang_analyzer_dump_int(res); //expected-warning{{2147483647 S32b}}
      return;
    }
 
@@ -160,7 +160,7 @@ void test_bool_assign(void)
 {
     int res;
 
-    // Reproduce issue from GH#111147. __builtin_*_overflow funcions
+    // Reproduce issue from GH#111147. __builtin_*_overflow functions
     // should return _Bool, but not int.
     _Bool ret = __builtin_mul_overflow(10, 20, &res); // no crash
 }

clang/test/Analysis/builtin_overflow_notes.c

@@ -19,12 +19,16 @@ void test_no_overflow_note(int a, int b)
 
 void test_overflow_note(int a, int b)
 {
-   int res; // expected-note{{'res' declared without an initial value}}
+   int res;
 
    if (__builtin_add_overflow(a, b, &res)) { // expected-note {{Assuming overflow}}
                                              // expected-note@-1 {{Taking true branch}}
-     int var = res; // expected-warning{{Assigned value is uninitialized}}
-                    // expected-note@-1 {{Assigned value is uninitialized}}
+     if (res) { // expected-note {{Assuming 'res' is not equal to 0}}
+                // expected-note@-1 {{Taking true branch}}
+        int *ptr = 0; // expected-note {{'ptr' initialized to a null pointer value}}
+        int var = *(int *) ptr; //expected-warning {{Dereference of null pointer}}
+                                //expected-note@-1 {{Dereference of null pointer}}
+     }
      return;
    }
 }