From a017ed04cc9bcc75b3c3ef35c923dbe7dc4606f8 Mon Sep 17 00:00:00 2001 From: Pavel Skripkin Date: Thu, 3 Oct 2024 13:27:25 +0300 Subject: [PATCH] [analyzer] Model overflow builtins (#102602) Add basic support for `builtin_*_overflow` primitives. These helps a lot for checking custom calloc-like functions with inlinable body. Without such support code like ```c #include #include static void *myMalloc(size_t a1, size_t a2) { size_t res; if (__builtin_mul_overflow(a1, a2, &res)) return NULL; return malloc(res); } void test(void) { char *ptr = myMalloc(10, 1); ptr[20] = 10; } ```` does not trigger any warnings. --- clang/docs/ReleaseNotes.rst | 2 + .../Checkers/BuiltinFunctionChecker.cpp | 203 +++++++++++++++++- clang/test/Analysis/builtin_overflow.c | 157 ++++++++++++++ clang/test/Analysis/builtin_overflow_notes.c | 30 +++ .../test/Analysis/out-of-bounds-diagnostics.c | 15 ++ clang/test/Analysis/taint-tester.c | 16 ++ 6 files changed, 421 insertions(+), 2 deletions(-) create mode 100644 clang/test/Analysis/builtin_overflow.c create mode 100644 clang/test/Analysis/builtin_overflow_notes.c diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 329ba37d6791c3..44d5f348ed2d54 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -628,6 +628,8 @@ Static Analyzer New features ^^^^^^^^^^^^ +- Now CSA models `__builtin_*_overflow` functions. (#GH102602) + - MallocChecker now checks for ``ownership_returns(class, idx)`` and ``ownership_takes(class, idx)`` attributes with class names different from "malloc". Clang static analyzer now reports an error if class of allocation and deallocation function mismatches. diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp index b198b1c2ff4d11..69d8e968283b37 100644 --- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp @@ -16,21 +16,93 @@ #include "clang/Basic/Builtins.h" #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/StaticAnalyzer/Checkers/Taint.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h" #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h" using namespace clang; using namespace ento; +using namespace taint; namespace { +QualType getSufficientTypeForOverflowOp(CheckerContext &C, const QualType &T) { + // Calling a builtin with a non-integer type result produces compiler error. + assert(T->isIntegerType()); + + ASTContext &ACtx = C.getASTContext(); + + unsigned BitWidth = ACtx.getIntWidth(T); + return ACtx.getIntTypeForBitwidth(BitWidth * 2, T->isSignedIntegerType()); +} + +QualType getOverflowBuiltinResultType(const CallEvent &Call) { + // Calling a builtin with an incorrect argument count produces compiler error. + assert(Call.getNumArgs() == 3); + + return Call.getArgExpr(2)->getType()->getPointeeType(); +} + +QualType getOverflowBuiltinResultType(const CallEvent &Call, CheckerContext &C, + unsigned BI) { + // Calling a builtin with an incorrect argument count produces compiler error. + assert(Call.getNumArgs() == 3); + + ASTContext &ACtx = C.getASTContext(); + + switch (BI) { + case Builtin::BI__builtin_smul_overflow: + case Builtin::BI__builtin_ssub_overflow: + case Builtin::BI__builtin_sadd_overflow: + return ACtx.IntTy; + case Builtin::BI__builtin_smull_overflow: + case Builtin::BI__builtin_ssubl_overflow: + case Builtin::BI__builtin_saddl_overflow: + return ACtx.LongTy; + case Builtin::BI__builtin_smulll_overflow: + case Builtin::BI__builtin_ssubll_overflow: + case Builtin::BI__builtin_saddll_overflow: + return ACtx.LongLongTy; + case Builtin::BI__builtin_umul_overflow: + case Builtin::BI__builtin_usub_overflow: + case Builtin::BI__builtin_uadd_overflow: + return ACtx.UnsignedIntTy; + case Builtin::BI__builtin_umull_overflow: + case Builtin::BI__builtin_usubl_overflow: + case Builtin::BI__builtin_uaddl_overflow: + return ACtx.UnsignedLongTy; + case Builtin::BI__builtin_umulll_overflow: + case Builtin::BI__builtin_usubll_overflow: + case Builtin::BI__builtin_uaddll_overflow: + return ACtx.UnsignedLongLongTy; + case Builtin::BI__builtin_mul_overflow: + case Builtin::BI__builtin_sub_overflow: + case Builtin::BI__builtin_add_overflow: + return getOverflowBuiltinResultType(Call); + default: + assert(false && "Unknown overflow builtin"); + return ACtx.IntTy; + } +} + class BuiltinFunctionChecker : public Checker { public: bool evalCall(const CallEvent &Call, CheckerContext &C) const; + void handleOverflowBuiltin(const CallEvent &Call, CheckerContext &C, + BinaryOperator::Opcode Op, + QualType ResultType) const; + const NoteTag *createBuiltinNoOverflowNoteTag(CheckerContext &C, + bool BothFeasible, SVal Arg1, + SVal Arg2, SVal Result) const; + const NoteTag *createBuiltinOverflowNoteTag(CheckerContext &C) const; + std::pair checkOverflow(CheckerContext &C, SVal RetVal, + QualType Res) const; private: // From: clang/include/clang/Basic/Builtins.def @@ -50,6 +122,102 @@ class BuiltinFunctionChecker : public Checker { } // namespace +const NoteTag *BuiltinFunctionChecker::createBuiltinNoOverflowNoteTag( + CheckerContext &C, bool BothFeasible, SVal Arg1, SVal Arg2, + SVal Result) const { + return C.getNoteTag([Result, Arg1, Arg2, BothFeasible]( + PathSensitiveBugReport &BR, llvm::raw_ostream &OS) { + if (!BR.isInteresting(Result)) + return; + + // Propagate interestingness to input argumets if result is interesting. + BR.markInteresting(Arg1); + BR.markInteresting(Arg2); + + if (BothFeasible) + OS << "Assuming no overflow"; + }); +} + +const NoteTag * +BuiltinFunctionChecker::createBuiltinOverflowNoteTag(CheckerContext &C) const { + return C.getNoteTag([](PathSensitiveBugReport &BR, + llvm::raw_ostream &OS) { OS << "Assuming overflow"; }, + /*isPrunable=*/true); +} + +std::pair +BuiltinFunctionChecker::checkOverflow(CheckerContext &C, SVal RetVal, + QualType Res) const { + // Calling a builtin with a non-integer type result produces compiler error. + assert(Res->isIntegerType()); + + unsigned BitWidth = C.getASTContext().getIntWidth(Res); + bool IsUnsigned = Res->isUnsignedIntegerType(); + + auto MinValType = llvm::APSInt::getMinValue(BitWidth, IsUnsigned); + auto MaxValType = llvm::APSInt::getMaxValue(BitWidth, IsUnsigned); + nonloc::ConcreteInt MinVal{MinValType}; + nonloc::ConcreteInt MaxVal{MaxValType}; + + SValBuilder &SVB = C.getSValBuilder(); + ProgramStateRef State = C.getState(); + SVal IsLeMax = SVB.evalBinOp(State, BO_LE, RetVal, MaxVal, Res); + SVal IsGeMin = SVB.evalBinOp(State, BO_GE, RetVal, MinVal, Res); + + auto [MayNotOverflow, MayOverflow] = + State->assume(IsLeMax.castAs()); + auto [MayNotUnderflow, MayUnderflow] = + State->assume(IsGeMin.castAs()); + + return {MayOverflow || MayUnderflow, MayNotOverflow && MayNotUnderflow}; +} + +void BuiltinFunctionChecker::handleOverflowBuiltin(const CallEvent &Call, + CheckerContext &C, + BinaryOperator::Opcode Op, + QualType ResultType) const { + // Calling a builtin with an incorrect argument count produces compiler error. + assert(Call.getNumArgs() == 3); + + ProgramStateRef State = C.getState(); + SValBuilder &SVB = C.getSValBuilder(); + const Expr *CE = Call.getOriginExpr(); + + SVal Arg1 = Call.getArgSVal(0); + SVal Arg2 = Call.getArgSVal(1); + + SVal RetValMax = SVB.evalBinOp(State, Op, Arg1, Arg2, + getSufficientTypeForOverflowOp(C, ResultType)); + SVal RetVal = SVB.evalBinOp(State, Op, Arg1, Arg2, ResultType); + + auto [Overflow, NotOverflow] = checkOverflow(C, RetValMax, ResultType); + if (NotOverflow) { + ProgramStateRef StateNoOverflow = + State->BindExpr(CE, C.getLocationContext(), SVB.makeTruthVal(false)); + + if (auto L = Call.getArgSVal(2).getAs()) { + StateNoOverflow = + StateNoOverflow->bindLoc(*L, RetVal, C.getLocationContext()); + + // Propagate taint if any of the argumets were tainted + if (isTainted(State, Arg1) || isTainted(State, Arg2)) + StateNoOverflow = addTaint(StateNoOverflow, *L); + } + + C.addTransition( + StateNoOverflow, + createBuiltinNoOverflowNoteTag( + C, /*BothFeasible=*/NotOverflow && Overflow, Arg1, Arg2, RetVal)); + } + + if (Overflow) { + C.addTransition( + State->BindExpr(CE, C.getLocationContext(), SVB.makeTruthVal(true)), + createBuiltinOverflowNoteTag(C)); + } +} + bool BuiltinFunctionChecker::isBuiltinLikeFunction( const CallEvent &Call) const { const auto *FD = llvm::dyn_cast_or_null(Call.getDecl()); @@ -82,10 +250,41 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent &Call, return true; } - switch (FD->getBuiltinID()) { + unsigned BI = FD->getBuiltinID(); + + switch (BI) { default: return false; - + case Builtin::BI__builtin_mul_overflow: + case Builtin::BI__builtin_smul_overflow: + case Builtin::BI__builtin_smull_overflow: + case Builtin::BI__builtin_smulll_overflow: + case Builtin::BI__builtin_umul_overflow: + case Builtin::BI__builtin_umull_overflow: + case Builtin::BI__builtin_umulll_overflow: + handleOverflowBuiltin(Call, C, BO_Mul, + getOverflowBuiltinResultType(Call, C, BI)); + return true; + case Builtin::BI__builtin_sub_overflow: + case Builtin::BI__builtin_ssub_overflow: + case Builtin::BI__builtin_ssubl_overflow: + case Builtin::BI__builtin_ssubll_overflow: + case Builtin::BI__builtin_usub_overflow: + case Builtin::BI__builtin_usubl_overflow: + case Builtin::BI__builtin_usubll_overflow: + handleOverflowBuiltin(Call, C, BO_Sub, + getOverflowBuiltinResultType(Call, C, BI)); + return true; + case Builtin::BI__builtin_add_overflow: + case Builtin::BI__builtin_sadd_overflow: + case Builtin::BI__builtin_saddl_overflow: + case Builtin::BI__builtin_saddll_overflow: + case Builtin::BI__builtin_uadd_overflow: + case Builtin::BI__builtin_uaddl_overflow: + case Builtin::BI__builtin_uaddll_overflow: + handleOverflowBuiltin(Call, C, BO_Add, + getOverflowBuiltinResultType(Call, C, BI)); + return true; case Builtin::BI__builtin_assume: case Builtin::BI__assume: { assert (Call.getNumArgs() > 0); diff --git a/clang/test/Analysis/builtin_overflow.c b/clang/test/Analysis/builtin_overflow.c new file mode 100644 index 00000000000000..5c61795661d095 --- /dev/null +++ b/clang/test/Analysis/builtin_overflow.c @@ -0,0 +1,157 @@ +// RUN: %clang_analyze_cc1 -triple x86_64-unknown-unknown -verify %s \ +// RUN: -analyzer-checker=core,debug.ExprInspection + +#define __UINT_MAX__ (__INT_MAX__ * 2U + 1U) +#define __INT_MIN__ (-__INT_MAX__ - 1) + +void clang_analyzer_dump_int(int); +void clang_analyzer_dump_long(long); +void clang_analyzer_eval(int); +void clang_analyzer_warnIfReached(void); + +void test_add_nooverflow(void) +{ + int res; + + if (__builtin_add_overflow(10, 20, &res)) { + clang_analyzer_warnIfReached(); + return; + } + + clang_analyzer_dump_int(res); //expected-warning{{30 S32b}} +} + +void test_add_overflow(void) +{ + int res; + + if (__builtin_add_overflow(__INT_MAX__, 1, &res)) { + clang_analyzer_dump_int(res); //expected-warning{{1st function call argument is an uninitialized value}} + return; + } + + clang_analyzer_warnIfReached(); +} + +void test_add_underoverflow(void) +{ + int res; + + if (__builtin_add_overflow(__INT_MIN__, -1, &res)) { + clang_analyzer_dump_int(res); //expected-warning{{1st function call argument is an uninitialized value}} + return; + } + + clang_analyzer_warnIfReached(); +} + +void test_sub_underflow(void) +{ + int res; + + if (__builtin_sub_overflow(__INT_MIN__, 10, &res)) { + return; + } + + clang_analyzer_warnIfReached(); +} + +void test_sub_overflow(void) +{ + int res; + + if (__builtin_sub_overflow(__INT_MAX__, -1, &res)) { + return; + } + + clang_analyzer_warnIfReached(); +} + +void test_sub_nooverflow(void) +{ + int res; + + if (__builtin_sub_overflow(__INT_MAX__, 1, &res)) { + clang_analyzer_warnIfReached(); + return; + } + + clang_analyzer_dump_int(res); //expected-warning{{2147483646 S32b}} +} + +void test_mul_overflow(void) +{ + int res; + + if (__builtin_mul_overflow(__INT_MAX__, 2, &res)) { + return; + } + + clang_analyzer_warnIfReached(); +} + +void test_mul_underflow(void) +{ + int res; + + if (__builtin_mul_overflow(__INT_MIN__, -2, &res)) { + return; + } + + clang_analyzer_warnIfReached(); +} + +void test_mul_nooverflow(void) +{ + int res; + + if (__builtin_mul_overflow(10, -2, &res)) { + clang_analyzer_warnIfReached(); + return; + } + + clang_analyzer_dump_int(res); //expected-warning{{-20 S32b}} +} + +void test_nooverflow_diff_types(void) +{ + long res; + + // This is not an overflow, since result type is long. + if (__builtin_add_overflow(__INT_MAX__, 1, &res)) { + clang_analyzer_warnIfReached(); + return; + } + + clang_analyzer_dump_long(res); //expected-warning{{2147483648 S64b}} +} + +void test_uaddll_overflow_contraints(unsigned long a, unsigned long b) +{ + unsigned long long res; + + if (a != 10) + return; + if (b != 10) + return; + + if (__builtin_uaddll_overflow(a, b, &res)) { + clang_analyzer_warnIfReached(); + return; + } +} + +void test_uadd_overflow_contraints(unsigned a, unsigned b) +{ + unsigned res; + + if (a > 5) + return; + if (b != 10) + return; + + if (__builtin_uadd_overflow(a, b, &res)) { + clang_analyzer_warnIfReached(); + return; + } +} diff --git a/clang/test/Analysis/builtin_overflow_notes.c b/clang/test/Analysis/builtin_overflow_notes.c new file mode 100644 index 00000000000000..20f333a4a6cca5 --- /dev/null +++ b/clang/test/Analysis/builtin_overflow_notes.c @@ -0,0 +1,30 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-output text \ +// RUN: -verify %s + +void test_no_overflow_note(int a, int b) +{ + int res; + + if (__builtin_add_overflow(a, b, &res)) // expected-note {{Assuming no overflow}} + // expected-note@-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}} + } +} + +void test_overflow_note(int a, int b) +{ + int res; // expected-note{{'res' declared without an initial value}} + + if (__builtin_add_overflow(a, b, &res)) { // expected-note {{Assuming overflow}} + // expected-note@-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; + } +} diff --git a/clang/test/Analysis/out-of-bounds-diagnostics.c b/clang/test/Analysis/out-of-bounds-diagnostics.c index 8ecad7036c3314..65bc28f58276fd 100644 --- a/clang/test/Analysis/out-of-bounds-diagnostics.c +++ b/clang/test/Analysis/out-of-bounds-diagnostics.c @@ -278,6 +278,21 @@ int *mallocRegion(void) { return mem; } +int *custom_calloc(size_t a, size_t b) { + size_t res; + + return __builtin_mul_overflow(a, b, &res) ? 0 : malloc(res); +} + +int *mallocRegionOverflow(void) { + int *mem = (int*)custom_calloc(10, sizeof(int)); + + mem[20] = 10; + // expected-warning@-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)); diff --git a/clang/test/Analysis/taint-tester.c b/clang/test/Analysis/taint-tester.c index 479a96c92ececd..66d54eab19f39f 100644 --- a/clang/test/Analysis/taint-tester.c +++ b/clang/test/Analysis/taint-tester.c @@ -196,3 +196,19 @@ void noCrashTest(void) { __builtin___memcpy_chk(pointer2, pointer1, 0, 0); // no-crash } } + +void builtin_overflow_test(void) { + int input, input2, res; + + scanf("%d", &input); + scanf("%d", &input2); + + if (__builtin_add_overflow(input, 10, &res)) // expected-warning + {{tainted}} + return; + + if (__builtin_add_overflow(10, input, &res)) // expected-warning + {{tainted}} + return; + + if (__builtin_add_overflow(input2, input, &res)) // expected-warning + {{tainted}} + return; +}