Skip to content

Commit 2e067c8

Browse files
committed
clang/csa: simplify code and fix dangling reference in checkOverflow
1 parent 23923df commit 2e067c8

File tree

2 files changed

+99
-28
lines changed

2 files changed

+99
-28
lines changed

clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp

+15-22
Original file line numberDiff line numberDiff line change
@@ -48,40 +48,40 @@ QualType getOverflowBuiltinResultType(const CallEvent &Call, CheckerContext &C,
4848
unsigned BI) {
4949
assert(Call.getNumArgs() == 3);
5050

51-
ASTContext &Ast = C.getASTContext();
51+
ASTContext &ACtx = C.getASTContext();
5252

5353
switch (BI) {
5454
case Builtin::BI__builtin_smul_overflow:
5555
case Builtin::BI__builtin_ssub_overflow:
5656
case Builtin::BI__builtin_sadd_overflow:
57-
return Ast.IntTy;
57+
return ACtx.IntTy;
5858
case Builtin::BI__builtin_smull_overflow:
5959
case Builtin::BI__builtin_ssubl_overflow:
6060
case Builtin::BI__builtin_saddl_overflow:
61-
return Ast.LongTy;
61+
return ACtx.LongTy;
6262
case Builtin::BI__builtin_smulll_overflow:
6363
case Builtin::BI__builtin_ssubll_overflow:
6464
case Builtin::BI__builtin_saddll_overflow:
65-
return Ast.LongLongTy;
65+
return ACtx.LongLongTy;
6666
case Builtin::BI__builtin_umul_overflow:
6767
case Builtin::BI__builtin_usub_overflow:
6868
case Builtin::BI__builtin_uadd_overflow:
69-
return Ast.UnsignedIntTy;
69+
return ACtx.UnsignedIntTy;
7070
case Builtin::BI__builtin_umull_overflow:
7171
case Builtin::BI__builtin_usubl_overflow:
7272
case Builtin::BI__builtin_uaddl_overflow:
73-
return Ast.UnsignedLongTy;
73+
return ACtx.UnsignedLongTy;
7474
case Builtin::BI__builtin_umulll_overflow:
7575
case Builtin::BI__builtin_usubll_overflow:
7676
case Builtin::BI__builtin_uaddll_overflow:
77-
return Ast.UnsignedLongLongTy;
77+
return ACtx.UnsignedLongLongTy;
7878
case Builtin::BI__builtin_mul_overflow:
7979
case Builtin::BI__builtin_sub_overflow:
8080
case Builtin::BI__builtin_add_overflow:
8181
return getOverflowBuiltinResultType(Call);
8282
default:
8383
assert(false && "Unknown overflow builtin");
84-
return Ast.IntTy;
84+
return ACtx.IntTy;
8585
}
8686
}
8787

@@ -117,28 +117,21 @@ BuiltinFunctionChecker::checkOverflow(CheckerContext &C, SVal RetVal,
117117
QualType Res) const {
118118
ProgramStateRef State = C.getState();
119119
SValBuilder &SVB = C.getSValBuilder();
120-
auto SvalToBool = [&](SVal val) {
121-
return State->isNonNull(val).isConstrainedTrue();
122-
};
123120
ASTContext &ACtx = C.getASTContext();
124121

125122
assert(Res->isIntegerType());
126123

127124
unsigned BitWidth = ACtx.getIntWidth(Res);
128-
SVal MinType = nonloc::ConcreteInt(
129-
llvm::APSInt::getMinValue(BitWidth, Res->isUnsignedIntegerType()));
130-
SVal MaxType = nonloc::ConcreteInt(
131-
llvm::APSInt::getMaxValue(BitWidth, Res->isUnsignedIntegerType()));
125+
auto MinVal = llvm::APSInt::getMinValue(BitWidth, Res->isUnsignedIntegerType());
126+
auto MaxVal = llvm::APSInt::getMaxValue(BitWidth, Res->isUnsignedIntegerType());
132127

133-
bool IsGreaterMax =
134-
SvalToBool(SVB.evalBinOp(State, BO_GT, RetVal, MaxType, Res));
135-
bool IsLessMin =
136-
SvalToBool(SVB.evalBinOp(State, BO_LT, RetVal, MinType, Res));
128+
SVal IsLeMax = SVB.evalBinOp(State, BO_LE, RetVal, nonloc::ConcreteInt(MaxVal), Res);
129+
SVal IsGeMin = SVB.evalBinOp(State, BO_GE, RetVal, nonloc::ConcreteInt(MinVal), Res);
137130

138-
bool IsLeMax = SvalToBool(SVB.evalBinOp(State, BO_LE, RetVal, MaxType, Res));
139-
bool IsGeMin = SvalToBool(SVB.evalBinOp(State, BO_GE, RetVal, MinType, Res));
131+
auto [MayNotOverflow, MayOverflow] = State->assume(IsLeMax.castAs<DefinedOrUnknownSVal>());
132+
auto [MayNotUnderflow, MayUnderflow] = State->assume(IsGeMin.castAs<DefinedOrUnknownSVal>());
140133

141-
return {IsGreaterMax || IsLessMin, IsLeMax && IsGeMin};
134+
return {MayOverflow || MayUnderflow, MayNotOverflow && MayNotUnderflow};
142135
}
143136

