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 <paskrip...@gmail.com>
Date: Fri, 9 Aug 2024 14:37:47 +0300
Subject: [PATCH 01/12] 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@-1 {{Out of bound access to memory after the end of the
heap area}}
+ // expected-note@-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 <paskrip...@gmail.com>
Date: Sat, 10 Aug 2024 00:08:54 +0300
Subject: [PATCH 02/12] 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@-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 <paskrip...@gmail.com>
Date: Sun, 11 Aug 2024 21:06:50 +0300
Subject: [PATCH 03/12] 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 <paskrip...@gmail.com>
Date: Sun, 11 Aug 2024 21:08:48 +0300
Subject: [PATCH 04/12] 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 <paskrip...@gmail.com>
Date: Mon, 12 Aug 2024 17:23:35 +0300
Subject: [PATCH 05/12] 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 <paskrip...@gmail.com>
Date: Mon, 12 Aug 2024 22:58:45 +0300
Subject: [PATCH 06/12] 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 <paskrip...@gmail.com>
Date: Wed, 14 Aug 2024 00:06:52 +0300
Subject: [PATCH 07/12] 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.
>From e03fdb46c50b5c4c1892b30fb9fb6e0ce9515748 Mon Sep 17 00:00:00 2001
From: Pavel Skripkin <paskrip...@gmail.com>
Date: Mon, 2 Sep 2024 22:41:18 +0300
Subject: [PATCH 08/12] added note tag on non-overflow case
---
.../Checkers/BuiltinFunctionChecker.cpp | 25 ++++++++++++++++++-
clang/test/Analysis/builtin_overflow_notes.c | 19 ++++++++++++++
2 files changed, 43 insertions(+), 1 deletion(-)
create mode 100644 clang/test/Analysis/builtin_overflow_notes.c
diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
index 4c87ce9dfeed49..90be1f6f75fc15 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -97,6 +97,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;
std::pair<bool, bool> checkOverflow(CheckerContext &C, SVal RetVal,
QualType Res) const;
@@ -118,6 +121,23 @@ 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 overflow does not happen";
+ });
+}
+
std::pair<bool, bool>
BuiltinFunctionChecker::checkOverflow(CheckerContext &C, SVal RetVal,
QualType Res) const {
@@ -179,7 +199,10 @@ void BuiltinFunctionChecker::handleOverflowBuiltin(const
CallEvent &Call,
StateNoOverflow = addTaint(StateNoOverflow, *L);
}
- C.addTransition(StateNoOverflow);
+ const NoteTag *tag = createBuiltinNoOverflowNoteTag(
+ C, NotOverflow && Overflow, Arg1, Arg2, RetVal);
+
+ C.addTransition(StateNoOverflow, tag);
}
if (Overflow)
diff --git a/clang/test/Analysis/builtin_overflow_notes.c
b/clang/test/Analysis/builtin_overflow_notes.c
new file mode 100644
index 00000000000000..5d0f59dca1d4b3
--- /dev/null
+++ b/clang/test/Analysis/builtin_overflow_notes.c
@@ -0,0 +1,19 @@
+// 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
overflow does not happen}}
+ // expected-note@-1 {{Taking
false branch}}
+ return;
+ }
+
+ 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}}
+ }
+}
>From 2cd9158726ced46a45167eeceef58e5dfa5c9d82 Mon Sep 17 00:00:00 2001
From: Pavel Skripkin <paskrip...@gmail.com>
Date: Wed, 4 Sep 2024 00:56:24 +0300
Subject: [PATCH 09/12] add notes for overflow case
---
.../Checkers/BuiltinFunctionChecker.cpp | 17 +++++++++++++++--
clang/test/Analysis/builtin_overflow_notes.c | 17 ++++++++++++++---
2 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
index 90be1f6f75fc15..8f1a35460ce187 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -100,6 +100,7 @@ class BuiltinFunctionChecker : public Checker<eval::Call> {
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;
@@ -138,6 +139,15 @@ const NoteTag
*BuiltinFunctionChecker::createBuiltinNoOverflowNoteTag(
});
}
+const NoteTag *
+BuiltinFunctionChecker::createBuiltinOverflowNoteTag(CheckerContext &C) const {
+ return C.getNoteTag(
+ [](PathSensitiveBugReport &BR, llvm::raw_ostream &OS) {
+ OS << "Assuming overflow does happen";
+ },
+ /*isPrunable=*/true);
+}
+
std::pair<bool, bool>
BuiltinFunctionChecker::checkOverflow(CheckerContext &C, SVal RetVal,
QualType Res) const {
@@ -205,9 +215,12 @@ void BuiltinFunctionChecker::handleOverflowBuiltin(const
CallEvent &Call,
C.addTransition(StateNoOverflow, tag);
}
- if (Overflow)
+ if (Overflow) {
+ const NoteTag *tag = createBuiltinOverflowNoteTag(C);
C.addTransition(
- State->BindExpr(CE, C.getLocationContext(), SVB.makeTruthVal(true)));
+ State->BindExpr(CE, C.getLocationContext(), SVB.makeTruthVal(true)),
+ tag);
+ }
}
bool BuiltinFunctionChecker::isBuiltinLikeFunction(
diff --git a/clang/test/Analysis/builtin_overflow_notes.c
b/clang/test/Analysis/builtin_overflow_notes.c
index 5d0f59dca1d4b3..821443d05c281e 100644
--- a/clang/test/Analysis/builtin_overflow_notes.c
+++ b/clang/test/Analysis/builtin_overflow_notes.c
@@ -5,10 +5,9 @@ void test_no_overflow_note(int a, int b)
{
int res;
- if (__builtin_add_overflow(a, b, &res)) { // expected-note {{Assuming
overflow does not happen}}
- // expected-note@-1 {{Taking
false branch}}
+ if (__builtin_add_overflow(a, b, &res)) // expected-note {{Assuming
overflow does not happen}}
+ // expected-note@-1 {{Taking false
branch}}
return;
- }
if (res) { // expected-note {{Assuming 'res' is not equal to 0}}
// expected-note@-1 {{Taking true branch}}
@@ -17,3 +16,15 @@ void test_no_overflow_note(int a, int b)
//expected-note@-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 does happen}}
+ // expected-note@-1 {{Taking true
branch}}
+ int var = res; // expected-warning{{Assigned value is garbage or
undefined}}
+ // expected-note@-1 {{Assigned value is garbage or
undefined}}
+ return;
+ }
+}
>From bb82bfba197cc1abb03c0f434344f7aa3d50306e Mon Sep 17 00:00:00 2001
From: Pavel Skripkin <paskrip...@gmail.com>
Date: Thu, 12 Sep 2024 14:57:09 +0300
Subject: [PATCH 10/12] Update clang/docs/ReleaseNotes.rst
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Co-authored-by: Donát Nagy <donat.n...@ericsson.com>
---
clang/docs/ReleaseNotes.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1bcfdf337d28d6..604d0d49f6306d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -327,7 +327,7 @@ Static Analyzer
New features
^^^^^^^^^^^^
-- Now CSA models `builtin_*_overflow` functions.
+- 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
>From dd89f8b51d620b3001cdd21cc45fb0e4d4cc08af Mon Sep 17 00:00:00 2001
From: Pavel Skripkin <paskrip...@gmail.com>
Date: Wed, 2 Oct 2024 19:39:17 +0300
Subject: [PATCH 11/12] apply style review
---
clang/docs/ReleaseNotes.rst | 2 +-
.../Checkers/BuiltinFunctionChecker.cpp | 51 ++++++++-----------
clang/test/Analysis/builtin_overflow.c | 4 +-
clang/test/Analysis/builtin_overflow_notes.c | 4 +-
4 files changed, 27 insertions(+), 34 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 604d0d49f6306d..8737b776c09d8a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -327,7 +327,7 @@ Static Analyzer
New features
^^^^^^^^^^^^
-- Now CSA models `__builtin_*_overflow` functions.
+- Now CSA models `__builtin_*_overflow` functions. (#GH102602)
- 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
diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
index 8f1a35460ce187..860db3985be1d4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -98,7 +98,7 @@ class BuiltinFunctionChecker : public Checker<eval::Call> {
BinaryOperator::Opcode Op,
QualType ResultType) const;
const NoteTag *createBuiltinNoOverflowNoteTag(CheckerContext &C,
- bool bothFeasible, SVal Arg1,
+ bool BothFeasible, SVal Arg1,
SVal Arg2, SVal Result) const;
const NoteTag *createBuiltinOverflowNoteTag(CheckerContext &C) const;
std::pair<bool, bool> checkOverflow(CheckerContext &C, SVal RetVal,
@@ -123,9 +123,9 @@ class BuiltinFunctionChecker : public Checker<eval::Call> {
} // namespace
const NoteTag *BuiltinFunctionChecker::createBuiltinNoOverflowNoteTag(
- CheckerContext &C, bool bothFeasible, SVal Arg1, SVal Arg2,
+ CheckerContext &C, bool BothFeasible, SVal Arg1, SVal Arg2,
SVal Result) const {
- return C.getNoteTag([Result, Arg1, Arg2, bothFeasible](
+ return C.getNoteTag([Result, Arg1, Arg2, BothFeasible](
PathSensitiveBugReport &BR, llvm::raw_ostream &OS) {
if (!BR.isInteresting(Result))
return;
@@ -134,40 +134,34 @@ const NoteTag
*BuiltinFunctionChecker::createBuiltinNoOverflowNoteTag(
BR.markInteresting(Arg1);
BR.markInteresting(Arg2);
- if (bothFeasible)
- OS << "Assuming overflow does not happen";
+ 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 does happen";
- },
- /*isPrunable=*/true);
+ 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 {
- ProgramStateRef State = C.getState();
- 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);
- auto MinVal =
- llvm::APSInt::getMinValue(BitWidth, Res->isUnsignedIntegerType());
- auto MaxVal =
- llvm::APSInt::getMaxValue(BitWidth, Res->isUnsignedIntegerType());
+ unsigned BitWidth = C.getASTContext().getIntWidth(Res);
+ bool IsUnsigned = 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);
+ nonloc::ConcreteInt MinVal{llvm::APSInt::getMinValue(BitWidth, IsUnsigned)};
+ nonloc::ConcreteInt MaxVal{llvm::APSInt::getMaxValue(BitWidth, IsUnsigned)};
+
+ 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>());
@@ -209,17 +203,16 @@ void BuiltinFunctionChecker::handleOverflowBuiltin(const
CallEvent &Call,
StateNoOverflow = addTaint(StateNoOverflow, *L);
}
- const NoteTag *tag = createBuiltinNoOverflowNoteTag(
- C, NotOverflow && Overflow, Arg1, Arg2, RetVal);
-
- C.addTransition(StateNoOverflow, tag);
+ C.addTransition(
+ StateNoOverflow,
+ createBuiltinNoOverflowNoteTag(
+ C, /*BothFeasible=*/NotOverflow && Overflow, Arg1, Arg2, RetVal));
}
if (Overflow) {
- const NoteTag *tag = createBuiltinOverflowNoteTag(C);
C.addTransition(
State->BindExpr(CE, C.getLocationContext(), SVB.makeTruthVal(true)),
- tag);
+ createBuiltinOverflowNoteTag(C));
}
}
diff --git a/clang/test/Analysis/builtin_overflow.c
b/clang/test/Analysis/builtin_overflow.c
index dcf3c3309faa87..5c61795661d095 100644
--- a/clang/test/Analysis/builtin_overflow.c
+++ b/clang/test/Analysis/builtin_overflow.c
@@ -79,7 +79,7 @@ void test_sub_nooverflow(void)
clang_analyzer_dump_int(res); //expected-warning{{2147483646 S32b}}
}
-void test_mul_overrflow(void)
+void test_mul_overflow(void)
{
int res;
@@ -90,7 +90,7 @@ void test_mul_overrflow(void)
clang_analyzer_warnIfReached();
}
-void test_mul_underrflow(void)
+void test_mul_underflow(void)
{
int res;
diff --git a/clang/test/Analysis/builtin_overflow_notes.c
b/clang/test/Analysis/builtin_overflow_notes.c
index 821443d05c281e..20f333a4a6cca5 100644
--- a/clang/test/Analysis/builtin_overflow_notes.c
+++ b/clang/test/Analysis/builtin_overflow_notes.c
@@ -5,7 +5,7 @@ void test_no_overflow_note(int a, int b)
{
int res;
- if (__builtin_add_overflow(a, b, &res)) // expected-note {{Assuming
overflow does not happen}}
+ if (__builtin_add_overflow(a, b, &res)) // expected-note {{Assuming no
overflow}}
// expected-note@-1 {{Taking false
branch}}
return;
@@ -21,7 +21,7 @@ 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 does happen}}
+ 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 garbage or
undefined}}
// expected-note@-1 {{Assigned value is garbage or
undefined}}
>From 41bf101e2472fe7c11c3055d2136504b9ad6c3e9 Mon Sep 17 00:00:00 2001
From: Pavel Skripkin <paskrip...@gmail.com>
Date: Thu, 3 Oct 2024 00:41:02 +0300
Subject: [PATCH 12/12] fix dangling reference bug
---
.../lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
index 860db3985be1d4..69d8e968283b37 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -155,8 +155,10 @@ BuiltinFunctionChecker::checkOverflow(CheckerContext &C,
SVal RetVal,
unsigned BitWidth = C.getASTContext().getIntWidth(Res);
bool IsUnsigned = Res->isUnsignedIntegerType();
- nonloc::ConcreteInt MinVal{llvm::APSInt::getMinValue(BitWidth, IsUnsigned)};
- nonloc::ConcreteInt MaxVal{llvm::APSInt::getMaxValue(BitWidth, IsUnsigned)};
+ 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();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
