diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp index 6bad9a93a3016..f46282a73fbe4 100644 --- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -2336,20 +2336,16 @@ NonLoc RegionStoreManager::createLazyBinding(RegionBindingsConstRef B, return svalBuilder.makeLazyCompoundVal(StoreRef(B.asStore(), *this), R); } -static bool isRecordEmpty(const RecordDecl *RD) { - if (!RD->field_empty()) - return false; - if (const CXXRecordDecl *CRD = dyn_cast(RD)) - return CRD->getNumBases() == 0; - return true; -} - SVal RegionStoreManager::getBindingForStruct(RegionBindingsConstRef B, const TypedValueRegion *R) { const RecordDecl *RD = R->getValueType()->castAs()->getDecl(); - if (!RD->getDefinition() || isRecordEmpty(RD)) + if (!RD->getDefinition()) return UnknownVal(); + // We also create a LCV for copying empty structs because then the store + // behavior doesn't depend on the struct layout. + // This way even an empty struct can carry taint, no matter if creduce drops + // the last field member or not. return createLazyBinding(B, R); } diff --git a/clang/test/Analysis/ctor-trivial-copy.cpp b/clang/test/Analysis/ctor-trivial-copy.cpp index 5ed188aa8f1ea..a1209c24136e2 100644 --- a/clang/test/Analysis/ctor-trivial-copy.cpp +++ b/clang/test/Analysis/ctor-trivial-copy.cpp @@ -1,8 +1,12 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-inlining=constructors -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-inlining=constructors -verify %s \ +// RUN: 2>&1 | FileCheck %s -template -void clang_analyzer_dump(T&); +void clang_analyzer_printState(); +template void clang_analyzer_dump_lref(T&); +template void clang_analyzer_dump_val(T); +template T conjure(); +template void nop(const Ts &... args) {} struct aggr { int x; @@ -15,20 +19,103 @@ struct empty { void test_copy_return() { aggr s1 = {1, 2}; aggr const& cr1 = aggr(s1); - clang_analyzer_dump(cr1); // expected-warning-re {{&lifetime_extended_object{aggr, cr1, S{{[0-9]+}}} }} + clang_analyzer_dump_lref(cr1); // expected-warning-re {{&lifetime_extended_object{aggr, cr1, S{{[0-9]+}}} }} empty s2; empty const& cr2 = empty{s2}; - clang_analyzer_dump(cr2); // expected-warning-re {{&lifetime_extended_object{empty, cr2, S{{[0-9]+}}} }} + clang_analyzer_dump_lref(cr2); // expected-warning-re {{&lifetime_extended_object{empty, cr2, S{{[0-9]+}}} }} } void test_assign_return() { aggr s1 = {1, 2}; aggr d1; - clang_analyzer_dump(d1 = s1); // expected-warning {{&d1 }} + clang_analyzer_dump_lref(d1 = s1); // expected-warning {{&d1 }} empty s2; empty d2; - clang_analyzer_dump(d2 = s2); // expected-warning {{&d2 }} was Unknown + clang_analyzer_dump_lref(d2 = s2); // expected-warning {{&d2 }} was Unknown } + +namespace trivial_struct_copy { + +void _01_empty_structs() { + clang_analyzer_dump_val(conjure()); // expected-warning {{lazyCompoundVal}} + empty Empty = conjure(); + empty Empty2 = Empty; + empty Empty3 = Empty2; + // All of these should refer to the exact same LCV, because all of + // these trivial copies refer to the original conjured value. + // There were Unknown before: + clang_analyzer_dump_val(Empty); // expected-warning {{lazyCompoundVal}} + clang_analyzer_dump_val(Empty2); // expected-warning {{lazyCompoundVal}} + clang_analyzer_dump_val(Empty3); // expected-warning {{lazyCompoundVal}} + + // Notice that we don't have entries for the copies, only for the original "Empty" object. + // That binding was added by the opaque "conjure" call, directly constructing to the variable "Empty" by copy-elision. + // The copies are not present because the ExprEngine skips the evalBind of empty structs. + // And even if such binds would reach the Store, the Store copies small structs by copying the individual fields, + // and there are no fields within "Empty", thus we wouldn't have any entries anyways. + clang_analyzer_printState(); + // CHECK: "store": { "pointer": "0x{{[0-9a-f]+}}", "items": [ + // CHECK-NEXT: { "cluster": "GlobalInternalSpaceRegion", "pointer": "0x{{[0-9a-f]+}}", "items": [ + // CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "conj_$ + // CHECK-NEXT: ]}, + // CHECK-NEXT: { "cluster": "GlobalSystemSpaceRegion", "pointer": "0x{{[0-9a-f]+}}", "items": [ + // CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "conj_$ + // CHECK-NEXT: ]}, + // CHECK-NEXT: { "cluster": "Empty", "pointer": "0x{{[0-9a-f]+}}", "items": [ + // CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "[[EMPTY_CONJ:conj_\$[0-9]+{int, LC[0-9]+, S[0-9]+, #[0-9]+}]]" } + // CHECK-NEXT: ]} + // CHECK-NEXT: ]}, + + nop(Empty, Empty2, Empty3); +} + +void _02_structs_with_members() { + clang_analyzer_dump_val(conjure()); // expected-warning {{lazyCompoundVal}} + aggr Aggr = conjure(); + aggr Aggr2 = Aggr; + aggr Aggr3 = Aggr2; + // All of these should refer to the exact same LCV, because all of + // these trivial copies refer to the original conjured value. + clang_analyzer_dump_val(Aggr); // expected-warning {{lazyCompoundVal}} + clang_analyzer_dump_val(Aggr2); // expected-warning {{lazyCompoundVal}} + clang_analyzer_dump_val(Aggr3); // expected-warning {{lazyCompoundVal}} + + // We have fields in the struct we copy, thus we also have the entries for the copies + // (and for all of their fields). + clang_analyzer_printState(); + // CHECK: "store": { "pointer": "0x{{[0-9a-f]+}}", "items": [ + // CHECK-NEXT: { "cluster": "GlobalInternalSpaceRegion", "pointer": "0x{{[0-9a-f]+}}", "items": [ + // CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "conj_$ + // CHECK-NEXT: ]}, + // CHECK-NEXT: { "cluster": "GlobalSystemSpaceRegion", "pointer": "0x{{[0-9a-f]+}}", "items": [ + // CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "conj_$ + // CHECK-NEXT: ]}, + // CHECK-NEXT: { "cluster": "Aggr", "pointer": "0x{{[0-9a-f]+}}", "items": [ + // CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "[[AGGR_CONJ:conj_\$[0-9]+{int, LC[0-9]+, S[0-9]+, #[0-9]+}]]" } + // CHECK-NEXT: ]}, + // CHECK-NEXT: { "cluster": "Aggr2", "pointer": "0x{{[0-9a-f]+}}", "items": [ + // CHECK-NEXT: { "kind": "Direct", "offset": 0, "value": "derived_${{[0-9]+}}{[[AGGR_CONJ]],Aggr.x}" }, + // CHECK-NEXT: { "kind": "Direct", "offset": 32, "value": "derived_${{[0-9]+}}{[[AGGR_CONJ]],Aggr.y}" } + // CHECK-NEXT: ]}, + // CHECK-NEXT: { "cluster": "Aggr3", "pointer": "0x{{[0-9a-f]+}}", "items": [ + // CHECK-NEXT: { "kind": "Direct", "offset": 0, "value": "derived_${{[0-9]+}}{[[AGGR_CONJ]],Aggr.x}" }, + // CHECK-NEXT: { "kind": "Direct", "offset": 32, "value": "derived_${{[0-9]+}}{[[AGGR_CONJ]],Aggr.y}" } + // CHECK-NEXT: ]} + // CHECK-NEXT: ]}, + + nop(Aggr, Aggr2, Aggr3); +} + +// Tests that use `clang_analyzer_printState()` must share the analysis entry +// point, and have a strict ordering between. This is to meet the different +// `clang_analyzer_printState()` calls in a fixed relative ordering, thus +// FileCheck could check the stdouts. +void entrypoint() { + _01_empty_structs(); + _02_structs_with_members(); +} + +} // namespace trivial_struct_copy