Author: Vince Bridgers Date: 2021-10-06T05:18:27-05:00 New Revision: b29186c08ae230d0decbca67565be68919c6b24d
URL: https://github.com/llvm/llvm-project/commit/b29186c08ae230d0decbca67565be68919c6b24d DIFF: https://github.com/llvm/llvm-project/commit/b29186c08ae230d0decbca67565be68919c6b24d.diff LOG: [analyzer] canonicalize special case of structure/pointer deref This simple change addresses a special case of structure/pointer aliasing that produced different symbolvals, leading to false positives during analysis. The reproducer is as simple as this. ```lang=C++ struct s { int v; }; void foo(struct s *ps) { struct s ss = *ps; clang_analyzer_dump(ss.v); // reg_$1<int Element{SymRegion{reg_$0<struct s *ps>},0 S64b,struct s}.v> clang_analyzer_dump(ps->v); //reg_$3<int SymRegion{reg_$0<struct s *ps>}.v> clang_analyzer_eval(ss.v == ps->v); // UNKNOWN } ``` Acks: Many thanks to @steakhal and @martong for the group debug session. Reviewed By: steakhal, martong Differential Revision: https://reviews.llvm.org/D110625 Added: Modified: clang/lib/StaticAnalyzer/Core/Store.cpp clang/test/Analysis/ptr-arith.c Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Core/Store.cpp b/clang/lib/StaticAnalyzer/Core/Store.cpp index b867b0746f90f..f7f0b6c71abf5 100644 --- a/clang/lib/StaticAnalyzer/Core/Store.cpp +++ b/clang/lib/StaticAnalyzer/Core/Store.cpp @@ -442,6 +442,19 @@ SVal StoreManager::getLValueIvar(const ObjCIvarDecl *decl, SVal base) { SVal StoreManager::getLValueElement(QualType elementType, NonLoc Offset, SVal Base) { + + // Special case, if index is 0, return the same type as if + // this was not an array dereference. + if (Offset.isZeroConstant()) { + QualType BT = Base.getType(this->Ctx); + if (!BT.isNull() && !elementType.isNull()) { + QualType PointeeTy = BT->getPointeeType(); + if (!PointeeTy.isNull() && + PointeeTy.getCanonicalType() == elementType.getCanonicalType()) + return Base; + } + } + // If the base is an unknown or undefined value, just return it back. // FIXME: For absolute pointer addresses, we just return that value back as // well, although in reality we should return the offset added to that diff --git a/clang/test/Analysis/ptr-arith.c b/clang/test/Analysis/ptr-arith.c index c0798f94504ed..54347d3bd3748 100644 --- a/clang/test/Analysis/ptr-arith.c +++ b/clang/test/Analysis/ptr-arith.c @@ -2,6 +2,7 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.FixedAddr,alpha.core.PointerArithm,alpha.core.PointerSub,debug.ExprInspection -analyzer-store=region -Wno-pointer-to-int-cast -verify -triple i686-apple-darwin9 -Wno-tautological-pointer-compare -analyzer-config eagerly-assume=false %s void clang_analyzer_eval(int); +void clang_analyzer_dump(int); void f1() { int a[10]; @@ -330,3 +331,59 @@ float test_nowarning_on_vector_deref() { simd_float2 x = {0, 1}; return x[1]; // no-warning } + +struct s { + int v; +}; + +// These three expressions should produce the same sym vals. +void struct_pointer_canon(struct s *ps) { + struct s ss = *ps; + clang_analyzer_dump((*ps).v); + // expected-warning-re@-1{{reg_${{[[:digit:]]+}}<int SymRegion{reg_${{[[:digit:]]+}}<struct s * ps>}.v>}} + clang_analyzer_dump(ps[0].v); + // expected-warning-re@-1{{reg_${{[[:digit:]]+}}<int SymRegion{reg_${{[[:digit:]]+}}<struct s * ps>}.v>}} + clang_analyzer_dump(ps->v); + // expected-warning-re@-1{{reg_${{[[:digit:]]+}}<int SymRegion{reg_${{[[:digit:]]+}}<struct s * ps>}.v>}} + clang_analyzer_eval((*ps).v == ps[0].v); // expected-warning{{TRUE}} + clang_analyzer_eval((*ps).v == ps->v); // expected-warning{{TRUE}} + clang_analyzer_eval(ps[0].v == ps->v); // expected-warning{{TRUE}} +} + +void struct_pointer_canon_bidim(struct s **ps) { + struct s ss = **ps; + clang_analyzer_eval(&(ps[0][0].v) == &((*ps)->v)); // expected-warning{{TRUE}} +} + +typedef struct s T1; +typedef struct s T2; +void struct_pointer_canon_typedef(T1 *ps) { + T2 ss = *ps; + clang_analyzer_dump((*ps).v); + // expected-warning-re@-1{{reg_${{[[:digit:]]+}}<int SymRegion{reg_${{[[:digit:]]+}}<T1 * ps>}.v>}} + clang_analyzer_dump(ps[0].v); + // expected-warning-re@-1{{reg_${{[[:digit:]]+}}<int SymRegion{reg_${{[[:digit:]]+}}<T1 * ps>}.v>}} + clang_analyzer_dump(ps->v); + // expected-warning-re@-1{{reg_${{[[:digit:]]+}}<int SymRegion{reg_${{[[:digit:]]+}}<T1 * ps>}.v>}} + clang_analyzer_eval((*ps).v == ps[0].v); // expected-warning{{TRUE}} + clang_analyzer_eval((*ps).v == ps->v); // expected-warning{{TRUE}} + clang_analyzer_eval(ps[0].v == ps->v); // expected-warning{{TRUE}} +} + +void struct_pointer_canon_bidim_typedef(T1 **ps) { + T2 ss = **ps; + clang_analyzer_eval(&(ps[0][0].v) == &((*ps)->v)); // expected-warning{{TRUE}} +} + +void struct_pointer_canon_const(const struct s *ps) { + struct s ss = *ps; + clang_analyzer_dump((*ps).v); + // expected-warning-re@-1{{reg_${{[[:digit:]]+}}<int SymRegion{reg_${{[[:digit:]]+}}<const struct s * ps>}.v>}} + clang_analyzer_dump(ps[0].v); + // expected-warning-re@-1{{reg_${{[[:digit:]]+}}<int SymRegion{reg_${{[[:digit:]]+}}<const struct s * ps>}.v>}} + clang_analyzer_dump(ps->v); + // expected-warning-re@-1{{reg_${{[[:digit:]]+}}<int SymRegion{reg_${{[[:digit:]]+}}<const struct s * ps>}.v>}} + clang_analyzer_eval((*ps).v == ps[0].v); // expected-warning{{TRUE}} + clang_analyzer_eval((*ps).v == ps->v); // expected-warning{{TRUE}} + clang_analyzer_eval(ps[0].v == ps->v); // expected-warning{{TRUE}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