144137
void BuiltinFunctionChecker::handleOverflowBuiltin(const CallEvent &Call,

clang/test/Analysis/builtin_overflow.c

+84-6
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
// RUN: %clang_analyze_cc1 -triple x86_64-unknown-unknown -verify %s \
22
// RUN: -analyzer-checker=core,debug.ExprInspection
33

4-
#define __UINT_MAX__ (__INT_MAX__ * 2U + 1U)
4+
#define __UINT_MAX__ (__INT_MAX__ * 2U + 1U)
5+
#define __INT_MIN__ (-__INT_MAX__ - 1)
56

67
void clang_analyzer_dump_int(int);
8+
void clang_analyzer_dump_long(long);
79
void clang_analyzer_eval(int);
810
void clang_analyzer_warnIfReached(void);
911

@@ -31,21 +33,97 @@ void test_add_overflow(void)
3133
clang_analyzer_warnIfReached();
3234
}
3335

34-
void test_add_overflow_contraints(int a, int b)
36+
void test_add_underoverflow(void)
3537
{
3638
int res;
3739

38-
if (a != 10)
40+
if (__builtin_add_overflow(__INT_MIN__, -1, &res)) {
41+
clang_analyzer_dump_int(res); //expected-warning{{1st function call argument is an uninitialized value}}
42+
return;
43+
}
44+
45+
clang_analyzer_warnIfReached();
46+
}
47+
48+
void test_sub_underflow(void)
49+
{
50+
int res;
51+
52+
if (__builtin_sub_overflow(__INT_MIN__, 10, &res)) {
53+
return;
54+
}
55+
56+
clang_analyzer_warnIfReached();
57+
}
58+
59+
void test_sub_overflow(void)
60+
{
61+
int res;
62+
63+
if (__builtin_sub_overflow(__INT_MAX__, -1, &res)) {
64+
return;
65+
}
66+
67+
clang_analyzer_warnIfReached();
68+
}
69+
70+
void test_sub_nooverflow(void)
71+
{
72+
int res;
73+
74+
if (__builtin_sub_overflow(__INT_MAX__, 1, &res)) {
75+
clang_analyzer_warnIfReached();
76+
return;
77+
}
78+
79+
clang_analyzer_dump_int(res); //expected-warning{{2147483646 S32b}}
80+
}
81+
82+
void test_mul_overrflow(void)
83+
{
84+
int res;
85+
86+
if (__builtin_mul_overflow(__INT_MAX__, 2, &res)) {
3987
return;
40-
if (b != 0)
88+
}
89+
90+
clang_analyzer_warnIfReached();
91+
}
92+
93+
void test_mul_underrflow(void)
94+
{
95+
int res;
96+
97+
if (__builtin_mul_overflow(__INT_MIN__, -2, &res)) {
4198
return;
99+
}
42100

43-
if (__builtin_add_overflow(a, b, &res)) {
101+
clang_analyzer_warnIfReached();
102+
}
103+
104+
void test_mul_nooverflow(void)
105+
{
106+
int res;
107+
108+
if (__builtin_mul_overflow(10, -2, &res)) {
109+
clang_analyzer_warnIfReached();
110+
return;
111+
}
112+
113+
clang_analyzer_dump_int(res); //expected-warning{{-20 S32b}}
114+
}
115+
116+
void test_nooverflow_diff_types(void)
117+
{
118+
long res;
119+
120+
// This is not an overflow, since result type is long.
121+
if (__builtin_add_overflow(__INT_MAX__, 1, &res)) {
44122
clang_analyzer_warnIfReached();
45123
return;
46124
}
47125

48-
clang_analyzer_dump_int(res); //expected-warning{{10 S32b}}
126+
clang_analyzer_dump_long(res); //expected-warning{{2147483648 S64b}}
49127
}
50128

51129
void test_uaddll_overflow_contraints(unsigned long a, unsigned long b)

0 commit comments

Comments
 (0)