[PATCH] D126281: [analyzer] Fix symbol simplification assertion failure

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 431635.
martong marked an inline comment as done.
martong added a comment.

- Add clang_analyzer_warnIfReached() to the test and remove unused debug function decls


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126281/new/

https://reviews.llvm.org/D126281

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/symbol-simplification-assertion.c


Index: clang/test/Analysis/symbol-simplification-assertion.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/symbol-simplification-assertion.c
@@ -0,0 +1,25 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=true \
+// RUN:   -verify
+
+// Here we test that no assertion is fired during symbol simplification.
+// Related issue: https://github.com/llvm/llvm-project/issues/55546
+
+extern void abort() __attribute__((__noreturn__));
+#define assert(expr) ((expr) ? (void)(0) : abort())
+
+void clang_analyzer_warnIfReached();
+
+int a, b, c;
+long L, L1;
+void test() {
+  assert(a + L + 1 + b != c);
+  assert(L == a);
+  assert(c == 0);
+  L1 = 0;
+  assert(a + L1 + 1 + b != c);
+  assert(a == 0); // no-assertion
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -2508,12 +2508,6 @@
   SymbolSet ClsMembers = getClassMembers(State);
   assert(ClsMembers.contains(Old));
 
-  // We don't remove `Old`'s Sym->Class relation for two reasons:
-  // 1) This way constraints for the old symbol can still be found via it's
-  // equivalence class that it used to be the member of.
-  // 2) Performance and resource reasons. We can spare one removal and thus one
-  // additional tree in the forest of `ClassMap`.
-
   // Remove `Old`'s Class->Sym relation.
   SymbolSet::Factory &F = getMembersFactory(State);
   ClassMembersTy::Factory &EMFactory = State->get_context<ClassMembers>();
@@ -2527,6 +2521,12 @@
   ClassMembersMap = EMFactory.add(ClassMembersMap, *this, ClsMembers);
   State = State->set<ClassMembers>(ClassMembersMap);
 
+  // Remove `Old`'s Sym->Class relation.
+  ClassMapTy Classes = State->get<ClassMap>();
+  ClassMapTy::Factory &CMF = State->get_context<ClassMap>();
+  Classes = CMF.remove(Classes, Old);
+  State = State->set<ClassMap>(Classes);
+
   return State;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D126281.431635.patch
Type: text/x-patch
Size: 2243 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220524/f5f012f0/attachment.bin>