[clang] [analyzer] Teach analzer about ms __analyzer_assume(bool) and friends (PR #80456)
https://github.com/steakhal updated
https://github.com/llvm/llvm-project/pull/80456
>From 3a11db7ce1e91daacb86e183e7137db7a6101c9b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Joly?=
Date: Tue, 9 Aug 2022 23:21:18 +0200
Subject: [PATCH] [analyzer] Model Microsoft "__assume" in the same way as
clang "__builtin_assume"
---
.../Checkers/BuiltinFunctionChecker.cpp| 3 ++-
clang/test/Analysis/builtin-functions.cpp | 18 ++
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
index 61521c259ca90..01e46fa8591c0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -44,7 +44,8 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent ,
default:
return false;
- case Builtin::BI__builtin_assume: {
+ case Builtin::BI__builtin_assume:
+ case Builtin::BI__assume: {
assert (Call.getNumArgs() > 0);
SVal Arg = Call.getArgSVal(0);
if (Arg.isUndef())
diff --git a/clang/test/Analysis/builtin-functions.cpp
b/clang/test/Analysis/builtin-functions.cpp
index 37e522049b174..8719193e405c4 100644
--- a/clang/test/Analysis/builtin-functions.cpp
+++ b/clang/test/Analysis/builtin-functions.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10
-analyzer-checker=core,debug.ExprInspection %s -std=c++11 -verify
+// RUN: %clang_analyze_cc1 -triple x86_64-pc-windows-msvc19.11.0
-fms-extensions -analyzer-checker=core,debug.ExprInspection %s -std=c++11
-verify
void clang_analyzer_eval(bool);
void clang_analyzer_warnIfReached();
@@ -65,6 +66,23 @@ void g(int i) {
}
}
+#ifdef _WIN32
+namespace ms {
+void f(int i) {
+ __assume(i < 10);
+ clang_analyzer_eval(i < 15); // expected-warning {{TRUE}}
+}
+
+void g(int i) {
+ if (i > 5) {
+__assume(i < 5);
+clang_analyzer_warnIfReached(); // Assumtion contradicts constraints.
+// We give up the analysis on this path.
+ }
+}
+} // namespace ms
+#endif
+
void test_constant_p(void *ptr) {
int i = 1;
const int j = 2;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Teach analzer about ms __analyzer_assume(bool) and friends (PR #80456)
https://github.com/NagyDonat approved this pull request. LGTM, thanks for the update. https://github.com/llvm/llvm-project/pull/80456 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Teach analzer about ms __analyzer_assume(bool) and friends (PR #80456)
steakhal wrote: > The code LGTM with some minor remarks. (Disclaimer: I'm not familiar with > these MS functions.) > > I'm not sure whether these "builtin by Microsoft" functions are in scope for > "our" BuiltinFunctionChecker which previously only checked functions that are > recognized as `Builtin::BI__something` by Clang. (However, I can believe that > there is no better place for them and I don't think that they would cause > problems here.) For me, this file would be where I would look to see how `__assume` is eval called. This is actually an unknown function in regular mode, but with `-fms-extensions` it's actually defined. Check the AST for this [example](https://godbolt.org/z/1xdbP8scx). So, techniquely, it's not a builtin, but feels like it. About the remarks, yes, let's do [Clean As You Code](https://docs.sonarsource.com/sonarqube/latest/user-guide/clean-as-you-code/#what-is-clean-as-you-code)! I'm all in. > Hey! > > Thanks for looking into this! > > Did you actually encounter this call in the wild? The reason I ask, because > their definition looks like this in the current version of `sal.h`: > > ``` > #ifndef __analysis_assume // [ > #ifdef _PREFAST_ // [ > #define __analysis_assume(expr) __assume(expr) > #else // ][ > #define __analysis_assume(expr) > #endif // ] > #endif // ] > > #ifndef _Analysis_assume_ // [ > #ifdef _PREFAST_ // [ > #define _Analysis_assume_(expr) __assume(expr) > #else // ][ > #define _Analysis_assume_(expr) > #endif // ] > #endif // ] > ``` > > The basic idea is, when MSVC's analyzer is invoked, `_PREFAST_` is defined, > and these macros should expand to `__assume(expr)`. This makes me a bit > surprised if the original names are present in the preprocessed code. > > There is no difference between `__analysis_assume` and `_Analysis_assume_`. > The former is following the naming conventions in SAL 1, the latter is > following the conventions of SAL 2. The latter is preferred in user code, but > MSVC still supports both spellings. I think I've seen FPs on ChakraCore and on the DotnetRuntime and on other Windows-related projects defining some of their assert macros by wrapping `__analysis_assume`. I can't exactly recall ATM where exactly I've seen that, but I'll do a measurment to confirm that this PR actually improves the situation. Anyways, it turns out the FP I wanted to fix actually expands into using `__builtin_trap()`, which is still not modeled in CSA :face_with_spiral_eyes: I'll create a separate PR for handling that. I think this PR stands on it's own, and improves the situation - (I'll check and come back of course). I hope `__assume()` takes a single parameter (of any type) and returns `void` though. Sorry for not doing my homework in the beginning, and thanks for catching that! I though it's gonna be an easy one and I fire off a PR on my tea-break. https://github.com/llvm/llvm-project/pull/80456 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Teach analzer about ms __analyzer_assume(bool) and friends (PR #80456)
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
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 {
public:
bool evalCall(const CallEvent , CheckerContext ) const;
+
+private:
+ const CallDescriptionSet MicrosoftAnalysisAssume{
+ {{"__analysis_assume"}, 1},
+ {{"_Analysis_assume_"}, 1},
+ };
+
+ void evalCallAssume(const CallEvent , CheckerContext ) const;
};
}
+void BuiltinFunctionChecker::evalCallAssume(const CallEvent ,
+CheckerContext ) 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(), 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 ,
CheckerContext ) const {
ProgramStateRef state = C.getState();
@@ -39,29 +71,20 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent ,
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(), 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] Teach analzer about ms __analyzer_assume(bool) and friends (PR #80456)
Xazax-hun wrote: Hey! Thanks for looking into this! Did you actually encounter this call in the wild? The reason I ask, because their definition looks like this in the current version of `sal.h`: ``` #ifndef __analysis_assume // [ #ifdef _PREFAST_ // [ #define __analysis_assume(expr) __assume(expr) #else // ][ #define __analysis_assume(expr) #endif // ] #endif // ] #ifndef _Analysis_assume_ // [ #ifdef _PREFAST_ // [ #define _Analysis_assume_(expr) __assume(expr) #else // ][ #define _Analysis_assume_(expr) #endif // ] #endif // ] ``` The basic idea is, when MSVC's analyzer is invoked, `_PREFAST_` is defined, and these macros should expand to `__assume(expr)`. This makes me a bit surprised if the original names are present in the preprocessed code. There is no difference between `__analysis_assume` and `_Analysis_assume_`. The former is following the naming conventions in SAL 1, the latter is following the conventions of SAL 2. The latter is preferred in user code, but MSVC's toolchain still supports both spellings. https://github.com/llvm/llvm-project/pull/80456 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Teach analzer about ms __analyzer_assume(bool) and friends (PR #80456)
https://github.com/NagyDonat edited https://github.com/llvm/llvm-project/pull/80456 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Teach analzer about ms __analyzer_assume(bool) and friends (PR #80456)
@@ -26,10 +27,41 @@ namespace {
class BuiltinFunctionChecker : public Checker {
public:
bool evalCall(const CallEvent , CheckerContext ) const;
+
+private:
+ const CallDescriptionSet MicrosoftAnalysisAssume{
+ {{"__analysis_assume"}, 1},
+ {{"_Analysis_assume_"}, 1},
+ };
+
+ void evalCallAssume(const CallEvent , CheckerContext ) const;
};
}
+void BuiltinFunctionChecker::evalCallAssume(const CallEvent ,
+CheckerContext ) 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(), true);
+
+ // FIXME: do we want to warn here? Not right now. The most reports might
+ // come from infeasible paths, thus being false positives.
NagyDonat wrote:
I'm pretty sure that we don't want to warn here. This analyzer-specific
hint/override is presumably used in code that's confusing for the analyzer, so
let's trust it.
https://github.com/llvm/llvm-project/pull/80456
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Teach analzer about ms __analyzer_assume(bool) and friends (PR #80456)
@@ -26,10 +27,41 @@ namespace {
class BuiltinFunctionChecker : public Checker {
public:
bool evalCall(const CallEvent , CheckerContext ) const;
+
+private:
+ const CallDescriptionSet MicrosoftAnalysisAssume{
+ {{"__analysis_assume"}, 1},
+ {{"_Analysis_assume_"}, 1},
+ };
+
+ void evalCallAssume(const CallEvent , CheckerContext ) const;
};
}
+void BuiltinFunctionChecker::evalCallAssume(const CallEvent ,
+CheckerContext ) 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(), 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;
NagyDonat wrote:
```suggestion
```
This is the last line of a `void` function.
https://github.com/llvm/llvm-project/pull/80456
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Teach analzer about ms __analyzer_assume(bool) and friends (PR #80456)
@@ -26,10 +27,41 @@ namespace {
class BuiltinFunctionChecker : public Checker {
public:
bool evalCall(const CallEvent , CheckerContext ) const;
+
+private:
+ const CallDescriptionSet MicrosoftAnalysisAssume{
+ {{"__analysis_assume"}, 1},
+ {{"_Analysis_assume_"}, 1},
+ };
+
+ void evalCallAssume(const CallEvent , CheckerContext ) const;
};
}
+void BuiltinFunctionChecker::evalCallAssume(const CallEvent ,
+CheckerContext ) const {
+ assert(Call.getNumArgs() > 0);
+ assert(Call.getResultType()->isVoidType());
+ SVal Arg = Call.getArgSVal(0);
+
+ if (Arg.isUndef())
+return; // Return true to model purity.
NagyDonat wrote:
`Return true` is inaccurate/confusing after a valueless `return;`
https://github.com/llvm/llvm-project/pull/80456
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Teach analzer about ms __analyzer_assume(bool) and friends (PR #80456)
https://github.com/NagyDonat edited https://github.com/llvm/llvm-project/pull/80456 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Teach analzer about ms __analyzer_assume(bool) and friends (PR #80456)
https://github.com/NagyDonat commented: The code LGTM with some minor remarks, but I'm not familiar with these MS functions. I'm not sure whether these "builtin by Microsoft" functions are in scope for "our" BuiltinFunctionChecker which previously only checked functions that are recognized as `Builtin::BI__something` by Clang. (However, I can believe that there is no better place for them and I don't think that they would cause problems here.) https://github.com/llvm/llvm-project/pull/80456 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Teach analzer about ms __analyzer_assume(bool) and friends (PR #80456)
steakhal wrote: Let me know if this is correct @Xazax-hun. You probably have more insights on these APIs than me ;) https://github.com/llvm/llvm-project/pull/80456 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Teach analzer about ms __analyzer_assume(bool) and friends (PR #80456)
llvmbot wrote:
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Balazs Benics (steakhal)
Changes
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/80456.diff
2 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
(+40-17)
- (modified) clang/test/Analysis/builtin-functions.cpp (+22)
``diff
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 {
public:
bool evalCall(const CallEvent , CheckerContext ) const;
+
+private:
+ const CallDescriptionSet MicrosoftAnalysisAssume{
+ {{"__analysis_assume"}, 1},
+ {{"_Analysis_assume_"}, 1},
+ };
+
+ void evalCallAssume(const CallEvent , CheckerContext ) const;
};
}
+void BuiltinFunctionChecker::evalCallAssume(const CallEvent ,
+CheckerContext ) 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(), 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 ,
CheckerContext ) const {
ProgramStateRef state = C.getState();
@@ -39,29 +71,20 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent ,
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(), 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] Teach analzer about ms __analyzer_assume(bool) and friends (PR #80456)
https://github.com/steakhal created
https://github.com/llvm/llvm-project/pull/80456
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.
>From 9065aec18b5b9c4d922b0650e709e71ed31b5a45 Mon Sep 17 00:00:00 2001
From: Balazs Benics
Date: Fri, 2 Feb 2024 16:24:21 +0100
Subject: [PATCH] [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 {
public:
bool evalCall(const CallEvent , CheckerContext ) const;
+
+private:
+ const CallDescriptionSet MicrosoftAnalysisAssume{
+ {{"__analysis_assume"}, 1},
+ {{"_Analysis_assume_"}, 1},
+ };
+
+ void evalCallAssume(const CallEvent , CheckerContext ) const;
};
}
+void BuiltinFunctionChecker::evalCallAssume(const CallEvent ,
+CheckerContext ) 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(), 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 ,
CheckerContext ) const {
ProgramStateRef state = C.getState();
@@ -39,29 +71,20 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent ,
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(), 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}}
