Skip to content

Commit b29186c

Browse files
vabridgerseinvbri
authored andcommitted
[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
1 parent c11e7b5 commit b29186c

File tree

2 files changed

+70
-0
lines changed

2 files changed

+70
-0
lines changed

clang/lib/StaticAnalyzer/Core/Store.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,19 @@ SVal StoreManager::getLValueIvar(const ObjCIvarDecl *decl, SVal base) {
442442

443443
SVal StoreManager::getLValueElement(QualType elementType, NonLoc Offset,
444444
SVal Base) {
445+
446+
// Special case, if index is 0, return the same type as if
447+
// this was not an array dereference.
448+
if (Offset.isZeroConstant()) {
449+
QualType BT = Base.getType(this->Ctx);
450+
if (!BT.isNull() && !elementType.isNull()) {
451+
QualType PointeeTy = BT->getPointeeType();
452+
if (!PointeeTy.isNull() &&
453+
PointeeTy.getCanonicalType() == elementType.getCanonicalType())
454+
return Base;
455+
}
456+
}
457+
445458
// If the base is an unknown or undefined value, just return it back.
446459
// FIXME: For absolute pointer addresses, we just return that value back as
447460
// well, although in reality we should return the offset added to that

clang/test/Analysis/ptr-arith.c

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// 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
33

44
void clang_analyzer_eval(int);
5+
void clang_analyzer_dump(int);
56

67
void f1() {
78
int a[10];
@@ -330,3 +331,59 @@ float test_nowarning_on_vector_deref() {
330331
simd_float2 x = {0, 1};
331332
return x[1]; // no-warning
332333
}
334+
335+
struct s {
336+
int v;
337+
};
338+
339+
// These three expressions should produce the same sym vals.
340+
void struct_pointer_canon(struct s *ps) {
341+
struct s ss = *ps;
342+
clang_analyzer_dump((*ps).v);
343+
// expected-warning-re@-1{{reg_${{[[:digit:]]+}}<int SymRegion{reg_${{[[:digit:]]+}}<struct s * ps>}.v>}}
344+
clang_analyzer_dump(ps[0].v);
345+
// expected-warning-re@-1{{reg_${{[[:digit:]]+}}<int SymRegion{reg_${{[[:digit:]]+}}<struct s * ps>}.v>}}
346+
clang_analyzer_dump(ps->v);
347+
// expected-warning-re@-1{{reg_${{[[:digit:]]+}}<int SymRegion{reg_${{[[:digit:]]+}}<struct s * ps>}.v>}}
348+
clang_analyzer_eval((*ps).v == ps[0].v); // expected-warning{{TRUE}}
349+
clang_analyzer_eval((*ps).v == ps->v); // expected-warning{{TRUE}}
350+
clang_analyzer_eval(ps[0].v == ps->v); // expected-warning{{TRUE}}
351+
}
352+
353+
void struct_pointer_canon_bidim(struct s **ps) {
354+
struct s ss = **ps;
355+
clang_analyzer_eval(&(ps[0][0].v) == &((*ps)->v)); // expected-warning{{TRUE}}
356+
}
357+
358+
typedef struct s T1;
359+
typedef struct s T2;
360+
void struct_pointer_canon_typedef(T1 *ps) {
361+
T2 ss = *ps;
362+
clang_analyzer_dump((*ps).v);
363+
// expected-warning-re@-1{{reg_${{[[:digit:]]+}}<int SymRegion{reg_${{[[:digit:]]+}}<T1 * ps>}.v>}}
364+
clang_analyzer_dump(ps[0].v);
365+
// expected-warning-re@-1{{reg_${{[[:digit:]]+}}<int SymRegion{reg_${{[[:digit:]]+}}<T1 * ps>}.v>}}
366+
clang_analyzer_dump(ps->v);
367+
// expected-warning-re@-1{{reg_${{[[:digit:]]+}}<int SymRegion{reg_${{[[:digit:]]+}}<T1 * ps>}.v>}}
368+
clang_analyzer_eval((*ps).v == ps[0].v); // expected-warning{{TRUE}}
369+
clang_analyzer_eval((*ps).v == ps->v); // expected-warning{{TRUE}}
370+
clang_analyzer_eval(ps[0].v == ps->v); // expected-warning{{TRUE}}
371+
}
372+
373+
void struct_pointer_canon_bidim_typedef(T1 **ps) {
374+
T2 ss = **ps;
375+
clang_analyzer_eval(&(ps[0][0].v) == &((*ps)->v)); // expected-warning{{TRUE}}
376+
}
377+
378+
void struct_pointer_canon_const(const struct s *ps) {
379+
struct s ss = *ps;
380+
clang_analyzer_dump((*ps).v);
381+
// expected-warning-re@-1{{reg_${{[[:digit:]]+}}<int SymRegion{reg_${{[[:digit:]]+}}<const struct s * ps>}.v>}}
382+
clang_analyzer_dump(ps[0].v);
383+
// expected-warning-re@-1{{reg_${{[[:digit:]]+}}<int SymRegion{reg_${{[[:digit:]]+}}<const struct s * ps>}.v>}}
384+
clang_analyzer_dump(ps->v);
385+
// expected-warning-re@-1{{reg_${{[[:digit:]]+}}<int SymRegion{reg_${{[[:digit:]]+}}<const struct s * ps>}.v>}}
386+
clang_analyzer_eval((*ps).v == ps[0].v); // expected-warning{{TRUE}}
387+
clang_analyzer_eval((*ps).v == ps->v); // expected-warning{{TRUE}}
388+
clang_analyzer_eval(ps[0].v == ps->v); // expected-warning{{TRUE}}
389+
}

0 commit comments

Comments
 (0)