| Message ID | alpine.DEB.2.02.1805181416001.9066@stedding.saclay.inria.fr |
|---|---|
| State | New |
| Headers | show |
| Series | Aliasing 'this' in a C++ constructor | expand |
On Fri, May 18, 2018 at 2:25 PM Marc Glisse <marc.glisse@inria.fr> wrote: > Hello, > this lets alias analysis handle the implicit 'this' parameter in C++ > constructors as if it was restrict. > Bootstrap+regtest on powerpc64le-unknown-linux-gnu. OK. Please give C++ folks the chance to chime in on the semantics. Thanks, Richard. > 2018-05-18 Marc Glisse <marc.glisse@inria.fr> > PR c++/82899 > gcc/ > * tree-ssa-structalias.c (create_variable_info_for_1): Extra argument. > (intra_create_variable_infos): Handle C++ constructors. > gcc/testsuite/ > * g++.dg/pr82899.C: New testcase. > -- > Marc Glisse
(Cc: said C++ folks) On Fri, 18 May 2018, Richard Biener wrote: > On Fri, May 18, 2018 at 2:25 PM Marc Glisse <marc.glisse@inria.fr> wrote: > >> Hello, > >> this lets alias analysis handle the implicit 'this' parameter in C++ >> constructors as if it was restrict. > >> Bootstrap+regtest on powerpc64le-unknown-linux-gnu. > > OK. Please give C++ folks the chance to chime in on the semantics. > > Thanks, > Richard. > >> 2018-05-18 Marc Glisse <marc.glisse@inria.fr> > >> PR c++/82899 >> gcc/ >> * tree-ssa-structalias.c (create_variable_info_for_1): Extra > argument. >> (intra_create_variable_infos): Handle C++ constructors. > >> gcc/testsuite/ >> * g++.dg/pr82899.C: New testcase. > >> -- >> Marc Glisse
On 05/18/2018 08:34 AM, Marc Glisse wrote: > (Cc: said C++ folks) > > On Fri, 18 May 2018, Richard Biener wrote: > >> On Fri, May 18, 2018 at 2:25 PM Marc Glisse <marc.glisse@inria.fr> wrote: >> >>> Hello, >> >>> this lets alias analysis handle the implicit 'this' parameter in C++ >>> constructors as if it was restrict. I think the following bizarre code is nevertheless well-formed: struct selfie { selfie *me; selfie (selfie *ptr) : me (ptr) {} }; selfie bob (&bob); nathan
On Fri, 18 May 2018, Nathan Sidwell wrote: > On 05/18/2018 08:34 AM, Marc Glisse wrote: >> (Cc: said C++ folks) >> >> On Fri, 18 May 2018, Richard Biener wrote: >> >>> On Fri, May 18, 2018 at 2:25 PM Marc Glisse <marc.glisse@inria.fr> wrote: >>> >>>> Hello, >>> >>>> this lets alias analysis handle the implicit 'this' parameter in C++ >>>> constructors as if it was restrict. > > I think the following bizarre code is nevertheless well-formed: > > struct selfie { > selfie *me; > selfie (selfie *ptr) : me (ptr) {} > }; > > selfie bob (&bob); As long as you do not dereference ptr in the constructor, that shouldn't contradict 'restrict'. The PR gives this quote from the standard: "During the construction of an object, if the value of the object or any of its subobjects is accessed through a glvalue that is not obtained, directly or indirectly, from the constructor’s this pointer, the value of the object or subobject thus obtained is unspecified." which reads quite close to saying that 'this' is restrict.
On 05/18/2018 08:53 AM, Marc Glisse wrote: > As long as you do not dereference ptr in the constructor, that shouldn't > contradict 'restrict'. The PR gives this quote from the standard: > > "During the construction of an object, if the value of the object or any > of its subobjects is accessed through a glvalue that is not obtained, > directly or indirectly, from the constructor’s this pointer, the value > of the object or subobject thus obtained is unspecified." > > which reads quite close to saying that 'this' is restrict. Indeed it is, thanks. what about comparisons to this? I thought restrict implied such a comparison was 'never the same'? ie. if the ctor was: selfie (selfie *ptr) : me (ptr==this ? 0 : ptr) {} nathan
On Fri, 18 May 2018, Nathan Sidwell wrote: > On 05/18/2018 08:53 AM, Marc Glisse wrote: > >> As long as you do not dereference ptr in the constructor, that shouldn't >> contradict 'restrict'. The PR gives this quote from the standard: >> >> "During the construction of an object, if the value of the object or any of >> its subobjects is accessed through a glvalue that is not obtained, directly >> or indirectly, from the constructor’s this pointer, the value of the object >> or subobject thus obtained is unspecified." >> >> which reads quite close to saying that 'this' is restrict. > Indeed it is, thanks. > > what about comparisons to this? I thought restrict implied such a comparison > was 'never the same'? > > ie. if the ctor was: > selfie (selfie *ptr) : me (ptr==this ? 0 : ptr) {} It is tempting to think so, but I don't see any language to that effect. C11 has this example: void h(int n, int * restrict p, int * restrict q, int * restrict r) { int i; for (i = 0; i < n; i++) p[i] = q[i] + r[i]; } h(100, a, b, b) --> valid (because no write) We have https://gcc.gnu.org/bugzilla/show_bug.cgi?id=13962 about using alias information to simplify pointer comparisons.
On Fri, May 18, 2018 at 8:34 AM, Marc Glisse <marc.glisse@inria.fr> wrote: > On Fri, 18 May 2018, Richard Biener wrote: >> On Fri, May 18, 2018 at 2:25 PM Marc Glisse <marc.glisse@inria.fr> wrote: >> >>> this lets alias analysis handle the implicit 'this' parameter in C++ >>> constructors as if it was restrict. >> >> OK. Please give C++ folks the chance to chime in on the semantics. > > (Cc: said C++ folks) I suppose it's technically valid to write something like void f() { A a; a.~A(); new (&a) A(a); } but that's pretty pathological and useless, so I'm not concerned about messing with it. Jason
On Fri, May 18, 2018 at 09:02:08AM -0400, Nathan Sidwell wrote: > On 05/18/2018 08:53 AM, Marc Glisse wrote: > > > As long as you do not dereference ptr in the constructor, that shouldn't > > contradict 'restrict'. The PR gives this quote from the standard: > > > > "During the construction of an object, if the value of the object or any > > of its subobjects is accessed through a glvalue that is not obtained, > > directly or indirectly, from the constructor’s this pointer, the value > > of the object or subobject thus obtained is unspecified." > > > > which reads quite close to saying that 'this' is restrict. > Indeed it is, thanks. > > what about comparisons to this? I thought restrict implied such a > comparison was 'never the same'? > > ie. if the ctor was: > selfie (selfie *ptr) : me (ptr==this ? 0 : ptr) {} But what is invalid on: struct S { int foo (S *); int a; } s { 2 }; int S::foo (S *x) { int b = this->a; x->a = 5; b += this->a; return b; } int main () { if (s.foo (&s) != 7) abort (); } I think if you make this a restrict pointer, this will be miscompiled. Jakub
On Fri, May 18, 2018 at 9:53 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Fri, May 18, 2018 at 09:02:08AM -0400, Nathan Sidwell wrote: >> On 05/18/2018 08:53 AM, Marc Glisse wrote: >> >> > As long as you do not dereference ptr in the constructor, that shouldn't >> > contradict 'restrict'. The PR gives this quote from the standard: >> > >> > "During the construction of an object, if the value of the object or any >> > of its subobjects is accessed through a glvalue that is not obtained, >> > directly or indirectly, from the constructor’s this pointer, the value >> > of the object or subobject thus obtained is unspecified." >> > >> > which reads quite close to saying that 'this' is restrict. >> Indeed it is, thanks. >> >> what about comparisons to this? I thought restrict implied such a >> comparison was 'never the same'? >> >> ie. if the ctor was: >> selfie (selfie *ptr) : me (ptr==this ? 0 : ptr) {} > > But what is invalid on: > struct S { int foo (S *); int a; } s { 2 }; > int S::foo (S *x) > { > int b = this->a; > x->a = 5; > b += this->a; > return b; > } > int main () > { > if (s.foo (&s) != 7) > abort (); > } > > I think if you make this a restrict pointer, this will be miscompiled. We're only talking about the 'this' pointer in a constructor, not a normal member function like foo. Jason
On Fri, 18 May 2018, Jakub Jelinek wrote: > On Fri, May 18, 2018 at 09:02:08AM -0400, Nathan Sidwell wrote: >> On 05/18/2018 08:53 AM, Marc Glisse wrote: >> >>> As long as you do not dereference ptr in the constructor, that shouldn't >>> contradict 'restrict'. The PR gives this quote from the standard: >>> >>> "During the construction of an object, if the value of the object or any >>> of its subobjects is accessed through a glvalue that is not obtained, >>> directly or indirectly, from the constructor’s this pointer, the value >>> of the object or subobject thus obtained is unspecified." >>> >>> which reads quite close to saying that 'this' is restrict. >> Indeed it is, thanks. >> >> what about comparisons to this? I thought restrict implied such a >> comparison was 'never the same'? >> >> ie. if the ctor was: >> selfie (selfie *ptr) : me (ptr==this ? 0 : ptr) {} > > But what is invalid on: > struct S { int foo (S *); int a; } s { 2 }; > int S::foo (S *x) > { > int b = this->a; > x->a = 5; > b += this->a; > return b; > } > int main () > { > if (s.foo (&s) != 7) > abort (); > } > > I think if you make this a restrict pointer, this will be miscompiled. The patch is only for constructors, not for all member functions.
On 05/18/2018 09:21 AM, Marc Glisse wrote: >> what about comparisons to this? I thought restrict implied such a >> comparison was 'never the same'? >> >> ie. if the ctor was: >> selfie (selfie *ptr) : me (ptr==this ? 0 : ptr) {} > > It is tempting to think so, but I don't see any language to that effect. Ok then, thanks for the explanations. nathan
On Fri, May 18, 2018 at 09:57:42AM -0400, Jason Merrill wrote: > > But what is invalid on: > > struct S { int foo (S *); int a; } s { 2 }; > > int S::foo (S *x) > > { > > int b = this->a; > > x->a = 5; > > b += this->a; > > return b; > > } > > int main () > > { > > if (s.foo (&s) != 7) > > abort (); > > } > > > > I think if you make this a restrict pointer, this will be miscompiled. > > We're only talking about the 'this' pointer in a constructor, not a > normal member function like foo. Oops, sorry, missed that. Make it then: struct S { S (S *); int a; } s (&s); S::S (S *s) { this->a = 2; s->a = 3; if (this->a != 3) abort (); } Or is it invalid to dereference s before it has been constructed? Jakub
On 05/18/2018 10:12 AM, Jakub Jelinek wrote:
> Or is it invalid to dereference s before it has been constructed?
Yes, Marc found:
"During the construction of an object, if the value of the object or any
of its subobjects is accessed through a glvalue that is not obtained,
directly or indirectly, from the constructor’s this pointer, the value
of the object or subobject thus obtained is unspecified."
nathan
Index: gcc/testsuite/g++.dg/pr82899.C =================================================================== --- gcc/testsuite/g++.dg/pr82899.C (nonexistent) +++ gcc/testsuite/g++.dg/pr82899.C (working copy) @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-O -fdump-tree-optimized" } */ + +struct A { + int i; + A(A&); +}; +int X; +A::A(A&a):i(42){ + a.i=0; + X=i; +} + +/* { dg-final { scan-tree-dump "X = 42;" "optimized" } } */ Index: gcc/tree-ssa-structalias.c =================================================================== --- gcc/tree-ssa-structalias.c (revision 260347) +++ gcc/tree-ssa-structalias.c (working copy) @@ -5928,25 +5928,28 @@ check_for_overlaps (vec<fieldoff_s> fiel return true; lastoffset = fo->offset; } return false; } /* Create a varinfo structure for NAME and DECL, and add it to VARMAP. This will also create any varinfo structures necessary for fields of DECL. DECL is a function parameter if HANDLE_PARAM is set. HANDLED_STRUCT_TYPE is used to register struct types reached by following - restrict pointers. This is needed to prevent infinite recursion. */ + restrict pointers. This is needed to prevent infinite recursion. + If ADD_RESTRICT, pretend that the pointer NAME is restrict even if DECL + does not advertise it. */ static varinfo_t create_variable_info_for_1 (tree decl, const char *name, bool add_id, - bool handle_param, bitmap handled_struct_type) + bool handle_param, bitmap handled_struct_type, + bool add_restrict = false) { varinfo_t vi, newvi; tree decl_type = TREE_TYPE (decl); tree declsize = DECL_P (decl) ? DECL_SIZE (decl) : TYPE_SIZE (decl_type); auto_vec<fieldoff_s> fieldstack; fieldoff_s *fo; unsigned int i; if (!declsize || !tree_fits_uhwi_p (declsize)) @@ -6006,21 +6009,21 @@ create_variable_info_for_1 (tree decl, c if (fieldstack.length () == 0 || fieldstack.length () > MAX_FIELDS_FOR_FIELD_SENSITIVE) { vi = new_var_info (decl, name, add_id); vi->offset = 0; vi->may_have_pointers = true; vi->fullsize = tree_to_uhwi (declsize); vi->size = vi->fullsize; vi->is_full_var = true; if (POINTER_TYPE_P (decl_type) - && TYPE_RESTRICT (decl_type)) + && (TYPE_RESTRICT (decl_type) || add_restrict)) vi->only_restrict_pointers = 1; if (vi->only_restrict_pointers && !type_contains_placeholder_p (TREE_TYPE (decl_type)) && handle_param && !bitmap_bit_p (handled_struct_type, TYPE_UID (TREE_TYPE (decl_type)))) { varinfo_t rvi; tree heapvar = build_fake_var_decl (TREE_TYPE (decl_type)); DECL_EXTERNAL (heapvar) = 1; @@ -6235,35 +6238,38 @@ make_param_constraints (varinfo_t vi) } /* Create varinfo structures for all of the variables in the function for intraprocedural mode. */ static void intra_create_variable_infos (struct function *fn) { tree t; bitmap handled_struct_type = NULL; + bool this_parm_in_ctor = DECL_CXX_CONSTRUCTOR_P (fn->decl); /* For each incoming pointer argument arg, create the constraint ARG = NONLOCAL or a dummy variable if it is a restrict qualified passed-by-reference argument. */ for (t = DECL_ARGUMENTS (fn->decl); t; t = DECL_CHAIN (t)) { if (handled_struct_type == NULL) handled_struct_type = BITMAP_ALLOC (NULL); varinfo_t p = create_variable_info_for_1 (t, alias_get_name (t), false, true, - handled_struct_type); + handled_struct_type, this_parm_in_ctor); insert_vi_for_tree (t, p); make_param_constraints (p); + + this_parm_in_ctor = false; } if (handled_struct_type != NULL) BITMAP_FREE (handled_struct_type); /* Add a constraint for a result decl that is passed by reference. */ if (DECL_RESULT (fn->decl) && DECL_BY_REFERENCE (DECL_RESULT (fn->decl))) { varinfo_t p, result_vi = get_vi_for_tree (DECL_RESULT (fn->decl));