[clang] [analyzer] Make pointer/null comparison commutative for addresses of fields of symbolic regions (PR #209875)

[Lucas Ly Ba via cfe-commits]

https://github.com/lucasly-ba updated https://github.com/llvm/llvm-project/pull/209875

>From d0bcc0c2097d90fc82cb05fb045acd900196052e Mon Sep 17 00:00:00 2001
From: Lucas Ly Ba <hi at lucaslyba.com>
Date: Wed, 15 Jul 2026 21:20:37 +0200
Subject: [PATCH] [analyzer] Make pointer/null comparison commutative for field
 addresses

evalBinOpLL's loc::ConcreteInt case called getAsLocSymbol() without
IncludeBaseRegions, so the address of a field of a symbolic region
(e.g. &r->s) wasn't treated as symbol-based. A swapped null check
`nullptr == &r->s` then fell through to the "address of a non-symbolic
region is non-null" special case, contradicting the constraint recorded
by `&r->s == nullptr` and producing a spurious core.NullDereference.

Pass IncludeBaseRegions=true so both orderings build the same constraint.

Fixes #206798
---
 .../StaticAnalyzer/Core/SimpleSValBuilder.cpp |  2 +-
 clang/test/Analysis/issue-206798.cpp          | 43 +++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/Analysis/issue-206798.cpp

diff --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index 53ceabc621c19..7d154cbc840ac 100644
--- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -861,7 +861,7 @@ SVal SimpleSValBuilder::evalBinOpLL(ProgramStateRef state,
 
     // If one of the operands is a symbol and the other is a constant,
     // build an expression for use by the constraint manager.
-    if (SymbolRef rSym = rhs.getAsLocSymbol()) {
+    if (SymbolRef rSym = rhs.getAsLocSymbol(true)) {
       if (op == BO_Cmp)
         return UnknownVal();
 
diff --git a/clang/test/Analysis/issue-206798.cpp b/clang/test/Analysis/issue-206798.cpp
new file mode 100644
index 0000000000000..b9827df0cf71c
--- /dev/null
+++ b/clang/test/Analysis/issue-206798.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+struct S {
+  int n;
+};
+struct T {
+  S s;
+};
+
+// &r->s is the address of a field of a symbolic region, so it is symbol-based.
+// A swapped null check (`nullptr == p`) must build the same constraint as the
+// usual `p == nullptr`; otherwise the analyzer contradicts itself and reports a
+// null dereference that cannot happen.
+int gh206798_swapped(T *r) {
+  S *p = &r->s;
+  if (!p) {
+  }
+  if (nullptr == p)
+    return 0;
+  return p->n; // no-warning: p is known non-null after the check
+}
+
+int gh206798_normal(T *r) {
+  S *p = &r->s;
+  if (!p) {
+  }
+  if (p == nullptr)
+    return 0;
+  return p->n; // no-warning: p is known non-null after the check
+}
+
+// A genuine null dereference must still be flagged in either order.
+int stillReportsRealBug_swapped(S *p) {
+  if (nullptr == p)
+    return p->n; // expected-warning{{Access to field 'n' results in a dereference of a null pointer (loaded from variable 'p')}}
+  return 0;
+}
+
+int stillReportsRealBug_normal(S *p) {
+  if (p == nullptr)
+    return p->n; // expected-warning{{Access to field 'n' results in a dereference of a null pointer (loaded from variable 'p')}}
+  return 0;
+}