[clang] 43eb18e - [analyzer] Do list initialization for CXXNewExpr with initializer list arg (#127702)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 28 08:42:30 PST 2025
Author: Michael Flanders
Date: 2025-02-28T17:42:26+01:00
New Revision: 43eb18e51f5582b73665306a45c640a880976ec1
URL: https://github.com/llvm/llvm-project/commit/43eb18e51f5582b73665306a45c640a880976ec1
DIFF: https://github.com/llvm/llvm-project/commit/43eb18e51f5582b73665306a45c640a880976ec1.diff
LOG: [analyzer] Do list initialization for CXXNewExpr with initializer list arg (#127702)
Fixes #116444.
Closed #127700 because I accidentally updated it in github UI.
### Current vs expected behavior
Previously, the result of a `CXXNewExpr` was not always list initialized
when using an initializer list.
In this example:
```
struct S { int x; };
void F() {
S *s = new S{1};
delete s;
}
```
there would be a binding of `s` to `compoundVal{1}`, but this isn't used
during later field binding lookup. After this PR, there is instead a
binding of `s->x` to `1`. This is the cause of #116444 since the field
binding lookup returns undefined in some cases currently.
### Changes
This PR swaps around the handling of typed value regions (seems to be
the usual region type when doing non-CXX-new-expr list initialization)
and symbolic regions (the result of the CXX new expr), so that symbolic
regions also get list initialized. In the below snippet, it swaps the
order of the two conditionals.
https://github.com/llvm/llvm-project/blob/8529bd7b964cc9fafe8fece84f7bd12dacb09560/clang/lib/StaticAnalyzer/Core/RegionStore.cpp#L2426-L2448
### Followup work
This PR only makes CSA do list init for `CXXNewExpr`s. After this, I
would like to make some changes to `RegionStoreMananger::bind` in how it
handles list initialization generally.
I've added some straightforward test cases here for the `new` expr with
a list initializer. I started adding some more before realizing that the
current general (not just `new` expr) list initialization could be
changed to handle more cases like list initialization of unions and
arrays (like https://github.com/llvm/llvm-project/issues/54910). Lmk if
it is preferred to then leave these test cases out for now.
Added:
clang/test/Analysis/new-user-defined.cpp
Modified:
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/test/Analysis/initializer.cpp
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index 550a276c66c71..79cb5a07701fd 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2533,6 +2533,15 @@ RegionStoreManager::bind(LimitedRegionBindingsConstRef B, Loc L, SVal V) {
const MemRegion *R = MemRegVal->getRegion();
+ // Binding directly to a symbolic region should be treated as binding
+ // to element 0.
+ if (const auto *SymReg = dyn_cast<SymbolicRegion>(R)) {
+ QualType Ty = SymReg->getPointeeStaticType();
+ if (Ty->isVoidType())
+ Ty = StateMgr.getContext().CharTy;
+ R = GetElementZeroRegion(SymReg, Ty);
+ }
+
// Check if the region is a struct region.
if (const TypedValueRegion* TR = dyn_cast<TypedValueRegion>(R)) {
QualType Ty = TR->getValueType();
@@ -2546,15 +2555,6 @@ RegionStoreManager::bind(LimitedRegionBindingsConstRef B, Loc L, SVal V) {
return bindAggregate(B, TR, V);
}
- // Binding directly to a symbolic region should be treated as binding
- // to element 0.
- if (const auto *SymReg = dyn_cast<SymbolicRegion>(R)) {
- QualType Ty = SymReg->getPointeeStaticType();
- if (Ty->isVoidType())
- Ty = StateMgr.getContext().CharTy;
- R = GetElementZeroRegion(SymReg, Ty);
- }
-
assert((!isa<CXXThisRegion>(R) || !B.lookup(R)) &&
"'this' pointer is not an l-value and is not assignable");
diff --git a/clang/test/Analysis/initializer.cpp b/clang/test/Analysis/initializer.cpp
index f50afff25d245..713e121168571 100644
--- a/clang/test/Analysis/initializer.cpp
+++ b/clang/test/Analysis/initializer.cpp
@@ -254,6 +254,224 @@ void foo() {
}
} // namespace CXX17_aggregate_construction
+namespace newexpr_init_list_initialization {
+template <class FirstT, class... Rest>
+void escape(FirstT first, Rest... args);
+
+struct S {
+ int foo;
+ int bar;
+};
+void none_designated() {
+ S *s = new S{13,1};
+ clang_analyzer_eval(13 == s->foo); // expected-warning{{TRUE}}
+ clang_analyzer_eval(1 == s->bar); // expected-warning{{TRUE}}
+ delete s;
+}
+void none_designated_swapped() {
+ S *s = new S{1,13};
+ clang_analyzer_eval(1 == s->foo); // expected-warning{{TRUE}}
+ clang_analyzer_eval(13 == s->bar); // expected-warning{{TRUE}}
+ delete s;
+}
+void one_designated_one_not() {
+ S *s = new S{ 1, .bar = 13 };
+ clang_analyzer_eval(1 == s->foo); // expected-warning{{TRUE}}
+ clang_analyzer_eval(13 == s->bar); // expected-warning{{TRUE}}
+ delete s;
+}
+void all_designated() {
+ S *s = new S{
+ .foo = 13,
+ .bar = 1,
+ };
+ clang_analyzer_eval(13 == s->foo); // expected-warning{{TRUE}}
+ clang_analyzer_eval(1 == s->bar); // expected-warning{{TRUE}}
+ delete s;
+}
+void non_designated_array_of_aggr_struct() {
+ S *s = new S[2] { {1, 2}, {3, 4} };
+ clang_analyzer_eval(1 == s[0].foo); // expected-warning{{TRUE}}
+ clang_analyzer_eval(2 == s[0].bar); // expected-warning{{TRUE}}
+ clang_analyzer_eval(3 == s[1].foo); // expected-warning{{TRUE}}
+ clang_analyzer_eval(4 == s[1].bar); // expected-warning{{TRUE}}
+ delete[] s;
+}
+
+struct WithGaps {
+ int foo;
+ int bar;
+ int baz;
+};
+void out_of_order_designated_initializers_with_gaps() {
+ WithGaps *s = new WithGaps{
+ .foo = 13,
+ .baz = 1,
+ };
+ clang_analyzer_eval(13 == s->foo); // expected-warning{{TRUE}}
+ clang_analyzer_eval(0 == s->bar); // expected-warning{{TRUE}}
+ clang_analyzer_eval(1 == s->baz); // expected-warning{{TRUE}}
+ delete s;
+}
+
+// https://eel.is/c++draft/dcl.init.aggr#note-6:
+// Static data members, non-static data members of anonymous
+// union members, and unnamed bit-fields are not considered
+// elements of the aggregate.
+struct NonConsideredFields {
+ int i;
+ static int s;
+ int j;
+ int :17;
+ int k;
+};
+void considered_fields_initd() {
+ auto S = new NonConsideredFields { 1, 2, 3 };
+ clang_analyzer_eval(1 == S->i); // expected-warning{{TRUE}}
+ clang_analyzer_eval(2 == S->j); // expected-warning{{TRUE}}
+ clang_analyzer_eval(3 == S->k); // expected-warning{{TRUE}}
+ delete S;
+}
+
+#if __cplusplus >= 201703L
+enum Enum : int {
+};
+void list_init_enum() {
+ Enum *E = new Enum{53};
+ clang_analyzer_eval(53 == *E); // expected-warning{{TRUE}}
+ delete E;
+}
+#endif // __cplusplus >= 201703L
+
+class PubClass {
+public:
+ int foo;
+ int bar;
+};
+void public_class_designated_initializers() {
+ S *s = new S{
+ .foo = 13,
+ .bar = 1,
+ };
+ clang_analyzer_eval(13 == s->foo); // expected-warning{{TRUE}}
+ clang_analyzer_eval(1 == s->bar); // expected-warning{{TRUE}}
+ delete s;
+}
+
+union UnionTestTy {
+ int x;
+ char y;
+};
+void new_expr_aggr_init_union_no_designator() {
+ UnionTestTy *u = new UnionTestTy{};
+ clang_analyzer_eval(0 == u->x); // expected-warning{{UNKNOWN}} FIXME: should be TRUE
+ clang_analyzer_eval(u->y); // expected-warning{{UNKNOWN}} FIXME: should be undefined, warning
+ delete u;
+}
+void new_expr_aggr_init_union_designated_first_field() {
+ UnionTestTy *u = new UnionTestTy{ .x = 14 };
+ clang_analyzer_eval(14 == u->x); // expected-warning{{UNKNOWN}} FIXME: should be TRUE
+ clang_analyzer_eval(u->y); // expected-warning{{UNKNOWN}} FIXME: should be undefined, warning
+ delete u;
+}
+void new_expr_aggr_init_union_designated_non_first_field() {
+ UnionTestTy *u = new UnionTestTy{ .y = 3 };
+ clang_analyzer_eval(3 == u->y); // expected-warning{{UNKNOWN}} FIXME: should be TRUE
+ clang_analyzer_eval(u->x); // expected-warning{{UNKNOWN}} FIXME: should be undefined, warning
+ delete u;
+}
+
+union UnionTestTyWithDefaultMemberInit {
+ int x;
+ char y = 14;
+};
+void union_with_default_member_init_empty_init_list() {
+ auto U = new UnionTestTyWithDefaultMemberInit{};
+ // clang_analyzer_eval(14 == U->y); // FIXME: Should be true
+ clang_analyzer_eval(U->x); // expected-warning{{UNKNOWN}} FIXME: should be undefined, warning
+ delete U;
+}
+
+struct Inner {
+ int bar;
+};
+struct Nested {
+ int foo;
+ Inner inner;
+ int baz;
+};
+void nested_aggregates() {
+ auto N = new Nested{};
+ clang_analyzer_eval(0 == N->foo); // expected-warning{{TRUE}}
+ clang_analyzer_eval(0 == N->inner.bar); // expected-warning{{TRUE}}
+ clang_analyzer_eval(0 == N->baz); // expected-warning{{TRUE}}
+
+ auto N1 = new Nested{1};
+ clang_analyzer_eval(1 == N1->foo); // expected-warning{{TRUE}}
+ clang_analyzer_eval(0 == N1->inner.bar); // expected-warning{{TRUE}}
+ clang_analyzer_eval(0 == N1->baz); // expected-warning{{TRUE}}
+
+ auto N2 = new Nested{.baz = 14};
+ clang_analyzer_eval(0 == N2->foo); // expected-warning{{TRUE}}
+ clang_analyzer_eval(0 == N2->inner.bar); // expected-warning{{TRUE}}
+ clang_analyzer_eval(14 == N2->baz); // expected-warning{{TRUE}}
+
+ auto N3 = new Nested{1,2,3};
+ clang_analyzer_eval(1 == N3->foo); // expected-warning{{TRUE}}
+ clang_analyzer_eval(2 == N3->inner.bar); // expected-warning{{TRUE}}
+ clang_analyzer_eval(3 == N3->baz); // expected-warning{{TRUE}}
+
+ auto N4 = new Nested{1, {}, 3};
+ clang_analyzer_eval(1 == N4->foo); // expected-warning{{TRUE}}
+ clang_analyzer_eval(0 == N4->inner.bar); // expected-warning{{TRUE}}
+ clang_analyzer_eval(3 == N4->baz); // expected-warning{{TRUE}}
+
+ auto N5 = new Nested{{},{},{}};
+ clang_analyzer_eval(0 == N5->foo); // expected-warning{{TRUE}}
+ clang_analyzer_eval(0 == N5->inner.bar); // expected-warning{{TRUE}}
+ clang_analyzer_eval(0 == N5->baz); // expected-warning{{TRUE}}
+
+ auto N6 = new Nested{1, {.bar = 2}, 3};
+ clang_analyzer_eval(1 == N6->foo); // expected-warning{{TRUE}}
+ clang_analyzer_eval(2 == N6->inner.bar); // expected-warning{{TRUE}}
+ clang_analyzer_eval(3 == N6->baz); // expected-warning{{TRUE}}
+
+ auto N7 = new Nested{1, {2}, 3};
+ clang_analyzer_eval(1 == N7->foo); // expected-warning{{TRUE}}
+ clang_analyzer_eval(2 == N7->inner.bar); // expected-warning{{TRUE}}
+ clang_analyzer_eval(3 == N7->baz); // expected-warning{{TRUE}}
+
+ escape(N,N1,N2,N3,N4,N5,N6,N7);
+}
+} // namespace newexpr_init_list_initialization
+
+namespace placement_new_initializer_list_arg {
+struct S {
+ int x;
+};
+void aggregate_struct() {
+ S s;
+ S *s_ptr = new (&s) S{1};
+ clang_analyzer_eval(1 == s_ptr->x); // expected-warning{{TRUE}}
+
+ S vi;
+ S *vi_ptr = new (&vi) S{};
+ clang_analyzer_eval(0 == vi_ptr->x); // expected-warning{{TRUE}}
+
+ S di;
+ S *di_ptr = new (&di) S;
+ int z = di_ptr->x + 1; // expected-warning{{The left operand of '+' is a garbage value}}
+}
+void initialize_non_zeroth_element(S arr[2]) {
+ S *s = new (&arr[1]) S{1};
+ clang_analyzer_eval(1 == s->x); // expected-warning{{TRUE}}
+}
+void initialize_non_zeroth_argument_pointers(S *arr[2]) {
+ arr[1] = new (arr[1]) S{1};
+ clang_analyzer_eval(1 == arr[1]->x); // expected-warning{{TRUE}}
+}
+} // namespace placement_new_initializer_list_arg
+
namespace CXX17_transparent_init_list_exprs {
class A {};
diff --git a/clang/test/Analysis/new-user-defined.cpp b/clang/test/Analysis/new-user-defined.cpp
new file mode 100644
index 0000000000000..8987ac078bf2c
--- /dev/null
+++ b/clang/test/Analysis/new-user-defined.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_analyze_cc1 -verify %s\
+// RUN: -analyzer-checker=core,debug.ExprInspection
+
+void clang_analyzer_eval(bool);
+
+using size_t = decltype(sizeof(int));
+
+template <class FirstT, class... Rest>
+void escape(FirstT first, Rest... args);
+
+namespace CustomClassType {
+struct S {
+ int x;
+ static void* operator new(size_t size) {
+ return ::operator new(size);
+ }
+};
+void F() {
+ S *s = new S;
+ clang_analyzer_eval(s->x); // expected-warning{{UNKNOWN}} FIXME: should be an undefined warning
+
+ S *s2 = new S{};
+ clang_analyzer_eval(0 == s2->x); // expected-warning{{TRUE}}
+
+ S *s3 = new S{1};
+ clang_analyzer_eval(1 == s3->x); // expected-warning{{TRUE}}
+
+ escape(s, s2, s3);
+}
+} // namespace CustomClassType
More information about the cfe-commits
mailing list