https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/80456
>From 9065aec18b5b9c4d922b0650e709e71ed31b5a45 Mon Sep 17 00:00:00 2001 From: Balazs Benics <benicsbal...@gmail.com> Date: Fri, 2 Feb 2024 16:24:21 +0100 Subject: [PATCH 1/2] [analyzer] Teach analzer about ms __analyzer_assume(bool) and friends See the MS docs: https://learn.microsoft.com/en-us/windows-hardware/drivers/devtest/using-the--analysis-assume-function-to-suppress-false-defects https://learn.microsoft.com/en-us/cpp/code-quality/how-to-specify-additional-code-information-by-using-analysis-assume TBH, I don't really know what is the difference between the two APIs. --- .../Checkers/BuiltinFunctionChecker.cpp | 57 +++++++++++++------ clang/test/Analysis/builtin-functions.cpp | 22 +++++++ 2 files changed, 62 insertions(+), 17 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp index 61521c259ca90..ea874c1529b3b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp @@ -14,6 +14,7 @@ #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.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/DynamicExtent.h" @@ -26,10 +27,41 @@ namespace { class BuiltinFunctionChecker : public Checker<eval::Call> { public: bool evalCall(const CallEvent &Call, CheckerContext &C) const; + +private: + const CallDescriptionSet MicrosoftAnalysisAssume{ + {{"__analysis_assume"}, 1}, + {{"_Analysis_assume_"}, 1}, + }; + + void evalCallAssume(const CallEvent &Call, CheckerContext &C) const; }; } +void BuiltinFunctionChecker::evalCallAssume(const CallEvent &Call, + CheckerContext &C) const { + assert(Call.getNumArgs() > 0); + assert(Call.getResultType()->isVoidType()); + SVal Arg = Call.getArgSVal(0); + + if (Arg.isUndef()) + return; // Return true to model purity. + + ProgramStateRef State = C.getState(); + State = State->assume(Arg.castAs<DefinedOrUnknownSVal>(), true); + + // FIXME: do we want to warn here? Not right now. The most reports might + // come from infeasible paths, thus being false positives. + if (!State) { + C.generateSink(C.getState(), C.getPredecessor()); + return; + } + + C.addTransition(State); + return; +} + bool BuiltinFunctionChecker::evalCall(const CallEvent &Call, CheckerContext &C) const { ProgramStateRef state = C.getState(); @@ -39,29 +71,20 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent &Call, const LocationContext *LCtx = C.getLocationContext(); const Expr *CE = Call.getOriginExpr(); + bool ReturnsVoid = Call.getResultType()->isVoidType(); + + if (MicrosoftAnalysisAssume.contains(Call) && ReturnsVoid) { + evalCallAssume(Call, C); + return true; + } switch (FD->getBuiltinID()) { default: return false; - case Builtin::BI__builtin_assume: { - assert (Call.getNumArgs() > 0); - SVal Arg = Call.getArgSVal(0); - if (Arg.isUndef()) - return true; // Return true to model purity. - - state = state->assume(Arg.castAs<DefinedOrUnknownSVal>(), true); - // FIXME: do we want to warn here? Not right now. The most reports might - // come from infeasible paths, thus being false positives. - if (!state) { - C.generateSink(C.getState(), C.getPredecessor()); - return true; - } - - C.addTransition(state); + case Builtin::BI__builtin_assume: + evalCallAssume(Call, C); return true; - } - case Builtin::BI__builtin_unpredictable: case Builtin::BI__builtin_expect: case Builtin::BI__builtin_expect_with_probability: diff --git a/clang/test/Analysis/builtin-functions.cpp b/clang/test/Analysis/builtin-functions.cpp index 37e522049b174..4a26f82ffcb16 100644 --- a/clang/test/Analysis/builtin-functions.cpp +++ b/clang/test/Analysis/builtin-functions.cpp @@ -1,5 +1,7 @@ // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,debug.ExprInspection %s -std=c++11 -verify +void __analysis_assume(bool); +void _Analysis_assume_(bool); void clang_analyzer_eval(bool); void clang_analyzer_warnIfReached(); @@ -82,3 +84,23 @@ void test_constant_p(void *ptr) { clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}} clang_analyzer_eval(__builtin_constant_p(ptr == 0)); // expected-warning {{FALSE}} } + +void test_ms_analysis_assume(int *p) { + __analysis_assume(p); + if (!p) { + clang_analyzer_warnIfReached(); // no-warning: dead code. + } + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + __analysis_assume(false); + clang_analyzer_warnIfReached(); // no-warning: dead code. +} + +void test_ms_Analysis_assume_(int *p) { + _Analysis_assume_(p); + if (!p) { + clang_analyzer_warnIfReached(); // no-warning: dead code. + } + clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} + _Analysis_assume_(false); + clang_analyzer_warnIfReached(); // no-warning: dead code. +} >From a48cd8238bae4f6e78126de59e123c1e8312ed4d Mon Sep 17 00:00:00 2001 From: Balazs Benics <benicsbal...@gmail.com> Date: Sat, 3 Feb 2024 09:03:55 +0100 Subject: [PATCH 2/2] Address review comments --- .../Checkers/BuiltinFunctionChecker.cpp | 30 +++++++------------ clang/test/Analysis/builtin-functions.cpp | 17 ++--------- 2 files changed, 13 insertions(+), 34 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp index ea874c1529b3b..c240199e04c2b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp @@ -29,10 +29,7 @@ class BuiltinFunctionChecker : public Checker<eval::Call> { bool evalCall(const CallEvent &Call, CheckerContext &C) const; private: - const CallDescriptionSet MicrosoftAnalysisAssume{ - {{"__analysis_assume"}, 1}, - {{"_Analysis_assume_"}, 1}, - }; + const CallDescription MicrosoftAnalysisAssume{{"__assume"}, 1}; void evalCallAssume(const CallEvent &Call, CheckerContext &C) const; }; @@ -43,23 +40,16 @@ void BuiltinFunctionChecker::evalCallAssume(const CallEvent &Call, CheckerContext &C) const { assert(Call.getNumArgs() > 0); assert(Call.getResultType()->isVoidType()); - SVal Arg = Call.getArgSVal(0); - - if (Arg.isUndef()) - return; // Return true to model purity. - - ProgramStateRef State = C.getState(); - State = State->assume(Arg.castAs<DefinedOrUnknownSVal>(), true); - - // FIXME: do we want to warn here? Not right now. The most reports might - // come from infeasible paths, thus being false positives. - if (!State) { - C.generateSink(C.getState(), C.getPredecessor()); + auto Arg = Call.getArgSVal(0).getAs<DefinedOrUnknownSVal>(); + if (!Arg) return; - } - C.addTransition(State); - return; + ProgramStateRef OriginalState = C.getState(); + if (ProgramStateRef State = OriginalState->assume(*Arg, true)) { + C.addTransition(State); + } else { + C.generateSink(OriginalState, C.getPredecessor()); + } } bool BuiltinFunctionChecker::evalCall(const CallEvent &Call, @@ -73,7 +63,7 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent &Call, const Expr *CE = Call.getOriginExpr(); bool ReturnsVoid = Call.getResultType()->isVoidType(); - if (MicrosoftAnalysisAssume.contains(Call) && ReturnsVoid) { + if (MicrosoftAnalysisAssume.matches(Call) && ReturnsVoid) { evalCallAssume(Call, C); return true; } diff --git a/clang/test/Analysis/builtin-functions.cpp b/clang/test/Analysis/builtin-functions.cpp index 4a26f82ffcb16..cc67f350be4b8 100644 --- a/clang/test/Analysis/builtin-functions.cpp +++ b/clang/test/Analysis/builtin-functions.cpp @@ -1,7 +1,6 @@ // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,debug.ExprInspection %s -std=c++11 -verify -void __analysis_assume(bool); -void _Analysis_assume_(bool); +void __assume(bool); void clang_analyzer_eval(bool); void clang_analyzer_warnIfReached(); @@ -86,21 +85,11 @@ void test_constant_p(void *ptr) { } void test_ms_analysis_assume(int *p) { - __analysis_assume(p); + __assume(p); if (!p) { clang_analyzer_warnIfReached(); // no-warning: dead code. } clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} - __analysis_assume(false); - clang_analyzer_warnIfReached(); // no-warning: dead code. -} - -void test_ms_Analysis_assume_(int *p) { - _Analysis_assume_(p); - if (!p) { - clang_analyzer_warnIfReached(); // no-warning: dead code. - } - clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} - _Analysis_assume_(false); + __assume(false); clang_analyzer_warnIfReached(); // no-warning: dead code. } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits