[clang] 5e95571 - [analyzer] Do not reason about locations passed as inline asm input (#103714)

[via cfe-commits]

Author: Pavel Skripkin
Date: 2024-08-15T11:42:27+02:00
New Revision: 5e95571a90b1ec193b735b7312c5c2559d7ee5ea

URL: https://github.com/llvm/llvm-project/commit/5e95571a90b1ec193b735b7312c5c2559d7ee5ea
DIFF: https://github.com/llvm/llvm-project/commit/5e95571a90b1ec193b735b7312c5c2559d7ee5ea.diff

LOG: [analyzer] Do not reason about locations passed as inline asm input (#103714)

If pointer is passed as input operand for inline assembly, it's possible
that asm block will change memory behind this pointer. So if pointer is
passed inside inline asm block, it's better to not guess and assume
memory has unknown state.

Without such change, we observed a lot of FP with hand-written `memcpy`
and friends.

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
    clang/test/Analysis/asm.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 686310d38ebd58..493914f88ba9a5 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3807,6 +3807,14 @@ void ExprEngine::VisitGCCAsmStmt(const GCCAsmStmt *A, ExplodedNode *Pred,
       state = state->bindLoc(*LV, UnknownVal(), Pred->getLocationContext());
   }
 
+  // Do not reason about locations passed inside inline assembly.
+  for (const Expr *I : A->inputs()) {
+    SVal X = state->getSVal(I, Pred->getLocationContext());
+
+    if (std::optional<Loc> LV = X.getAs<Loc>())
+      state = state->bindLoc(*LV, UnknownVal(), Pred->getLocationContext());
+  }
+
   Bldr.generateNode(A, Pred, state);
 }
 

diff  --git a/clang/test/Analysis/asm.cpp b/clang/test/Analysis/asm.cpp
index 1180063502168f..3181aea870c8aa 100644
--- a/clang/test/Analysis/asm.cpp
+++ b/clang/test/Analysis/asm.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker debug.ExprInspection -fheinous-gnu-extensions -w %s -verify
+// RUN: %clang_analyze_cc1 -triple=x86_64-unknown-unknown \
+// RUN:      -analyzer-checker debug.ExprInspection,core -fheinous-gnu-extensions -w %s -verify
 
 int clang_analyzer_eval(int);
 
@@ -10,3 +11,32 @@ void testRValueOutput() {
   clang_analyzer_eval(global == 1); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(ref == 1);    // expected-warning{{UNKNOWN}}
 }
+
+void *MyMemcpy(void *d, const void *s, const int n) {
+  asm volatile (
+    "cld\n rep movsb\n"
+    :: "S" (s), "D" (d), "c" (n) : "memory"
+  );
+  return d;
+}
+
+void testInlineAsmMemcpy(void)
+{
+    int a, b = 10, c;
+    MyMemcpy(&a, &b, sizeof(b));
+    c = a; // no-warning
+}
+
+void testInlineAsmMemcpyArray(void)
+{
+    int a[10], b[10] = {}, c;
+    MyMemcpy(&a, &b, sizeof(b));
+    c = a[8]; // no-warning
+}
+
+void testInlineAsmMemcpyUninit(void)
+{
+    int a[10], b[10] = {}, c;
+    MyMemcpy(&a[1], &b[1], sizeof(b) - sizeof(b[1]));
+    c = a[0]; // expected-warning{{Assigned value is garbage or undefined}}
+}