[clang] [analyzer] Preserve LocAsInteger through identity operations (PR #225048)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Peiqi Li (voyager-jhk)

<details>
<summary>Changes</summary>

Preserve LocAsInteger values through identity operations on integers converted from pointers.

This fixes a false positive leak report from unix.Malloc.

Fixes #<!-- -->220972

---
Full diff: https://github.com/llvm/llvm-project/pull/225048.diff


3 Files Affected:

- (modified) clang/docs/ReleaseNotes.md (+3) 
- (modified) clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp (+24) 
- (modified) clang/test/Analysis/malloc.c (+47) 


``````````diff
diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md
index db710b64a6ae5..f0f1b7f65c35f 100644
--- a/clang/docs/ReleaseNotes.md
+++ b/clang/docs/ReleaseNotes.md
@@ -871,6 +871,9 @@ features cannot lower the translation-unit ABI level;
 
 #### Crash and bug fixes
 
+- Fixed a false positive `unix.Malloc` leak report for identity operations on
+  integers converted from pointers.
+
 % comment:
 % This is for the Static Analyzer.
 % Use `####` headings for subsections:
diff --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index 7d154cbc840ac..c742de27e7aaa 100644
--- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -230,6 +230,25 @@ SVal SimpleSValBuilder::MakeSymIntVal(const SymExpr *LHS,
   return makeNonLoc(LHS, op, *ConvertedRHS, resultTy);
 }
 
+// Returns whether X op RHS is equivalent to X.
+static bool isIdentityOperation(BinaryOperator::Opcode op,
+                                const llvm::APSInt &RHS) {
+  switch (op) {
+  case BO_Add:
+  case BO_Sub:
+  case BO_Or:
+  case BO_Xor:
+  case BO_Shl:
+  case BO_Shr:
+    return RHS == 0;
+  case BO_Mul:
+  case BO_Div:
+    return RHS == 1;
+  default:
+    return false;
+  }
+}
+
 // See if Sym is known to be a relation Rel with Bound.
 static bool isInRelation(BinaryOperator::Opcode Rel, SymbolRef Sym,
                          llvm::APSInt Bound, ProgramStateRef State) {
@@ -491,6 +510,11 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state,
                            rhs.castAs<nonloc::LocAsInteger>().getLoc(),
                            resultTy);
       case nonloc::ConcreteIntKind: {
+        // Preserve the location for identity operations.
+        if (isIdentityOperation(op,
+                                rhs.castAs<nonloc::ConcreteInt>().getValue()))
+          return evalCast(lhs, resultTy, QualType{});
+
         // FIXME: at the moment the implementation
         // of modeling "pointers as integers" is not complete.
         if (!BinaryOperator::isComparisonOp(op))
diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index 6c3dadfd16021..791946164b6d1 100644
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -1327,6 +1327,53 @@ void test_double_assign_ints_positive(void)
   void *ptr = malloc(16);
   (void*)(intptr_t)(uintptr_t)ptr; // expected-warning {{unused}}
 } // expected-warning {{leak}}
+
+// Regression test for GH#220972.
+void test_identity_op_on_int_cast_pointer(void)
+{
+  void *ptr = malloc(16);
+  if (ptr == 0)
+    return;
+
+  uintptr_t value = (uintptr_t)ptr;
+  value += 0;
+  clang_analyzer_eval(value == (uintptr_t)ptr); // expected-warning{{TRUE}}
+
+  free((void *)value); // no-warning
+}
+
+void test_identity_ops_on_int_cast_pointer(void)
+{
+  void *ptr = malloc(16);
+  if (ptr == 0)
+    return;
+
+  uintptr_t value = (uintptr_t)ptr;
+  value -= 0;
+  value |= 0;
+  value ^= 0;
+  value <<= 0;
+  value >>= 0;
+  value *= 1;
+  value /= 1;
+  clang_analyzer_eval(value == (uintptr_t)ptr); // expected-warning{{TRUE}}
+
+  free((void *)value); // no-warning
+}
+
+// Test identity operations with a narrower result type.
+void test_identity_op_with_narrower_result(void)
+{
+  void *ptr = malloc(16);
+  if (ptr == 0)
+    return;
+
+  unsigned value = (unsigned)(uintptr_t)ptr;
+  value += 0;
+  clang_analyzer_eval(value == (unsigned)(uintptr_t)ptr); // expected-warning{{TRUE}}
+
+  free(ptr); // no-warning
+}
 #endif
 
 void testCGContextNoLeak(void)

``````````

</details>


https://github.com/llvm/llvm-project/pull/225048