[clang] a017ed0 - [analyzer] Model overflow builtins (#102602)

via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 3 03:27:28 PDT 2024


Author: Pavel Skripkin
Date: 2024-10-03T12:27:25+02:00
New Revision: a017ed04cc9bcc75b3c3ef35c923dbe7dc4606f8

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

LOG: [analyzer] Model overflow builtins (#102602)

Add basic support for `builtin_*_overflow`  primitives.
 
These helps a lot for checking custom calloc-like functions with
inlinable body. Without such support code like

```c
#include <stddef.h>
#include <stdlib.h>

static void *myMalloc(size_t a1, size_t a2)
{
    size_t res;

    if (__builtin_mul_overflow(a1, a2, &res))
        return NULL;
    return malloc(res);
}

void test(void)
{
    char *ptr = myMalloc(10, 1);
    ptr[20] = 10;
}
````

does not trigger any warnings.

Added: 
    clang/test/Analysis/builtin_overflow.c
    clang/test/Analysis/builtin_overflow_notes.c

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
    clang/test/Analysis/out-of-bounds-diagnostics.c
    clang/test/Analysis/taint-tester.c

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 329ba37d6791c3..44d5f348ed2d54 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -628,6 +628,8 @@ Static Analyzer
 New features
 ^^^^^^^^^^^^
 
+- Now CSA models `__builtin_*_overflow` functions. (#GH102602)
+
 - MallocChecker now checks for ``ownership_returns(class, idx)`` and ``ownership_takes(class, idx)``
   attributes with class names 
diff erent from "malloc". Clang static analyzer now reports an error
   if class of allocation and deallocation function mismatches.

diff  --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
index b198b1c2ff4d11..69d8e968283b37 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -16,21 +16,93 @@
 
 #include "clang/Basic/Builtins.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Checkers/Taint.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 
 using namespace clang;
 using namespace ento;
+using namespace taint;
 
 namespace {
 
+QualType getSufficientTypeForOverflowOp(CheckerContext &C, const QualType &T) {
+  // Calling a builtin with a non-integer type result produces compiler error.
+  assert(T->isIntegerType());
+
+  ASTContext &ACtx = C.getASTContext();
+
+  unsigned BitWidth = ACtx.getIntWidth(T);
+  return ACtx.getIntTypeForBitwidth(BitWidth * 2, T->isSignedIntegerType());
+}
+
+QualType getOverflowBuiltinResultType(const CallEvent &Call) {
+  // Calling a builtin with an incorrect argument count produces compiler error.
+  assert(Call.getNumArgs() == 3);
+
+  return Call.getArgExpr(2)->getType()->getPointeeType();
+}
+
+QualType getOverflowBuiltinResultType(const CallEvent &Call, CheckerContext &C,
+                                      unsigned BI) {
+  // Calling a builtin with an incorrect argument count produces compiler error.
+  assert(Call.getNumArgs() == 3);
+
+  ASTContext &ACtx = C.getASTContext();
+
+  switch (BI) {
+  case Builtin::BI__builtin_smul_overflow:
+  case Builtin::BI__builtin_ssub_overflow:
+  case Builtin::BI__builtin_sadd_overflow:
+    return ACtx.IntTy;
+  case Builtin::BI__builtin_smull_overflow:
+  case Builtin::BI__builtin_ssubl_overflow:
+  case Builtin::BI__builtin_saddl_overflow:
+    return ACtx.LongTy;
+  case Builtin::BI__builtin_smulll_overflow:
+  case Builtin::BI__builtin_ssubll_overflow:
+  case Builtin::BI__builtin_saddll_overflow:
+    return ACtx.LongLongTy;
+  case Builtin::BI__builtin_umul_overflow:
+  case Builtin::BI__builtin_usub_overflow:
+  case Builtin::BI__builtin_uadd_overflow:
+    return ACtx.UnsignedIntTy;
+  case Builtin::BI__builtin_umull_overflow:
+  case Builtin::BI__builtin_usubl_overflow:
+  case Builtin::BI__builtin_uaddl_overflow:
+    return ACtx.UnsignedLongTy;
+  case Builtin::BI__builtin_umulll_overflow:
+  case Builtin::BI__builtin_usubll_overflow:
+  case Builtin::BI__builtin_uaddll_overflow:
+    return ACtx.UnsignedLongLongTy;
+  case Builtin::BI__builtin_mul_overflow:
+  case Builtin::BI__builtin_sub_overflow:
+  case Builtin::BI__builtin_add_overflow:
+    return getOverflowBuiltinResultType(Call);
+  default:
+    assert(false && "Unknown overflow builtin");
+    return ACtx.IntTy;
+  }
+}
+
 class BuiltinFunctionChecker : public Checker<eval::Call> {
 public:
   bool evalCall(const CallEvent &Call, CheckerContext &C) const;
+  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;
+  std::pair<bool, bool> checkOverflow(CheckerContext &C, SVal RetVal,
+                                      QualType Res) const;
 
 private:
   // From: clang/include/clang/Basic/Builtins.def
@@ -50,6 +122,102 @@ class BuiltinFunctionChecker : public Checker<eval::Call> {
 
 } // namespace
 
+const NoteTag *BuiltinFunctionChecker::createBuiltinNoOverflowNoteTag(
+    CheckerContext &C, bool BothFeasible, SVal Arg1, SVal Arg2,
+    SVal Result) const {
+  return C.getNoteTag([Result, Arg1, Arg2, BothFeasible](
+                          PathSensitiveBugReport &BR, llvm::raw_ostream &OS) {
+    if (!BR.isInteresting(Result))
+      return;
+
+    // Propagate interestingness to input argumets if result is interesting.
+    BR.markInteresting(Arg1);
+    BR.markInteresting(Arg2);
+
+    if (BothFeasible)
+      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 {
+  // Calling a builtin with a non-integer type result produces compiler error.
+  assert(Res->isIntegerType());
+
+  unsigned BitWidth = C.getASTContext().getIntWidth(Res);
+  bool IsUnsigned = Res->isUnsignedIntegerType();
+
+  auto MinValType = llvm::APSInt::getMinValue(BitWidth, IsUnsigned);
+  auto MaxValType = llvm::APSInt::getMaxValue(BitWidth, IsUnsigned);
+  nonloc::ConcreteInt MinVal{MinValType};
+  nonloc::ConcreteInt MaxVal{MaxValType};
+
+  SValBuilder &SVB = C.getSValBuilder();
+  ProgramStateRef State = C.getState();
+  SVal IsLeMax = SVB.evalBinOp(State, BO_LE, RetVal, MaxVal, Res);
+  SVal IsGeMin = SVB.evalBinOp(State, BO_GE, RetVal, MinVal, Res);
+
+  auto [MayNotOverflow, MayOverflow] =
+      State->assume(IsLeMax.castAs<DefinedOrUnknownSVal>());
+  auto [MayNotUnderflow, MayUnderflow] =
+      State->assume(IsGeMin.castAs<DefinedOrUnknownSVal>());
+
+  return {MayOverflow || MayUnderflow, MayNotOverflow && MayNotUnderflow};
+}
+
+void BuiltinFunctionChecker::handleOverflowBuiltin(const CallEvent &Call,
+                                                   CheckerContext &C,
+                                                   BinaryOperator::Opcode Op,
+                                                   QualType ResultType) const {
+  // Calling a builtin with an incorrect argument count produces compiler error.
+  assert(Call.getNumArgs() == 3);
+
+  ProgramStateRef State = C.getState();
+  SValBuilder &SVB = C.getSValBuilder();
+  const Expr *CE = Call.getOriginExpr();
+
+  SVal Arg1 = Call.getArgSVal(0);
+  SVal Arg2 = Call.getArgSVal(1);
+
+  SVal RetValMax = SVB.evalBinOp(State, Op, Arg1, Arg2,
+                                 getSufficientTypeForOverflowOp(C, ResultType));
+  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));
+
+    if (auto L = Call.getArgSVal(2).getAs<Loc>()) {
+      StateNoOverflow =
+          StateNoOverflow->bindLoc(*L, RetVal, C.getLocationContext());
+
+      // Propagate taint if any of the argumets were tainted
+      if (isTainted(State, Arg1) || isTainted(State, Arg2))
+        StateNoOverflow = addTaint(StateNoOverflow, *L);
+    }
+
+    C.addTransition(
+        StateNoOverflow,
+        createBuiltinNoOverflowNoteTag(
+            C, /*BothFeasible=*/NotOverflow && Overflow, Arg1, Arg2, RetVal));
+  }
+
+  if (Overflow) {
+    C.addTransition(
+        State->BindExpr(CE, C.getLocationContext(), SVB.makeTruthVal(true)),
+        createBuiltinOverflowNoteTag(C));
+  }
+}
+
 bool BuiltinFunctionChecker::isBuiltinLikeFunction(
     const CallEvent &Call) const {
   const auto *FD = llvm::dyn_cast_or_null<FunctionDecl>(Call.getDecl());
@@ -82,10 +250,41 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent &Call,
     return true;
   }
 
-  switch (FD->getBuiltinID()) {
+  unsigned BI = FD->getBuiltinID();
+
+  switch (BI) {
   default:
     return false;
-
+  case Builtin::BI__builtin_mul_overflow:
+  case Builtin::BI__builtin_smul_overflow:
+  case Builtin::BI__builtin_smull_overflow:
+  case Builtin::BI__builtin_smulll_overflow:
+  case Builtin::BI__builtin_umul_overflow:
+  case Builtin::BI__builtin_umull_overflow:
+  case Builtin::BI__builtin_umulll_overflow:
+    handleOverflowBuiltin(Call, C, BO_Mul,
+                          getOverflowBuiltinResultType(Call, C, BI));
+    return true;
+  case Builtin::BI__builtin_sub_overflow:
+  case Builtin::BI__builtin_ssub_overflow:
+  case Builtin::BI__builtin_ssubl_overflow:
+  case Builtin::BI__builtin_ssubll_overflow:
+  case Builtin::BI__builtin_usub_overflow:
+  case Builtin::BI__builtin_usubl_overflow:
+  case Builtin::BI__builtin_usubll_overflow:
+    handleOverflowBuiltin(Call, C, BO_Sub,
+                          getOverflowBuiltinResultType(Call, C, BI));
+    return true;
+  case Builtin::BI__builtin_add_overflow:
+  case Builtin::BI__builtin_sadd_overflow:
+  case Builtin::BI__builtin_saddl_overflow:
+  case Builtin::BI__builtin_saddll_overflow:
+  case Builtin::BI__builtin_uadd_overflow:
+  case Builtin::BI__builtin_uaddl_overflow:
+  case Builtin::BI__builtin_uaddll_overflow:
+    handleOverflowBuiltin(Call, C, BO_Add,
+                          getOverflowBuiltinResultType(Call, C, BI));
+    return true;
   case Builtin::BI__builtin_assume:
   case Builtin::BI__assume: {
     assert (Call.getNumArgs() > 0);

diff  --git a/clang/test/Analysis/builtin_overflow.c b/clang/test/Analysis/builtin_overflow.c
new file mode 100644
index 00000000000000..5c61795661d095
--- /dev/null
+++ b/clang/test/Analysis/builtin_overflow.c
@@ -0,0 +1,157 @@
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-unknown -verify %s \
+// RUN:   -analyzer-checker=core,debug.ExprInspection
+
+#define __UINT_MAX__ (__INT_MAX__ * 2U + 1U)
+#define __INT_MIN__  (-__INT_MAX__ - 1)
+
+void clang_analyzer_dump_int(int);
+void clang_analyzer_dump_long(long);
+void clang_analyzer_eval(int);
+void clang_analyzer_warnIfReached(void);
+
+void test_add_nooverflow(void)
+{
+   int res;
+
+   if (__builtin_add_overflow(10, 20, &res)) {
+     clang_analyzer_warnIfReached();
+     return;
+   }
+
+   clang_analyzer_dump_int(res); //expected-warning{{30 S32b}}
+}
+
+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}}
+     return;
+   }
+
+   clang_analyzer_warnIfReached();
+}
+
+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}}
+     return;
+   }
+
+   clang_analyzer_warnIfReached();
+}
+
+void test_sub_underflow(void)
+{
+   int res;
+
+   if (__builtin_sub_overflow(__INT_MIN__, 10, &res)) {
+     return;
+   }
+
+   clang_analyzer_warnIfReached();
+}
+
+void test_sub_overflow(void)
+{
+   int res;
+
+   if (__builtin_sub_overflow(__INT_MAX__, -1, &res)) {
+     return;
+   }
+
+   clang_analyzer_warnIfReached();
+}
+
+void test_sub_nooverflow(void)
+{
+   int res;
+
+   if (__builtin_sub_overflow(__INT_MAX__, 1, &res)) {
+     clang_analyzer_warnIfReached();
+     return;
+   }
+
+   clang_analyzer_dump_int(res); //expected-warning{{2147483646 S32b}}
+}
+
+void test_mul_overflow(void)
+{
+   int res;
+
+   if (__builtin_mul_overflow(__INT_MAX__, 2, &res)) {
+     return;
+   }
+
+   clang_analyzer_warnIfReached();
+}
+
+void test_mul_underflow(void)
+{
+   int res;
+
+   if (__builtin_mul_overflow(__INT_MIN__, -2, &res)) {
+     return;
+   }
+
+   clang_analyzer_warnIfReached();
+}
+
+void test_mul_nooverflow(void)
+{
+   int res;
+
+   if (__builtin_mul_overflow(10, -2, &res)) {
+     clang_analyzer_warnIfReached();
+     return;
+   }
+
+   clang_analyzer_dump_int(res); //expected-warning{{-20 S32b}}
+}
+
+void test_nooverflow_
diff _types(void)
+{
+   long res;
+
+   // This is not an overflow, since result type is long.
+   if (__builtin_add_overflow(__INT_MAX__, 1, &res)) {
+     clang_analyzer_warnIfReached();
+     return;
+   }
+
+   clang_analyzer_dump_long(res); //expected-warning{{2147483648 S64b}}
+}
+
+void test_uaddll_overflow_contraints(unsigned long a, unsigned long b)
+{
+   unsigned long long res;
+
+   if (a != 10)
+     return;
+   if (b != 10)
+     return;
+
+   if (__builtin_uaddll_overflow(a, b, &res)) {
+     clang_analyzer_warnIfReached();
+     return;
+   }
+}
+
+void test_uadd_overflow_contraints(unsigned a, unsigned b)
+{
+   unsigned res;
+
+   if (a > 5)
+     return;
+   if (b != 10)
+     return;
+
+   if (__builtin_uadd_overflow(a, b, &res)) {
+     clang_analyzer_warnIfReached();
+     return;
+   }
+}

diff  --git a/clang/test/Analysis/builtin_overflow_notes.c b/clang/test/Analysis/builtin_overflow_notes.c
new file mode 100644
index 00000000000000..20f333a4a6cca5
--- /dev/null
+++ b/clang/test/Analysis/builtin_overflow_notes.c
@@ -0,0 +1,30 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-output text \
+// RUN:   -verify %s
+
+void test_no_overflow_note(int a, int b)
+{
+   int res;
+
+   if (__builtin_add_overflow(a, b, &res)) // expected-note {{Assuming no overflow}}
+                                           // expected-note at -1 {{Taking false branch}}
+     return;
+
+   if (res) { // expected-note {{Assuming 'res' is not equal to 0}}
+              // expected-note at -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 at -1 {{Dereference of null pointer}}
+   }
+}
+
+void test_overflow_note(int a, int b)
+{
+   int res; // expected-note{{'res' declared without an initial value}}
+
+   if (__builtin_add_overflow(a, b, &res)) { // expected-note {{Assuming overflow}}
+                                             // expected-note at -1 {{Taking true branch}}
+     int var = res; // expected-warning{{Assigned value is garbage or undefined}}
+                    // expected-note at -1 {{Assigned value is garbage or undefined}}
+     return;
+   }
+}

diff  --git a/clang/test/Analysis/out-of-bounds-diagnostics.c b/clang/test/Analysis/out-of-bounds-diagnostics.c
index 8ecad7036c3314..65bc28f58276fd 100644
--- a/clang/test/Analysis/out-of-bounds-diagnostics.c
+++ b/clang/test/Analysis/out-of-bounds-diagnostics.c
@@ -278,6 +278,21 @@ int *mallocRegion(void) {
   return mem;
 }
 
+int *custom_calloc(size_t a, size_t b) {
+  size_t res;
+
+  return __builtin_mul_overflow(a, b, &res) ? 0 : malloc(res);
+}
+
+int *mallocRegionOverflow(void) {
+  int *mem = (int*)custom_calloc(10, sizeof(int));
+
+  mem[20] = 10;
+  // expected-warning at -1 {{Out of bound access to memory after the end of the heap area}}
+  // expected-note at -2 {{Access of the heap area at index 20, while it holds only 10 'int' elements}}
+  return mem;
+}
+
 int *mallocRegionDeref(void) {
   int *mem = (int*)malloc(2*sizeof(int));
 

diff  --git a/clang/test/Analysis/taint-tester.c b/clang/test/Analysis/taint-tester.c
index 479a96c92ececd..66d54eab19f39f 100644
--- a/clang/test/Analysis/taint-tester.c
+++ b/clang/test/Analysis/taint-tester.c
@@ -196,3 +196,19 @@ void noCrashTest(void) {
     __builtin___memcpy_chk(pointer2, pointer1, 0, 0); // no-crash
   }
 }
+
+void builtin_overflow_test(void) {
+  int input, input2, res;
+
+  scanf("%d", &input);
+  scanf("%d", &input2);
+
+  if (__builtin_add_overflow(input, 10, &res)) // expected-warning + {{tainted}}
+    return;
+
+  if (__builtin_add_overflow(10, input, &res)) // expected-warning + {{tainted}}
+    return;
+
+  if (__builtin_add_overflow(input2, input, &res)) // expected-warning + {{tainted}}
+    return;
+}


        


More information about the cfe-commits mailing list