[clang] [analyzer] Model overflow builtins (PR #102602)

Pavel Skripkin via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 13 14:07:22 PDT 2024


https://github.com/pskrgag updated https://github.com/llvm/llvm-project/pull/102602

>From 7b4f999b39f4308cab253204e6be41ea7a70f695 Mon Sep 17 00:00:00 2001
From: Pavel Skripkin <paskripkin at gmail.com>
Date: Fri, 9 Aug 2024 14:37:47 +0300
Subject: [PATCH 1/7] clang/csa: add initial support for builtin overflow

---
 .../Checkers/BuiltinFunctionChecker.cpp       | 124 +++++++++++++++++-
 clang/test/Analysis/builtin_overflow.c        |  65 +++++++++
 .../test/Analysis/out-of-bounds-diagnostics.c |  17 +++
 3 files changed, 204 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/Analysis/builtin_overflow.c

diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
index b198b1c2ff4d11..0c8b9fa3d947b0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -21,16 +21,67 @@
 #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;
 
 namespace {
 
+QualType getOverflowBuiltinResultType(const CallEvent &Call) {
+  assert(Call.getNumArgs() == 3);
+
+  return Call.getArgExpr(2)->getType()->getPointeeType();
+}
+
+QualType getOverflowBuiltinResultType(const CallEvent &Call, CheckerContext &C,
+                                      unsigned BI) {
+  assert(Call.getNumArgs() == 3);
+
+  ASTContext &Ast = C.getASTContext();
+
+  switch (BI) {
+  case Builtin::BI__builtin_smul_overflow:
+  case Builtin::BI__builtin_ssub_overflow:
+  case Builtin::BI__builtin_sadd_overflow:
+    return Ast.IntTy;
+  case Builtin::BI__builtin_smull_overflow:
+  case Builtin::BI__builtin_ssubl_overflow:
+  case Builtin::BI__builtin_saddl_overflow:
+    return Ast.LongTy;
+  case Builtin::BI__builtin_smulll_overflow:
+  case Builtin::BI__builtin_ssubll_overflow:
+  case Builtin::BI__builtin_saddll_overflow:
+    return Ast.LongLongTy;
+  case Builtin::BI__builtin_umul_overflow:
+  case Builtin::BI__builtin_usub_overflow:
+  case Builtin::BI__builtin_uadd_overflow:
+    return Ast.UnsignedIntTy;
+  case Builtin::BI__builtin_umull_overflow:
+  case Builtin::BI__builtin_usubl_overflow:
+  case Builtin::BI__builtin_uaddl_overflow:
+    return Ast.UnsignedLongTy;
+  case Builtin::BI__builtin_umulll_overflow:
+  case Builtin::BI__builtin_usubll_overflow:
+  case Builtin::BI__builtin_uaddll_overflow:
+    return Ast.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");
+  }
+}
+
 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;
 
 private:
   // From: clang/include/clang/Basic/Builtins.def
@@ -50,6 +101,44 @@ class BuiltinFunctionChecker : public Checker<eval::Call> {
 
 } // namespace
 
+void BuiltinFunctionChecker::HandleOverflowBuiltin(const CallEvent &Call,
+                                                   CheckerContext &C,
+                                                   BinaryOperator::Opcode Op,
+                                                   QualType ResultType) const {
+  // All __builtin_*_overflow functions take 3 argumets.
+  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 RetVal = SVB.evalBinOp(State, Op, Arg1, Arg2, ResultType);
+
+  // TODO: Handle overflows with values that known to overflow. Like INT_MAX + 1
+  // should not produce state for non-overflow case and threat it as
+  // unreachable.
+
+  // Handle non-overflow case.
+  {
+    ProgramStateRef StateSuccess =
+        State->BindExpr(CE, C.getLocationContext(), SVB.makeTruthVal(false));
+
+    if (auto L = Call.getArgSVal(2).getAs<Loc>())
+      StateSuccess = StateSuccess->bindLoc(*L, RetVal, C.getLocationContext());
+
+    C.addTransition(StateSuccess);
+  }
+
+  // Handle overflow case.
+  {
+    C.addTransition(
+        State->BindExpr(CE, C.getLocationContext(), SVB.makeTruthVal(true)));
+  }
+}
+
 bool BuiltinFunctionChecker::isBuiltinLikeFunction(
     const CallEvent &Call) const {
   const auto *FD = llvm::dyn_cast_or_null<FunctionDecl>(Call.getDecl());
@@ -82,10 +171,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..3b8639aa450046
--- /dev/null
+++ b/clang/test/Analysis/builtin_overflow.c
@@ -0,0 +1,65 @@
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-unknown -verify %s \
+// RUN:   -analyzer-checker=core,debug.ExprInspection
+
+#define NULL ((void *)0)
+#define INT_MAX __INT_MAX__
+
+void clang_analyzer_dump_int(int);
+
+void test1(void)
+{
+   int res;
+
+   if (__builtin_add_overflow(10, 20, &res)) {
+     clang_analyzer_dump_int(res); //expected-warning{{1st function call argument is an uninitialized value}}
+     return;
+   }
+
+   clang_analyzer_dump_int(res); //expected-warning{{30}}
+}
+
+void test2(void)
+{
+   int res;
+
+   __builtin_add_overflow(10, 20, &res);
+   clang_analyzer_dump_int(res); //expected-warning{{1st function call argument is an uninitialized value}} expected-warning{{S32b}}
+}
+
+void test3(void)
+{
+   int res;
+
+   if (__builtin_sub_overflow(10, 20, &res)) {
+     clang_analyzer_dump_int(res); //expected-warning{{1st function call argument is an uninitialized value}}
+     return;
+   }
+
+   clang_analyzer_dump_int(res); //expected-warning{{-10}}
+}
+
+void test4(void)
+{
+   int res;
+
+   if (__builtin_sub_overflow(10, 20, &res)) {
+     clang_analyzer_dump_int(res); //expected-warning{{1st function call argument is an uninitialized value}}
+     return;
+   }
+
+   if (res != -10) {
+     *(volatile char *)NULL; //no warning
+   }
+}
+
+void test5(void)
+{
+   int res;
+
+   if (__builtin_mul_overflow(10, 20, &res)) {
+     clang_analyzer_dump_int(res); //expected-warning{{1st function call argument is an uninitialized value}}
+     return;
+   }
+
+   clang_analyzer_dump_int(res); //expected-warning{{200}}
+}
diff --git a/clang/test/Analysis/out-of-bounds-diagnostics.c b/clang/test/Analysis/out-of-bounds-diagnostics.c
index de70e483add1c0..73b56e7a8ba4db 100644
--- a/clang/test/Analysis/out-of-bounds-diagnostics.c
+++ b/clang/test/Analysis/out-of-bounds-diagnostics.c
@@ -278,6 +278,23 @@ int *mallocRegion(void) {
   return mem;
 }
 
+int *custom_calloc(size_t a, size_t b) {
+  size_t res;
+  if (__builtin_mul_overflow(a, b, &res))
+    return 0;
+
+  return malloc(res);
+}
+
+int *mallocRegionOverflow(void) {
+  int *mem = (int*)custom_calloc(4, 10);
+
+  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));
 

>From 23923dfbd029eddfca5c7d9adb1876578c12c42c Mon Sep 17 00:00:00 2001
From: Pavel Skripkin <paskripkin at gmail.com>
Date: Sat, 10 Aug 2024 00:08:54 +0300
Subject: [PATCH 2/7] clang/csa: address review and try to use symbolic values
 to split state

---
 .../Checkers/BuiltinFunctionChecker.cpp       | 64 +++++++++++++----
 clang/test/Analysis/builtin_overflow.c        | 68 ++++++++++++-------
 .../test/Analysis/out-of-bounds-diagnostics.c |  6 +-
 3 files changed, 95 insertions(+), 43 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
index 0c8b9fa3d947b0..d048c02dd60012 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -30,6 +30,14 @@ using namespace ento;
 
 namespace {
 
+QualType getSufficientTypeForOverflowOp(CheckerContext &C, const QualType &T) {
+  assert(T->isIntegerType());
+  ASTContext &ACtx = C.getASTContext();
+
+  unsigned BitWidth = ACtx.getIntWidth(T);
+  return ACtx.getIntTypeForBitwidth(BitWidth * 2, T->isSignedIntegerType());
+}
+
 QualType getOverflowBuiltinResultType(const CallEvent &Call) {
   assert(Call.getNumArgs() == 3);
 
@@ -73,15 +81,18 @@ QualType getOverflowBuiltinResultType(const CallEvent &Call, CheckerContext &C,
     return getOverflowBuiltinResultType(Call);
   default:
     assert(false && "Unknown overflow builtin");
+    return Ast.IntTy;
   }
 }
 
 class BuiltinFunctionChecker : public Checker<eval::Call> {
 public:
   bool evalCall(const CallEvent &Call, CheckerContext &C) const;
-  void HandleOverflowBuiltin(const CallEvent &Call, CheckerContext &C,
+  void handleOverflowBuiltin(const CallEvent &Call, CheckerContext &C,
                              BinaryOperator::Opcode Op,
                              QualType ResultType) const;
+  std::pair<bool, bool> checkOverflow(CheckerContext &C, SVal RetVal,
+                                      QualType Res) const;
 
 private:
   // From: clang/include/clang/Basic/Builtins.def
@@ -101,7 +112,36 @@ class BuiltinFunctionChecker : public Checker<eval::Call> {
 
 } // namespace
 
-void BuiltinFunctionChecker::HandleOverflowBuiltin(const CallEvent &Call,
+std::pair<bool, bool>
+BuiltinFunctionChecker::checkOverflow(CheckerContext &C, SVal RetVal,
+                                      QualType Res) const {
+  ProgramStateRef State = C.getState();
+  SValBuilder &SVB = C.getSValBuilder();
+  auto SvalToBool = [&](SVal val) {
+    return State->isNonNull(val).isConstrainedTrue();
+  };
+  ASTContext &ACtx = C.getASTContext();
+
+  assert(Res->isIntegerType());
+
+  unsigned BitWidth = ACtx.getIntWidth(Res);
+  SVal MinType = nonloc::ConcreteInt(
+      llvm::APSInt::getMinValue(BitWidth, Res->isUnsignedIntegerType()));
+  SVal MaxType = nonloc::ConcreteInt(
+      llvm::APSInt::getMaxValue(BitWidth, Res->isUnsignedIntegerType()));
+
+  bool IsGreaterMax =
+      SvalToBool(SVB.evalBinOp(State, BO_GT, RetVal, MaxType, Res));
+  bool IsLessMin =
+      SvalToBool(SVB.evalBinOp(State, BO_LT, RetVal, MinType, Res));
+
+  bool IsLeMax = SvalToBool(SVB.evalBinOp(State, BO_LE, RetVal, MaxType, Res));
+  bool IsGeMin = SvalToBool(SVB.evalBinOp(State, BO_GE, RetVal, MinType, Res));
+
+  return {IsGreaterMax || IsLessMin, IsLeMax && IsGeMin};
+}
+
+void BuiltinFunctionChecker::handleOverflowBuiltin(const CallEvent &Call,
                                                    CheckerContext &C,
                                                    BinaryOperator::Opcode Op,
                                                    QualType ResultType) const {
@@ -115,14 +155,12 @@ void BuiltinFunctionChecker::HandleOverflowBuiltin(const CallEvent &Call,
   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);
 
-  // TODO: Handle overflows with values that known to overflow. Like INT_MAX + 1
-  // should not produce state for non-overflow case and threat it as
-  // unreachable.
-
-  // Handle non-overflow case.
-  {
+  auto [Overflow, NotOverflow] = checkOverflow(C, RetValMax, ResultType);
+  if (NotOverflow) {
     ProgramStateRef StateSuccess =
         State->BindExpr(CE, C.getLocationContext(), SVB.makeTruthVal(false));
 
@@ -132,11 +170,9 @@ void BuiltinFunctionChecker::HandleOverflowBuiltin(const CallEvent &Call,
     C.addTransition(StateSuccess);
   }
 
-  // Handle overflow case.
-  {
+  if (Overflow)
     C.addTransition(
         State->BindExpr(CE, C.getLocationContext(), SVB.makeTruthVal(true)));
-  }
 }
 
 bool BuiltinFunctionChecker::isBuiltinLikeFunction(
@@ -183,7 +219,7 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent &Call,
   case Builtin::BI__builtin_umul_overflow:
   case Builtin::BI__builtin_umull_overflow:
   case Builtin::BI__builtin_umulll_overflow:
-    HandleOverflowBuiltin(Call, C, BO_Mul,
+    handleOverflowBuiltin(Call, C, BO_Mul,
                           getOverflowBuiltinResultType(Call, C, BI));
     return true;
   case Builtin::BI__builtin_sub_overflow:
@@ -193,7 +229,7 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent &Call,
   case Builtin::BI__builtin_usub_overflow:
   case Builtin::BI__builtin_usubl_overflow:
   case Builtin::BI__builtin_usubll_overflow:
-    HandleOverflowBuiltin(Call, C, BO_Sub,
+    handleOverflowBuiltin(Call, C, BO_Sub,
                           getOverflowBuiltinResultType(Call, C, BI));
     return true;
   case Builtin::BI__builtin_add_overflow:
@@ -203,7 +239,7 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent &Call,
   case Builtin::BI__builtin_uadd_overflow:
   case Builtin::BI__builtin_uaddl_overflow:
   case Builtin::BI__builtin_uaddll_overflow:
-    HandleOverflowBuiltin(Call, C, BO_Add,
+    handleOverflowBuiltin(Call, C, BO_Add,
                           getOverflowBuiltinResultType(Call, C, BI));
     return true;
   case Builtin::BI__builtin_assume:
diff --git a/clang/test/Analysis/builtin_overflow.c b/clang/test/Analysis/builtin_overflow.c
index 3b8639aa450046..695c8e66e8b842 100644
--- a/clang/test/Analysis/builtin_overflow.c
+++ b/clang/test/Analysis/builtin_overflow.c
@@ -1,65 +1,83 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-unknown-unknown -verify %s \
 // RUN:   -analyzer-checker=core,debug.ExprInspection
 
-#define NULL ((void *)0)
-#define INT_MAX __INT_MAX__
+#define __UINT_MAX__ (__INT_MAX__  * 2U + 1U)
 
 void clang_analyzer_dump_int(int);
+void clang_analyzer_eval(int);
+void clang_analyzer_warnIfReached(void);
 
-void test1(void)
+void test_add_nooverflow(void)
 {
    int res;
 
    if (__builtin_add_overflow(10, 20, &res)) {
-     clang_analyzer_dump_int(res); //expected-warning{{1st function call argument is an uninitialized value}}
+     clang_analyzer_warnIfReached();
      return;
    }
 
-   clang_analyzer_dump_int(res); //expected-warning{{30}}
+   clang_analyzer_dump_int(res); //expected-warning{{30 S32b}}
 }
 
-void test2(void)
+void test_add_overflow(void)
 {
    int res;
 
-   __builtin_add_overflow(10, 20, &res);
-   clang_analyzer_dump_int(res); //expected-warning{{1st function call argument is an uninitialized value}} expected-warning{{S32b}}
+   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 test3(void)
+void test_add_overflow_contraints(int a, int b)
 {
    int res;
 
-   if (__builtin_sub_overflow(10, 20, &res)) {
-     clang_analyzer_dump_int(res); //expected-warning{{1st function call argument is an uninitialized value}}
+   if (a != 10)
+     return;
+   if (b != 0)
+     return;
+
+   if (__builtin_add_overflow(a, b, &res)) {
+     clang_analyzer_warnIfReached();
      return;
    }
 
-   clang_analyzer_dump_int(res); //expected-warning{{-10}}
+   clang_analyzer_dump_int(res); //expected-warning{{10 S32b}}
 }
 
-void test4(void)
+void test_uaddll_overflow_contraints(unsigned long a, unsigned long b)
 {
-   int res;
+   unsigned long long res;
 
-   if (__builtin_sub_overflow(10, 20, &res)) {
-     clang_analyzer_dump_int(res); //expected-warning{{1st function call argument is an uninitialized value}}
+   if (a != 10)
+     return;
+   if (b != 10)
      return;
-   }
 
-   if (res != -10) {
-     *(volatile char *)NULL; //no warning
+   if (__builtin_uaddll_overflow(a, b, &res)) {
+     clang_analyzer_warnIfReached();
+     return;
    }
 }
 
-void test5(void)
+void test_uadd_overflow_contraints(unsigned a, unsigned b)
 {
-   int res;
+   unsigned res;
 
-   if (__builtin_mul_overflow(10, 20, &res)) {
-     clang_analyzer_dump_int(res); //expected-warning{{1st function call argument is an uninitialized value}}
+   if (a > 10)
      return;
-   }
+   if (b > 10)
+     return;
+
+   // clang_analyzer_eval(a + b < 30); <--- Prints 1 and 0, but why ???
 
-   clang_analyzer_dump_int(res); //expected-warning{{200}}
+   if (__builtin_uadd_overflow(a, b, &res)) {
+     /* clang_analyzer_warnIfReached(); */
+     return;
+   }
 }
+
+// TODO: more tests after figuring out what's going on.
diff --git a/clang/test/Analysis/out-of-bounds-diagnostics.c b/clang/test/Analysis/out-of-bounds-diagnostics.c
index 73b56e7a8ba4db..3231fbc1a1be00 100644
--- a/clang/test/Analysis/out-of-bounds-diagnostics.c
+++ b/clang/test/Analysis/out-of-bounds-diagnostics.c
@@ -280,14 +280,12 @@ int *mallocRegion(void) {
 
 int *custom_calloc(size_t a, size_t b) {
   size_t res;
-  if (__builtin_mul_overflow(a, b, &res))
-    return 0;
 
-  return malloc(res);
+  return __builtin_mul_overflow(a, b, &res) ? 0 : malloc(res);
 }
 
 int *mallocRegionOverflow(void) {
-  int *mem = (int*)custom_calloc(4, 10);
+  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}}

>From 2e067c8befb0dea0c74f62033044de159a6493bf Mon Sep 17 00:00:00 2001
From: Pavel Skripkin <paskripkin at gmail.com>
Date: Sun, 11 Aug 2024 21:06:50 +0300
Subject: [PATCH 3/7] clang/csa: simplify code and fix dangling reference in
 checkOverflow

---
 .../Checkers/BuiltinFunctionChecker.cpp       | 37 ++++----
 clang/test/Analysis/builtin_overflow.c        | 90 +++++++++++++++++--
 2 files changed, 99 insertions(+), 28 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
index d048c02dd60012..5d7760c1317a84 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -48,40 +48,40 @@ QualType getOverflowBuiltinResultType(const CallEvent &Call, CheckerContext &C,
                                       unsigned BI) {
   assert(Call.getNumArgs() == 3);
 
-  ASTContext &Ast = C.getASTContext();
+  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 Ast.IntTy;
+    return ACtx.IntTy;
   case Builtin::BI__builtin_smull_overflow:
   case Builtin::BI__builtin_ssubl_overflow:
   case Builtin::BI__builtin_saddl_overflow:
-    return Ast.LongTy;
+    return ACtx.LongTy;
   case Builtin::BI__builtin_smulll_overflow:
   case Builtin::BI__builtin_ssubll_overflow:
   case Builtin::BI__builtin_saddll_overflow:
-    return Ast.LongLongTy;
+    return ACtx.LongLongTy;
   case Builtin::BI__builtin_umul_overflow:
   case Builtin::BI__builtin_usub_overflow:
   case Builtin::BI__builtin_uadd_overflow:
-    return Ast.UnsignedIntTy;
+    return ACtx.UnsignedIntTy;
   case Builtin::BI__builtin_umull_overflow:
   case Builtin::BI__builtin_usubl_overflow:
   case Builtin::BI__builtin_uaddl_overflow:
-    return Ast.UnsignedLongTy;
+    return ACtx.UnsignedLongTy;
   case Builtin::BI__builtin_umulll_overflow:
   case Builtin::BI__builtin_usubll_overflow:
   case Builtin::BI__builtin_uaddll_overflow:
-    return Ast.UnsignedLongLongTy;
+    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 Ast.IntTy;
+    return ACtx.IntTy;
   }
 }
 
@@ -117,28 +117,21 @@ BuiltinFunctionChecker::checkOverflow(CheckerContext &C, SVal RetVal,
                                       QualType Res) const {
   ProgramStateRef State = C.getState();
   SValBuilder &SVB = C.getSValBuilder();
-  auto SvalToBool = [&](SVal val) {
-    return State->isNonNull(val).isConstrainedTrue();
-  };
   ASTContext &ACtx = C.getASTContext();
 
   assert(Res->isIntegerType());
 
   unsigned BitWidth = ACtx.getIntWidth(Res);
-  SVal MinType = nonloc::ConcreteInt(
-      llvm::APSInt::getMinValue(BitWidth, Res->isUnsignedIntegerType()));
-  SVal MaxType = nonloc::ConcreteInt(
-      llvm::APSInt::getMaxValue(BitWidth, Res->isUnsignedIntegerType()));
+  auto MinVal = llvm::APSInt::getMinValue(BitWidth, Res->isUnsignedIntegerType());
+  auto MaxVal = llvm::APSInt::getMaxValue(BitWidth, Res->isUnsignedIntegerType());
 
-  bool IsGreaterMax =
-      SvalToBool(SVB.evalBinOp(State, BO_GT, RetVal, MaxType, Res));
-  bool IsLessMin =
-      SvalToBool(SVB.evalBinOp(State, BO_LT, RetVal, MinType, Res));
+  SVal IsLeMax = SVB.evalBinOp(State, BO_LE, RetVal, nonloc::ConcreteInt(MaxVal), Res);
+  SVal IsGeMin = SVB.evalBinOp(State, BO_GE, RetVal, nonloc::ConcreteInt(MinVal), Res);
 
-  bool IsLeMax = SvalToBool(SVB.evalBinOp(State, BO_LE, RetVal, MaxType, Res));
-  bool IsGeMin = SvalToBool(SVB.evalBinOp(State, BO_GE, RetVal, MinType, Res));
+  auto [MayNotOverflow, MayOverflow] = State->assume(IsLeMax.castAs<DefinedOrUnknownSVal>());
+  auto [MayNotUnderflow, MayUnderflow] = State->assume(IsGeMin.castAs<DefinedOrUnknownSVal>());
 
-  return {IsGreaterMax || IsLessMin, IsLeMax && IsGeMin};
+  return {MayOverflow || MayUnderflow, MayNotOverflow && MayNotUnderflow};
 }
 
 void BuiltinFunctionChecker::handleOverflowBuiltin(const CallEvent &Call,
diff --git a/clang/test/Analysis/builtin_overflow.c b/clang/test/Analysis/builtin_overflow.c
index 695c8e66e8b842..765f1b816495ee 100644
--- a/clang/test/Analysis/builtin_overflow.c
+++ b/clang/test/Analysis/builtin_overflow.c
@@ -1,9 +1,11 @@
 // 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 __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);
 
@@ -31,21 +33,97 @@ void test_add_overflow(void)
    clang_analyzer_warnIfReached();
 }
 
-void test_add_overflow_contraints(int a, int b)
+void test_add_underoverflow(void)
 {
    int res;
 
-   if (a != 10)
+   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_overrflow(void)
+{
+   int res;
+
+   if (__builtin_mul_overflow(__INT_MAX__, 2, &res)) {
      return;
-   if (b != 0)
+   }
+
+   clang_analyzer_warnIfReached();
+}
+
+void test_mul_underrflow(void)
+{
+   int res;
+
+   if (__builtin_mul_overflow(__INT_MIN__, -2, &res)) {
      return;
+   }
 
-   if (__builtin_add_overflow(a, b, &res)) {
+   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_int(res); //expected-warning{{10 S32b}}
+   clang_analyzer_dump_long(res); //expected-warning{{2147483648 S64b}}
 }
 
 void test_uaddll_overflow_contraints(unsigned long a, unsigned long b)

>From d1bb65e010fb1833520b93dfe70afb5739bba3cb Mon Sep 17 00:00:00 2001
From: Pavel Skripkin <paskripkin at gmail.com>
Date: Sun, 11 Aug 2024 21:08:48 +0300
Subject: [PATCH 4/7] fix formatting

---
 .../Checkers/BuiltinFunctionChecker.cpp       | 22 ++++++++++++-------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
index 5d7760c1317a84..1507973c63e974 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -122,14 +122,20 @@ BuiltinFunctionChecker::checkOverflow(CheckerContext &C, SVal RetVal,
   assert(Res->isIntegerType());
 
   unsigned BitWidth = ACtx.getIntWidth(Res);
-  auto MinVal = llvm::APSInt::getMinValue(BitWidth, Res->isUnsignedIntegerType());
-  auto MaxVal = llvm::APSInt::getMaxValue(BitWidth, Res->isUnsignedIntegerType());
-
-  SVal IsLeMax = SVB.evalBinOp(State, BO_LE, RetVal, nonloc::ConcreteInt(MaxVal), Res);
-  SVal IsGeMin = SVB.evalBinOp(State, BO_GE, RetVal, nonloc::ConcreteInt(MinVal), Res);
-
-  auto [MayNotOverflow, MayOverflow] = State->assume(IsLeMax.castAs<DefinedOrUnknownSVal>());
-  auto [MayNotUnderflow, MayUnderflow] = State->assume(IsGeMin.castAs<DefinedOrUnknownSVal>());
+  auto MinVal =
+      llvm::APSInt::getMinValue(BitWidth, Res->isUnsignedIntegerType());
+  auto MaxVal =
+      llvm::APSInt::getMaxValue(BitWidth, Res->isUnsignedIntegerType());
+
+  SVal IsLeMax =
+      SVB.evalBinOp(State, BO_LE, RetVal, nonloc::ConcreteInt(MaxVal), Res);
+  SVal IsGeMin =
+      SVB.evalBinOp(State, BO_GE, RetVal, nonloc::ConcreteInt(MinVal), Res);
+
+  auto [MayNotOverflow, MayOverflow] =
+      State->assume(IsLeMax.castAs<DefinedOrUnknownSVal>());
+  auto [MayNotUnderflow, MayUnderflow] =
+      State->assume(IsGeMin.castAs<DefinedOrUnknownSVal>());
 
   return {MayOverflow || MayUnderflow, MayNotOverflow && MayNotUnderflow};
 }

>From c170268b5a0bb58a94861e0f7dc3f48a877269c4 Mon Sep 17 00:00:00 2001
From: Pavel Skripkin <paskripkin at gmail.com>
Date: Mon, 12 Aug 2024 17:23:35 +0300
Subject: [PATCH 5/7] retrigger CI

---
 clang/test/Analysis/builtin_overflow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/test/Analysis/builtin_overflow.c b/clang/test/Analysis/builtin_overflow.c
index 765f1b816495ee..19a2917423be66 100644
--- a/clang/test/Analysis/builtin_overflow.c
+++ b/clang/test/Analysis/builtin_overflow.c
@@ -150,7 +150,7 @@ void test_uadd_overflow_contraints(unsigned a, unsigned b)
    if (b > 10)
      return;
 
-   // clang_analyzer_eval(a + b < 30); <--- Prints 1 and 0, but why ???
+   /* clang_analyzer_eval(a + b < 30); <--- Prints 1 and 0, but why ??? */
 
    if (__builtin_uadd_overflow(a, b, &res)) {
      /* clang_analyzer_warnIfReached(); */

>From b2b0b98a7883564ae32b8d031c1112bd27628c21 Mon Sep 17 00:00:00 2001
From: Pavel Skripkin <paskripkin at gmail.com>
Date: Mon, 12 Aug 2024 22:58:45 +0300
Subject: [PATCH 6/7] clang/csa: add comments about wrong builtin arguments and
 propogate taint

---
 .../Checkers/BuiltinFunctionChecker.cpp       | 23 +++++++++++++++----
 clang/test/Analysis/builtin_overflow.c        | 10 +++-----
 clang/test/Analysis/taint-tester.c            | 16 +++++++++++++
 3 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
index 1507973c63e974..4c87ce9dfeed49 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -16,6 +16,7 @@
 
 #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"
@@ -27,11 +28,14 @@
 
 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);
@@ -39,6 +43,7 @@ QualType getSufficientTypeForOverflowOp(CheckerContext &C, const QualType &T) {
 }
 
 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();
@@ -46,6 +51,7 @@ QualType getOverflowBuiltinResultType(const CallEvent &Call) {
 
 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();
@@ -119,6 +125,7 @@ BuiltinFunctionChecker::checkOverflow(CheckerContext &C, SVal RetVal,
   SValBuilder &SVB = C.getSValBuilder();
   ASTContext &ACtx = C.getASTContext();
 
+  // Calling a builtin with a non-integer type result produces compiler error.
   assert(Res->isIntegerType());
 
   unsigned BitWidth = ACtx.getIntWidth(Res);
@@ -144,7 +151,7 @@ void BuiltinFunctionChecker::handleOverflowBuiltin(const CallEvent &Call,
                                                    CheckerContext &C,
                                                    BinaryOperator::Opcode Op,
                                                    QualType ResultType) const {
-  // All __builtin_*_overflow functions take 3 argumets.
+  // Calling a builtin with an incorrect argument count produces compiler error.
   assert(Call.getNumArgs() == 3);
 
   ProgramStateRef State = C.getState();
@@ -160,13 +167,19 @@ void BuiltinFunctionChecker::handleOverflowBuiltin(const CallEvent &Call,
 
   auto [Overflow, NotOverflow] = checkOverflow(C, RetValMax, ResultType);
   if (NotOverflow) {
-    ProgramStateRef StateSuccess =
+    ProgramStateRef StateNoOverflow =
         State->BindExpr(CE, C.getLocationContext(), SVB.makeTruthVal(false));
 
-    if (auto L = Call.getArgSVal(2).getAs<Loc>())
-      StateSuccess = StateSuccess->bindLoc(*L, RetVal, C.getLocationContext());
+    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(StateSuccess);
+    C.addTransition(StateNoOverflow);
   }
 
   if (Overflow)
diff --git a/clang/test/Analysis/builtin_overflow.c b/clang/test/Analysis/builtin_overflow.c
index 19a2917423be66..dcf3c3309faa87 100644
--- a/clang/test/Analysis/builtin_overflow.c
+++ b/clang/test/Analysis/builtin_overflow.c
@@ -145,17 +145,13 @@ void test_uadd_overflow_contraints(unsigned a, unsigned b)
 {
    unsigned res;
 
-   if (a > 10)
+   if (a > 5)
      return;
-   if (b > 10)
+   if (b != 10)
      return;
 
-   /* clang_analyzer_eval(a + b < 30); <--- Prints 1 and 0, but why ??? */
-
    if (__builtin_uadd_overflow(a, b, &res)) {
-     /* clang_analyzer_warnIfReached(); */
+     clang_analyzer_warnIfReached();
      return;
    }
 }
-
-// TODO: more tests after figuring out what's going on.
diff --git a/clang/test/Analysis/taint-tester.c b/clang/test/Analysis/taint-tester.c
index 302349fb662ddb..fb0fe78f1ae654 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;
+}

>From 412baea90119c5e30b929de9a186d48159e94af7 Mon Sep 17 00:00:00 2001
From: Pavel Skripkin <paskripkin at gmail.com>
Date: Wed, 14 Aug 2024 00:06:52 +0300
Subject: [PATCH 7/7] add release note

---
 clang/docs/ReleaseNotes.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 6796a619ba97f8..1bcfdf337d28d6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -327,6 +327,8 @@ Static Analyzer
 New features
 ^^^^^^^^^^^^
 
+- Now CSA models `builtin_*_overflow` functions.
+
 - MallocChecker now checks for ``ownership_returns(class, idx)`` and ``ownership_takes(class, idx)``
   attributes with class names different from "malloc". Clang static analyzer now reports an error
   if class of allocation and deallocation function mismatches.



More information about the cfe-commits mailing list