forked from wangliu-iscas/gcc-patch
-
Notifications
You must be signed in to change notification settings - Fork 3
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
c++: Catch indirect change of active union member in constexpr [PR101…
…631] On Wed, Aug 30, 2023 at 04:28:18PM -0400, Jason Merrill wrote: > On 8/29/23 09:35, Nathaniel Shead wrote: > > This is an attempt to improve the constexpr machinery's handling of > > union lifetime by catching more cases that cause UB. Is this approach > > OK? > > > > I'd also like some feedback on a couple of pain points with this > > implementation; in particular, is there a good way to detect if a type > > has a non-deleted trivial constructor? I've used 'is_trivially_xible' in > > this patch, but that also checks for a trivial destructor which by my > > reading of [class.union.general]p5 is possibly incorrect. Checking for a > > trivial default constructor doesn't seem too hard but I couldn't find a > > good way of checking if that constructor is deleted. > > I guess the simplest would be > > (TYPE_HAS_TRIVIAL_DFLT (t) && locate_ctor (t)) > > because locate_ctor returns null for a deleted default ctor. It would be > good to make this a separate predicate. > > > I'm also generally unsatisfied with the additional complexity with the > > third 'refs' argument in 'cxx_eval_store_expression' being pushed and > > popped; would it be better to replace this with a vector of some > > specific structure type for the data that needs to be passed on? > > Perhaps, but what you have here is fine. Another possibility would be to > just have a vec of the refs and extract the index from the ref later as > needed. > > Jason > Thanks for the feedback. I've kept the refs as-is for now. I've also cleaned up a couple of other typos I'd had with comments and diagnostics. Bootstrapped and regtested on x86_64-pc-linux-gnu. -- 8< -- This patch adds checks for attempting to change the active member of a union by methods other than a member access expression. To be able to properly distinguish `*(&u.a) = ` from `u.a = `, this patch redoes the solution for c++/59950 to avoid extranneous *&; it seems that the only case that needed the workaround was when copying empty classes. Additionally, this patch ensures that constructors for a union field mark that field as the active member before entering the call itself; this ensures that modifications of the field within the constructor's body don't cause false positives (as these will not appear to be member access expressions). This means that we no longer need to start the lifetime of empty union members after the constructor body completes. PR c++/101631 gcc/cp/ChangeLog: * call.cc (build_over_call): Fold more indirect refs for trivial assignment op. * class.cc (type_has_non_deleted_trivial_default_ctor): Create. * constexpr.cc (cxx_eval_call_expression): Start lifetime of union member before entering constructor. (cxx_eval_store_expression): Check for accessing inactive union member indirectly. * cp-tree.h (type_has_non_deleted_trivial_default_ctor): Forward declare. gcc/testsuite/ChangeLog: * g++.dg/cpp2a/constexpr-union2.C: New test. * g++.dg/cpp2a/constexpr-union3.C: New test. * g++.dg/cpp2a/constexpr-union4.C: New test. * g++.dg/cpp2a/constexpr-union5.C: New test. Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
- Loading branch information
1 parent
c2d62cd
commit 96e2798
Showing
8 changed files
with
246 additions
and
38 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| // PR c++/101631 | ||
| // { dg-do compile { target c++20 } } | ||
|
|
||
| struct sso { | ||
| union { | ||
| int buf[10]; | ||
| int* alloc; | ||
| }; | ||
| }; | ||
|
|
||
| constexpr bool direct() { | ||
| sso val; | ||
| val.alloc = nullptr; | ||
| val.buf[5] = 42; | ||
| return true; | ||
| } | ||
| constexpr bool ok = direct(); | ||
|
|
||
|
|
||
| constexpr void perform_assignment(int& left, int right) noexcept { | ||
| left = right; // { dg-error "accessing .+ member instead of initialized" } | ||
| } | ||
|
|
||
| constexpr bool indirect() { | ||
| sso val; | ||
| val.alloc = nullptr; | ||
| perform_assignment(val.buf[5], 42); // { dg-message "in .constexpr. expansion" } | ||
| return true; | ||
| } | ||
| constexpr bool err = indirect(); // { dg-message "in .constexpr. expansion" } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| // { dg-do compile { target c++20 } } | ||
|
|
||
| struct S | ||
| { | ||
| union { | ||
| char buf[8]; | ||
| char* ptr; | ||
| }; | ||
| unsigned len; | ||
|
|
||
| constexpr S(const char* s, unsigned n) | ||
| { | ||
| char* p; | ||
| if (n > 7) | ||
| p = ptr = new char[n+1]; | ||
| else | ||
| p = buf; | ||
| for (len = 0; len < n; ++len) | ||
| p[len] = s[len]; // { dg-error "accessing uninitialized member" } | ||
| p[len] = '\0'; | ||
| } | ||
|
|
||
| constexpr ~S() | ||
| { | ||
| if (len > 7) | ||
| delete[] ptr; | ||
| } | ||
| }; | ||
|
|
||
| constexpr bool test1() | ||
| { | ||
| S s("test", 4); // { dg-message "in .constexpr. expansion" } | ||
| return true; | ||
| } | ||
|
|
||
| constexpr bool a = test1(); // { dg-message "in .constexpr. expansion" } | ||
|
|
||
|
|
||
| constexpr bool test2() | ||
| { | ||
| S s("hello world", 11); | ||
| return true; | ||
| } | ||
|
|
||
| constexpr bool b = test2(); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| // { dg-do compile { target c++20 } } | ||
|
|
||
| // from [class.union.general] p5 | ||
|
|
||
| union A { int x; int y[4]; }; | ||
| struct B { A a; }; | ||
| union C { B b; int k; }; | ||
| constexpr int f() { | ||
| C c; // does not start lifetime of any union member | ||
| c.b.a.y[3] = 4; // OK, S(c.b.a.y[3]) contains c.b and c.b.a.y; | ||
| // creates objects to hold union members c.b and c.b.a.y | ||
| return c.b.a.y[3]; // OK, c.b.a.y refers to newly created object (see [basic.life]) | ||
| } | ||
| constexpr int a = f(); | ||
|
|
||
| struct X { const int a; int b; }; | ||
| union Y { X x; int k; };// { dg-message "does not implicitly begin its lifetime" } | ||
| constexpr int g() { | ||
| Y y = { { 1, 2 } }; // OK, y.x is active union member ([class.mem]) | ||
| int n = y.x.a; | ||
| y.k = 4; // OK, ends lifetime of y.x, y.k is active member of union | ||
|
|
||
| y.x.b = n; // { dg-error "accessing .* member instead of initialized .* member" } | ||
| // undefined behavior: y.x.b modified outside its lifetime, | ||
| // S(y.x.b) is empty because X's default constructor is deleted, | ||
| // so union member y.x's lifetime does not implicitly start | ||
| return 0; | ||
| } | ||
| constexpr int b = g(); // { dg-message "in .constexpr. expansion" } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| // { dg-do compile { target c++20 } } | ||
|
|
||
| union U { int a; int b; int c[2]; }; | ||
|
|
||
| constexpr int test1() { | ||
| U u; | ||
| u.a = 10; | ||
| *&u.b = 20; // { dg-error "accessing" } | ||
| return u.b; | ||
| } | ||
| constexpr int a = test1(); // { dg-message "in .constexpr. expansion" } | ||
|
|
||
| constexpr int test2() { | ||
| U u; | ||
| u.a = 10; | ||
| (0, u.b) = 20; // { dg-error "accessing" } | ||
| return u.b; | ||
| } | ||
| constexpr int b = test2(); // { dg-message "in .constexpr. expansion" } | ||
|
|
||
| constexpr int test3() { | ||
| U u; | ||
| u.a = 0; | ||
| int* p = &u.b; | ||
| p[u.a] = 10; // { dg-error "accessing" } | ||
| return u.b; | ||
| } | ||
| constexpr int c = test3(); // { dg-message "in .constexpr. expansion" } | ||
|
|
||
| constexpr int test4() { | ||
| U u; | ||
| u.a = 0; | ||
| int* p = &u.b; | ||
| u.a[p] = 10; // { dg-error "accessing" } | ||
| return u.b; | ||
| } | ||
| constexpr int d = test4(); // { dg-message "in .constexpr. expansion" } | ||
|
|
||
| struct S { U u[10]; }; | ||
| constexpr int test5() { | ||
| S s; | ||
| s.u[4].a = 10; | ||
| 6[s.u].b = 15; | ||
| return 4[s.u].a + s.u[6].b; | ||
| } | ||
| static_assert(test5() == 25); | ||
|
|
||
| constexpr int test6() { | ||
| U u; | ||
| u.a = 5; | ||
| u.c[0] = 3; | ||
| 1[u.c] = 8; | ||
| return 1[u.c] + u.c[0]; | ||
| } | ||
| static_assert(test6() == 11); |