[clang] [analyzer] Teach analzer about ms __analyzer_assume(bool) and friends (PR #80456)

Balazs Benics via cfe-commits cfe-commits at lists.llvm.org
Sat Feb 3 00:04:12 PST 2024


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 <benicsbalazs at 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 <benicsbalazs at 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.
 }



More information about the cfe-commits mailing list