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 <[email protected]> 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; +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
