Skip to content

[analyzer] Allow copying empty structs (1/4) #115916

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 14, 2024
Merged

[analyzer] Allow copying empty structs (1/4) #115916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions clang/lib/StaticAnalyzer/Core/RegionStore.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<CXXRecordDecl>(RD))
return CRD->getNumBases() == 0;
return true;
}

SVal RegionStoreManager::getBindingForStruct(RegionBindingsConstRef B,
const TypedValueRegion *R) {
const RecordDecl *RD = R->getValueType()->castAs<RecordType>()->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);
}

Expand Down
101 changes: 94 additions & 7 deletions clang/test/Analysis/ctor-trivial-copy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<typename T>
void clang_analyzer_dump(T&);
void clang_analyzer_printState();
template<typename T> void clang_analyzer_dump_lref(T&);
template<typename T> void clang_analyzer_dump_val(T);
template <typename T> T conjure();
template <typename... Ts> void nop(const Ts &... args) {}

struct aggr {
int x;
Expand All @@ -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<empty>()); // expected-warning {{lazyCompoundVal}}
empty Empty = conjure<empty>();
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<aggr>()); // expected-warning {{lazyCompoundVal}}
aggr Aggr = conjure<aggr>();
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
Loading