| Message ID | 20190731192659.GP32749@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | C++ PATCH for c++/91264 - detect modifying const objects in constexpr | expand |
On 7/31/19 3:26 PM, Marek Polacek wrote: > One of the features of constexpr is that it doesn't allow UB; and such UB must > be detected at compile-time. So running your code in a context that requires > a constant expression should ensure that the code in question is free of UB. > In effect, constexpr can serve as a sanitizer. E.g. this article describes in > in more detail: > <https://shafik.github.io/c++/undefined%20behavior/2019/05/11/explporing_undefined_behavior_using_constexpr.html> > > [dcl.type.cv]p4 says "Any attempt to modify a const object during its lifetime > results in undefined behavior." However, as the article above points out, we > aren't detecting that case in constexpr evaluation. > > This patch fixes that. It's not that easy, though, because we have to keep in > mind [class.ctor]p5: > "A constructor can be invoked for a const, volatile or const volatile object. > const and volatile semantics are not applied on an object under construction. > They come into effect when the constructor for the most derived object ends." > > I handled this by keeping a hash set which tracks objects under construction. > I considered other options, such as going up call_stack, but that wouldn't > work with trivial constructor/op=. It was also interesting to find out that > the definition of TREE_HAS_CONSTRUCTOR says "When appearing in a FIELD_DECL, > it means that this field has been duly initialized in its constructor" though > nowhere in the codebase do we set TREE_HAS_CONSTRUCTOR on a FIELD_DECL as far > as I can see. Unfortunately, using this bit proved useless for my needs here. > Also, be mindful of mutable subobjects. > > Does this approach look like an appropriate strategy for tracking objects' > construction? For scalar objects, we should be able to rely on INIT_EXPR vs. MODIFY_EXPR to distinguish between initialization and modification; for class objects, I wonder about setting a flag on the CONSTRUCTOR after initialization is complete to indicate that the value is now constant. Jason
On Mon, Aug 05, 2019 at 03:54:19PM -0400, Jason Merrill wrote: > On 7/31/19 3:26 PM, Marek Polacek wrote: > > One of the features of constexpr is that it doesn't allow UB; and such UB must > > be detected at compile-time. So running your code in a context that requires > > a constant expression should ensure that the code in question is free of UB. > > In effect, constexpr can serve as a sanitizer. E.g. this article describes in > > in more detail: > > <https://shafik.github.io/c++/undefined%20behavior/2019/05/11/explporing_undefined_behavior_using_constexpr.html> > > > > [dcl.type.cv]p4 says "Any attempt to modify a const object during its lifetime > > results in undefined behavior." However, as the article above points out, we > > aren't detecting that case in constexpr evaluation. > > > > This patch fixes that. It's not that easy, though, because we have to keep in > > mind [class.ctor]p5: > > "A constructor can be invoked for a const, volatile or const volatile object. > > const and volatile semantics are not applied on an object under construction. > > They come into effect when the constructor for the most derived object ends." > > > > I handled this by keeping a hash set which tracks objects under construction. > > I considered other options, such as going up call_stack, but that wouldn't > > work with trivial constructor/op=. It was also interesting to find out that > > the definition of TREE_HAS_CONSTRUCTOR says "When appearing in a FIELD_DECL, > > it means that this field has been duly initialized in its constructor" though > > nowhere in the codebase do we set TREE_HAS_CONSTRUCTOR on a FIELD_DECL as far > > as I can see. Unfortunately, using this bit proved useless for my needs here. > > > Also, be mindful of mutable subobjects. > > > > Does this approach look like an appropriate strategy for tracking objects' > > construction? > > For scalar objects, we should be able to rely on INIT_EXPR vs. MODIFY_EXPR > to distinguish between initialization and modification; for class objects, I This is already true: only class object go into the hash set. > wonder about setting a flag on the CONSTRUCTOR after initialization is > complete to indicate that the value is now constant. But here we're not dealing with CONSTRUCTORs in the gcc sense (i.e. exprs with TREE_CODE == CONSTRUCTOR). We have a CALL_EXPR like Y::Y ((struct Y *) &y), which initializes the object "y". Setting a flag on the CALL_EXPR or its underlying function decl wouldn't help. (Also, all 6 TREE_LANG_FLAGs for a CONSTRUCTOR are used.) Am I missing something? Marek
Hi, On 31/07/19 21:26, Marek Polacek wrote: > +static void > +modifying_const_object_error (tree expr, tree obj) > +{ > + location_t loc = cp_expr_loc_or_loc (expr, input_location); Nit: now we have cp_expr_loc_input_loc Paolo.
On Tue, Aug 06, 2019 at 09:38:35PM +0200, Paolo Carlini wrote: > Hi, > > On 31/07/19 21:26, Marek Polacek wrote: > > +static void > > +modifying_const_object_error (tree expr, tree obj) > > +{ > > + location_t loc = cp_expr_loc_or_loc (expr, input_location); > > Nit: now we have cp_expr_loc_input_loc Ah yeah, and I was meaning to update it! Will fix. Marek
On 8/6/19 3:20 PM, Marek Polacek wrote: > On Mon, Aug 05, 2019 at 03:54:19PM -0400, Jason Merrill wrote: >> On 7/31/19 3:26 PM, Marek Polacek wrote: >>> One of the features of constexpr is that it doesn't allow UB; and such UB must >>> be detected at compile-time. So running your code in a context that requires >>> a constant expression should ensure that the code in question is free of UB. >>> In effect, constexpr can serve as a sanitizer. E.g. this article describes in >>> in more detail: >>> <https://shafik.github.io/c++/undefined%20behavior/2019/05/11/explporing_undefined_behavior_using_constexpr.html> >>> >>> [dcl.type.cv]p4 says "Any attempt to modify a const object during its lifetime >>> results in undefined behavior." However, as the article above points out, we >>> aren't detecting that case in constexpr evaluation. >>> >>> This patch fixes that. It's not that easy, though, because we have to keep in >>> mind [class.ctor]p5: >>> "A constructor can be invoked for a const, volatile or const volatile object. >>> const and volatile semantics are not applied on an object under construction. >>> They come into effect when the constructor for the most derived object ends." >>> >>> I handled this by keeping a hash set which tracks objects under construction. >>> I considered other options, such as going up call_stack, but that wouldn't >>> work with trivial constructor/op=. It was also interesting to find out that >>> the definition of TREE_HAS_CONSTRUCTOR says "When appearing in a FIELD_DECL, >>> it means that this field has been duly initialized in its constructor" though >>> nowhere in the codebase do we set TREE_HAS_CONSTRUCTOR on a FIELD_DECL as far >>> as I can see. Unfortunately, using this bit proved useless for my needs here. >> >>> Also, be mindful of mutable subobjects. >>> >>> Does this approach look like an appropriate strategy for tracking objects' >>> construction? >> >> For scalar objects, we should be able to rely on INIT_EXPR vs. MODIFY_EXPR >> to distinguish between initialization and modification; for class objects, I > > This is already true: only class object go into the hash set. > >> wonder about setting a flag on the CONSTRUCTOR after initialization is >> complete to indicate that the value is now constant. > > But here we're not dealing with CONSTRUCTORs in the gcc sense (i.e. exprs with > TREE_CODE == CONSTRUCTOR). We have a CALL_EXPR like Y::Y ((struct Y *) &y), > which initializes the object "y". Setting a flag on the CALL_EXPR or its underlying > function decl wouldn't help. > > Am I missing something? I was thinking that where in your current patch you call remove_object_under_construction, we could instead mark the object's value CONSTRUCTOR as immutable. > (Also, all 6 TREE_LANG_FLAGs for a CONSTRUCTOR are used.) TREE_READONLY seems suitable. Jason
On Thu, Aug 08, 2019 at 11:06:17AM -0400, Jason Merrill wrote: > On 8/6/19 3:20 PM, Marek Polacek wrote: > > On Mon, Aug 05, 2019 at 03:54:19PM -0400, Jason Merrill wrote: > > > On 7/31/19 3:26 PM, Marek Polacek wrote: > > > > One of the features of constexpr is that it doesn't allow UB; and such UB must > > > > be detected at compile-time. So running your code in a context that requires > > > > a constant expression should ensure that the code in question is free of UB. > > > > In effect, constexpr can serve as a sanitizer. E.g. this article describes in > > > > in more detail: > > > > <https://shafik.github.io/c++/undefined%20behavior/2019/05/11/explporing_undefined_behavior_using_constexpr.html> > > > > > > > > [dcl.type.cv]p4 says "Any attempt to modify a const object during its lifetime > > > > results in undefined behavior." However, as the article above points out, we > > > > aren't detecting that case in constexpr evaluation. > > > > > > > > This patch fixes that. It's not that easy, though, because we have to keep in > > > > mind [class.ctor]p5: > > > > "A constructor can be invoked for a const, volatile or const volatile object. > > > > const and volatile semantics are not applied on an object under construction. > > > > They come into effect when the constructor for the most derived object ends." > > > > > > > > I handled this by keeping a hash set which tracks objects under construction. > > > > I considered other options, such as going up call_stack, but that wouldn't > > > > work with trivial constructor/op=. It was also interesting to find out that > > > > the definition of TREE_HAS_CONSTRUCTOR says "When appearing in a FIELD_DECL, > > > > it means that this field has been duly initialized in its constructor" though > > > > nowhere in the codebase do we set TREE_HAS_CONSTRUCTOR on a FIELD_DECL as far > > > > as I can see. Unfortunately, using this bit proved useless for my needs here. > > > > > > > Also, be mindful of mutable subobjects. > > > > > > > > Does this approach look like an appropriate strategy for tracking objects' > > > > construction? > > > > > > For scalar objects, we should be able to rely on INIT_EXPR vs. MODIFY_EXPR > > > to distinguish between initialization and modification; for class objects, I > > > > This is already true: only class object go into the hash set. > > > > > wonder about setting a flag on the CONSTRUCTOR after initialization is > > > complete to indicate that the value is now constant. > > > > But here we're not dealing with CONSTRUCTORs in the gcc sense (i.e. exprs with > > TREE_CODE == CONSTRUCTOR). We have a CALL_EXPR like Y::Y ((struct Y *) &y), > > which initializes the object "y". Setting a flag on the CALL_EXPR or its underlying > > function decl wouldn't help. > > > > Am I missing something? > > I was thinking that where in your current patch you call > remove_object_under_construction, we could instead mark the object's value > CONSTRUCTOR as immutable. Ah, what you meant was to look at DECL_INITIAL of the object we're constructing, which could be a CONSTRUCTOR. Unfortunately, this DECL_INITIAL is null (in all the new tests when doing remove_object_under_construction), so there's nothing to mark as TREE_READONLY :/. Marek
On Thu, Aug 8, 2019 at 3:25 PM Marek Polacek <polacek@redhat.com> wrote: > > On Thu, Aug 08, 2019 at 11:06:17AM -0400, Jason Merrill wrote: > > On 8/6/19 3:20 PM, Marek Polacek wrote: > > > On Mon, Aug 05, 2019 at 03:54:19PM -0400, Jason Merrill wrote: > > > > On 7/31/19 3:26 PM, Marek Polacek wrote: > > > > > One of the features of constexpr is that it doesn't allow UB; and such UB must > > > > > be detected at compile-time. So running your code in a context that requires > > > > > a constant expression should ensure that the code in question is free of UB. > > > > > In effect, constexpr can serve as a sanitizer. E.g. this article describes in > > > > > in more detail: > > > > > <https://shafik.github.io/c++/undefined%20behavior/2019/05/11/explporing_undefined_behavior_using_constexpr.html> > > > > > > > > > > [dcl.type.cv]p4 says "Any attempt to modify a const object during its lifetime > > > > > results in undefined behavior." However, as the article above points out, we > > > > > aren't detecting that case in constexpr evaluation. > > > > > > > > > > This patch fixes that. It's not that easy, though, because we have to keep in > > > > > mind [class.ctor]p5: > > > > > "A constructor can be invoked for a const, volatile or const volatile object. > > > > > const and volatile semantics are not applied on an object under construction. > > > > > They come into effect when the constructor for the most derived object ends." > > > > > > > > > > I handled this by keeping a hash set which tracks objects under construction. > > > > > I considered other options, such as going up call_stack, but that wouldn't > > > > > work with trivial constructor/op=. It was also interesting to find out that > > > > > the definition of TREE_HAS_CONSTRUCTOR says "When appearing in a FIELD_DECL, > > > > > it means that this field has been duly initialized in its constructor" though > > > > > nowhere in the codebase do we set TREE_HAS_CONSTRUCTOR on a FIELD_DECL as far > > > > > as I can see. Unfortunately, using this bit proved useless for my needs here. > > > > > > > > > Also, be mindful of mutable subobjects. > > > > > > > > > > Does this approach look like an appropriate strategy for tracking objects' > > > > > construction? > > > > > > > > For scalar objects, we should be able to rely on INIT_EXPR vs. MODIFY_EXPR > > > > to distinguish between initialization and modification; for class objects, I > > > > > > This is already true: only class object go into the hash set. > > > > > > > wonder about setting a flag on the CONSTRUCTOR after initialization is > > > > complete to indicate that the value is now constant. > > > > > > But here we're not dealing with CONSTRUCTORs in the gcc sense (i.e. exprs with > > > TREE_CODE == CONSTRUCTOR). We have a CALL_EXPR like Y::Y ((struct Y *) &y), > > > which initializes the object "y". Setting a flag on the CALL_EXPR or its underlying > > > function decl wouldn't help. > > > > > > Am I missing something? > > > > I was thinking that where in your current patch you call > > remove_object_under_construction, we could instead mark the object's value > > CONSTRUCTOR as immutable. > > Ah, what you meant was to look at DECL_INITIAL of the object we're > constructing, which could be a CONSTRUCTOR. Unfortunately, this > DECL_INITIAL is null (in all the new tests when doing > remove_object_under_construction), so there's nothing to mark as TREE_READONLY :/. There's a value in ctx->values, isn't there? Jason
On Wed, Aug 14, 2019 at 02:50:13PM -0400, Jason Merrill wrote: > On Thu, Aug 8, 2019 at 3:25 PM Marek Polacek <polacek@redhat.com> wrote: > > > > On Thu, Aug 08, 2019 at 11:06:17AM -0400, Jason Merrill wrote: > > > On 8/6/19 3:20 PM, Marek Polacek wrote: > > > > On Mon, Aug 05, 2019 at 03:54:19PM -0400, Jason Merrill wrote: > > > > > On 7/31/19 3:26 PM, Marek Polacek wrote: > > > > > > One of the features of constexpr is that it doesn't allow UB; and such UB must > > > > > > be detected at compile-time. So running your code in a context that requires > > > > > > a constant expression should ensure that the code in question is free of UB. > > > > > > In effect, constexpr can serve as a sanitizer. E.g. this article describes in > > > > > > in more detail: > > > > > > <https://shafik.github.io/c++/undefined%20behavior/2019/05/11/explporing_undefined_behavior_using_constexpr.html> > > > > > > > > > > > > [dcl.type.cv]p4 says "Any attempt to modify a const object during its lifetime > > > > > > results in undefined behavior." However, as the article above points out, we > > > > > > aren't detecting that case in constexpr evaluation. > > > > > > > > > > > > This patch fixes that. It's not that easy, though, because we have to keep in > > > > > > mind [class.ctor]p5: > > > > > > "A constructor can be invoked for a const, volatile or const volatile object. > > > > > > const and volatile semantics are not applied on an object under construction. > > > > > > They come into effect when the constructor for the most derived object ends." > > > > > > > > > > > > I handled this by keeping a hash set which tracks objects under construction. > > > > > > I considered other options, such as going up call_stack, but that wouldn't > > > > > > work with trivial constructor/op=. It was also interesting to find out that > > > > > > the definition of TREE_HAS_CONSTRUCTOR says "When appearing in a FIELD_DECL, > > > > > > it means that this field has been duly initialized in its constructor" though > > > > > > nowhere in the codebase do we set TREE_HAS_CONSTRUCTOR on a FIELD_DECL as far > > > > > > as I can see. Unfortunately, using this bit proved useless for my needs here. > > > > > > > > > > > Also, be mindful of mutable subobjects. > > > > > > > > > > > > Does this approach look like an appropriate strategy for tracking objects' > > > > > > construction? > > > > > > > > > > For scalar objects, we should be able to rely on INIT_EXPR vs. MODIFY_EXPR > > > > > to distinguish between initialization and modification; for class objects, I > > > > > > > > This is already true: only class object go into the hash set. > > > > > > > > > wonder about setting a flag on the CONSTRUCTOR after initialization is > > > > > complete to indicate that the value is now constant. > > > > > > > > But here we're not dealing with CONSTRUCTORs in the gcc sense (i.e. exprs with > > > > TREE_CODE == CONSTRUCTOR). We have a CALL_EXPR like Y::Y ((struct Y *) &y), > > > > which initializes the object "y". Setting a flag on the CALL_EXPR or its underlying > > > > function decl wouldn't help. > > > > > > > > Am I missing something? > > > > > > I was thinking that where in your current patch you call > > > remove_object_under_construction, we could instead mark the object's value > > > CONSTRUCTOR as immutable. > > > > Ah, what you meant was to look at DECL_INITIAL of the object we're > > constructing, which could be a CONSTRUCTOR. Unfortunately, this > > DECL_INITIAL is null (in all the new tests when doing > > remove_object_under_construction), so there's nothing to mark as TREE_READONLY :/. > > There's a value in ctx->values, isn't there? Doesn't seem to be the case for e.g. struct A { int n; constexpr A() : n(1) { n = 2; } }; struct B { const A a; constexpr B(bool b) { if (b) const_cast<A &>(a).n = 3; // { dg-error "modifying a const object" } } }; constexpr B b(false); static_assert(b.a.n == 2, ""); Here we're constructing "b", its ctx->values->get(new_obj) is initially "{}". In the middle of constructing "b", we construct "b.a", but that has nothing in ctx->values. Marek
On 8/15/19 5:34 PM, Marek Polacek wrote: > On Wed, Aug 14, 2019 at 02:50:13PM -0400, Jason Merrill wrote: >> On Thu, Aug 8, 2019 at 3:25 PM Marek Polacek <polacek@redhat.com> wrote: >>> >>> On Thu, Aug 08, 2019 at 11:06:17AM -0400, Jason Merrill wrote: >>>> On 8/6/19 3:20 PM, Marek Polacek wrote: >>>>> On Mon, Aug 05, 2019 at 03:54:19PM -0400, Jason Merrill wrote: >>>>>> On 7/31/19 3:26 PM, Marek Polacek wrote: >>>>>>> One of the features of constexpr is that it doesn't allow UB; and such UB must >>>>>>> be detected at compile-time. So running your code in a context that requires >>>>>>> a constant expression should ensure that the code in question is free of UB. >>>>>>> In effect, constexpr can serve as a sanitizer. E.g. this article describes in >>>>>>> in more detail: >>>>>>> <https://shafik.github.io/c++/undefined%20behavior/2019/05/11/explporing_undefined_behavior_using_constexpr.html> >>>>>>> >>>>>>> [dcl.type.cv]p4 says "Any attempt to modify a const object during its lifetime >>>>>>> results in undefined behavior." However, as the article above points out, we >>>>>>> aren't detecting that case in constexpr evaluation. >>>>>>> >>>>>>> This patch fixes that. It's not that easy, though, because we have to keep in >>>>>>> mind [class.ctor]p5: >>>>>>> "A constructor can be invoked for a const, volatile or const volatile object. >>>>>>> const and volatile semantics are not applied on an object under construction. >>>>>>> They come into effect when the constructor for the most derived object ends." >>>>>>> >>>>>>> I handled this by keeping a hash set which tracks objects under construction. >>>>>>> I considered other options, such as going up call_stack, but that wouldn't >>>>>>> work with trivial constructor/op=. It was also interesting to find out that >>>>>>> the definition of TREE_HAS_CONSTRUCTOR says "When appearing in a FIELD_DECL, >>>>>>> it means that this field has been duly initialized in its constructor" though >>>>>>> nowhere in the codebase do we set TREE_HAS_CONSTRUCTOR on a FIELD_DECL as far >>>>>>> as I can see. Unfortunately, using this bit proved useless for my needs here. >>>>>> >>>>>>> Also, be mindful of mutable subobjects. >>>>>>> >>>>>>> Does this approach look like an appropriate strategy for tracking objects' >>>>>>> construction? >>>>>> >>>>>> For scalar objects, we should be able to rely on INIT_EXPR vs. MODIFY_EXPR >>>>>> to distinguish between initialization and modification; for class objects, I >>>>> >>>>> This is already true: only class object go into the hash set. >>>>> >>>>>> wonder about setting a flag on the CONSTRUCTOR after initialization is >>>>>> complete to indicate that the value is now constant. >>>>> >>>>> But here we're not dealing with CONSTRUCTORs in the gcc sense (i.e. exprs with >>>>> TREE_CODE == CONSTRUCTOR). We have a CALL_EXPR like Y::Y ((struct Y *) &y), >>>>> which initializes the object "y". Setting a flag on the CALL_EXPR or its underlying >>>>> function decl wouldn't help. >>>>> >>>>> Am I missing something? >>>> >>>> I was thinking that where in your current patch you call >>>> remove_object_under_construction, we could instead mark the object's value >>>> CONSTRUCTOR as immutable. >>> >>> Ah, what you meant was to look at DECL_INITIAL of the object we're >>> constructing, which could be a CONSTRUCTOR. Unfortunately, this >>> DECL_INITIAL is null (in all the new tests when doing >>> remove_object_under_construction), so there's nothing to mark as TREE_READONLY :/. >> >> There's a value in ctx->values, isn't there? > > Doesn't seem to be the case for e.g. > > struct A { > int n; > constexpr A() : n(1) { n = 2; } > }; > > struct B { > const A a; > constexpr B(bool b) { > if (b) > const_cast<A &>(a).n = 3; // { dg-error "modifying a const object" } > } > }; > > constexpr B b(false); > static_assert(b.a.n == 2, ""); > > Here we're constructing "b", its ctx->values->get(new_obj) is initially > "{}". In the middle of constructing "b", we construct "b.a", but that > has nothing in ctx->values. Right, subobjects aren't in ctx->values. In cxx_eval_call_expression we have if (DECL_CONSTRUCTOR_P (fun)) /* This can be null for a subobject constructor call, in which case what we care about is the initialization side-effects rather than the value. We could get at the value by evaluating *this, but we don't bother; there's no need to put such a call in the hash table. */ result = lval ? ctx->object : ctx->ctor; Your patch already finds *this (b.a) and puts it in new_obj; if it's const we can evaluate it to get the CONSTRUCTOR to set TREE_READONLY on. Jason
On Thu, Aug 15, 2019 at 08:21:25PM -0400, Jason Merrill wrote: > On 8/15/19 5:34 PM, Marek Polacek wrote: > > On Wed, Aug 14, 2019 at 02:50:13PM -0400, Jason Merrill wrote: > > > On Thu, Aug 8, 2019 at 3:25 PM Marek Polacek <polacek@redhat.com> wrote: > > > > > > > > On Thu, Aug 08, 2019 at 11:06:17AM -0400, Jason Merrill wrote: > > > > > On 8/6/19 3:20 PM, Marek Polacek wrote: > > > > > > On Mon, Aug 05, 2019 at 03:54:19PM -0400, Jason Merrill wrote: > > > > > > > On 7/31/19 3:26 PM, Marek Polacek wrote: > > > > > > > > One of the features of constexpr is that it doesn't allow UB; and such UB must > > > > > > > > be detected at compile-time. So running your code in a context that requires > > > > > > > > a constant expression should ensure that the code in question is free of UB. > > > > > > > > In effect, constexpr can serve as a sanitizer. E.g. this article describes in > > > > > > > > in more detail: > > > > > > > > <https://shafik.github.io/c++/undefined%20behavior/2019/05/11/explporing_undefined_behavior_using_constexpr.html> > > > > > > > > > > > > > > > > [dcl.type.cv]p4 says "Any attempt to modify a const object during its lifetime > > > > > > > > results in undefined behavior." However, as the article above points out, we > > > > > > > > aren't detecting that case in constexpr evaluation. > > > > > > > > > > > > > > > > This patch fixes that. It's not that easy, though, because we have to keep in > > > > > > > > mind [class.ctor]p5: > > > > > > > > "A constructor can be invoked for a const, volatile or const volatile object. > > > > > > > > const and volatile semantics are not applied on an object under construction. > > > > > > > > They come into effect when the constructor for the most derived object ends." > > > > > > > > > > > > > > > > I handled this by keeping a hash set which tracks objects under construction. > > > > > > > > I considered other options, such as going up call_stack, but that wouldn't > > > > > > > > work with trivial constructor/op=. It was also interesting to find out that > > > > > > > > the definition of TREE_HAS_CONSTRUCTOR says "When appearing in a FIELD_DECL, > > > > > > > > it means that this field has been duly initialized in its constructor" though > > > > > > > > nowhere in the codebase do we set TREE_HAS_CONSTRUCTOR on a FIELD_DECL as far > > > > > > > > as I can see. Unfortunately, using this bit proved useless for my needs here. > > > > > > > > > > > > > > > Also, be mindful of mutable subobjects. > > > > > > > > > > > > > > > > Does this approach look like an appropriate strategy for tracking objects' > > > > > > > > construction? > > > > > > > > > > > > > > For scalar objects, we should be able to rely on INIT_EXPR vs. MODIFY_EXPR > > > > > > > to distinguish between initialization and modification; for class objects, I > > > > > > > > > > > > This is already true: only class object go into the hash set. > > > > > > > > > > > > > wonder about setting a flag on the CONSTRUCTOR after initialization is > > > > > > > complete to indicate that the value is now constant. > > > > > > > > > > > > But here we're not dealing with CONSTRUCTORs in the gcc sense (i.e. exprs with > > > > > > TREE_CODE == CONSTRUCTOR). We have a CALL_EXPR like Y::Y ((struct Y *) &y), > > > > > > which initializes the object "y". Setting a flag on the CALL_EXPR or its underlying > > > > > > function decl wouldn't help. > > > > > > > > > > > > Am I missing something? > > > > > > > > > > I was thinking that where in your current patch you call > > > > > remove_object_under_construction, we could instead mark the object's value > > > > > CONSTRUCTOR as immutable. > > > > > > > > Ah, what you meant was to look at DECL_INITIAL of the object we're > > > > constructing, which could be a CONSTRUCTOR. Unfortunately, this > > > > DECL_INITIAL is null (in all the new tests when doing > > > > remove_object_under_construction), so there's nothing to mark as TREE_READONLY :/. > > > > > > There's a value in ctx->values, isn't there? > > > > Doesn't seem to be the case for e.g. > > > > struct A { > > int n; > > constexpr A() : n(1) { n = 2; } > > }; > > > > struct B { > > const A a; > > constexpr B(bool b) { > > if (b) > > const_cast<A &>(a).n = 3; // { dg-error "modifying a const object" } > > } > > }; > > > > constexpr B b(false); > > static_assert(b.a.n == 2, ""); > > > > Here we're constructing "b", its ctx->values->get(new_obj) is initially > > "{}". In the middle of constructing "b", we construct "b.a", but that > > has nothing in ctx->values. > > Right, subobjects aren't in ctx->values. In cxx_eval_call_expression we > have > > if (DECL_CONSTRUCTOR_P (fun)) > /* This can be null for a subobject constructor call, in > > which case what we care about is the initialization > > side-effects rather than the value. We could get at the > > value by evaluating *this, but we don't bother; there's > > no need to put such a call in the hash table. */ > result = lval ? ctx->object : ctx->ctor; > > Your patch already finds *this (b.a) and puts it in new_obj; if it's const > we can evaluate it to get the CONSTRUCTOR to set TREE_READONLY on. Ah, got it! This patch uses setting TREE_READONLY to achieve what I was after. I also needed to set TREE_READONLY in cxx_eval_constant_expression/DECL_EXPR. The additional evaluating will only happen for const-qual objects so I hope not very often. Any further comments? Thanks, Bootstrapped/regtested on x86_64-linux, ok for trunk? 2019-08-16 Marek Polacek <polacek@redhat.com> PR c++/91264 - detect modifying const objects in constexpr. * constexpr.c (object_under_construction_p): New function. (modifying_const_object_error): Likewise. (cxx_eval_call_expression): Set TREE_READONLY on a CONSTRUCTOR of a const-qualified object after it's been fully constructed. (modifying_const_object_p): New function. (cxx_eval_store_expression): Detect modifying a const object during constant expression evaluation. (cxx_eval_increment_expression): Use a better location when building up the store. (cxx_eval_constant_expression) <case DECL_EXPR>: Mark a constant object's constructor TREE_READONLY. * g++.dg/cpp1y/constexpr-tracking-const1.C: New test. * g++.dg/cpp1y/constexpr-tracking-const2.C: New test. * g++.dg/cpp1y/constexpr-tracking-const3.C: New test. * g++.dg/cpp1y/constexpr-tracking-const4.C: New test. * g++.dg/cpp1y/constexpr-tracking-const5.C: New test. * g++.dg/cpp1y/constexpr-tracking-const6.C: New test. * g++.dg/cpp1y/constexpr-tracking-const7.C: New test. * g++.dg/cpp1y/constexpr-tracking-const8.C: New test. * g++.dg/cpp1y/constexpr-tracking-const9.C: New test. * g++.dg/cpp1y/constexpr-tracking-const10.C: New test. * g++.dg/cpp1y/constexpr-tracking-const11.C: New test. diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c index 23f2a027e2f..0bd190cc366 100644 --- gcc/cp/constexpr.c +++ gcc/cp/constexpr.c @@ -1575,6 +1575,41 @@ clear_no_implicit_zero (tree ctor) } } +/* Return true if OBJ is an object currently under construction. */ + +static bool +object_under_construction_p (const constexpr_ctx *ctx, tree obj) +{ + if (!CLASS_TYPE_P (TREE_TYPE (obj))) + return false; + + tree *ctor = ctx->values->get (obj); + /* Subobjects won't be stored in ctx->values but we can get + its CONSTRUCTOR by evaluating *this. */ + if (ctor == NULL) + { + bool dummy1 = false, dummy2 = false; + tree e = cxx_eval_constant_expression (ctx, obj, /*lval*/false, &dummy1, + &dummy2); + return !TREE_READONLY (e); + } + else + return !TREE_READONLY (*ctor); +} + +/* Complain about a const object OBJ being modified in a constant expression. + EXPR is the MODIFY_EXPR expression performing the modification. */ + +static void +modifying_const_object_error (tree expr, tree obj) +{ + location_t loc = cp_expr_loc_or_input_loc (expr); + auto_diagnostic_group d; + error_at (loc, "modifying a const object %qE is not allowed in " + "a constant expression", TREE_OPERAND (expr, 0)); + inform (location_of (obj), "originally declared %<const%> here"); +} + /* Subroutine of cxx_eval_constant_expression. Evaluate the call expression tree T in the context of OLD_CALL expression evaluation. */ @@ -1775,6 +1810,19 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, depth_ok = push_cx_call_context (t); + /* Remember the object we are constructing. */ + tree new_obj = NULL_TREE; + if (DECL_CONSTRUCTOR_P (fun)) + { + /* In a constructor, it should be the first `this' argument. + At this point it has already been evaluated in the call + to cxx_bind_parameters_in_call. */ + new_obj = TREE_VEC_ELT (new_call.bindings, 0); + STRIP_NOPS (new_obj); + if (TREE_CODE (new_obj) == ADDR_EXPR) + new_obj = TREE_OPERAND (new_obj, 0); + } + tree result = NULL_TREE; constexpr_call *entry = NULL; @@ -1910,6 +1958,29 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, } } + /* At this point, the object's constructor will have run, so + the object is no longer under construction, and its possible + 'const' semantics now apply. Make a note of this fact by + marking the CONSTRUCTOR TREE_READONLY. */ + if (new_obj + && CLASS_TYPE_P (TREE_TYPE (new_obj)) + && CP_TYPE_CONST_P (TREE_TYPE (new_obj))) + { + tree *ctor = ctx->values->get (new_obj); + /* Subobjects won't be stored in ctx->values but we can get + its CONSTRUCTOR by evaluating *this. */ + if (ctor == NULL) + { + tree e = cxx_eval_constant_expression (ctx, new_obj, + /*lval*/false, + non_constant_p, + overflow_p); + TREE_READONLY (e) = true; + } + else + TREE_READONLY (*ctor) = true; + } + /* Forget the saved values of the callee's SAVE_EXPRs. */ unsigned int i; tree save_expr; @@ -3724,6 +3795,36 @@ maybe_simplify_trivial_copy (tree &target, tree &init) } } +/* Return true if we are modifying something that is const during constant + expression evaluation. CODE is the code of the statement, OBJ is the + object in question, MUTABLE_P is true if one of the subobjects were + declared mutable. */ + +static bool +modifying_const_object_p (const constexpr_ctx *ctx, tree_code code, tree obj, + bool mutable_p) +{ + /* If this is initialization, there's no problem. */ + if (code != MODIFY_EXPR) + return false; + + tree type = TREE_TYPE (obj); + + /* [basic.type.qualifier] "A const object is an object of type + const T or a non-mutable subobject of a const object." */ + return ((TREE_READONLY (obj) || CP_TYPE_CONST_P (type) + /* If it's an aggregate and any field is const, then it is + effectively const. */ + || (CLASS_TYPE_P (type) && C_TYPE_FIELDS_READONLY (type))) + && !mutable_p + /* [class.ctor]p5 "A constructor can be invoked for a const, + volatile, or const volatile object. const and volatile + semantics are not applied on an object under construction. + They come into effect when the constructor for the most + derived object ends." */ + && !object_under_construction_p (ctx, obj)); +} + /* Evaluate an INIT_EXPR or MODIFY_EXPR. */ static tree @@ -3773,6 +3874,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, /* Find the underlying variable. */ releasing_vec refs; tree object = NULL_TREE; + bool mutable_p = false; for (tree probe = target; object == NULL_TREE; ) { switch (TREE_CODE (probe)) @@ -3783,6 +3885,17 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, { tree ob = TREE_OPERAND (probe, 0); tree elt = TREE_OPERAND (probe, 1); + if (DECL_P (elt) && DECL_MUTABLE_P (elt)) + mutable_p = true; + if (evaluated + && modifying_const_object_p (ctx, TREE_CODE (t), probe, + mutable_p)) + { + if (!ctx->quiet) + modifying_const_object_error (t, probe); + *non_constant_p = true; + return t; + } if (TREE_CODE (probe) == ARRAY_REF) { elt = eval_and_check_array_index (ctx, probe, false, @@ -3811,6 +3924,14 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, } } + if (modifying_const_object_p (ctx, TREE_CODE (t), object, mutable_p)) + { + if (!ctx->quiet) + modifying_const_object_error (t, object); + *non_constant_p = true; + return t; + } + /* And then find/build up our initializer for the path to the subobject we're initializing. */ tree *valp; @@ -4063,7 +4184,8 @@ cxx_eval_increment_expression (const constexpr_ctx *ctx, tree t, VERIFY_CONSTANT (mod); /* Storing the modified value. */ - tree store = build2 (MODIFY_EXPR, type, op, mod); + tree store = build2_loc (cp_expr_loc_or_loc (t, input_location), + MODIFY_EXPR, type, op, mod); cxx_eval_constant_expression (ctx, store, true, non_constant_p, overflow_p); ggc_free (store); @@ -4650,6 +4772,11 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, non_constant_p, overflow_p); /* Don't share a CONSTRUCTOR that might be changed. */ init = unshare_constructor (init); + /* Remember that a constant object's constructor has already + ran. */ + if (CLASS_TYPE_P (TREE_TYPE (r)) + && CP_TYPE_CONST_P (TREE_TYPE (r))) + TREE_READONLY (init) = true; ctx->values->put (r, init); } else if (ctx == &new_ctx) diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const1.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const1.C new file mode 100644 index 00000000000..e081a535659 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const1.C @@ -0,0 +1,72 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +constexpr void +mod (int &r) +{ + r = 99; // { dg-error "modifying a const object" } +} + +constexpr int +fn1 () +{ + const int i = 0; // { dg-message "originally declared" } + mod (const_cast<int &>(i)); // { dg-message "in .constexpr. expansion of " } + return i; +} + +constexpr int i1 = fn1 (); // { dg-message "in .constexpr. expansion of " } + +constexpr int +fn2 () +{ + const int i = 5; // { dg-message "originally declared" } + const_cast<int &>(i) = 10; // { dg-error "modifying a const object" } + return i; +} + +constexpr int i2 = fn2 (); // { dg-message "in .constexpr. expansion of " } + +constexpr int +fn3 () +{ + const int i = 5; // { dg-message "originally declared" } + ++const_cast<int &>(i); // { dg-error "modifying a const object" } + return i; +} + +constexpr int i3 = fn3 (); // { dg-message "in .constexpr. expansion of " } + +constexpr int +fn4 () +{ + const int i = 5; // { dg-message "originally declared" } + const_cast<int &>(i)--; // { dg-error "modifying a const object" } + return i; +} + +constexpr int i4 = fn4 (); // { dg-message "in .constexpr. expansion of " } + +constexpr int +fn5 () +{ + const int i = 5; // { dg-message "originally declared" } + const_cast<int &>(i) += 2; // { dg-error "modifying a const object" } + return i; +} + +constexpr int i5 = fn5 (); // { dg-message "in .constexpr. expansion of " } + +constexpr int +fn6 () +{ + // This is OK. + int i = 3; + const int *cip = &i; + int *ip = const_cast<int *>(cip); + *ip = 4; + return i; +} + +constexpr int i6 = fn6 (); +static_assert(i6 == 4, ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const10.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const10.C new file mode 100644 index 00000000000..53acb37beb8 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const10.C @@ -0,0 +1,22 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct B { + B() = default; + int i; +}; + +constexpr B bar() +{ + constexpr B b = B(); // { dg-message "originally declared" } + B *p = const_cast<B*>(&b); + + p->i = 11; // { dg-error "modifying a const object" } + + return *p; +} + +void foo() +{ + constexpr B y = bar(); // { dg-message "in .constexpr. expansion of" } +} diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const11.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const11.C new file mode 100644 index 00000000000..2b351cd013a --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const11.C @@ -0,0 +1,16 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct S { + int a = 1; + int * ptr = &a; +}; + +constexpr bool f() { + auto const s = S{}; // { dg-message "originally declared" } + *s.ptr = 2; // { dg-error "modifying a const object" } + return s.a == 2; +} + +static_assert(f(), ""); // { dg-error "non-constant condition" } +// { dg-message "in 'constexpr' expansion of" "" { target *-*-* } .-1 } diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const2.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const2.C new file mode 100644 index 00000000000..9803309cace --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const2.C @@ -0,0 +1,23 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct X { + int j; + constexpr X() : j(0) { } +}; + +struct Y { + X x; + constexpr Y() : x{} { } +}; + +constexpr void +g () +{ + const Y y; // { dg-message "originally declared" } + Y *p = const_cast<Y *>(&y); + p->x.j = 99; // { dg-error "modifying a const object" } +} + +static_assert((g() , 1), ""); // { dg-error "non-constant condition" } +// { dg-message "in 'constexpr' expansion of" "" { target *-*-* } .-1 } diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const3.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const3.C new file mode 100644 index 00000000000..6853775c1e2 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const3.C @@ -0,0 +1,22 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct A { + int n; + constexpr A() : n(1) { n = 2; } +}; + +struct B { + const A a; + constexpr B(bool b) { + if (b) + const_cast<A &>(a).n = 3; // { dg-error "modifying a const object" } + } +}; + +constexpr B b(false); +static_assert(b.a.n == 2, ""); + +constexpr B b2(true); // { dg-message "in .constexpr. expansion of " } +// { dg-message "originally declared" "" { target *-*-* } .-1 } +static_assert((b2.a.n, 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const4.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const4.C new file mode 100644 index 00000000000..8263a7cc505 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const4.C @@ -0,0 +1,17 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct A { + const int n; + constexpr A() : n(1) { } +}; +struct B { + A a; + constexpr B() { + int *p = const_cast<int *>(&a.n); + *p = 3; // { dg-error "modifying a const object" } + } +}; +constexpr B b; // { dg-message "in .constexpr. expansion of " } +// { dg-message "originally declared" "" { target *-*-* } .-1 } +static_assert((b.a.n, 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const5.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const5.C new file mode 100644 index 00000000000..bea54fb4fde --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const5.C @@ -0,0 +1,17 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct A { + mutable int n; + constexpr A() : n(1) { n = 2; } +}; + +struct B { + const A a; + constexpr B() { + const_cast<A &>(a).n = 3; + } +}; + +constexpr B b{}; +static_assert((b.a.n, 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const6.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const6.C new file mode 100644 index 00000000000..07a7f835e11 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const6.C @@ -0,0 +1,22 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct X { + mutable int j; + constexpr X() : j(0) { } +}; + +struct Y { + X x; + constexpr Y() : x{} { } +}; + +constexpr void +g () +{ + const Y y; + Y *p = const_cast<Y *>(&y); + p->x.j = 99; +} + +static_assert((g() , 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const7.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const7.C new file mode 100644 index 00000000000..922e8ff126f --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const7.C @@ -0,0 +1,23 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct D { int n; }; + +struct C { const D d; }; + +struct A { + C c; + constexpr A() : c{} { } +}; + +struct B { + A a; + constexpr B() { + int &r = const_cast<int &>(a.c.d.n); + r = 3; // { dg-error "modifying a const object" } + } +}; + +constexpr B b{}; // { dg-message "in .constexpr. expansion of " } +// { dg-message "originally declared" "" { target *-*-* } .-1 } +static_assert((b.a.c.d.n, 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const8.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const8.C new file mode 100644 index 00000000000..2b3fe793f83 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const8.C @@ -0,0 +1,23 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct B { + int i; + double d; +}; + +constexpr B bar() +{ + constexpr B b = {10,10.10}; // { dg-message "originally declared" } + B *p = const_cast<B*>(&b); + + p->i = 11; // { dg-error "modifying a const object" } + p->d = 11.11; + + return *p; +} + +void foo() +{ + constexpr B y = bar(); // { dg-message "in .constexpr. expansion of" } +} diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const9.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const9.C new file mode 100644 index 00000000000..0edec4d05cf --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const9.C @@ -0,0 +1,23 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct B { + int i; + double d; +}; + +constexpr B bar() +{ + constexpr B b{}; // { dg-message "originally declared" } + B *p = const_cast<B*>(&b); + + p->i = 11; // { dg-error "modifying a const object" } + p->d = 11.11; + + return *p; +} + +void foo() +{ + constexpr B y = bar(); // { dg-message "in .constexpr. expansion of" } +}
On 8/16/19 5:11 AM, Marek Polacek wrote: > On Thu, Aug 15, 2019 at 08:21:25PM -0400, Jason Merrill wrote: >> On 8/15/19 5:34 PM, Marek Polacek wrote: >>> On Wed, Aug 14, 2019 at 02:50:13PM -0400, Jason Merrill wrote: >>>> On Thu, Aug 8, 2019 at 3:25 PM Marek Polacek <polacek@redhat.com> wrote: >>>>> >>>>> On Thu, Aug 08, 2019 at 11:06:17AM -0400, Jason Merrill wrote: >>>>>> On 8/6/19 3:20 PM, Marek Polacek wrote: >>>>>>> On Mon, Aug 05, 2019 at 03:54:19PM -0400, Jason Merrill wrote: >>>>>>>> On 7/31/19 3:26 PM, Marek Polacek wrote: >>>>>>>>> One of the features of constexpr is that it doesn't allow UB; and such UB must >>>>>>>>> be detected at compile-time. So running your code in a context that requires >>>>>>>>> a constant expression should ensure that the code in question is free of UB. >>>>>>>>> In effect, constexpr can serve as a sanitizer. E.g. this article describes in >>>>>>>>> in more detail: >>>>>>>>> <https://shafik.github.io/c++/undefined%20behavior/2019/05/11/explporing_undefined_behavior_using_constexpr.html> >>>>>>>>> >>>>>>>>> [dcl.type.cv]p4 says "Any attempt to modify a const object during its lifetime >>>>>>>>> results in undefined behavior." However, as the article above points out, we >>>>>>>>> aren't detecting that case in constexpr evaluation. >>>>>>>>> >>>>>>>>> This patch fixes that. It's not that easy, though, because we have to keep in >>>>>>>>> mind [class.ctor]p5: >>>>>>>>> "A constructor can be invoked for a const, volatile or const volatile object. >>>>>>>>> const and volatile semantics are not applied on an object under construction. >>>>>>>>> They come into effect when the constructor for the most derived object ends." >>>>>>>>> >>>>>>>>> I handled this by keeping a hash set which tracks objects under construction. >>>>>>>>> I considered other options, such as going up call_stack, but that wouldn't >>>>>>>>> work with trivial constructor/op=. It was also interesting to find out that >>>>>>>>> the definition of TREE_HAS_CONSTRUCTOR says "When appearing in a FIELD_DECL, >>>>>>>>> it means that this field has been duly initialized in its constructor" though >>>>>>>>> nowhere in the codebase do we set TREE_HAS_CONSTRUCTOR on a FIELD_DECL as far >>>>>>>>> as I can see. Unfortunately, using this bit proved useless for my needs here. >>>>>>>> >>>>>>>>> Also, be mindful of mutable subobjects. >>>>>>>>> >>>>>>>>> Does this approach look like an appropriate strategy for tracking objects' >>>>>>>>> construction? >>>>>>>> >>>>>>>> For scalar objects, we should be able to rely on INIT_EXPR vs. MODIFY_EXPR >>>>>>>> to distinguish between initialization and modification; for class objects, I >>>>>>> >>>>>>> This is already true: only class object go into the hash set. >>>>>>> >>>>>>>> wonder about setting a flag on the CONSTRUCTOR after initialization is >>>>>>>> complete to indicate that the value is now constant. >>>>>>> >>>>>>> But here we're not dealing with CONSTRUCTORs in the gcc sense (i.e. exprs with >>>>>>> TREE_CODE == CONSTRUCTOR). We have a CALL_EXPR like Y::Y ((struct Y *) &y), >>>>>>> which initializes the object "y". Setting a flag on the CALL_EXPR or its underlying >>>>>>> function decl wouldn't help. >>>>>>> >>>>>>> Am I missing something? >>>>>> >>>>>> I was thinking that where in your current patch you call >>>>>> remove_object_under_construction, we could instead mark the object's value >>>>>> CONSTRUCTOR as immutable. >>>>> >>>>> Ah, what you meant was to look at DECL_INITIAL of the object we're >>>>> constructing, which could be a CONSTRUCTOR. Unfortunately, this >>>>> DECL_INITIAL is null (in all the new tests when doing >>>>> remove_object_under_construction), so there's nothing to mark as TREE_READONLY :/. >>>> >>>> There's a value in ctx->values, isn't there? >>> >>> Doesn't seem to be the case for e.g. >>> >>> struct A { >>> int n; >>> constexpr A() : n(1) { n = 2; } >>> }; >>> >>> struct B { >>> const A a; >>> constexpr B(bool b) { >>> if (b) >>> const_cast<A &>(a).n = 3; // { dg-error "modifying a const object" } >>> } >>> }; >>> >>> constexpr B b(false); >>> static_assert(b.a.n == 2, ""); >>> >>> Here we're constructing "b", its ctx->values->get(new_obj) is initially >>> "{}". In the middle of constructing "b", we construct "b.a", but that >>> has nothing in ctx->values. >> >> Right, subobjects aren't in ctx->values. In cxx_eval_call_expression we >> have >> >> if (DECL_CONSTRUCTOR_P (fun)) >> /* This can be null for a subobject constructor call, in >> >> which case what we care about is the initialization >> >> side-effects rather than the value. We could get at the >> >> value by evaluating *this, but we don't bother; there's >> >> no need to put such a call in the hash table. */ >> result = lval ? ctx->object : ctx->ctor; >> >> Your patch already finds *this (b.a) and puts it in new_obj; if it's const >> we can evaluate it to get the CONSTRUCTOR to set TREE_READONLY on. > > Ah, got it! This patch uses setting TREE_READONLY to achieve what I was after. > I also needed to set TREE_READONLY in cxx_eval_constant_expression/DECL_EXPR. > The additional evaluating will only happen for const-qual objects so I hope not > very often. > > Any further comments? Thanks, > > @@ -1910,6 +1958,29 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, > > + /* At this point, the object's constructor will have run, so > + the object is no longer under construction, and its possible > + 'const' semantics now apply. Make a note of this fact by > + marking the CONSTRUCTOR TREE_READONLY. */ > + if (new_obj > + && CLASS_TYPE_P (TREE_TYPE (new_obj)) > + && CP_TYPE_CONST_P (TREE_TYPE (new_obj))) > + { > + tree *ctor = ctx->values->get (new_obj); I don't think trying ctx->values->get first is a win, let's go straight to cxx_eval_constant_expression. > +/* Return true if we are modifying something that is const during constant > + expression evaluation. CODE is the code of the statement, OBJ is the > + object in question, MUTABLE_P is true if one of the subobjects were > + declared mutable. */ > + > +static bool > +modifying_const_object_p (const constexpr_ctx *ctx, tree_code code, tree obj, > + bool mutable_p) > +{ > + /* If this is initialization, there's no problem. */ > + if (code != MODIFY_EXPR) > + return false; > + > + tree type = TREE_TYPE (obj); > + > + /* [basic.type.qualifier] "A const object is an object of type > + const T or a non-mutable subobject of a const object." */ > + return ((TREE_READONLY (obj) || CP_TYPE_CONST_P (type) > + /* If it's an aggregate and any field is const, then it is > + effectively const. */ > + || (CLASS_TYPE_P (type) && C_TYPE_FIELDS_READONLY (type))) This seems wrong; if one field is const, we can still modify other fields. I don't see a test for that case. > @@ -3783,6 +3885,17 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, > { > tree ob = TREE_OPERAND (probe, 0); > tree elt = TREE_OPERAND (probe, 1); > + if (DECL_P (elt) && DECL_MUTABLE_P (elt)) > + mutable_p = true; > + if (evaluated > + && modifying_const_object_p (ctx, TREE_CODE (t), probe, > + mutable_p)) > + { > + if (!ctx->quiet) > + modifying_const_object_error (t, probe); > + *non_constant_p = true; > + return t; > + } What if there's a mutable member further down, i.e. struct A { mutable int i; }; struct B { A a; }; const B b; b.a.i = 42; ? And also... > @@ -3811,6 +3924,14 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, > > + if (modifying_const_object_p (ctx, TREE_CODE (t), object, mutable_p)) > + { > + if (!ctx->quiet) > + modifying_const_object_error (t, object); > + *non_constant_p = true; > + return t; > + } ...we are already collecting the CONSTRUCTORs that we're dealing with in the "ctors" stack, we shouldn't need to evaluate object at this point. I'd expect the topmost class-type CONSTRUCTOR on the stack (if any) to be the one we want to look at. I'd think you could do away with much of modifying_const_object_p. > @@ -4650,6 +4772,11 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, > non_constant_p, overflow_p); > /* Don't share a CONSTRUCTOR that might be changed. */ > init = unshare_constructor (init); > + /* Remember that a constant object's constructor has already > + ran. */ "has...run" Jason
On Fri, Aug 16, 2019 at 05:40:39PM -0700, Jason Merrill wrote: > On 8/16/19 5:11 AM, Marek Polacek wrote: > > On Thu, Aug 15, 2019 at 08:21:25PM -0400, Jason Merrill wrote: > > > On 8/15/19 5:34 PM, Marek Polacek wrote: > > > > On Wed, Aug 14, 2019 at 02:50:13PM -0400, Jason Merrill wrote: > > > > > On Thu, Aug 8, 2019 at 3:25 PM Marek Polacek <polacek@redhat.com> wrote: > > > > > > > > > > > > On Thu, Aug 08, 2019 at 11:06:17AM -0400, Jason Merrill wrote: > > > > > > > On 8/6/19 3:20 PM, Marek Polacek wrote: > > > > > > > > On Mon, Aug 05, 2019 at 03:54:19PM -0400, Jason Merrill wrote: > > > > > > > > > On 7/31/19 3:26 PM, Marek Polacek wrote: > > > > > > > > > > One of the features of constexpr is that it doesn't allow UB; and such UB must > > > > > > > > > > be detected at compile-time. So running your code in a context that requires > > > > > > > > > > a constant expression should ensure that the code in question is free of UB. > > > > > > > > > > In effect, constexpr can serve as a sanitizer. E.g. this article describes in > > > > > > > > > > in more detail: > > > > > > > > > > <https://shafik.github.io/c++/undefined%20behavior/2019/05/11/explporing_undefined_behavior_using_constexpr.html> > > > > > > > > > > > > > > > > > > > > [dcl.type.cv]p4 says "Any attempt to modify a const object during its lifetime > > > > > > > > > > results in undefined behavior." However, as the article above points out, we > > > > > > > > > > aren't detecting that case in constexpr evaluation. > > > > > > > > > > > > > > > > > > > > This patch fixes that. It's not that easy, though, because we have to keep in > > > > > > > > > > mind [class.ctor]p5: > > > > > > > > > > "A constructor can be invoked for a const, volatile or const volatile object. > > > > > > > > > > const and volatile semantics are not applied on an object under construction. > > > > > > > > > > They come into effect when the constructor for the most derived object ends." > > > > > > > > > > > > > > > > > > > > I handled this by keeping a hash set which tracks objects under construction. > > > > > > > > > > I considered other options, such as going up call_stack, but that wouldn't > > > > > > > > > > work with trivial constructor/op=. It was also interesting to find out that > > > > > > > > > > the definition of TREE_HAS_CONSTRUCTOR says "When appearing in a FIELD_DECL, > > > > > > > > > > it means that this field has been duly initialized in its constructor" though > > > > > > > > > > nowhere in the codebase do we set TREE_HAS_CONSTRUCTOR on a FIELD_DECL as far > > > > > > > > > > as I can see. Unfortunately, using this bit proved useless for my needs here. > > > > > > > > > > > > > > > > > > > Also, be mindful of mutable subobjects. > > > > > > > > > > > > > > > > > > > > Does this approach look like an appropriate strategy for tracking objects' > > > > > > > > > > construction? > > > > > > > > > > > > > > > > > > For scalar objects, we should be able to rely on INIT_EXPR vs. MODIFY_EXPR > > > > > > > > > to distinguish between initialization and modification; for class objects, I > > > > > > > > > > > > > > > > This is already true: only class object go into the hash set. > > > > > > > > > > > > > > > > > wonder about setting a flag on the CONSTRUCTOR after initialization is > > > > > > > > > complete to indicate that the value is now constant. > > > > > > > > > > > > > > > > But here we're not dealing with CONSTRUCTORs in the gcc sense (i.e. exprs with > > > > > > > > TREE_CODE == CONSTRUCTOR). We have a CALL_EXPR like Y::Y ((struct Y *) &y), > > > > > > > > which initializes the object "y". Setting a flag on the CALL_EXPR or its underlying > > > > > > > > function decl wouldn't help. > > > > > > > > > > > > > > > > Am I missing something? > > > > > > > > > > > > > > I was thinking that where in your current patch you call > > > > > > > remove_object_under_construction, we could instead mark the object's value > > > > > > > CONSTRUCTOR as immutable. > > > > > > > > > > > > Ah, what you meant was to look at DECL_INITIAL of the object we're > > > > > > constructing, which could be a CONSTRUCTOR. Unfortunately, this > > > > > > DECL_INITIAL is null (in all the new tests when doing > > > > > > remove_object_under_construction), so there's nothing to mark as TREE_READONLY :/. > > > > > > > > > > There's a value in ctx->values, isn't there? > > > > > > > > Doesn't seem to be the case for e.g. > > > > > > > > struct A { > > > > int n; > > > > constexpr A() : n(1) { n = 2; } > > > > }; > > > > > > > > struct B { > > > > const A a; > > > > constexpr B(bool b) { > > > > if (b) > > > > const_cast<A &>(a).n = 3; // { dg-error "modifying a const object" } > > > > } > > > > }; > > > > > > > > constexpr B b(false); > > > > static_assert(b.a.n == 2, ""); > > > > > > > > Here we're constructing "b", its ctx->values->get(new_obj) is initially > > > > "{}". In the middle of constructing "b", we construct "b.a", but that > > > > has nothing in ctx->values. > > > > > > Right, subobjects aren't in ctx->values. In cxx_eval_call_expression we > > > have > > > > > > if (DECL_CONSTRUCTOR_P (fun)) > > > /* This can be null for a subobject constructor call, in > > > > > > which case what we care about is the initialization > > > > > > side-effects rather than the value. We could get at the > > > > > > value by evaluating *this, but we don't bother; there's > > > > > > no need to put such a call in the hash table. */ > > > result = lval ? ctx->object : ctx->ctor; > > > > > > Your patch already finds *this (b.a) and puts it in new_obj; if it's const > > > we can evaluate it to get the CONSTRUCTOR to set TREE_READONLY on. > > > > Ah, got it! This patch uses setting TREE_READONLY to achieve what I was after. > > I also needed to set TREE_READONLY in cxx_eval_constant_expression/DECL_EXPR. > > The additional evaluating will only happen for const-qual objects so I hope not > > very often. > > > > Any further comments? Thanks, > > > > @@ -1910,6 +1958,29 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, > > > > + /* At this point, the object's constructor will have run, so > > + the object is no longer under construction, and its possible > > + 'const' semantics now apply. Make a note of this fact by > > + marking the CONSTRUCTOR TREE_READONLY. */ > > + if (new_obj > > + && CLASS_TYPE_P (TREE_TYPE (new_obj)) > > + && CP_TYPE_CONST_P (TREE_TYPE (new_obj))) > > + { > > + tree *ctor = ctx->values->get (new_obj); > > I don't think trying ctx->values->get first is a win, let's go straight to > cxx_eval_constant_expression. Done. > > +/* Return true if we are modifying something that is const during constant > > + expression evaluation. CODE is the code of the statement, OBJ is the > > + object in question, MUTABLE_P is true if one of the subobjects were > > + declared mutable. */ > > + > > +static bool > > +modifying_const_object_p (const constexpr_ctx *ctx, tree_code code, tree obj, > > + bool mutable_p) > > +{ > > + /* If this is initialization, there's no problem. */ > > + if (code != MODIFY_EXPR) > > + return false; > > + > > + tree type = TREE_TYPE (obj); > > + > > + /* [basic.type.qualifier] "A const object is an object of type > > + const T or a non-mutable subobject of a const object." */ > > + return ((TREE_READONLY (obj) || CP_TYPE_CONST_P (type) > > + /* If it's an aggregate and any field is const, then it is > > + effectively const. */ > > + || (CLASS_TYPE_P (type) && C_TYPE_FIELDS_READONLY (type))) > > This seems wrong; if one field is const, we can still modify other fields. I > don't see a test for that case. I took it from cp_build_modify_expr but it's not really needed, so dropped. > > @@ -3783,6 +3885,17 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, > > { > > tree ob = TREE_OPERAND (probe, 0); > > tree elt = TREE_OPERAND (probe, 1); > > + if (DECL_P (elt) && DECL_MUTABLE_P (elt)) > > + mutable_p = true; > > + if (evaluated > > + && modifying_const_object_p (ctx, TREE_CODE (t), probe, > > + mutable_p)) > > + { > > + if (!ctx->quiet) > > + modifying_const_object_error (t, probe); > > + *non_constant_p = true; > > + return t; > > + } > > What if there's a mutable member further down, i.e. > > struct A { mutable int i; }; > struct B { A a; }; > const B b; > b.a.i = 42; > > ? And also... This works. We check mutable going from the innermost object to the outermost, so if we have b.a.n, we check n first. Some of the new tests are exercising this. > > @@ -3811,6 +3924,14 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, > > + if (modifying_const_object_p (ctx, TREE_CODE (t), object, mutable_p)) > > + { > > + if (!ctx->quiet) > > + modifying_const_object_error (t, object); > > + *non_constant_p = true; > > + return t; > > + } > > ...we are already collecting the CONSTRUCTORs that we're dealing with in the > "ctors" stack, we shouldn't need to evaluate object at this point. I'd > expect the topmost class-type CONSTRUCTOR on the stack (if any) to be the > one we want to look at. I'd think you could do away with much of > modifying_const_object_p. Oh, got it; wish I'd thought of that sooner. We can't just check the topmost CONSTRUCTOR though, the yuge comment I've added has an example when that would not be enough. And yes, I could 86 most of modifying_const_object_p. > > @@ -4650,6 +4772,11 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, > > non_constant_p, overflow_p); > > /* Don't share a CONSTRUCTOR that might be changed. */ > > init = unshare_constructor (init); > > + /* Remember that a constant object's constructor has already > > + ran. */ > > "has...run" Yikes. Thanks for the review, it allowed me to make the patch more concise. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2019-08-18 Marek Polacek <polacek@redhat.com> PR c++/91264 - detect modifying const objects in constexpr. * constexpr.c (modifying_const_object_error): New function. (cxx_eval_call_expression): Set TREE_READONLY on a CONSTRUCTOR of a const-qualified object after it's been fully constructed. (modifying_const_object_p): New function. (cxx_eval_store_expression): Detect modifying a const object during constant expression evaluation. (cxx_eval_increment_expression): Use a better location when building up the store. (cxx_eval_constant_expression) <case DECL_EXPR>: Mark a constant object's constructor TREE_READONLY. * g++.dg/cpp1y/constexpr-tracking-const1.C: New test. * g++.dg/cpp1y/constexpr-tracking-const2.C: New test. * g++.dg/cpp1y/constexpr-tracking-const3.C: New test. * g++.dg/cpp1y/constexpr-tracking-const4.C: New test. * g++.dg/cpp1y/constexpr-tracking-const5.C: New test. * g++.dg/cpp1y/constexpr-tracking-const6.C: New test. * g++.dg/cpp1y/constexpr-tracking-const7.C: New test. * g++.dg/cpp1y/constexpr-tracking-const8.C: New test. * g++.dg/cpp1y/constexpr-tracking-const9.C: New test. * g++.dg/cpp1y/constexpr-tracking-const10.C: New test. * g++.dg/cpp1y/constexpr-tracking-const11.C: New test. * g++.dg/cpp1y/constexpr-tracking-const12.C: New test. * g++.dg/cpp1y/constexpr-tracking-const13.C: New test. * g++.dg/cpp1y/constexpr-tracking-const14.C: New test. diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c index 23f2a027e2f..e5ed326efca 100644 --- gcc/cp/constexpr.c +++ gcc/cp/constexpr.c @@ -1575,6 +1575,19 @@ clear_no_implicit_zero (tree ctor) } } +/* Complain about a const object OBJ being modified in a constant expression. + EXPR is the MODIFY_EXPR expression performing the modification. */ + +static void +modifying_const_object_error (tree expr, tree obj) +{ + location_t loc = cp_expr_loc_or_input_loc (expr); + auto_diagnostic_group d; + error_at (loc, "modifying a const object %qE is not allowed in " + "a constant expression", TREE_OPERAND (expr, 0)); + inform (location_of (obj), "originally declared %<const%> here"); +} + /* Subroutine of cxx_eval_constant_expression. Evaluate the call expression tree T in the context of OLD_CALL expression evaluation. */ @@ -1775,6 +1788,19 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, depth_ok = push_cx_call_context (t); + /* Remember the object we are constructing. */ + tree new_obj = NULL_TREE; + if (DECL_CONSTRUCTOR_P (fun)) + { + /* In a constructor, it should be the first `this' argument. + At this point it has already been evaluated in the call + to cxx_bind_parameters_in_call. */ + new_obj = TREE_VEC_ELT (new_call.bindings, 0); + STRIP_NOPS (new_obj); + if (TREE_CODE (new_obj) == ADDR_EXPR) + new_obj = TREE_OPERAND (new_obj, 0); + } + tree result = NULL_TREE; constexpr_call *entry = NULL; @@ -1910,6 +1936,23 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, } } + /* At this point, the object's constructor will have run, so + the object is no longer under construction, and its possible + 'const' semantics now apply. Make a note of this fact by + marking the CONSTRUCTOR TREE_READONLY. */ + if (new_obj + && CLASS_TYPE_P (TREE_TYPE (new_obj)) + && CP_TYPE_CONST_P (TREE_TYPE (new_obj))) + { + /* Subobjects might not be stored in ctx->values but we can + get its CONSTRUCTOR by evaluating *this. */ + tree e = cxx_eval_constant_expression (ctx, new_obj, + /*lval*/false, + non_constant_p, + overflow_p); + TREE_READONLY (e) = true; + } + /* Forget the saved values of the callee's SAVE_EXPRs. */ unsigned int i; tree save_expr; @@ -3724,6 +3767,26 @@ maybe_simplify_trivial_copy (tree &target, tree &init) } } +/* Return true if we are modifying something that is const during constant + expression evaluation. CODE is the code of the statement, OBJ is the + object in question, MUTABLE_P is true if one of the subobjects were + declared mutable. */ + +static bool +modifying_const_object_p (tree_code code, tree obj, bool mutable_p) +{ + /* If this is initialization, there's no problem. */ + if (code != MODIFY_EXPR) + return false; + + /* [basic.type.qualifier] "A const object is an object of type + const T or a non-mutable subobject of a const object." */ + if (mutable_p) + return false; + + return (TREE_READONLY (obj) || CP_TYPE_CONST_P (TREE_TYPE (obj))); +} + /* Evaluate an INIT_EXPR or MODIFY_EXPR. */ static tree @@ -3773,6 +3836,9 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, /* Find the underlying variable. */ releasing_vec refs; tree object = NULL_TREE; + /* If we're modifying a const object, save it. */ + tree const_object_being_modified = NULL_TREE; + bool mutable_p = false; for (tree probe = target; object == NULL_TREE; ) { switch (TREE_CODE (probe)) @@ -3783,6 +3849,12 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, { tree ob = TREE_OPERAND (probe, 0); tree elt = TREE_OPERAND (probe, 1); + if (DECL_P (elt) && DECL_MUTABLE_P (elt)) + mutable_p = true; + if (evaluated + && modifying_const_object_p (TREE_CODE (t), probe, mutable_p) + && const_object_being_modified == NULL_TREE) + const_object_being_modified = probe; if (TREE_CODE (probe) == ARRAY_REF) { elt = eval_and_check_array_index (ctx, probe, false, @@ -3811,6 +3883,10 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, } } + if (modifying_const_object_p (TREE_CODE (t), object, mutable_p) + && const_object_being_modified == NULL_TREE) + const_object_being_modified = object; + /* And then find/build up our initializer for the path to the subobject we're initializing. */ tree *valp; @@ -3950,6 +4026,61 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, valp = &cep->value; } + /* Detect modifying a constant object in constexpr evaluation. + We have found a const object that is being modified. Figure out + if we need to issue an error. Consider + + struct A { + int n; + constexpr A() : n(1) { n = 2; } // #1 + }; + struct B { + const A a; + constexpr B() { a.n = 3; } // #2 + }; + constexpr B b{}; + + #1 is OK, since we're modifying an object under construction, but + #2 is wrong, since "a" is const and has been fully constructed. + We track the "under construction" state by using the TREE_READONLY + bit in the object's CONSTRUCTOR. For the example above, the *ctors + stack at the point of #2 will look like: + + ctors[0] = {.a={.n=2}} TREE_READONLY = 0 + ctors[1] = {.n=2} TREE_READONLY = 1 + + and we're modifying "b.a", so we search the stack and see that the + constructor for "b.a" has already run. */ + if (const_object_being_modified) + { + bool fail = false; + if (!CLASS_TYPE_P (TREE_TYPE (const_object_being_modified))) + fail = true; + else + { + /* [class.ctor]p5 "A constructor can be invoked for a const, + volatile, or const volatile object. const and volatile + semantics are not applied on an object under construction. + They come into effect when the constructor for the most + derived object ends." */ + tree elt; + unsigned int i; + FOR_EACH_VEC_ELT (*ctors, i, elt) + if (CLASS_TYPE_P (TREE_TYPE (elt)) && TREE_READONLY (elt)) + { + fail = true; + break; + } + } + if (fail) + { + if (!ctx->quiet) + modifying_const_object_error (t, const_object_being_modified); + *non_constant_p = true; + return t; + } + } + if (!preeval) { /* Create a new CONSTRUCTOR in case evaluation of the initializer @@ -4063,7 +4194,8 @@ cxx_eval_increment_expression (const constexpr_ctx *ctx, tree t, VERIFY_CONSTANT (mod); /* Storing the modified value. */ - tree store = build2 (MODIFY_EXPR, type, op, mod); + tree store = build2_loc (cp_expr_loc_or_loc (t, input_location), + MODIFY_EXPR, type, op, mod); cxx_eval_constant_expression (ctx, store, true, non_constant_p, overflow_p); ggc_free (store); @@ -4650,6 +4782,11 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, non_constant_p, overflow_p); /* Don't share a CONSTRUCTOR that might be changed. */ init = unshare_constructor (init); + /* Remember that a constant object's constructor has already + run. */ + if (CLASS_TYPE_P (TREE_TYPE (r)) + && CP_TYPE_CONST_P (TREE_TYPE (r))) + TREE_READONLY (init) = true; ctx->values->put (r, init); } else if (ctx == &new_ctx) diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const1.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const1.C new file mode 100644 index 00000000000..e081a535659 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const1.C @@ -0,0 +1,72 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +constexpr void +mod (int &r) +{ + r = 99; // { dg-error "modifying a const object" } +} + +constexpr int +fn1 () +{ + const int i = 0; // { dg-message "originally declared" } + mod (const_cast<int &>(i)); // { dg-message "in .constexpr. expansion of " } + return i; +} + +constexpr int i1 = fn1 (); // { dg-message "in .constexpr. expansion of " } + +constexpr int +fn2 () +{ + const int i = 5; // { dg-message "originally declared" } + const_cast<int &>(i) = 10; // { dg-error "modifying a const object" } + return i; +} + +constexpr int i2 = fn2 (); // { dg-message "in .constexpr. expansion of " } + +constexpr int +fn3 () +{ + const int i = 5; // { dg-message "originally declared" } + ++const_cast<int &>(i); // { dg-error "modifying a const object" } + return i; +} + +constexpr int i3 = fn3 (); // { dg-message "in .constexpr. expansion of " } + +constexpr int +fn4 () +{ + const int i = 5; // { dg-message "originally declared" } + const_cast<int &>(i)--; // { dg-error "modifying a const object" } + return i; +} + +constexpr int i4 = fn4 (); // { dg-message "in .constexpr. expansion of " } + +constexpr int +fn5 () +{ + const int i = 5; // { dg-message "originally declared" } + const_cast<int &>(i) += 2; // { dg-error "modifying a const object" } + return i; +} + +constexpr int i5 = fn5 (); // { dg-message "in .constexpr. expansion of " } + +constexpr int +fn6 () +{ + // This is OK. + int i = 3; + const int *cip = &i; + int *ip = const_cast<int *>(cip); + *ip = 4; + return i; +} + +constexpr int i6 = fn6 (); +static_assert(i6 == 4, ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const10.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const10.C new file mode 100644 index 00000000000..53acb37beb8 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const10.C @@ -0,0 +1,22 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct B { + B() = default; + int i; +}; + +constexpr B bar() +{ + constexpr B b = B(); // { dg-message "originally declared" } + B *p = const_cast<B*>(&b); + + p->i = 11; // { dg-error "modifying a const object" } + + return *p; +} + +void foo() +{ + constexpr B y = bar(); // { dg-message "in .constexpr. expansion of" } +} diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const11.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const11.C new file mode 100644 index 00000000000..2b351cd013a --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const11.C @@ -0,0 +1,16 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct S { + int a = 1; + int * ptr = &a; +}; + +constexpr bool f() { + auto const s = S{}; // { dg-message "originally declared" } + *s.ptr = 2; // { dg-error "modifying a const object" } + return s.a == 2; +} + +static_assert(f(), ""); // { dg-error "non-constant condition" } +// { dg-message "in 'constexpr' expansion of" "" { target *-*-* } .-1 } diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const12.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const12.C new file mode 100644 index 00000000000..d83e2794f7f --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const12.C @@ -0,0 +1,17 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct A { + const int n; + int m; + constexpr A() : n(1), m(2) { } +}; +struct B { + A a; + constexpr B() { + int *p = &a.m; + *p = 3; + } +}; +constexpr B b; +static_assert(b.a.m == 3, ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const13.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const13.C new file mode 100644 index 00000000000..bc7faa3c250 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const13.C @@ -0,0 +1,20 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct A { + mutable int i; + constexpr A() : i(0) { } +}; +struct B { + A a; + constexpr B() : a{} { } +}; + +constexpr void +g () +{ + const B b; + b.a.i = 42; +} + +static_assert((g(), 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const14.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const14.C new file mode 100644 index 00000000000..45c4fcf50be --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const14.C @@ -0,0 +1,38 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct F { + const int f; + constexpr F() : f(9) { } +}; + +struct C { + int n; + const F f; + constexpr C() : n(1) { n = 66; } +}; + +struct A { + int r; + const C c; + constexpr A() : r(11) { r = 14; const_cast<C &>(c).n = 42; } // { dg-error "modifying a const object" } +}; + +struct D { + const A a; + constexpr D() { } // { dg-message "in .constexpr. expansion of" } +}; + +struct E { + const D d; + constexpr E() { } // { dg-message "in .constexpr. expansion of" } +}; + +struct B { + const E e; + constexpr B(bool) { } // { dg-message "in .constexpr. expansion of" } +}; + +constexpr B b(false); // { dg-message "in .constexpr. expansion of" } +// { dg-message "originally declared" "" { target *-*-* } .-1 } +static_assert(b.e.d.a.c.n == 2, ""); // { dg-error "non-constant condition" } diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const2.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const2.C new file mode 100644 index 00000000000..9803309cace --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const2.C @@ -0,0 +1,23 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct X { + int j; + constexpr X() : j(0) { } +}; + +struct Y { + X x; + constexpr Y() : x{} { } +}; + +constexpr void +g () +{ + const Y y; // { dg-message "originally declared" } + Y *p = const_cast<Y *>(&y); + p->x.j = 99; // { dg-error "modifying a const object" } +} + +static_assert((g() , 1), ""); // { dg-error "non-constant condition" } +// { dg-message "in 'constexpr' expansion of" "" { target *-*-* } .-1 } diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const3.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const3.C new file mode 100644 index 00000000000..6853775c1e2 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const3.C @@ -0,0 +1,22 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct A { + int n; + constexpr A() : n(1) { n = 2; } +}; + +struct B { + const A a; + constexpr B(bool b) { + if (b) + const_cast<A &>(a).n = 3; // { dg-error "modifying a const object" } + } +}; + +constexpr B b(false); +static_assert(b.a.n == 2, ""); + +constexpr B b2(true); // { dg-message "in .constexpr. expansion of " } +// { dg-message "originally declared" "" { target *-*-* } .-1 } +static_assert((b2.a.n, 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const4.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const4.C new file mode 100644 index 00000000000..8263a7cc505 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const4.C @@ -0,0 +1,17 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct A { + const int n; + constexpr A() : n(1) { } +}; +struct B { + A a; + constexpr B() { + int *p = const_cast<int *>(&a.n); + *p = 3; // { dg-error "modifying a const object" } + } +}; +constexpr B b; // { dg-message "in .constexpr. expansion of " } +// { dg-message "originally declared" "" { target *-*-* } .-1 } +static_assert((b.a.n, 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const5.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const5.C new file mode 100644 index 00000000000..bea54fb4fde --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const5.C @@ -0,0 +1,17 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct A { + mutable int n; + constexpr A() : n(1) { n = 2; } +}; + +struct B { + const A a; + constexpr B() { + const_cast<A &>(a).n = 3; + } +}; + +constexpr B b{}; +static_assert((b.a.n, 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const6.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const6.C new file mode 100644 index 00000000000..54d83b17d39 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const6.C @@ -0,0 +1,22 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct X { + mutable int j; + constexpr X() : j(0) { } +}; + +struct Y { + X x; + constexpr Y() : x{} { } +}; + +constexpr void +g () +{ + const Y y; + Y *p = const_cast<Y *>(&y); + p->x.j = 99; +} + +static_assert((g(), 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const7.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const7.C new file mode 100644 index 00000000000..922e8ff126f --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const7.C @@ -0,0 +1,23 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct D { int n; }; + +struct C { const D d; }; + +struct A { + C c; + constexpr A() : c{} { } +}; + +struct B { + A a; + constexpr B() { + int &r = const_cast<int &>(a.c.d.n); + r = 3; // { dg-error "modifying a const object" } + } +}; + +constexpr B b{}; // { dg-message "in .constexpr. expansion of " } +// { dg-message "originally declared" "" { target *-*-* } .-1 } +static_assert((b.a.c.d.n, 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const8.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const8.C new file mode 100644 index 00000000000..2b3fe793f83 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const8.C @@ -0,0 +1,23 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct B { + int i; + double d; +}; + +constexpr B bar() +{ + constexpr B b = {10,10.10}; // { dg-message "originally declared" } + B *p = const_cast<B*>(&b); + + p->i = 11; // { dg-error "modifying a const object" } + p->d = 11.11; + + return *p; +} + +void foo() +{ + constexpr B y = bar(); // { dg-message "in .constexpr. expansion of" } +} diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const9.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const9.C new file mode 100644 index 00000000000..0edec4d05cf --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const9.C @@ -0,0 +1,23 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct B { + int i; + double d; +}; + +constexpr B bar() +{ + constexpr B b{}; // { dg-message "originally declared" } + B *p = const_cast<B*>(&b); + + p->i = 11; // { dg-error "modifying a const object" } + p->d = 11.11; + + return *p; +} + +void foo() +{ + constexpr B y = bar(); // { dg-message "in .constexpr. expansion of" } +}
On 8/18/19 6:52 AM, Marek Polacek wrote: > On Fri, Aug 16, 2019 at 05:40:39PM -0700, Jason Merrill wrote: >> On 8/16/19 5:11 AM, Marek Polacek wrote: >>> On Thu, Aug 15, 2019 at 08:21:25PM -0400, Jason Merrill wrote: >>>> On 8/15/19 5:34 PM, Marek Polacek wrote: >>>>> On Wed, Aug 14, 2019 at 02:50:13PM -0400, Jason Merrill wrote: >>>>>> On Thu, Aug 8, 2019 at 3:25 PM Marek Polacek <polacek@redhat.com> wrote: >>>>>>> >>>>>>> On Thu, Aug 08, 2019 at 11:06:17AM -0400, Jason Merrill wrote: >>>>>>>> On 8/6/19 3:20 PM, Marek Polacek wrote: >>>>>>>>> On Mon, Aug 05, 2019 at 03:54:19PM -0400, Jason Merrill wrote: >>>>>>>>>> On 7/31/19 3:26 PM, Marek Polacek wrote: >>>>>>>>>>> One of the features of constexpr is that it doesn't allow UB; and such UB must >>>>>>>>>>> be detected at compile-time. So running your code in a context that requires >>>>>>>>>>> a constant expression should ensure that the code in question is free of UB. >>>>>>>>>>> In effect, constexpr can serve as a sanitizer. E.g. this article describes in >>>>>>>>>>> in more detail: >>>>>>>>>>> <https://shafik.github.io/c++/undefined%20behavior/2019/05/11/explporing_undefined_behavior_using_constexpr.html> >>>>>>>>>>> >>>>>>>>>>> [dcl.type.cv]p4 says "Any attempt to modify a const object during its lifetime >>>>>>>>>>> results in undefined behavior." However, as the article above points out, we >>>>>>>>>>> aren't detecting that case in constexpr evaluation. >>>>>>>>>>> >>>>>>>>>>> This patch fixes that. It's not that easy, though, because we have to keep in >>>>>>>>>>> mind [class.ctor]p5: >>>>>>>>>>> "A constructor can be invoked for a const, volatile or const volatile object. >>>>>>>>>>> const and volatile semantics are not applied on an object under construction. >>>>>>>>>>> They come into effect when the constructor for the most derived object ends." >>>>>>>>>>> >>>>>>>>>>> I handled this by keeping a hash set which tracks objects under construction. >>>>>>>>>>> I considered other options, such as going up call_stack, but that wouldn't >>>>>>>>>>> work with trivial constructor/op=. It was also interesting to find out that >>>>>>>>>>> the definition of TREE_HAS_CONSTRUCTOR says "When appearing in a FIELD_DECL, >>>>>>>>>>> it means that this field has been duly initialized in its constructor" though >>>>>>>>>>> nowhere in the codebase do we set TREE_HAS_CONSTRUCTOR on a FIELD_DECL as far >>>>>>>>>>> as I can see. Unfortunately, using this bit proved useless for my needs here. >>>>>>>>>> >>>>>>>>>>> Also, be mindful of mutable subobjects. >>>>>>>>>>> >>>>>>>>>>> Does this approach look like an appropriate strategy for tracking objects' >>>>>>>>>>> construction? >>>>>>>>>> >>>>>>>>>> For scalar objects, we should be able to rely on INIT_EXPR vs. MODIFY_EXPR >>>>>>>>>> to distinguish between initialization and modification; for class objects, I >>>>>>>>> >>>>>>>>> This is already true: only class object go into the hash set. >>>>>>>>> >>>>>>>>>> wonder about setting a flag on the CONSTRUCTOR after initialization is >>>>>>>>>> complete to indicate that the value is now constant. >>>>>>>>> >>>>>>>>> But here we're not dealing with CONSTRUCTORs in the gcc sense (i.e. exprs with >>>>>>>>> TREE_CODE == CONSTRUCTOR). We have a CALL_EXPR like Y::Y ((struct Y *) &y), >>>>>>>>> which initializes the object "y". Setting a flag on the CALL_EXPR or its underlying >>>>>>>>> function decl wouldn't help. >>>>>>>>> >>>>>>>>> Am I missing something? >>>>>>>> >>>>>>>> I was thinking that where in your current patch you call >>>>>>>> remove_object_under_construction, we could instead mark the object's value >>>>>>>> CONSTRUCTOR as immutable. >>>>>>> >>>>>>> Ah, what you meant was to look at DECL_INITIAL of the object we're >>>>>>> constructing, which could be a CONSTRUCTOR. Unfortunately, this >>>>>>> DECL_INITIAL is null (in all the new tests when doing >>>>>>> remove_object_under_construction), so there's nothing to mark as TREE_READONLY :/. >>>>>> >>>>>> There's a value in ctx->values, isn't there? >>>>> >>>>> Doesn't seem to be the case for e.g. >>>>> >>>>> struct A { >>>>> int n; >>>>> constexpr A() : n(1) { n = 2; } >>>>> }; >>>>> >>>>> struct B { >>>>> const A a; >>>>> constexpr B(bool b) { >>>>> if (b) >>>>> const_cast<A &>(a).n = 3; // { dg-error "modifying a const object" } >>>>> } >>>>> }; >>>>> >>>>> constexpr B b(false); >>>>> static_assert(b.a.n == 2, ""); >>>>> >>>>> Here we're constructing "b", its ctx->values->get(new_obj) is initially >>>>> "{}". In the middle of constructing "b", we construct "b.a", but that >>>>> has nothing in ctx->values. >>>> >>>> Right, subobjects aren't in ctx->values. In cxx_eval_call_expression we >>>> have >>>> >>>> if (DECL_CONSTRUCTOR_P (fun)) >>>> /* This can be null for a subobject constructor call, in >>>> >>>> which case what we care about is the initialization >>>> >>>> side-effects rather than the value. We could get at the >>>> >>>> value by evaluating *this, but we don't bother; there's >>>> >>>> no need to put such a call in the hash table. */ >>>> result = lval ? ctx->object : ctx->ctor; >>>> >>>> Your patch already finds *this (b.a) and puts it in new_obj; if it's const >>>> we can evaluate it to get the CONSTRUCTOR to set TREE_READONLY on. >>> >>> Ah, got it! This patch uses setting TREE_READONLY to achieve what I was after. >>> I also needed to set TREE_READONLY in cxx_eval_constant_expression/DECL_EXPR. >>> The additional evaluating will only happen for const-qual objects so I hope not >>> very often. >>> >>> Any further comments? Thanks, >>> >>> @@ -1910,6 +1958,29 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, >>> >>> + /* At this point, the object's constructor will have run, so >>> + the object is no longer under construction, and its possible >>> + 'const' semantics now apply. Make a note of this fact by >>> + marking the CONSTRUCTOR TREE_READONLY. */ >>> + if (new_obj >>> + && CLASS_TYPE_P (TREE_TYPE (new_obj)) >>> + && CP_TYPE_CONST_P (TREE_TYPE (new_obj))) >>> + { >>> + tree *ctor = ctx->values->get (new_obj); >> >> I don't think trying ctx->values->get first is a win, let's go straight to >> cxx_eval_constant_expression. > > Done. > >>> +/* Return true if we are modifying something that is const during constant >>> + expression evaluation. CODE is the code of the statement, OBJ is the >>> + object in question, MUTABLE_P is true if one of the subobjects were >>> + declared mutable. */ >>> + >>> +static bool >>> +modifying_const_object_p (const constexpr_ctx *ctx, tree_code code, tree obj, >>> + bool mutable_p) >>> +{ >>> + /* If this is initialization, there's no problem. */ >>> + if (code != MODIFY_EXPR) >>> + return false; >>> + >>> + tree type = TREE_TYPE (obj); >>> + >>> + /* [basic.type.qualifier] "A const object is an object of type >>> + const T or a non-mutable subobject of a const object." */ >>> + return ((TREE_READONLY (obj) || CP_TYPE_CONST_P (type) >>> + /* If it's an aggregate and any field is const, then it is >>> + effectively const. */ >>> + || (CLASS_TYPE_P (type) && C_TYPE_FIELDS_READONLY (type))) >> >> This seems wrong; if one field is const, we can still modify other fields. I >> don't see a test for that case. > > I took it from cp_build_modify_expr but it's not really needed, so dropped. > >>> @@ -3783,6 +3885,17 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, >>> { >>> tree ob = TREE_OPERAND (probe, 0); >>> tree elt = TREE_OPERAND (probe, 1); >>> + if (DECL_P (elt) && DECL_MUTABLE_P (elt)) >>> + mutable_p = true; >>> + if (evaluated >>> + && modifying_const_object_p (ctx, TREE_CODE (t), probe, >>> + mutable_p)) >>> + { >>> + if (!ctx->quiet) >>> + modifying_const_object_error (t, probe); >>> + *non_constant_p = true; >>> + return t; >>> + } >> >> What if there's a mutable member further down, i.e. >> >> struct A { mutable int i; }; >> struct B { A a; }; >> const B b; >> b.a.i = 42; >> >> ? And also... > > This works. We check mutable going from the innermost object to the outermost, > so if we have b.a.n, we check n first. Some of the new tests are exercising this. > >>> @@ -3811,6 +3924,14 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, >>> + if (modifying_const_object_p (ctx, TREE_CODE (t), object, mutable_p)) >>> + { >>> + if (!ctx->quiet) >>> + modifying_const_object_error (t, object); >>> + *non_constant_p = true; >>> + return t; >>> + } >> >> ...we are already collecting the CONSTRUCTORs that we're dealing with in the >> "ctors" stack, we shouldn't need to evaluate object at this point. I'd >> expect the topmost class-type CONSTRUCTOR on the stack (if any) to be the >> one we want to look at. I'd think you could do away with much of >> modifying_const_object_p. > > Oh, got it; wish I'd thought of that sooner. We can't just check the topmost > CONSTRUCTOR though, the yuge comment I've added has an example when that would > not be enough. > > And yes, I could 86 most of modifying_const_object_p. > >>> @@ -4650,6 +4772,11 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, >>> non_constant_p, overflow_p); >>> /* Don't share a CONSTRUCTOR that might be changed. */ >>> init = unshare_constructor (init); >>> + /* Remember that a constant object's constructor has already >>> + ran. */ >> >> "has...run" > > Yikes. > > Thanks for the review, it allowed me to make the patch more concise. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? Looking pretty good, just a couple of nits: > + We track the "under construction" state by using the TREE_READONLY > + bit in the object's CONSTRUCTOR. Only incidentally; what we track using the TREE_READONLY bit is whether the object is read-only. > + /* [class.ctor]p5 "A constructor can be invoked for a const, > + volatile, or const volatile object. const and volatile > + semantics are not applied on an object under construction. > + They come into effect when the constructor for the most > + derived object ends." */ > + tree elt; > + unsigned int i; > + FOR_EACH_VEC_ELT (*ctors, i, elt) > + if (CLASS_TYPE_P (TREE_TYPE (elt)) && TREE_READONLY (elt)) Would it make sense to look here for the CONSTRUCTOR with the same type as const_object_being_modified and set "fail" (or not) based only on that CONSTRUCTOR? Jason
On Sun, Aug 18, 2019 at 05:08:07PM -0700, Jason Merrill wrote: > On 8/18/19 6:52 AM, Marek Polacek wrote: > > On Fri, Aug 16, 2019 at 05:40:39PM -0700, Jason Merrill wrote: > > > On 8/16/19 5:11 AM, Marek Polacek wrote: > > > > On Thu, Aug 15, 2019 at 08:21:25PM -0400, Jason Merrill wrote: > > > > > On 8/15/19 5:34 PM, Marek Polacek wrote: > > > > > > On Wed, Aug 14, 2019 at 02:50:13PM -0400, Jason Merrill wrote: > > > > > > > On Thu, Aug 8, 2019 at 3:25 PM Marek Polacek <polacek@redhat.com> wrote: > > > > > > > > > > > > > > > > On Thu, Aug 08, 2019 at 11:06:17AM -0400, Jason Merrill wrote: > > > > > > > > > On 8/6/19 3:20 PM, Marek Polacek wrote: > > > > > > > > > > On Mon, Aug 05, 2019 at 03:54:19PM -0400, Jason Merrill wrote: > > > > > > > > > > > On 7/31/19 3:26 PM, Marek Polacek wrote: > > > > > > > > > > > > One of the features of constexpr is that it doesn't allow UB; and such UB must > > > > > > > > > > > > be detected at compile-time. So running your code in a context that requires > > > > > > > > > > > > a constant expression should ensure that the code in question is free of UB. > > > > > > > > > > > > In effect, constexpr can serve as a sanitizer. E.g. this article describes in > > > > > > > > > > > > in more detail: > > > > > > > > > > > > <https://shafik.github.io/c++/undefined%20behavior/2019/05/11/explporing_undefined_behavior_using_constexpr.html> > > > > > > > > > > > > > > > > > > > > > > > > [dcl.type.cv]p4 says "Any attempt to modify a const object during its lifetime > > > > > > > > > > > > results in undefined behavior." However, as the article above points out, we > > > > > > > > > > > > aren't detecting that case in constexpr evaluation. > > > > > > > > > > > > > > > > > > > > > > > > This patch fixes that. It's not that easy, though, because we have to keep in > > > > > > > > > > > > mind [class.ctor]p5: > > > > > > > > > > > > "A constructor can be invoked for a const, volatile or const volatile object. > > > > > > > > > > > > const and volatile semantics are not applied on an object under construction. > > > > > > > > > > > > They come into effect when the constructor for the most derived object ends." > > > > > > > > > > > > > > > > > > > > > > > > I handled this by keeping a hash set which tracks objects under construction. > > > > > > > > > > > > I considered other options, such as going up call_stack, but that wouldn't > > > > > > > > > > > > work with trivial constructor/op=. It was also interesting to find out that > > > > > > > > > > > > the definition of TREE_HAS_CONSTRUCTOR says "When appearing in a FIELD_DECL, > > > > > > > > > > > > it means that this field has been duly initialized in its constructor" though > > > > > > > > > > > > nowhere in the codebase do we set TREE_HAS_CONSTRUCTOR on a FIELD_DECL as far > > > > > > > > > > > > as I can see. Unfortunately, using this bit proved useless for my needs here. > > > > > > > > > > > > > > > > > > > > > > > Also, be mindful of mutable subobjects. > > > > > > > > > > > > > > > > > > > > > > > > Does this approach look like an appropriate strategy for tracking objects' > > > > > > > > > > > > construction? > > > > > > > > > > > > > > > > > > > > > > For scalar objects, we should be able to rely on INIT_EXPR vs. MODIFY_EXPR > > > > > > > > > > > to distinguish between initialization and modification; for class objects, I > > > > > > > > > > > > > > > > > > > > This is already true: only class object go into the hash set. > > > > > > > > > > > > > > > > > > > > > wonder about setting a flag on the CONSTRUCTOR after initialization is > > > > > > > > > > > complete to indicate that the value is now constant. > > > > > > > > > > > > > > > > > > > > But here we're not dealing with CONSTRUCTORs in the gcc sense (i.e. exprs with > > > > > > > > > > TREE_CODE == CONSTRUCTOR). We have a CALL_EXPR like Y::Y ((struct Y *) &y), > > > > > > > > > > which initializes the object "y". Setting a flag on the CALL_EXPR or its underlying > > > > > > > > > > function decl wouldn't help. > > > > > > > > > > > > > > > > > > > > Am I missing something? > > > > > > > > > > > > > > > > > > I was thinking that where in your current patch you call > > > > > > > > > remove_object_under_construction, we could instead mark the object's value > > > > > > > > > CONSTRUCTOR as immutable. > > > > > > > > > > > > > > > > Ah, what you meant was to look at DECL_INITIAL of the object we're > > > > > > > > constructing, which could be a CONSTRUCTOR. Unfortunately, this > > > > > > > > DECL_INITIAL is null (in all the new tests when doing > > > > > > > > remove_object_under_construction), so there's nothing to mark as TREE_READONLY :/. > > > > > > > > > > > > > > There's a value in ctx->values, isn't there? > > > > > > > > > > > > Doesn't seem to be the case for e.g. > > > > > > > > > > > > struct A { > > > > > > int n; > > > > > > constexpr A() : n(1) { n = 2; } > > > > > > }; > > > > > > > > > > > > struct B { > > > > > > const A a; > > > > > > constexpr B(bool b) { > > > > > > if (b) > > > > > > const_cast<A &>(a).n = 3; // { dg-error "modifying a const object" } > > > > > > } > > > > > > }; > > > > > > > > > > > > constexpr B b(false); > > > > > > static_assert(b.a.n == 2, ""); > > > > > > > > > > > > Here we're constructing "b", its ctx->values->get(new_obj) is initially > > > > > > "{}". In the middle of constructing "b", we construct "b.a", but that > > > > > > has nothing in ctx->values. > > > > > > > > > > Right, subobjects aren't in ctx->values. In cxx_eval_call_expression we > > > > > have > > > > > > > > > > if (DECL_CONSTRUCTOR_P (fun)) > > > > > /* This can be null for a subobject constructor call, in > > > > > > > > > > which case what we care about is the initialization > > > > > > > > > > side-effects rather than the value. We could get at the > > > > > > > > > > value by evaluating *this, but we don't bother; there's > > > > > > > > > > no need to put such a call in the hash table. */ > > > > > result = lval ? ctx->object : ctx->ctor; > > > > > > > > > > Your patch already finds *this (b.a) and puts it in new_obj; if it's const > > > > > we can evaluate it to get the CONSTRUCTOR to set TREE_READONLY on. > > > > > > > > Ah, got it! This patch uses setting TREE_READONLY to achieve what I was after. > > > > I also needed to set TREE_READONLY in cxx_eval_constant_expression/DECL_EXPR. > > > > The additional evaluating will only happen for const-qual objects so I hope not > > > > very often. > > > > > > > > Any further comments? Thanks, > > > > > > > > @@ -1910,6 +1958,29 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, > > > > > > > > + /* At this point, the object's constructor will have run, so > > > > + the object is no longer under construction, and its possible > > > > + 'const' semantics now apply. Make a note of this fact by > > > > + marking the CONSTRUCTOR TREE_READONLY. */ > > > > + if (new_obj > > > > + && CLASS_TYPE_P (TREE_TYPE (new_obj)) > > > > + && CP_TYPE_CONST_P (TREE_TYPE (new_obj))) > > > > + { > > > > + tree *ctor = ctx->values->get (new_obj); > > > > > > I don't think trying ctx->values->get first is a win, let's go straight to > > > cxx_eval_constant_expression. > > > > Done. > > > > > > +/* Return true if we are modifying something that is const during constant > > > > + expression evaluation. CODE is the code of the statement, OBJ is the > > > > + object in question, MUTABLE_P is true if one of the subobjects were > > > > + declared mutable. */ > > > > + > > > > +static bool > > > > +modifying_const_object_p (const constexpr_ctx *ctx, tree_code code, tree obj, > > > > + bool mutable_p) > > > > +{ > > > > + /* If this is initialization, there's no problem. */ > > > > + if (code != MODIFY_EXPR) > > > > + return false; > > > > + > > > > + tree type = TREE_TYPE (obj); > > > > + > > > > + /* [basic.type.qualifier] "A const object is an object of type > > > > + const T or a non-mutable subobject of a const object." */ > > > > + return ((TREE_READONLY (obj) || CP_TYPE_CONST_P (type) > > > > + /* If it's an aggregate and any field is const, then it is > > > > + effectively const. */ > > > > + || (CLASS_TYPE_P (type) && C_TYPE_FIELDS_READONLY (type))) > > > > > > This seems wrong; if one field is const, we can still modify other fields. I > > > don't see a test for that case. > > > > I took it from cp_build_modify_expr but it's not really needed, so dropped. > > > > > > @@ -3783,6 +3885,17 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, > > > > { > > > > tree ob = TREE_OPERAND (probe, 0); > > > > tree elt = TREE_OPERAND (probe, 1); > > > > + if (DECL_P (elt) && DECL_MUTABLE_P (elt)) > > > > + mutable_p = true; > > > > + if (evaluated > > > > + && modifying_const_object_p (ctx, TREE_CODE (t), probe, > > > > + mutable_p)) > > > > + { > > > > + if (!ctx->quiet) > > > > + modifying_const_object_error (t, probe); > > > > + *non_constant_p = true; > > > > + return t; > > > > + } > > > > > > What if there's a mutable member further down, i.e. > > > > > > struct A { mutable int i; }; > > > struct B { A a; }; > > > const B b; > > > b.a.i = 42; > > > > > > ? And also... > > > > This works. We check mutable going from the innermost object to the outermost, > > so if we have b.a.n, we check n first. Some of the new tests are exercising this. > > > > > > @@ -3811,6 +3924,14 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, > > > > + if (modifying_const_object_p (ctx, TREE_CODE (t), object, mutable_p)) > > > > + { > > > > + if (!ctx->quiet) > > > > + modifying_const_object_error (t, object); > > > > + *non_constant_p = true; > > > > + return t; > > > > + } > > > > > > ...we are already collecting the CONSTRUCTORs that we're dealing with in the > > > "ctors" stack, we shouldn't need to evaluate object at this point. I'd > > > expect the topmost class-type CONSTRUCTOR on the stack (if any) to be the > > > one we want to look at. I'd think you could do away with much of > > > modifying_const_object_p. > > > > Oh, got it; wish I'd thought of that sooner. We can't just check the topmost > > CONSTRUCTOR though, the yuge comment I've added has an example when that would > > not be enough. > > > > And yes, I could 86 most of modifying_const_object_p. > > > > > > @@ -4650,6 +4772,11 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, > > > > non_constant_p, overflow_p); > > > > /* Don't share a CONSTRUCTOR that might be changed. */ > > > > init = unshare_constructor (init); > > > > + /* Remember that a constant object's constructor has already > > > > + ran. */ > > > > > > "has...run" > > > > Yikes. > > > > Thanks for the review, it allowed me to make the patch more concise. > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > Looking pretty good, just a couple of nits: > > > + We track the "under construction" state by using the TREE_READONLY > > + bit in the object's CONSTRUCTOR. > > Only incidentally; what we track using the TREE_READONLY bit is whether the > object is read-only. Indeed, that wording was imprecise. I've reworded it. > > + /* [class.ctor]p5 "A constructor can be invoked for a const, > > + volatile, or const volatile object. const and volatile > > + semantics are not applied on an object under construction. > > + They come into effect when the constructor for the most > > + derived object ends." */ > > + tree elt; > > + unsigned int i; > > + FOR_EACH_VEC_ELT (*ctors, i, elt) > > + if (CLASS_TYPE_P (TREE_TYPE (elt)) && TREE_READONLY (elt)) > > Would it make sense to look here for the CONSTRUCTOR with the same type as > const_object_being_modified and set "fail" (or not) based only on that > CONSTRUCTOR? That works too! It's the only contstructor we care about, because if it's readonly, we know we have to fail, and if it isn't, we know that the outer constructor can't have been constructed yet, since it subsumes the current object's constructor. Regtest/bootstrap still running, but all the new tests still pass as before. Ok if that passes? 2019-08-18 Marek Polacek <polacek@redhat.com> PR c++/91264 - detect modifying const objects in constexpr. * constexpr.c (modifying_const_object_error): New function. (cxx_eval_call_expression): Set TREE_READONLY on a CONSTRUCTOR of a const-qualified object after it's been fully constructed. (modifying_const_object_p): New function. (cxx_eval_store_expression): Detect modifying a const object during constant expression evaluation. (cxx_eval_increment_expression): Use a better location when building up the store. (cxx_eval_constant_expression) <case DECL_EXPR>: Mark a constant object's constructor TREE_READONLY. * g++.dg/cpp1y/constexpr-tracking-const1.C: New test. * g++.dg/cpp1y/constexpr-tracking-const2.C: New test. * g++.dg/cpp1y/constexpr-tracking-const3.C: New test. * g++.dg/cpp1y/constexpr-tracking-const4.C: New test. * g++.dg/cpp1y/constexpr-tracking-const5.C: New test. * g++.dg/cpp1y/constexpr-tracking-const6.C: New test. * g++.dg/cpp1y/constexpr-tracking-const7.C: New test. * g++.dg/cpp1y/constexpr-tracking-const8.C: New test. * g++.dg/cpp1y/constexpr-tracking-const9.C: New test. * g++.dg/cpp1y/constexpr-tracking-const10.C: New test. * g++.dg/cpp1y/constexpr-tracking-const11.C: New test. * g++.dg/cpp1y/constexpr-tracking-const12.C: New test. * g++.dg/cpp1y/constexpr-tracking-const13.C: New test. * g++.dg/cpp1y/constexpr-tracking-const14.C: New test. diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c index 23f2a027e2f..dbd0dc3b445 100644 --- gcc/cp/constexpr.c +++ gcc/cp/constexpr.c @@ -1575,6 +1575,19 @@ clear_no_implicit_zero (tree ctor) } } +/* Complain about a const object OBJ being modified in a constant expression. + EXPR is the MODIFY_EXPR expression performing the modification. */ + +static void +modifying_const_object_error (tree expr, tree obj) +{ + location_t loc = cp_expr_loc_or_input_loc (expr); + auto_diagnostic_group d; + error_at (loc, "modifying a const object %qE is not allowed in " + "a constant expression", TREE_OPERAND (expr, 0)); + inform (location_of (obj), "originally declared %<const%> here"); +} + /* Subroutine of cxx_eval_constant_expression. Evaluate the call expression tree T in the context of OLD_CALL expression evaluation. */ @@ -1775,6 +1788,19 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, depth_ok = push_cx_call_context (t); + /* Remember the object we are constructing. */ + tree new_obj = NULL_TREE; + if (DECL_CONSTRUCTOR_P (fun)) + { + /* In a constructor, it should be the first `this' argument. + At this point it has already been evaluated in the call + to cxx_bind_parameters_in_call. */ + new_obj = TREE_VEC_ELT (new_call.bindings, 0); + STRIP_NOPS (new_obj); + if (TREE_CODE (new_obj) == ADDR_EXPR) + new_obj = TREE_OPERAND (new_obj, 0); + } + tree result = NULL_TREE; constexpr_call *entry = NULL; @@ -1910,6 +1936,23 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, } } + /* At this point, the object's constructor will have run, so + the object is no longer under construction, and its possible + 'const' semantics now apply. Make a note of this fact by + marking the CONSTRUCTOR TREE_READONLY. */ + if (new_obj + && CLASS_TYPE_P (TREE_TYPE (new_obj)) + && CP_TYPE_CONST_P (TREE_TYPE (new_obj))) + { + /* Subobjects might not be stored in ctx->values but we can + get its CONSTRUCTOR by evaluating *this. */ + tree e = cxx_eval_constant_expression (ctx, new_obj, + /*lval*/false, + non_constant_p, + overflow_p); + TREE_READONLY (e) = true; + } + /* Forget the saved values of the callee's SAVE_EXPRs. */ unsigned int i; tree save_expr; @@ -3724,6 +3767,26 @@ maybe_simplify_trivial_copy (tree &target, tree &init) } } +/* Return true if we are modifying something that is const during constant + expression evaluation. CODE is the code of the statement, OBJ is the + object in question, MUTABLE_P is true if one of the subobjects were + declared mutable. */ + +static bool +modifying_const_object_p (tree_code code, tree obj, bool mutable_p) +{ + /* If this is initialization, there's no problem. */ + if (code != MODIFY_EXPR) + return false; + + /* [basic.type.qualifier] "A const object is an object of type + const T or a non-mutable subobject of a const object." */ + if (mutable_p) + return false; + + return (TREE_READONLY (obj) || CP_TYPE_CONST_P (TREE_TYPE (obj))); +} + /* Evaluate an INIT_EXPR or MODIFY_EXPR. */ static tree @@ -3773,6 +3836,9 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, /* Find the underlying variable. */ releasing_vec refs; tree object = NULL_TREE; + /* If we're modifying a const object, save it. */ + tree const_object_being_modified = NULL_TREE; + bool mutable_p = false; for (tree probe = target; object == NULL_TREE; ) { switch (TREE_CODE (probe)) @@ -3783,6 +3849,12 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, { tree ob = TREE_OPERAND (probe, 0); tree elt = TREE_OPERAND (probe, 1); + if (DECL_P (elt) && DECL_MUTABLE_P (elt)) + mutable_p = true; + if (evaluated + && modifying_const_object_p (TREE_CODE (t), probe, mutable_p) + && const_object_being_modified == NULL_TREE) + const_object_being_modified = probe; if (TREE_CODE (probe) == ARRAY_REF) { elt = eval_and_check_array_index (ctx, probe, false, @@ -3811,6 +3883,10 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, } } + if (modifying_const_object_p (TREE_CODE (t), object, mutable_p) + && const_object_being_modified == NULL_TREE) + const_object_being_modified = object; + /* And then find/build up our initializer for the path to the subobject we're initializing. */ tree *valp; @@ -3950,6 +4026,62 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, valp = &cep->value; } + /* Detect modifying a constant object in constexpr evaluation. + We have found a const object that is being modified. Figure out + if we need to issue an error. Consider + + struct A { + int n; + constexpr A() : n(1) { n = 2; } // #1 + }; + struct B { + const A a; + constexpr B() { a.n = 3; } // #2 + }; + constexpr B b{}; + + #1 is OK, since we're modifying an object under construction, but + #2 is wrong, since "a" is const and has been fully constructed. + To track it, we use the TREE_READONLY bit in the object's CONSTRUCTOR + which means that the object is read-only. For the example above, the + *ctors stack at the point of #2 will look like: + + ctors[0] = {.a={.n=2}} TREE_READONLY = 0 + ctors[1] = {.n=2} TREE_READONLY = 1 + + and we're modifying "b.a", so we search the stack and see if the + constructor for "b.a" has already run. */ + if (const_object_being_modified) + { + bool fail = false; + if (!CLASS_TYPE_P (TREE_TYPE (const_object_being_modified))) + fail = true; + else + { + /* [class.ctor]p5 "A constructor can be invoked for a const, + volatile, or const volatile object. const and volatile + semantics are not applied on an object under construction. + They come into effect when the constructor for the most + derived object ends." */ + tree elt; + unsigned int i; + FOR_EACH_VEC_ELT (*ctors, i, elt) + if (same_type_ignoring_top_level_qualifiers_p + (TREE_TYPE (const_object_being_modified), TREE_TYPE (elt))) + { + fail = TREE_READONLY (elt); + break; + } + } + if (fail) + { + if (!ctx->quiet) + modifying_const_object_error (t, const_object_being_modified); + *non_constant_p = true; + return t; + } + } + if (!preeval) { /* Create a new CONSTRUCTOR in case evaluation of the initializer @@ -4063,7 +4195,8 @@ cxx_eval_increment_expression (const constexpr_ctx *ctx, tree t, VERIFY_CONSTANT (mod); /* Storing the modified value. */ - tree store = build2 (MODIFY_EXPR, type, op, mod); + tree store = build2_loc (cp_expr_loc_or_loc (t, input_location), + MODIFY_EXPR, type, op, mod); cxx_eval_constant_expression (ctx, store, true, non_constant_p, overflow_p); ggc_free (store); @@ -4650,6 +4783,11 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, non_constant_p, overflow_p); /* Don't share a CONSTRUCTOR that might be changed. */ init = unshare_constructor (init); + /* Remember that a constant object's constructor has already + run. */ + if (CLASS_TYPE_P (TREE_TYPE (r)) + && CP_TYPE_CONST_P (TREE_TYPE (r))) + TREE_READONLY (init) = true; ctx->values->put (r, init); } else if (ctx == &new_ctx) diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const1.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const1.C new file mode 100644 index 00000000000..e081a535659 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const1.C @@ -0,0 +1,72 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +constexpr void +mod (int &r) +{ + r = 99; // { dg-error "modifying a const object" } +} + +constexpr int +fn1 () +{ + const int i = 0; // { dg-message "originally declared" } + mod (const_cast<int &>(i)); // { dg-message "in .constexpr. expansion of " } + return i; +} + +constexpr int i1 = fn1 (); // { dg-message "in .constexpr. expansion of " } + +constexpr int +fn2 () +{ + const int i = 5; // { dg-message "originally declared" } + const_cast<int &>(i) = 10; // { dg-error "modifying a const object" } + return i; +} + +constexpr int i2 = fn2 (); // { dg-message "in .constexpr. expansion of " } + +constexpr int +fn3 () +{ + const int i = 5; // { dg-message "originally declared" } + ++const_cast<int &>(i); // { dg-error "modifying a const object" } + return i; +} + +constexpr int i3 = fn3 (); // { dg-message "in .constexpr. expansion of " } + +constexpr int +fn4 () +{ + const int i = 5; // { dg-message "originally declared" } + const_cast<int &>(i)--; // { dg-error "modifying a const object" } + return i; +} + +constexpr int i4 = fn4 (); // { dg-message "in .constexpr. expansion of " } + +constexpr int +fn5 () +{ + const int i = 5; // { dg-message "originally declared" } + const_cast<int &>(i) += 2; // { dg-error "modifying a const object" } + return i; +} + +constexpr int i5 = fn5 (); // { dg-message "in .constexpr. expansion of " } + +constexpr int +fn6 () +{ + // This is OK. + int i = 3; + const int *cip = &i; + int *ip = const_cast<int *>(cip); + *ip = 4; + return i; +} + +constexpr int i6 = fn6 (); +static_assert(i6 == 4, ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const10.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const10.C new file mode 100644 index 00000000000..53acb37beb8 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const10.C @@ -0,0 +1,22 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct B { + B() = default; + int i; +}; + +constexpr B bar() +{ + constexpr B b = B(); // { dg-message "originally declared" } + B *p = const_cast<B*>(&b); + + p->i = 11; // { dg-error "modifying a const object" } + + return *p; +} + +void foo() +{ + constexpr B y = bar(); // { dg-message "in .constexpr. expansion of" } +} diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const11.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const11.C new file mode 100644 index 00000000000..2b351cd013a --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const11.C @@ -0,0 +1,16 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct S { + int a = 1; + int * ptr = &a; +}; + +constexpr bool f() { + auto const s = S{}; // { dg-message "originally declared" } + *s.ptr = 2; // { dg-error "modifying a const object" } + return s.a == 2; +} + +static_assert(f(), ""); // { dg-error "non-constant condition" } +// { dg-message "in 'constexpr' expansion of" "" { target *-*-* } .-1 } diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const12.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const12.C new file mode 100644 index 00000000000..d83e2794f7f --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const12.C @@ -0,0 +1,17 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct A { + const int n; + int m; + constexpr A() : n(1), m(2) { } +}; +struct B { + A a; + constexpr B() { + int *p = &a.m; + *p = 3; + } +}; +constexpr B b; +static_assert(b.a.m == 3, ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const13.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const13.C new file mode 100644 index 00000000000..bc7faa3c250 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const13.C @@ -0,0 +1,20 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct A { + mutable int i; + constexpr A() : i(0) { } +}; +struct B { + A a; + constexpr B() : a{} { } +}; + +constexpr void +g () +{ + const B b; + b.a.i = 42; +} + +static_assert((g(), 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const14.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const14.C new file mode 100644 index 00000000000..45c4fcf50be --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const14.C @@ -0,0 +1,38 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct F { + const int f; + constexpr F() : f(9) { } +}; + +struct C { + int n; + const F f; + constexpr C() : n(1) { n = 66; } +}; + +struct A { + int r; + const C c; + constexpr A() : r(11) { r = 14; const_cast<C &>(c).n = 42; } // { dg-error "modifying a const object" } +}; + +struct D { + const A a; + constexpr D() { } // { dg-message "in .constexpr. expansion of" } +}; + +struct E { + const D d; + constexpr E() { } // { dg-message "in .constexpr. expansion of" } +}; + +struct B { + const E e; + constexpr B(bool) { } // { dg-message "in .constexpr. expansion of" } +}; + +constexpr B b(false); // { dg-message "in .constexpr. expansion of" } +// { dg-message "originally declared" "" { target *-*-* } .-1 } +static_assert(b.e.d.a.c.n == 2, ""); // { dg-error "non-constant condition" } diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const2.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const2.C new file mode 100644 index 00000000000..9803309cace --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const2.C @@ -0,0 +1,23 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct X { + int j; + constexpr X() : j(0) { } +}; + +struct Y { + X x; + constexpr Y() : x{} { } +}; + +constexpr void +g () +{ + const Y y; // { dg-message "originally declared" } + Y *p = const_cast<Y *>(&y); + p->x.j = 99; // { dg-error "modifying a const object" } +} + +static_assert((g() , 1), ""); // { dg-error "non-constant condition" } +// { dg-message "in 'constexpr' expansion of" "" { target *-*-* } .-1 } diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const3.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const3.C new file mode 100644 index 00000000000..6853775c1e2 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const3.C @@ -0,0 +1,22 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct A { + int n; + constexpr A() : n(1) { n = 2; } +}; + +struct B { + const A a; + constexpr B(bool b) { + if (b) + const_cast<A &>(a).n = 3; // { dg-error "modifying a const object" } + } +}; + +constexpr B b(false); +static_assert(b.a.n == 2, ""); + +constexpr B b2(true); // { dg-message "in .constexpr. expansion of " } +// { dg-message "originally declared" "" { target *-*-* } .-1 } +static_assert((b2.a.n, 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const4.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const4.C new file mode 100644 index 00000000000..8263a7cc505 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const4.C @@ -0,0 +1,17 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct A { + const int n; + constexpr A() : n(1) { } +}; +struct B { + A a; + constexpr B() { + int *p = const_cast<int *>(&a.n); + *p = 3; // { dg-error "modifying a const object" } + } +}; +constexpr B b; // { dg-message "in .constexpr. expansion of " } +// { dg-message "originally declared" "" { target *-*-* } .-1 } +static_assert((b.a.n, 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const5.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const5.C new file mode 100644 index 00000000000..bea54fb4fde --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const5.C @@ -0,0 +1,17 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct A { + mutable int n; + constexpr A() : n(1) { n = 2; } +}; + +struct B { + const A a; + constexpr B() { + const_cast<A &>(a).n = 3; + } +}; + +constexpr B b{}; +static_assert((b.a.n, 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const6.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const6.C new file mode 100644 index 00000000000..54d83b17d39 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const6.C @@ -0,0 +1,22 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct X { + mutable int j; + constexpr X() : j(0) { } +}; + +struct Y { + X x; + constexpr Y() : x{} { } +}; + +constexpr void +g () +{ + const Y y; + Y *p = const_cast<Y *>(&y); + p->x.j = 99; +} + +static_assert((g(), 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const7.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const7.C new file mode 100644 index 00000000000..922e8ff126f --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const7.C @@ -0,0 +1,23 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct D { int n; }; + +struct C { const D d; }; + +struct A { + C c; + constexpr A() : c{} { } +}; + +struct B { + A a; + constexpr B() { + int &r = const_cast<int &>(a.c.d.n); + r = 3; // { dg-error "modifying a const object" } + } +}; + +constexpr B b{}; // { dg-message "in .constexpr. expansion of " } +// { dg-message "originally declared" "" { target *-*-* } .-1 } +static_assert((b.a.c.d.n, 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const8.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const8.C new file mode 100644 index 00000000000..2b3fe793f83 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const8.C @@ -0,0 +1,23 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct B { + int i; + double d; +}; + +constexpr B bar() +{ + constexpr B b = {10,10.10}; // { dg-message "originally declared" } + B *p = const_cast<B*>(&b); + + p->i = 11; // { dg-error "modifying a const object" } + p->d = 11.11; + + return *p; +} + +void foo() +{ + constexpr B y = bar(); // { dg-message "in .constexpr. expansion of" } +} diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const9.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const9.C new file mode 100644 index 00000000000..0edec4d05cf --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const9.C @@ -0,0 +1,23 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct B { + int i; + double d; +}; + +constexpr B bar() +{ + constexpr B b{}; // { dg-message "originally declared" } + B *p = const_cast<B*>(&b); + + p->i = 11; // { dg-error "modifying a const object" } + p->d = 11.11; + + return *p; +} + +void foo() +{ + constexpr B y = bar(); // { dg-message "in .constexpr. expansion of" } +}
On Sun, Aug 18, 2019 at 08:37:14PM -0400, Marek Polacek wrote:
> Ok if that passes?
Which it did.
Ok, thanks. On Sun, Aug 18, 2019, 6:26 PM Marek Polacek <polacek@redhat.com> wrote: > On Sun, Aug 18, 2019 at 08:37:14PM -0400, Marek Polacek wrote: > > Ok if that passes? > > Which it did. >
diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c index c1b8b9b8a5d..e1ee72f0343 100644 --- gcc/cp/constexpr.c +++ gcc/cp/constexpr.c @@ -1575,6 +1575,59 @@ clear_no_implicit_zero (tree ctor) } } +/* A hash set used for tracking objects under construction. This is used + for detecting modifying constant objects during constant expression + evaluation. */ +static GTY(()) hash_set<tree> *objects_under_construction; + +/* Add OBJ to the hash set above. */ + +static void +new_object_under_construction (const_tree obj) +{ + if (!CLASS_TYPE_P (TREE_TYPE (obj)) + /* Modifying non-const objects is allowed any time. */ + || !CP_TYPE_CONST_P (TREE_TYPE (obj))) + return; + + if (!objects_under_construction) + objects_under_construction = hash_set<tree>::create_ggc (3); + objects_under_construction->add (CONST_CAST_TREE (obj)); +} + +/* Remove OBJ from the hash set. */ + +static void +remove_object_under_construction (const_tree obj) +{ + if (objects_under_construction) + objects_under_construction->remove (CONST_CAST_TREE (obj)); +} + +/* Return true if OBJ is an object currently under construction. */ + +static bool +object_under_construction_p (const_tree obj) +{ + if (!CLASS_TYPE_P (TREE_TYPE (obj))) + return false; + + return (objects_under_construction + && objects_under_construction->contains (CONST_CAST_TREE (obj))); +} + +/* Complain about a const object OBJ being modified in a constant expression. + EXPR is the MODIFY_EXPR expression performing the modification. */ + +static void +modifying_const_object_error (tree expr, tree obj) +{ + location_t loc = cp_expr_loc_or_loc (expr, input_location); + error_at (loc, "modifying a const object %qE is not allowed in " + "a constant expression", TREE_OPERAND (expr, 0)); + inform (location_of (obj), "originally declared %<const%> here"); +} + /* Subroutine of cxx_eval_constant_expression. Evaluate the call expression tree T in the context of OLD_CALL expression evaluation. */ @@ -1694,8 +1747,12 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, op = build1 (INDIRECT_REF, TREE_TYPE (TREE_TYPE (op)), op); tree set = build2 (MODIFY_EXPR, TREE_TYPE (op), op, init); new_ctx.call = &new_call; - return cxx_eval_constant_expression (&new_ctx, set, lval, + /* We're constructing object OP; save that. */ + new_object_under_construction (op); + init = cxx_eval_constant_expression (&new_ctx, set, lval, non_constant_p, overflow_p); + remove_object_under_construction (op); + return init; } } @@ -1775,6 +1832,20 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, depth_ok = push_cx_call_context (t); + /* Remember the object we are constructing. */ + tree new_obj = NULL_TREE; + if (DECL_CONSTRUCTOR_P (fun)) + { + /* In a constructor, it should be the first `this' argument. + At this point it has already been evaluated in the call + to cxx_bind_parameters_in_call. */ + new_obj = TREE_VEC_ELT (new_call.bindings, 0); + STRIP_NOPS (new_obj); + if (TREE_CODE (new_obj) == ADDR_EXPR) + new_obj = TREE_OPERAND (new_obj, 0); + new_object_under_construction (new_obj); + } + tree result = NULL_TREE; constexpr_call *entry = NULL; @@ -1910,6 +1981,11 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, } } + /* At this point, the object's constructor will have run, so + it's no longer under construction. */ + if (new_obj) + remove_object_under_construction (new_obj); + /* Forget the saved values of the callee's SAVE_EXPRs. */ unsigned int i; tree save_expr; @@ -3724,6 +3800,35 @@ maybe_simplify_trivial_copy (tree &target, tree &init) } } +/* Return true if we are modifying something that is const during constant + expression evaluation. CODE is the code of the statement, OBJ is the + object in question, MUTABLE_P is true if one of the subobjects were + declared mutable. */ + +static bool +modifying_const_object_p (tree_code code, tree obj, bool mutable_p) +{ + /* If this is initialization, there's no problem. */ + if (code != MODIFY_EXPR) + return false; + + tree type = TREE_TYPE (obj); + + /* [basic.type.qualifier] "A const object is an object of type + const T or a non-mutable subobject of a const object." */ + return ((TREE_READONLY (obj) || CP_TYPE_CONST_P (type) + /* If it's an aggregate and any field is const, then it is + effectively const. */ + || (CLASS_TYPE_P (type) && C_TYPE_FIELDS_READONLY (type))) + && !mutable_p + /* [class.ctor]p5 "A constructor can be invoked for a const, + volatile, or const volatile object. const and volatile + semantics are not applied on an object under construction. + They come into effect when the constructor for the most + derived object ends." */ + && !object_under_construction_p (obj)); +} + /* Evaluate an INIT_EXPR or MODIFY_EXPR. */ static tree @@ -3773,6 +3878,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, /* Find the underlying variable. */ releasing_vec refs; tree object = NULL_TREE; + bool mutable_p = false; for (tree probe = target; object == NULL_TREE; ) { switch (TREE_CODE (probe)) @@ -3783,6 +3889,16 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, { tree ob = TREE_OPERAND (probe, 0); tree elt = TREE_OPERAND (probe, 1); + if (DECL_P (elt) && DECL_MUTABLE_P (elt)) + mutable_p = true; + if (evaluated + && modifying_const_object_p (TREE_CODE (t), probe, mutable_p)) + { + if (!ctx->quiet) + modifying_const_object_error (t, probe); + *non_constant_p = true; + return t; + } if (TREE_CODE (probe) == ARRAY_REF) { elt = eval_and_check_array_index (ctx, probe, false, @@ -3811,6 +3927,14 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, } } + if (modifying_const_object_p (TREE_CODE (t), object, mutable_p)) + { + if (!ctx->quiet) + modifying_const_object_error (t, object); + *non_constant_p = true; + return t; + } + /* And then find/build up our initializer for the path to the subobject we're initializing. */ tree *valp; @@ -4063,7 +4187,8 @@ cxx_eval_increment_expression (const constexpr_ctx *ctx, tree t, VERIFY_CONSTANT (mod); /* Storing the modified value. */ - tree store = build2 (MODIFY_EXPR, type, op, mod); + tree store = build2_loc (cp_expr_loc_or_loc (t, input_location), + MODIFY_EXPR, type, op, mod); cxx_eval_constant_expression (ctx, store, true, non_constant_p, overflow_p); ggc_free (store); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const1.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const1.C new file mode 100644 index 00000000000..e081a535659 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const1.C @@ -0,0 +1,72 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +constexpr void +mod (int &r) +{ + r = 99; // { dg-error "modifying a const object" } +} + +constexpr int +fn1 () +{ + const int i = 0; // { dg-message "originally declared" } + mod (const_cast<int &>(i)); // { dg-message "in .constexpr. expansion of " } + return i; +} + +constexpr int i1 = fn1 (); // { dg-message "in .constexpr. expansion of " } + +constexpr int +fn2 () +{ + const int i = 5; // { dg-message "originally declared" } + const_cast<int &>(i) = 10; // { dg-error "modifying a const object" } + return i; +} + +constexpr int i2 = fn2 (); // { dg-message "in .constexpr. expansion of " } + +constexpr int +fn3 () +{ + const int i = 5; // { dg-message "originally declared" } + ++const_cast<int &>(i); // { dg-error "modifying a const object" } + return i; +} + +constexpr int i3 = fn3 (); // { dg-message "in .constexpr. expansion of " } + +constexpr int +fn4 () +{ + const int i = 5; // { dg-message "originally declared" } + const_cast<int &>(i)--; // { dg-error "modifying a const object" } + return i; +} + +constexpr int i4 = fn4 (); // { dg-message "in .constexpr. expansion of " } + +constexpr int +fn5 () +{ + const int i = 5; // { dg-message "originally declared" } + const_cast<int &>(i) += 2; // { dg-error "modifying a const object" } + return i; +} + +constexpr int i5 = fn5 (); // { dg-message "in .constexpr. expansion of " } + +constexpr int +fn6 () +{ + // This is OK. + int i = 3; + const int *cip = &i; + int *ip = const_cast<int *>(cip); + *ip = 4; + return i; +} + +constexpr int i6 = fn6 (); +static_assert(i6 == 4, ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const2.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const2.C new file mode 100644 index 00000000000..9803309cace --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const2.C @@ -0,0 +1,23 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct X { + int j; + constexpr X() : j(0) { } +}; + +struct Y { + X x; + constexpr Y() : x{} { } +}; + +constexpr void +g () +{ + const Y y; // { dg-message "originally declared" } + Y *p = const_cast<Y *>(&y); + p->x.j = 99; // { dg-error "modifying a const object" } +} + +static_assert((g() , 1), ""); // { dg-error "non-constant condition" } +// { dg-message "in 'constexpr' expansion of" "" { target *-*-* } .-1 } diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const3.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const3.C new file mode 100644 index 00000000000..6853775c1e2 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const3.C @@ -0,0 +1,22 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct A { + int n; + constexpr A() : n(1) { n = 2; } +}; + +struct B { + const A a; + constexpr B(bool b) { + if (b) + const_cast<A &>(a).n = 3; // { dg-error "modifying a const object" } + } +}; + +constexpr B b(false); +static_assert(b.a.n == 2, ""); + +constexpr B b2(true); // { dg-message "in .constexpr. expansion of " } +// { dg-message "originally declared" "" { target *-*-* } .-1 } +static_assert((b2.a.n, 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const4.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const4.C new file mode 100644 index 00000000000..8263a7cc505 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const4.C @@ -0,0 +1,17 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct A { + const int n; + constexpr A() : n(1) { } +}; +struct B { + A a; + constexpr B() { + int *p = const_cast<int *>(&a.n); + *p = 3; // { dg-error "modifying a const object" } + } +}; +constexpr B b; // { dg-message "in .constexpr. expansion of " } +// { dg-message "originally declared" "" { target *-*-* } .-1 } +static_assert((b.a.n, 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const5.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const5.C new file mode 100644 index 00000000000..bea54fb4fde --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const5.C @@ -0,0 +1,17 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct A { + mutable int n; + constexpr A() : n(1) { n = 2; } +}; + +struct B { + const A a; + constexpr B() { + const_cast<A &>(a).n = 3; + } +}; + +constexpr B b{}; +static_assert((b.a.n, 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const6.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const6.C new file mode 100644 index 00000000000..07a7f835e11 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const6.C @@ -0,0 +1,22 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct X { + mutable int j; + constexpr X() : j(0) { } +}; + +struct Y { + X x; + constexpr Y() : x{} { } +}; + +constexpr void +g () +{ + const Y y; + Y *p = const_cast<Y *>(&y); + p->x.j = 99; +} + +static_assert((g() , 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const7.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const7.C new file mode 100644 index 00000000000..922e8ff126f --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const7.C @@ -0,0 +1,23 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct D { int n; }; + +struct C { const D d; }; + +struct A { + C c; + constexpr A() : c{} { } +}; + +struct B { + A a; + constexpr B() { + int &r = const_cast<int &>(a.c.d.n); + r = 3; // { dg-error "modifying a const object" } + } +}; + +constexpr B b{}; // { dg-message "in .constexpr. expansion of " } +// { dg-message "originally declared" "" { target *-*-* } .-1 } +static_assert((b.a.c.d.n, 1), ""); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const8.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const8.C new file mode 100644 index 00000000000..2b3fe793f83 --- /dev/null +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const8.C @@ -0,0 +1,23 @@ +// PR c++/91264 +// { dg-do compile { target c++14 } } + +struct B { + int i; + double d; +}; + +constexpr B bar() +{ + constexpr B b = {10,10.10}; // { dg-message "originally declared" } + B *p = const_cast<B*>(&b); + + p->i = 11; // { dg-error "modifying a const object" } + p->d = 11.11; + + return *p; +} + +void foo() +{ + constexpr B y = bar(); // { dg-message "in .constexpr. expansion of" } +}