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 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@-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 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@-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 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 <paskrip...@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 <paskrip...@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 <paskrip...@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 <paskrip...@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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
