[PATCH] Add a new type attribute always_alias (PR79671)

[PATCH] Add a new type attribute always_alias (PR79671)

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 887 bytes --]

Hi,

this is related to the P1 codegen bug PR79671:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671

Therefore I would like to add this now although
the trunk is already at in stage 4.

I propose to add a new type attribute always_alias that
works like may_alias but unlike may_alias it should
also make instances alias anything.  It is also exposed
in C/C++ as __attribute__((always_alias)).

For C++17 the FE automatically introduces this attribute
at std::byte and all class/struct/union which contain
either a std::byte array or an unsigned char array.

Note that the patch [1] needs to be applied as well,
because otherwise the new attribute will trip several
failures in the libstdc++ testsuite.


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.


[1] https://gcc.gnu.org/ml/gcc-patches/2017-04/msg00189.html

[-- Attachment #2: changelog-always-alias.txt --]
[-- Type: text/plain, Size: 928 bytes --]

gcc
2017-04-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* doc/extend.texi: Document the always_alias type attribute.
	* alias.c (get_alias_set): Honor the always_alias attribute.
	(record_component_aliases): Don't ignore the always_alias attribute.
	* tree.c (build_pointer_type_for_mode,
	build_reference_type_for_mode): Add the always_alias attribute.

gcc/c-family
2017-04-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-attribs.c (c_common_attribute_tab): Add the always_alias attribute.

gcc/cp
2017-04-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* class.c (fixup_attribute_variants): Add the always_alias attribute.
	(finish_struct_1): Add an always_alias attribute if required by C++17.
	* decl.c (start_enum): Likewise.
	* pt.c (lookup_template_class_1): Add the always_alias attribute.
	* typeck2.c (cxx_type_contains_byte_buffer): New function.
	* cp-tree.h (cxx_type_contains_byte_buffer): Declare.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: patch-always-alias.diff --]
[-- Type: text/x-patch; name="patch-always-alias.diff", Size: 8513 bytes --]

Index: gcc/alias.c
===================================================================
--- gcc/alias.c	(revision 246678)
+++ gcc/alias.c	(working copy)
@@ -879,6 +879,10 @@ get_alias_set (tree t)
       t = TREE_TYPE (t);
     }
 
+  /* Honor the always_alias type attribute.  */
+  if (lookup_attribute ("always_alias", TYPE_ATTRIBUTES (t)))
+    return 0;
+
   /* Variant qualifiers don't affect the alias set, so get the main
      variant.  */
   t = TYPE_MAIN_VARIANT (t);
@@ -1234,7 +1238,9 @@ record_component_aliases (tree type)
 		/* VECTOR_TYPE and ARRAY_TYPE share the alias set with their
 		   element type and that type has to be normalized to void *,
 		   too, in the case it is a pointer. */
-		while (!canonical_type_used_p (t) && !POINTER_TYPE_P (t))
+		while (!canonical_type_used_p (t) && !POINTER_TYPE_P (t)
+		       && !lookup_attribute ("always_alias",
+					     TYPE_ATTRIBUTES (t)))
 		  {
 		    gcc_checking_assert (TYPE_STRUCTURAL_EQUALITY_P (t));
 		    t = TREE_TYPE (t);
Index: gcc/c-family/c-attribs.c
===================================================================
--- gcc/c-family/c-attribs.c	(revision 246678)
+++ gcc/c-family/c-attribs.c	(working copy)
@@ -265,6 +265,7 @@ const struct attribute_spec c_common_attribute_tab
   { "nothrow",                0, 0, true,  false, false,
 			      handle_nothrow_attribute, false },
   { "may_alias",	      0, 0, false, true, false, NULL, false },
+  { "always_alias",	      0, 0, false, true, false, NULL, false },
   { "cleanup",		      1, 1, true, false, false,
 			      handle_cleanup_attribute, false },
   { "warn_unused_result",     0, 0, false, true, true,
Index: gcc/cp/class.c
===================================================================
--- gcc/cp/class.c	(revision 246678)
+++ gcc/cp/class.c	(working copy)
@@ -2083,7 +2082,8 @@ fixup_attribute_variants (tree t)
   tree attrs = TYPE_ATTRIBUTES (t);
   unsigned align = TYPE_ALIGN (t);
   bool user_align = TYPE_USER_ALIGN (t);
-  bool may_alias = lookup_attribute ("may_alias", attrs);
+  bool may_alias = lookup_attribute ("may_alias", attrs)
+		   || lookup_attribute ("always_alias", attrs);
 
   if (may_alias)
     fixup_may_alias (t);
@@ -7345,6 +7346,15 @@ finish_struct_1 (tree t)
      the class or perform any other required target modifications.  */
   targetm.cxx.adjust_class_at_definition (t);
 
+  if (cxx_dialect >= cxx1z && cxx_type_contains_byte_buffer (t)
+      && !lookup_attribute ("always_alias", TYPE_ATTRIBUTES (t)))
+    {
+      TYPE_ATTRIBUTES (t)
+	= tree_cons (get_identifier ("always_alias"),
+		     NULL_TREE, TYPE_ATTRIBUTES (t));
+      fixup_attribute_variants (t);
+    }
+
   maybe_suppress_debug_info (t);
 
   if (flag_vtable_verify)
Index: gcc/cp/cp-tree.h
===================================================================
--- gcc/cp/cp-tree.h	(revision 246678)
+++ gcc/cp/cp-tree.h	(working copy)
@@ -6858,6 +6858,7 @@ extern tree finish_binary_fold_expr          (tree
 extern void require_complete_eh_spec_types	(tree, tree);
 extern void cxx_incomplete_type_diagnostic	(location_t, const_tree,
 						 const_tree, diagnostic_t);
+extern bool cxx_type_contains_byte_buffer	(tree);
 inline void
 cxx_incomplete_type_diagnostic (const_tree value, const_tree type,
 				diagnostic_t diag_kind)
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 246678)
+++ gcc/cp/decl.c	(working copy)
@@ -14081,10 +14081,15 @@ start_enum (tree name, tree enumtype, tree underly
 	  enumtype = pushtag (name, enumtype, /*tag_scope=*/ts_current);
 
 	  /* std::byte aliases anything.  */
-	  if (enumtype != error_mark_node
+	  if (cxx_dialect >= cxx1z
+	      && enumtype != error_mark_node
 	      && TYPE_CONTEXT (enumtype) == std_node
-	      && !strcmp ("byte", TYPE_NAME_STRING (enumtype)))
-	    TYPE_ALIAS_SET (enumtype) = 0;
+	      && !strcmp ("byte", TYPE_NAME_STRING (enumtype))
+	      && !lookup_attribute ("always_alias",
+				    TYPE_ATTRIBUTES (enumtype)))
+	    TYPE_ATTRIBUTES (enumtype)
+	      = tree_cons (get_identifier ("always_alias"),
+			   NULL_TREE, TYPE_ATTRIBUTES (enumtype));
 	}
       else
 	  enumtype = xref_tag (enum_type, name, /*tag_scope=*/ts_current,
Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c	(revision 246678)
+++ gcc/cp/pt.c	(working copy)
@@ -8851,9 +8851,9 @@ lookup_template_class_1 (tree d1, tree arglist, tr
       if (OVERLOAD_TYPE_P (t)
 	  && !DECL_ALIAS_TEMPLATE_P (gen_tmpl))
 	{
-	  static const char *tags[] = {"abi_tag", "may_alias"};
+	  static const char *tags[] = {"abi_tag", "may_alias", "always_alias"};
 
-	  for (unsigned ix = 0; ix != 2; ix++)
+	  for (unsigned ix = 0; ix < sizeof (tags) / sizeof (tags[0]); ix++)
 	    {
 	      tree attributes
 		= lookup_attribute (tags[ix], TYPE_ATTRIBUTES (template_type));
Index: gcc/cp/typeck2.c
===================================================================
--- gcc/cp/typeck2.c	(revision 246678)
+++ gcc/cp/typeck2.c	(working copy)
@@ -2234,5 +2234,29 @@ require_complete_eh_spec_types (tree fntype, tree
     }
 }
 
+/* True iff type either is or contains a byte buffer (which can be used for
+   storing any trivially copyable type).  */
+
+bool
+cxx_type_contains_byte_buffer (tree type)
+{
+  if (TREE_CODE (type) == ARRAY_TYPE
+      && (cxx_type_contains_byte_buffer (TREE_TYPE (type))
+	  || TREE_TYPE (type) == unsigned_char_type_node
+	  || (TREE_CODE (TREE_TYPE (type)) == ENUMERAL_TYPE
+	      && TYPE_CONTEXT (TREE_TYPE (type)) == std_node
+	      && !strcmp ("byte", TYPE_NAME_STRING (TREE_TYPE (type))))))
+    return true;
+
+  if (CLASS_TYPE_P (type))
+    for (tree field = next_initializable_field (TYPE_FIELDS (type));
+	 field;
+	 field = next_initializable_field (DECL_CHAIN (field)))
+      if (cxx_type_contains_byte_buffer (TREE_TYPE (field)))
+	return true;
+
+  return false;
+}
+
 \f
 #include "gt-cp-typeck2.h"
Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 246678)
+++ gcc/doc/extend.texi	(working copy)
@@ -6656,6 +6656,11 @@ declaration, the above program would abort when co
 @option{-fstrict-aliasing}, which is on by default at @option{-O2} or
 above.
 
+@item always_alias
+@cindex @code{always_alias} type attribute
+Same as @code{may_alias}, but additionally applies to instances of
+types with this attribute.
+
 @item packed
 @cindex @code{packed} type attribute
 This attribute, attached to @code{struct} or @code{union} type
Index: gcc/testsuite/c-c++-common/attr-always-alias-1.c
===================================================================
--- gcc/testsuite/c-c++-common/attr-always-alias-1.c	(revision 0)
+++ gcc/testsuite/c-c++-common/attr-always-alias-1.c	(working copy)
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wall" } */
+
+typedef int T __attribute__((always_alias));
+
+extern T t, v;
+extern T *p;
+extern int *p;
+
+extern int *p2;
+extern T *p2;
+
+void fn1 (T);
+void fn1 (int);
+
+void fn2 (int);
+void fn2 (T);
+
+/* Ensure that the composite types have always_alias.  */
+void
+f (long *i)
+{
+  *i = *(__typeof (*p) *) &p;
+  asm ("" : : "r" (*p));
+  *i = *(__typeof (*p2) *) &p2;
+  asm ("" : : "r" (*p2));
+  t = v;
+  asm ("" : : "r" (t));
+}
Index: gcc/tree.c
===================================================================
--- gcc/tree.c	(revision 246678)
+++ gcc/tree.c	(working copy)
@@ -8041,7 +8041,8 @@ build_pointer_type_for_mode (tree to_type, machine
 
   /* If the pointed-to type has the may_alias attribute set, force
      a TYPE_REF_CAN_ALIAS_ALL pointer to be generated.  */
-  if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
+  if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type))
+      || lookup_attribute ("always_alias", TYPE_ATTRIBUTES (to_type)))
     can_alias_all = true;
 
   /* In some cases, languages will have things that aren't a POINTER_TYPE
@@ -8110,7 +8111,8 @@ build_reference_type_for_mode (tree to_type, machi
 
   /* If the pointed-to type has the may_alias attribute set, force
      a TYPE_REF_CAN_ALIAS_ALL pointer to be generated.  */
-  if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
+  if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type))
+      || lookup_attribute ("always_alias", TYPE_ATTRIBUTES (to_type)))
     can_alias_all = true;
 
   /* In some cases, languages will have things that aren't a REFERENCE_TYPE

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Jakub Jelinek]

On Wed, Apr 05, 2017 at 09:46:09AM +0000, Bernd Edlinger wrote:
> this is related to the P1 codegen bug PR79671:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671
> 
> Therefore I would like to add this now although
> the trunk is already at in stage 4.
> 
> I propose to add a new type attribute always_alias that
> works like may_alias but unlike may_alias it should
> also make instances alias anything.  It is also exposed
> in C/C++ as __attribute__((always_alias)).

I have just two small comments, for review IMHO want agreement
between Jason and Richard on this.

One thing is whether it is a good idea to keep this info in the IL
as an attribute, especially when you add it automatically in the
C++ FE.  I see e.g. 25 spare bits in tree_type_common.  Don't say we
need to waste them, but I'd say in C++ unsigned char arrays are fairly
common and thus not having to create the attribute for them each time
and look it up might be beneficial.

Also, wonder if you need to mark all types containing such arrays,
if you couldn't just set that flag in C++ on unsigned char/std::byte
arrays (and on anything with that attribute), have that imply alias set
0 on it and then let the rest of alias machinery handle aggregate types
containing such fields.

	Jakub

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Richard Biener]

On April 5, 2017 3:28:32 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>On Wed, Apr 05, 2017 at 09:46:09AM +0000, Bernd Edlinger wrote:
>> this is related to the P1 codegen bug PR79671:
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671
>> 
>> Therefore I would like to add this now although
>> the trunk is already at in stage 4.
>> 
>> I propose to add a new type attribute always_alias that
>> works like may_alias but unlike may_alias it should
>> also make instances alias anything.  It is also exposed
>> in C/C++ as __attribute__((always_alias)).
>
>I have just two small comments, for review IMHO want agreement
>between Jason and Richard on this.
>
>One thing is whether it is a good idea to keep this info in the IL
>as an attribute, especially when you add it automatically in the
>C++ FE.  I see e.g. 25 spare bits in tree_type_common.  Don't say we
>need to waste them, but I'd say in C++ unsigned char arrays are fairly
>common and thus not having to create the attribute for them each time
>and look it up might be beneficial.

Agreed.

>Also, wonder if you need to mark all types containing such arrays,
>if you couldn't just set that flag in C++ on unsigned char/std::byte
>arrays (and on anything with that attribute), have that imply alias set
>0 on it and then let the rest of alias machinery handle aggregate types
>containing such fields.

Yes, I expected it to work like this (didn't look at the patch yet).

Richard.

>	Jakub

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Bernd Edlinger]

On 04/05/17 17:20, Richard Biener wrote:
>> Also, wonder if you need to mark all types containing such arrays,
>> if you couldn't just set that flag in C++ on unsigned char/std::byte
>> arrays (and on anything with that attribute), have that imply alias set
>> 0 on it and then let the rest of alias machinery handle aggregate types
>> containing such fields.
>
> Yes, I expected it to work like this (didn't look at the patch yet).
>

I want to allow *only* what the C++ standard requires or what Jason says
of course :), and not a single bit more, because it suppresses otherwise
correct optimizations.

So a struct with a std::byte member is not alias_set 0,
only the std::byte itself is, but an array of std::byte
is itself typeless_storage, and makes the whole structure
also typeless_storage, but that is not the usual way how
the alias machinery works, where such an attribute would not
propagate.  Currently I think the C++ FE can do the propagation
when the type is declared.  I would not imply that with the
typeless_storage attribute, because may_alias does not do that
either.


Bernd.

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Jason Merrill]

On Wed, Apr 5, 2017 at 1:41 PM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
> On 04/05/17 17:20, Richard Biener wrote:
>>> Also, wonder if you need to mark all types containing such arrays,
>>> if you couldn't just set that flag in C++ on unsigned char/std::byte
>>> arrays (and on anything with that attribute), have that imply alias set
>>> 0 on it and then let the rest of alias machinery handle aggregate types
>>> containing such fields.
>>
>> Yes, I expected it to work like this (didn't look at the patch yet).

My impression is that this is how GCC 6 worked, but GCC 7 decides to
ignore alias set 0 members.  Is that right?

> I want to allow *only* what the C++ standard requires or what Jason says
> of course :), and not a single bit more, because it suppresses otherwise
> correct optimizations.
>
> So a struct with a std::byte member is not alias_set 0,
> only the std::byte itself is, but an array of std::byte
> is itself typeless_storage, and makes the whole structure
> also typeless_storage

Well, only the array member, not the whole structure, but it may make
sense for GCC to treat the whole structure as such internally.

Jason

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Bernd Edlinger]

On 04/05/17 22:17, Jason Merrill wrote:
> On Wed, Apr 5, 2017 at 1:41 PM, Bernd Edlinger
> <bernd.edlinger@hotmail.de> wrote:
>> On 04/05/17 17:20, Richard Biener wrote:
>>>> Also, wonder if you need to mark all types containing such arrays,
>>>> if you couldn't just set that flag in C++ on unsigned char/std::byte
>>>> arrays (and on anything with that attribute), have that imply alias set
>>>> 0 on it and then let the rest of alias machinery handle aggregate types
>>>> containing such fields.
>>>
>>> Yes, I expected it to work like this (didn't look at the patch yet).
>
> My impression is that this is how GCC 6 worked, but GCC 7 decides to
> ignore alias set 0 members.  Is that right?
>

This is how I always thought it worked, until Jakub pointed out that
it is documented differently:

@item may_alias
@cindex @code{may_alias} type attribute
Accesses through pointers to types with this attribute are not subject
to type-based alias analysis, but are instead assumed to be able to 
alias any other type of objects.
In the context of section 6.5 paragraph 7 of the C99 standard,
an lvalue expression dereferencing such a pointer is treated like
having a character type.

=> So nobody said anything about accesses without pointers.



>> I want to allow *only* what the C++ standard requires or what Jason says
>> of course :), and not a single bit more, because it suppresses otherwise
>> correct optimizations.
>>
>> So a struct with a std::byte member is not alias_set 0,
>> only the std::byte itself is, but an array of std::byte
>> is itself typeless_storage, and makes the whole structure
>> also typeless_storage
>
> Well, only the array member, not the whole structure, but it may make
> sense for GCC to treat the whole structure as such internally.
>


Yes, that is only as close as I can get in the moment.

It does the same as may_alias but additionally objects
declared with that type have alias set 0, I just don't
know if I have yet found the right words so that it can
be understood.  Based on the feedback I have now written:

+@item typeless_storage
+@cindex @code{typeless_storage} type attribute
+An object declared with a type with this attribute behaves like a
+character type with respect to aliasing semantics.



Bernd.

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Pedro Alves]

On 04/05/2017 09:46 PM, Bernd Edlinger wrote:

> It does the same as may_alias but additionally objects
> declared with that type have alias set 0, I just don't
> know if I have yet found the right words so that it can
> be understood.  Based on the feedback I have now written:
> 
> +@item typeless_storage
> +@cindex @code{typeless_storage} type attribute
> +An object declared with a type with this attribute behaves like a
> +character type with respect to aliasing semantics.

I think including an example of what becomes valid with
such types but wouldn't be valid both without any
attribute and also with may_alias would help.

Thanks,
Pedro Alves

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Jonathan Wakely]

On 05/04/17 20:46 +0000, Bernd Edlinger wrote:
>It does the same as may_alias but additionally objects
>declared with that type have alias set 0, I just don't
>know if I have yet found the right words so that it can
>be understood.  Based on the feedback I have now written:
>
>+@item typeless_storage
>+@cindex @code{typeless_storage} type attribute
>+An object declared with a type with this attribute behaves like a
>+character type with respect to aliasing semantics.

This says that an object behaves like a type, which is a category
error. Don't you mean that a type declared with this attribute behaves
like a character type w.r.t aliasing semantics?

Or an object of a type with this attribute behaves like an object of
character type w.r.t aliasing semantics.

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Richard Biener]

On Wed, 5 Apr 2017, Jason Merrill wrote:

> On Wed, Apr 5, 2017 at 1:41 PM, Bernd Edlinger
> <bernd.edlinger@hotmail.de> wrote:
> > On 04/05/17 17:20, Richard Biener wrote:
> >>> Also, wonder if you need to mark all types containing such arrays,
> >>> if you couldn't just set that flag in C++ on unsigned char/std::byte
> >>> arrays (and on anything with that attribute), have that imply alias set
> >>> 0 on it and then let the rest of alias machinery handle aggregate types
> >>> containing such fields.
> >>
> >> Yes, I expected it to work like this (didn't look at the patch yet).
> 
> My impression is that this is how GCC 6 worked, but GCC 7 decides to
> ignore alias set 0 members.  Is that right?

Yes, in GCC 6 an access via an aggregate type that had an alias-set zero
member was using alias-set zero.

So we'd re-instantiate that behavior for aggregates containing
a type marked with the proposed new flag.

> > I want to allow *only* what the C++ standard requires or what Jason says
> > of course :), and not a single bit more, because it suppresses otherwise
> > correct optimizations.
> >
> > So a struct with a std::byte member is not alias_set 0,
> > only the std::byte itself is, but an array of std::byte
> > is itself typeless_storage, and makes the whole structure
> > also typeless_storage
> 
> Well, only the array member, not the whole structure, but it may make
> sense for GCC to treat the whole structure as such internally.

Yes, it's the easiest way to implement the required behavior.  But
I wouldn't document it that way but clearly state that only the
marked member behaves so but that for the purpose of accesses via
a container type the standards access rules apply as if the member
is accessed with a character type.

Richard.

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Bernd Edlinger]

On 04/05/17 15:28, Jakub Jelinek wrote:
> On Wed, Apr 05, 2017 at 09:46:09AM +0000, Bernd Edlinger wrote:
>> this is related to the P1 codegen bug PR79671:
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671
>>
>> Therefore I would like to add this now although
>> the trunk is already at in stage 4.
>>
>> I propose to add a new type attribute always_alias that
>> works like may_alias but unlike may_alias it should
>> also make instances alias anything.  It is also exposed
>> in C/C++ as __attribute__((always_alias)).
>
> I have just two small comments, for review IMHO want agreement
> between Jason and Richard on this.
>
> One thing is whether it is a good idea to keep this info in the IL
> as an attribute, especially when you add it automatically in the
> C++ FE.  I see e.g. 25 spare bits in tree_type_common.  Don't say we
> need to waste them, but I'd say in C++ unsigned char arrays are fairly
> common and thus not having to create the attribute for them each time
> and look it up might be beneficial.
>

Yes, but it is not clear if it will be available in LTO, for instance
we used to stream TYPE_ALIAS_SET == 0 information to let frontends mark
types that are opaque for TBAA.  This however did not work as intended,
because TYPE_ALIAS_SET == 0 was regularly lost in type merging.

But streaming an attribute works seamless, and is not lost in type
merging AFAICT.

> Also, wonder if you need to mark all types containing such arrays,
> if you couldn't just set that flag in C++ on unsigned char/std::byte
> arrays (and on anything with that attribute), have that imply alias set
> 0 on it and then let the rest of alias machinery handle aggregate types
> containing such fields.
>

Actually I set the flag only on C++ code with std::byte and
struct/union with embedded unsigned char or std::byte arrays,
but not on char arrays for instance, so I hope that it will
not pessimize the codegen too much.  The attribute does not
propagate from normal std::byte to the enclosing structure.

With LTO it is impossible to know which -std= option was used
and all C and C++ code is merged together.  So I was trying hard
to fin a way how it can work without breaking LTO.


Bernd.

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Jakub Jelinek]

On Wed, Apr 05, 2017 at 03:27:20PM +0000, Bernd Edlinger wrote:
> Yes, but it is not clear if it will be available in LTO, for instance
> we used to stream TYPE_ALIAS_SET == 0 information to let frontends mark
> types that are opaque for TBAA.  This however did not work as intended,
> because TYPE_ALIAS_SET == 0 was regularly lost in type merging.
> 
> But streaming an attribute works seamless, and is not lost in type
> merging AFAICT.

Streaming flags on types works seamlessly too, if you add the 2-3x one-liner
changes in the streamer.  Just look at some other type flag and do the same
thing.

	Jakub

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Florian Weimer]

On 04/05/2017 11:46 AM, Bernd Edlinger wrote:
> +@item always_alias
> +@cindex @code{always_alias} type attribute
> +Same as @code{may_alias}, but additionally applies to instances of
> +types with this attribute.

As a GCC user, I have to say that this doesn't really explain what the 
attribute does.  The C standard does not define what an “instance” of a 
type is.

Thanks,
Florian

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Richard Biener]

On April 5, 2017 4:50:32 PM GMT+02:00, Florian Weimer <fweimer@redhat.com> wrote:
>On 04/05/2017 11:46 AM, Bernd Edlinger wrote:
>> +@item always_alias
>> +@cindex @code{always_alias} type attribute
>> +Same as @code{may_alias}, but additionally applies to instances of
>> +types with this attribute.
>
>As a GCC user, I have to say that this doesn't really explain what the 
>attribute does.  The C standard does not define what an “instance” of a
>
>type is.

I don't like the name.  I'd name it 'typeless_storage' and document that when storage is accessed with a type with this attribute it it behaves like a character type with respect to semantics.

Richard.

>
>Thanks,
>Florian

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Bernd Edlinger]

On 04/05/17 17:23, Richard Biener wrote:
> On April 5, 2017 4:50:32 PM GMT+02:00, Florian Weimer <fweimer@redhat.com> wrote:
>> On 04/05/2017 11:46 AM, Bernd Edlinger wrote:
>>> +@item always_alias
>>> +@cindex @code{always_alias} type attribute
>>> +Same as @code{may_alias}, but additionally applies to instances of
>>> +types with this attribute.
>>
>> As a GCC user, I have to say that this doesn't really explain what the
>> attribute does.  The C standard does not define what an “instance” of a
>>
>> type is.
>
> I don't like the name.  I'd name it 'typeless_storage' and document that when storage is accessed with a type with this attribute it it behaves like a character type with respect to semantics.
>

you mean you like to name it __attribute__((typeless_storage)) ?

I wanted to say it behaves mostly like __attribute__((may_alias))
except that may_alias only has an effect on pointers and references
but not when accessing an object directly, (I hope you know what
I mean).


Bernd.


> Richard.
>
>>
>> Thanks,
>> Florian
>

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Jonathan Wakely]

On 05/04/17 15:38 +0000, Bernd Edlinger wrote:
>On 04/05/17 17:23, Richard Biener wrote:
>> On April 5, 2017 4:50:32 PM GMT+02:00, Florian Weimer <fweimer@redhat.com> wrote:
>>> On 04/05/2017 11:46 AM, Bernd Edlinger wrote:
>>>> +@item always_alias
>>>> +@cindex @code{always_alias} type attribute
>>>> +Same as @code{may_alias}, but additionally applies to instances of
>>>> +types with this attribute.
>>>
>>> As a GCC user, I have to say that this doesn't really explain what the
>>> attribute does.  The C standard does not define what an “instance” of a
>>>
>>> type is.

Agreed.


>> I don't like the name.  I'd name it 'typeless_storage' and document that when storage is accessed with a type with this attribute it it behaves like a character type with respect to semantics.
>>
>
>you mean you like to name it __attribute__((typeless_storage)) ?
>
>I wanted to say it behaves mostly like __attribute__((may_alias))
>except that may_alias only has an effect on pointers and references
>but not when accessing an object directly, (I hope you know what
>I mean).

And may_alias is not a very helpful name either.

I much prefer Richi's suggestion of typeless_storage.

But I'm not convinced we need the attribute at all. If a struct
containing an array of std::byte or unsigned char has the property
already then that's good. I don't think we need a non-portable way to
make other types behave the same. If you can change the code to use a
new GCC attribute then you can change the code to use an array of
unsigned char, and be portable.

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Jakub Jelinek]

On Wed, Apr 05, 2017 at 05:03:33PM +0100, Jonathan Wakely wrote:
> > I wanted to say it behaves mostly like __attribute__((may_alias))
> > except that may_alias only has an effect on pointers and references
> > but not when accessing an object directly, (I hope you know what
> > I mean).
> 
> And may_alias is not a very helpful name either.
> 
> I much prefer Richi's suggestion of typeless_storage.
> 
> But I'm not convinced we need the attribute at all. If a struct
> containing an array of std::byte or unsigned char has the property
> already then that's good. I don't think we need a non-portable way to
> make other types behave the same. If you can change the code to use a
> new GCC attribute then you can change the code to use an array of
> unsigned char, and be portable.

It will only work that way in C++ though, so if you want to achieve
the same in C, which doesn't have any special case for unsigned char
arrays nor std::byte, you need an attribute.

	Jakub

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Bernd Edlinger]

On 04/05/17 18:08, Jakub Jelinek wrote:
> On Wed, Apr 05, 2017 at 05:03:33PM +0100, Jonathan Wakely wrote:
>>> I wanted to say it behaves mostly like __attribute__((may_alias))
>>> except that may_alias only has an effect on pointers and references
>>> but not when accessing an object directly, (I hope you know what
>>> I mean).
>>
>> And may_alias is not a very helpful name either.
>>
>> I much prefer Richi's suggestion of typeless_storage.
>>
>> But I'm not convinced we need the attribute at all. If a struct
>> containing an array of std::byte or unsigned char has the property
>> already then that's good. I don't think we need a non-portable way to
>> make other types behave the same. If you can change the code to use a
>> new GCC attribute then you can change the code to use an array of
>> unsigned char, and be portable.
>
> It will only work that way in C++ though, so if you want to achieve
> the same in C, which doesn't have any special case for unsigned char
> arrays nor std::byte, you need an attribute.
>

Yes, exactly.  I really want to reach the deadline for gcc-7.
Fixing the name is certainly the most important first step,
and if everybody agrees on "typeless_storage", for the name
I can start with adjusting the name, and look into how
to use a spare type-flag that should be a mechanical change.


Bernd.

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 695 bytes --]

On 04/05/17 19:22, Bernd Edlinger wrote:
> On 04/05/17 18:08, Jakub Jelinek wrote:
>
> Yes, exactly.  I really want to reach the deadline for gcc-7.
> Fixing the name is certainly the most important first step,
> and if everybody agrees on "typeless_storage", for the name
> I can start with adjusting the name, and look into how
> to use a spare type-flag that should be a mechanical change.
>

Jakub, I just renamed the attribute and reworked the patch
as you suggested, reg-testing is not yet completed, but
it looks good so far.  I also added a few more tests.

I have changed the documentation as Richi suggested, but
I am not too sure what to say here.


Thanks
Bernd.

[-- Attachment #2: changelog-typeless-storage.txt --]
[-- Type: text/plain, Size: 1759 bytes --]

gcc
2017-04-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* doc/extend.texi: Document the typeless_storage type attribute.
	* alias.c (get_alias_set): Honor the typeless_storage attribute.
	(record_component_aliases): Don't ignore the typeless_storage
	attribute.
	* tree.c (build_pointer_type_for_mode,
	build_reference_type_for_mode): Handle the typeless_storage
	attribute.
	* print-tree.c (print_node): Likewise.
	* tree-streamer-in.c (unpack_ts_type_common_value_fields): Likewise.
	* tree-streamer-out.c (pack_ts_type_common_value_fields): Likewise.
	* tre.h (TYPE_TYPELESS_STORAGE): New access macro.
	* tree-core.h (tree_type_common::typeless_storage_flag): New flag.

gcc/c-family
2017-04-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-attribs.c (c_common_attribute_tab): Add the typeless_storage
	attribute.
	(handle_typeless_storage_attribute): New function.

gcc/cp
2017-04-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* class.c (fixup_attribute_variants): Handle the typeless_storage
	attribute.
	(finish_struct_1): Set the typeless_storage attribute if required
	by C++17.
	* decl.c (start_enum): Likewise.
	* pt.c (lookup_template_class_1): Handle the typeless_storage
	attribute.
	* typeck2.c (cxx_type_contains_byte_buffer): New function.
	* cp-tree.h (cxx_type_contains_byte_buffer): Declare.

gcc/lto
2017-04-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* lto.c (compare_tree_sccs_1, hash_tree): Handle the typeless_storage
	attribute.

gcc/testsuite
2017-04-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-c++-common/attr-typeless-storage-1.c: New test.
	* c-c++-common/attr-typeless-storage-2.c: New test.
	* gcc.c-torture/execute/typeless-storage-1.c: New test.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: patch-typeless-storage.diff --]
[-- Type: text/x-patch; name="patch-typeless-storage.diff", Size: 15148 bytes --]

Index: gcc/alias.c
===================================================================
--- gcc/alias.c	(revision 246678)
+++ gcc/alias.c	(working copy)
@@ -879,6 +879,10 @@ get_alias_set (tree t)
       t = TREE_TYPE (t);
     }
 
+  /* Honor the typeless_storage type attribute.  */
+  if (TYPE_TYPELESS_STORAGE (t))
+    return 0;
+
   /* Variant qualifiers don't affect the alias set, so get the main
      variant.  */
   t = TYPE_MAIN_VARIANT (t);
@@ -1234,7 +1238,8 @@ record_component_aliases (tree type)
 		/* VECTOR_TYPE and ARRAY_TYPE share the alias set with their
 		   element type and that type has to be normalized to void *,
 		   too, in the case it is a pointer. */
-		while (!canonical_type_used_p (t) && !POINTER_TYPE_P (t))
+		while (!canonical_type_used_p (t) && !POINTER_TYPE_P (t)
+		       && !TYPE_TYPELESS_STORAGE (t))
 		  {
 		    gcc_checking_assert (TYPE_STRUCTURAL_EQUALITY_P (t));
 		    t = TREE_TYPE (t);
Index: gcc/c-family/c-attribs.c
===================================================================
--- gcc/c-family/c-attribs.c	(revision 246678)
+++ gcc/c-family/c-attribs.c	(working copy)
@@ -113,6 +113,7 @@ static tree handle_vector_size_attribute (tree *,
 					  bool *);
 static tree handle_nonnull_attribute (tree *, tree, tree, int, bool *);
 static tree handle_nothrow_attribute (tree *, tree, tree, int, bool *);
+static tree handle_typeless_storage_attribute (tree *, tree, tree, int, bool *);
 static tree handle_cleanup_attribute (tree *, tree, tree, int, bool *);
 static tree handle_warn_unused_result_attribute (tree *, tree, tree, int,
 						 bool *);
@@ -265,6 +266,8 @@ const struct attribute_spec c_common_attribute_tab
   { "nothrow",                0, 0, true,  false, false,
 			      handle_nothrow_attribute, false },
   { "may_alias",	      0, 0, false, true, false, NULL, false },
+  { "typeless_storage",       0, 0, false, true, false,
+			      handle_typeless_storage_attribute, false },
   { "cleanup",		      1, 1, true, false, false,
 			      handle_cleanup_attribute, false },
   { "warn_unused_result",     0, 0, false, true, true,
@@ -2879,6 +2882,24 @@ handle_nothrow_attribute (tree *node, tree name, t
   return NULL_TREE;
 }
 
+/* Handle a "typeless_storage" attribute; arguments as in
+   struct attribute_spec.handler.  */
+
+static tree
+handle_typeless_storage_attribute (tree *node, tree name, tree ARG_UNUSED (args),
+				   int ARG_UNUSED (flags), bool *no_add_attrs)
+{
+  if (TYPE_P (*node))
+    TYPE_TYPELESS_STORAGE (*node) = 1;
+  else
+    {
+      warning (OPT_Wattributes, "%qE attribute ignored", name);
+      *no_add_attrs = true;
+    }
+
+  return NULL_TREE;
+}
+
 /* Handle a "cleanup" attribute; arguments as in
    struct attribute_spec.handler.  */
 
Index: gcc/cp/class.c
===================================================================
--- gcc/cp/class.c	(revision 246678)
+++ gcc/cp/class.c	(working copy)
@@ -2083,7 +2083,8 @@ fixup_attribute_variants (tree t)
   tree attrs = TYPE_ATTRIBUTES (t);
   unsigned align = TYPE_ALIGN (t);
   bool user_align = TYPE_USER_ALIGN (t);
-  bool may_alias = lookup_attribute ("may_alias", attrs);
+  bool may_alias = TYPE_TYPELESS_STORAGE (t)
+		   || lookup_attribute ("may_alias", attrs);
 
   if (may_alias)
     fixup_may_alias (t);
@@ -7345,6 +7348,12 @@ finish_struct_1 (tree t)
      the class or perform any other required target modifications.  */
   targetm.cxx.adjust_class_at_definition (t);
 
+  if (cxx_dialect >= cxx1z && cxx_type_contains_byte_buffer (t))
+    {
+      TYPE_TYPELESS_STORAGE (t) = 1;
+      fixup_attribute_variants (t);
+    }
+
   maybe_suppress_debug_info (t);
 
   if (flag_vtable_verify)
Index: gcc/cp/cp-tree.h
===================================================================
--- gcc/cp/cp-tree.h	(revision 246678)
+++ gcc/cp/cp-tree.h	(working copy)
@@ -6858,6 +6858,7 @@ extern tree finish_binary_fold_expr          (tree
 extern void require_complete_eh_spec_types	(tree, tree);
 extern void cxx_incomplete_type_diagnostic	(location_t, const_tree,
 						 const_tree, diagnostic_t);
+extern bool cxx_type_contains_byte_buffer	(tree);
 inline void
 cxx_incomplete_type_diagnostic (const_tree value, const_tree type,
 				diagnostic_t diag_kind)
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 246678)
+++ gcc/cp/decl.c	(working copy)
@@ -14081,10 +14081,11 @@ start_enum (tree name, tree enumtype, tree underly
 	  enumtype = pushtag (name, enumtype, /*tag_scope=*/ts_current);
 
 	  /* std::byte aliases anything.  */
-	  if (enumtype != error_mark_node
+	  if (cxx_dialect >= cxx1z
+	      && enumtype != error_mark_node
 	      && TYPE_CONTEXT (enumtype) == std_node
 	      && !strcmp ("byte", TYPE_NAME_STRING (enumtype)))
-	    TYPE_ALIAS_SET (enumtype) = 0;
+	    TYPE_TYPELESS_STORAGE (enumtype) = 1;
 	}
       else
 	  enumtype = xref_tag (enum_type, name, /*tag_scope=*/ts_current,
Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c	(revision 246678)
+++ gcc/cp/pt.c	(working copy)
@@ -8853,7 +8853,8 @@ lookup_template_class_1 (tree d1, tree arglist, tr
 	{
 	  static const char *tags[] = {"abi_tag", "may_alias"};
 
-	  for (unsigned ix = 0; ix != 2; ix++)
+	  TYPE_TYPELESS_STORAGE (t) |= TYPE_TYPELESS_STORAGE (template_type);
+	  for (unsigned ix = 0; ix < sizeof (tags) / sizeof (tags[0]); ix++)
 	    {
 	      tree attributes
 		= lookup_attribute (tags[ix], TYPE_ATTRIBUTES (template_type));
Index: gcc/cp/typeck2.c
===================================================================
--- gcc/cp/typeck2.c	(revision 246678)
+++ gcc/cp/typeck2.c	(working copy)
@@ -2234,5 +2234,29 @@ require_complete_eh_spec_types (tree fntype, tree
     }
 }
 
+/* True iff type either is or contains a byte buffer (which can be used for
+   storing any trivially copyable type).  */
+
+bool
+cxx_type_contains_byte_buffer (tree type)
+{
+  if (TREE_CODE (type) == ARRAY_TYPE
+      && (cxx_type_contains_byte_buffer (TREE_TYPE (type))
+	  || TREE_TYPE (type) == unsigned_char_type_node
+	  || (TREE_CODE (TREE_TYPE (type)) == ENUMERAL_TYPE
+	      && TYPE_CONTEXT (TREE_TYPE (type)) == std_node
+	      && !strcmp ("byte", TYPE_NAME_STRING (TREE_TYPE (type))))))
+    return true;
+
+  if (CLASS_TYPE_P (type))
+    for (tree field = next_initializable_field (TYPE_FIELDS (type));
+	 field;
+	 field = next_initializable_field (DECL_CHAIN (field)))
+      if (cxx_type_contains_byte_buffer (TREE_TYPE (field)))
+	return true;
+
+  return false;
+}
+
 \f
 #include "gt-cp-typeck2.h"
Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 246678)
+++ gcc/doc/extend.texi	(working copy)
@@ -6656,6 +6656,11 @@ declaration, the above program would abort when co
 @option{-fstrict-aliasing}, which is on by default at @option{-O2} or
 above.
 
+@item typeless_storage
+@cindex @code{typeless_storage} type attribute
+An object declared with a type with this attribute behaves like a
+character type with respect to aliasing semantics.
+
 @item packed
 @cindex @code{packed} type attribute
 This attribute, attached to @code{struct} or @code{union} type
Index: gcc/lto/lto.c
===================================================================
--- gcc/lto/lto.c	(revision 246678)
+++ gcc/lto/lto.c	(working copy)
@@ -1164,6 +1164,7 @@ compare_tree_sccs_1 (tree t1, tree t2, tree **map)
 	compare_values (TYPE_NONALIASED_COMPONENT);
       compare_values (TYPE_PACKED);
       compare_values (TYPE_RESTRICT);
+      compare_values (TYPE_TYPELESS_STORAGE);
       compare_values (TYPE_USER_ALIGN);
       compare_values (TYPE_READONLY);
       compare_values (TYPE_PRECISION);
Index: gcc/lto-streamer-out.c
===================================================================
--- gcc/lto-streamer-out.c	(revision 246678)
+++ gcc/lto-streamer-out.c	(working copy)
@@ -1134,6 +1134,7 @@ hash_tree (struct streamer_tree_cache_d *cache, ha
       hstate.add_flag (TYPE_NEEDS_CONSTRUCTING (t));
       hstate.add_flag (TYPE_PACKED (t));
       hstate.add_flag (TYPE_RESTRICT (t));
+      hstate.add_flag (TYPE_TYPELESS_STORAGE (t));
       hstate.add_flag (TYPE_USER_ALIGN (t));
       hstate.add_flag (TYPE_READONLY (t));
       if (RECORD_OR_UNION_TYPE_P (t))
Index: gcc/print-tree.c
===================================================================
--- gcc/print-tree.c	(revision 246678)
+++ gcc/print-tree.c	(working copy)
@@ -574,6 +574,9 @@ print_node (FILE *file, const char *prefix, tree n
       if (TYPE_RESTRICT (node))
 	fputs (" restrict", file);
 
+      if (TYPE_TYPELESS_STORAGE (node))
+	fputs (" typeless-storage", file);
+
       if (TYPE_LANG_FLAG_0 (node))
 	fputs (" type_0", file);
       if (TYPE_LANG_FLAG_1 (node))
Index: gcc/testsuite/c-c++-common/attr-typeless-storage-1.c
===================================================================
--- gcc/testsuite/c-c++-common/attr-typeless-storage-1.c	(revision 0)
+++ gcc/testsuite/c-c++-common/attr-typeless-storage-1.c	(working copy)
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wall" } */
+
+typedef int T __attribute__((typeless_storage));
+
+extern T t, v;
+extern T *p;
+extern int *p;
+
+extern int *p2;
+extern T *p2;
+
+void fn1 (T);
+void fn1 (int);
+
+void fn2 (int);
+void fn2 (T);
+
+/* Ensure that the composite types have typeless_storage.  */
+void
+f (long *i)
+{
+  *i = *(__typeof (*p) *) &p;
+  asm ("" : : "r" (*p));
+  *i = *(__typeof (*p2) *) &p2;
+  asm ("" : : "r" (*p2));
+  t = v;
+  asm ("" : : "r" (t));
+}
Index: gcc/testsuite/c-c++-common/attr-typeless-storage-2.c
===================================================================
--- gcc/testsuite/c-c++-common/attr-typeless-storage-2.c	(revision 0)
+++ gcc/testsuite/c-c++-common/attr-typeless-storage-2.c	(working copy)
@@ -0,0 +1,17 @@
+/* We used to reject this because types differentiating only in
+   TYPE_REF_CAN_ALIAS_ALL were deemed incompatible.  */
+/* { dg-do compile } */
+
+struct sockaddr;
+struct sockaddr *f (void);
+
+struct __attribute__((typeless_storage)) sockaddr { int j; };
+struct sockaddr *
+f (void)
+{
+  return
+#ifndef __cplusplus
+    (void *)
+#endif
+    0;
+}
Index: gcc/testsuite/gcc.c-torture/execute/typeless-storage-1.c
===================================================================
--- gcc/testsuite/gcc.c-torture/execute/typeless-storage-1.c	(revision 0)
+++ gcc/testsuite/gcc.c-torture/execute/typeless-storage-1.c	(working copy)
@@ -0,0 +1,20 @@
+/* Tests that the typeless_storage attribute works as expected.  */
+ 
+extern void abort(void);
+extern void exit(int);
+
+typedef int __attribute__((__typeless_storage__)) int_a;
+
+int
+main (void)
+{
+  int_a a = 0x12345678;
+  short *b = (short*) &a;
+
+  b[1] = 0;
+
+  if (a == 0x12345678)
+    abort();
+
+  exit(0);
+}
Index: gcc/tree-core.h
===================================================================
--- gcc/tree-core.h	(revision 246678)
+++ gcc/tree-core.h	(working copy)
@@ -1491,6 +1491,7 @@ struct GTY(()) tree_type_common {
   unsigned needs_constructing_flag : 1;
   unsigned transparent_aggr_flag : 1;
   unsigned restrict_flag : 1;
+  unsigned typeless_storage_flag : 1;
   unsigned contains_placeholder_bits : 2;
 
   ENUM_BITFIELD(machine_mode) mode : 8;
@@ -1511,7 +1512,7 @@ struct GTY(()) tree_type_common {
      so we need to store the value 32 (not 31, as we need the zero
      as well), hence six bits.  */
   unsigned align : 6;
-  unsigned spare : 25;
+  unsigned spare : 24;
   alias_set_type alias_set;
   tree pointer_to;
   tree reference_to;
Index: gcc/tree-streamer-in.c
===================================================================
--- gcc/tree-streamer-in.c	(revision 246678)
+++ gcc/tree-streamer-in.c	(working copy)
@@ -367,6 +367,7 @@ unpack_ts_type_common_value_fields (struct bitpack
   TYPE_NEEDS_CONSTRUCTING (expr) = (unsigned) bp_unpack_value (bp, 1);
   TYPE_PACKED (expr) = (unsigned) bp_unpack_value (bp, 1);
   TYPE_RESTRICT (expr) = (unsigned) bp_unpack_value (bp, 1);
+  TYPE_TYPELESS_STORAGE (expr) = (unsigned) bp_unpack_value (bp, 1);
   TYPE_USER_ALIGN (expr) = (unsigned) bp_unpack_value (bp, 1);
   TYPE_READONLY (expr) = (unsigned) bp_unpack_value (bp, 1);
   if (RECORD_OR_UNION_TYPE_P (expr))
Index: gcc/tree-streamer-out.c
===================================================================
--- gcc/tree-streamer-out.c	(revision 246678)
+++ gcc/tree-streamer-out.c	(working copy)
@@ -316,6 +316,7 @@ pack_ts_type_common_value_fields (struct bitpack_d
   bp_pack_value (bp, TYPE_NEEDS_CONSTRUCTING (expr), 1);
   bp_pack_value (bp, TYPE_PACKED (expr), 1);
   bp_pack_value (bp, TYPE_RESTRICT (expr), 1);
+  bp_pack_value (bp, TYPE_TYPELESS_STORAGE (expr), 1);
   bp_pack_value (bp, TYPE_USER_ALIGN (expr), 1);
   bp_pack_value (bp, TYPE_READONLY (expr), 1);
   /* We used to stream TYPE_ALIAS_SET == 0 information to let frontends mark
Index: gcc/tree.c
===================================================================
--- gcc/tree.c	(revision 246678)
+++ gcc/tree.c	(working copy)
@@ -8041,7 +8041,8 @@ build_pointer_type_for_mode (tree to_type, machine
 
   /* If the pointed-to type has the may_alias attribute set, force
      a TYPE_REF_CAN_ALIAS_ALL pointer to be generated.  */
-  if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
+  if (TYPE_TYPELESS_STORAGE (to_type)
+      || lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
     can_alias_all = true;
 
   /* In some cases, languages will have things that aren't a POINTER_TYPE
@@ -8110,7 +8111,8 @@ build_reference_type_for_mode (tree to_type, machi
 
   /* If the pointed-to type has the may_alias attribute set, force
      a TYPE_REF_CAN_ALIAS_ALL pointer to be generated.  */
-  if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
+  if (TYPE_TYPELESS_STORAGE (to_type)
+      || lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
     can_alias_all = true;
 
   /* In some cases, languages will have things that aren't a REFERENCE_TYPE
Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 246678)
+++ gcc/tree.h	(working copy)
@@ -1944,6 +1944,11 @@ extern machine_mode element_mode (const_tree t);
    the term.  */
 #define TYPE_RESTRICT(NODE) (TYPE_CHECK (NODE)->type_common.restrict_flag)
 
+/* Nonzero if the type should behave like a character type
+   with respect to aliasing sementics.  */
+#define TYPE_TYPELESS_STORAGE(NODE) \
+  (TYPE_CHECK (NODE)->type_common.typeless_storage_flag)
+
 /* If nonzero, type's name shouldn't be emitted into debug info.  */
 #define TYPE_NAMELESS(NODE) (TYPE_CHECK (NODE)->base.u.bits.nameless_flag)
 

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Sandra Loosemore]

On 04/05/2017 03:02 PM, Bernd Edlinger wrote:
> On 04/05/17 19:22, Bernd Edlinger wrote:
>> On 04/05/17 18:08, Jakub Jelinek wrote:
>>
>> Yes, exactly.  I really want to reach the deadline for gcc-7.
>> Fixing the name is certainly the most important first step,
>> and if everybody agrees on "typeless_storage", for the name
>> I can start with adjusting the name, and look into how
>> to use a spare type-flag that should be a mechanical change.
>>
>
> Jakub, I just renamed the attribute and reworked the patch
> as you suggested, reg-testing is not yet completed, but
> it looks good so far.  I also added a few more tests.
>
> I have changed the documentation as Richi suggested, but
> I am not too sure what to say here.
>

> Index: gcc/doc/extend.texi
> ===================================================================
> --- gcc/doc/extend.texi	(revision 246678)
> +++ gcc/doc/extend.texi	(working copy)
> @@ -6656,6 +6656,11 @@ declaration, the above program would abort when co
>  @option{-fstrict-aliasing}, which is on by default at @option{-O2} or
>  above.
>
> +@item typeless_storage
> +@cindex @code{typeless_storage} type attribute
> +An object declared with a type with this attribute behaves like a
> +character type with respect to aliasing semantics.
> +
>  @item packed
>  @cindex @code{packed} type attribute
>  This attribute, attached to @code{struct} or @code{union} type


"An object ....  behaves like a character type"?  I think you mean "as 
if it had character type".  Or maybe something simpler like "A type 
declared with this attribute has the same aliasing semantics as 
@code{char} type."

-Sandra

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Jakub Jelinek]

On Wed, Apr 05, 2017 at 05:17:24PM -0600, Sandra Loosemore wrote:
> > > Yes, exactly.  I really want to reach the deadline for gcc-7.
> > > Fixing the name is certainly the most important first step,
> > > and if everybody agrees on "typeless_storage", for the name
> > > I can start with adjusting the name, and look into how
> > > to use a spare type-flag that should be a mechanical change.
> > > 
> > 
> > Jakub, I just renamed the attribute and reworked the patch
> > as you suggested, reg-testing is not yet completed, but
> > it looks good so far.  I also added a few more tests.
> > 
> > I have changed the documentation as Richi suggested, but
> > I am not too sure what to say here.
> > 
> 
> > Index: gcc/doc/extend.texi
> > ===================================================================
> > --- gcc/doc/extend.texi	(revision 246678)
> > +++ gcc/doc/extend.texi	(working copy)
> > @@ -6656,6 +6656,11 @@ declaration, the above program would abort when co
> >  @option{-fstrict-aliasing}, which is on by default at @option{-O2} or
> >  above.
> > 
> > +@item typeless_storage
> > +@cindex @code{typeless_storage} type attribute
> > +An object declared with a type with this attribute behaves like a
> > +character type with respect to aliasing semantics.
> > +
> >  @item packed
> >  @cindex @code{packed} type attribute
> >  This attribute, attached to @code{struct} or @code{union} type
> 
> 
> "An object ....  behaves like a character type"?  I think you mean "as if it
> had character type".  Or maybe something simpler like "A type declared with
> this attribute has the same aliasing semantics as @code{char} type."

Well, that wording doesn't make much sense, because the problem is exactly
that alias semantics of the char type does not do what C++ needs from
unsigned char [N] or std::byte [N] arrays.  char type inside of a structure
no longer means you can put anything in there.  If you have effective type
of char and access it through lvalue of some type incompatible with it (see
the C99 6.5/7 rules), then it is still invalid, only if you access object
of any effective type through lvalue of char type, then it is valid.
Now, for objects with type with typeless_storage or for C++ unsigned char
or std::byte arrays, we basically want to say that accessing them through
lvalue of any type is well defined.  I'd probably document it just for
typeless_storage attribute, because for the unsigned char and std::byte
arrays it is C++17 that defines the exact rules and they are probably
less strict than that, so it would be good if we leave it open to refine
the unsigned char/std::byte array behavior later on to better match what
C++ requires.  Not sure if we want to document anything about accesses
through lvalue of type with typeless_storage attribute, whether that also
implies it is like accesses through lvalue of type char.  If yes, that
would be may_alias attribute behavior extended, so that it is not just
accesses through pointers/references to type with may_alias attribute
that are treated that way, but any accesses through lvalue with type
with typeless_storage attribute.

	Jakub

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Richard Biener]

On Wed, 5 Apr 2017, Bernd Edlinger wrote:

> On 04/05/17 19:22, Bernd Edlinger wrote:
> > On 04/05/17 18:08, Jakub Jelinek wrote:
> >
> > Yes, exactly.  I really want to reach the deadline for gcc-7.
> > Fixing the name is certainly the most important first step,
> > and if everybody agrees on "typeless_storage", for the name
> > I can start with adjusting the name, and look into how
> > to use a spare type-flag that should be a mechanical change.
> >
> 
> Jakub, I just renamed the attribute and reworked the patch
> as you suggested, reg-testing is not yet completed, but
> it looks good so far.  I also added a few more tests.
> 
> I have changed the documentation as Richi suggested, but
> I am not too sure what to say here.

The alias.c changes are not sufficient.  I think what you want is
sth like

Index: gcc/alias.c
===================================================================
--- gcc/alias.c	(revision 246678)
+++ gcc/alias.c	(working copy)
@@ -136,6 +136,9 @@ struct GTY(()) alias_set_entry {
   bool is_pointer;
   /* Nonzero if is_pointer or if one of childs have has_pointer set.  */
   bool has_pointer;
+  /* Nonzero if we have a child serving as typeless storage (or are
+     such storage ourselves).  */
+  bool has_typeless_storage;
 
   /* The children of the alias set.  These are not just the immediate
      children, but, in fact, all descendants.  So, if we have:
@@ -419,7 +422,8 @@ alias_set_subset_of (alias_set_type set1
   /* Check if set1 is a subset of set2.  */
   ase2 = get_alias_set_entry (set2);
   if (ase2 != 0
-      && (ase2->has_zero_child
+      && (ase2->has_typeless_storage
+	  || ase2->has_zero_child
 	  || (ase2->children && ase2->children->get (set1))))
     return true;
 
@@ -825,6 +829,7 @@ init_alias_set_entry (alias_set_type set
   ase->has_zero_child = false;
   ase->is_pointer = false;
   ase->has_pointer = false;
+  ase->has_typeless_storage = false;
   gcc_checking_assert (!get_alias_set_entry (set));
   (*alias_sets)[set] = ase;
   return ase;
@@ -955,6 +960,7 @@ get_alias_set (tree t)
      Just be pragmatic here and make sure the array and its element
      type get the same alias set assigned.  */
   else if (TREE_CODE (t) == ARRAY_TYPE
+	   && ! TYPE_TYPELESS_STORAGE (t)
 	   && (!TYPE_NONALIASED_COMPONENT (t)
 	       || TYPE_STRUCTURAL_EQUALITY_P (t)))
     set = get_alias_set (TREE_TYPE (t));
@@ -1094,6 +1100,15 @@ get_alias_set (tree t)
 
   TYPE_ALIAS_SET (t) = set;
 
+  if (TREE_CODE (t) == ARRAY_TYPE
+      && TYPE_TYPELESS_STORAGE (t))
+    {
+      alias_set_entry *ase = get_alias_set_entry (set);
+      if (!ase)
+	ase = init_alias_set_entry (set);
+      ase->has_typeless_storage = true;
+    }
+
   /* If this is an aggregate type or a complex type, we must record any
      component aliasing information.  */
   if (AGGREGATE_TYPE_P (t) || TREE_CODE (t) == COMPLEX_TYPE)
@@ -1173,6 +1188,8 @@ record_alias_subset (alias_set_type supe
 	    superset_entry->has_zero_child = true;
           if (subset_entry->has_pointer)
 	    superset_entry->has_pointer = true;
+	  if (subset_entry->has_typeless_storage)
+	    superset_entry->has_typeless_storage = true;
 
 	  if (subset_entry->children)
 	    {


please also restrict TYPE_TYPELESS_STORAGE to ARRAY_TYPEs (otherwise
more complications will arise).

Index: gcc/cp/class.c
===================================================================
--- gcc/cp/class.c      (revision 246678)
+++ gcc/cp/class.c      (working copy)
@@ -2083,7 +2083,8 @@ fixup_attribute_variants (tree t)
   tree attrs = TYPE_ATTRIBUTES (t);
   unsigned align = TYPE_ALIGN (t);
   bool user_align = TYPE_USER_ALIGN (t);
-  bool may_alias = lookup_attribute ("may_alias", attrs);
+  bool may_alias = TYPE_TYPELESS_STORAGE (t)
+                  || lookup_attribute ("may_alias", attrs);

   if (may_alias)
     fixup_may_alias (t);
@@ -7345,6 +7348,12 @@ finish_struct_1 (tree t)
      the class or perform any other required target modifications.  */
   targetm.cxx.adjust_class_at_definition (t);

+  if (cxx_dialect >= cxx1z && cxx_type_contains_byte_buffer (t))
+    {
+      TYPE_TYPELESS_STORAGE (t) = 1;
+      fixup_attribute_variants (t);
...

I don't think you need all this given alias.c only looks at
TYPE_MAIN_VARIANTs.

Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c       (revision 246678)
+++ gcc/cp/decl.c       (working copy)
@@ -14081,10 +14081,11 @@ start_enum (tree name, tree enumtype, tree 
underly
          enumtype = pushtag (name, enumtype, /*tag_scope=*/ts_current);

          /* std::byte aliases anything.  */
-         if (enumtype != error_mark_node
+         if (cxx_dialect >= cxx1z
+             && enumtype != error_mark_node
              && TYPE_CONTEXT (enumtype) == std_node
              && !strcmp ("byte", TYPE_NAME_STRING (enumtype)))
-           TYPE_ALIAS_SET (enumtype) = 0;
+           TYPE_TYPELESS_STORAGE (enumtype) = 1;
        }
       else

not needed (but also not sufficient - you need to handle arrays of
byte somewhere).

@@ -1491,6 +1491,7 @@ struct GTY(()) tree_type_common {
   unsigned needs_constructing_flag : 1;
   unsigned transparent_aggr_flag : 1;
   unsigned restrict_flag : 1;
+  unsigned typeless_storage_flag : 1;
   unsigned contains_placeholder_bits : 2;

   ENUM_BITFIELD(machine_mode) mode : 8;

bits are grouped in groups of 8 bits, this breaks it.

@@ -8041,7 +8041,8 @@ build_pointer_type_for_mode (tree to_type, machine

   /* If the pointed-to type has the may_alias attribute set, force
      a TYPE_REF_CAN_ALIAS_ALL pointer to be generated.  */
-  if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
+  if (TYPE_TYPELESS_STORAGE (to_type)
+      || lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
     can_alias_all = true;

   /* In some cases, languages will have things that aren't a POINTER_TYPE
@@ -8110,7 +8111,8 @@ build_reference_type_for_mode (tree to_type, machi

   /* If the pointed-to type has the may_alias attribute set, force
      a TYPE_REF_CAN_ALIAS_ALL pointer to be generated.  */
-  if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
+  if (TYPE_TYPELESS_STORAGE (to_type)
+      || lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
     can_alias_all = true;

   /* In some cases, languages will have things that aren't a

not needed.

+/* Nonzero if the type should behave like a character type
+   with respect to aliasing sementics.  */
+#define TYPE_TYPELESS_STORAGE(NODE) \
+  (TYPE_CHECK (NODE)->type_common.typeless_storage_flag)

ARRAY_TYPE_CHECK (NODE)->

Richard.

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Jakub Jelinek]

On Thu, Apr 06, 2017 at 09:47:10AM +0200, Richard Biener wrote:
> @@ -955,6 +960,7 @@ get_alias_set (tree t)
>       Just be pragmatic here and make sure the array and its element
>       type get the same alias set assigned.  */
>    else if (TREE_CODE (t) == ARRAY_TYPE
> +	   && ! TYPE_TYPELESS_STORAGE (t)
>  	   && (!TYPE_NONALIASED_COMPONENT (t)
>  	       || TYPE_STRUCTURAL_EQUALITY_P (t)))
>      set = get_alias_set (TREE_TYPE (t));
> @@ -1094,6 +1100,15 @@ get_alias_set (tree t)
>  
>    TYPE_ALIAS_SET (t) = set;
>  
> +  if (TREE_CODE (t) == ARRAY_TYPE
> +      && TYPE_TYPELESS_STORAGE (t))

Shouldn't TYPE_TYPELESS_STORAGE apply even for non-array types?
If somebody chooses to store anything in long long
__attribute__((typeless_storage)), so be it.  Or what kind of complications
do you see for that?

	Jakub

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Richard Biener]

On Thu, 6 Apr 2017, Jakub Jelinek wrote:

> On Thu, Apr 06, 2017 at 09:47:10AM +0200, Richard Biener wrote:
> > @@ -955,6 +960,7 @@ get_alias_set (tree t)
> >       Just be pragmatic here and make sure the array and its element
> >       type get the same alias set assigned.  */
> >    else if (TREE_CODE (t) == ARRAY_TYPE
> > +	   && ! TYPE_TYPELESS_STORAGE (t)
> >  	   && (!TYPE_NONALIASED_COMPONENT (t)
> >  	       || TYPE_STRUCTURAL_EQUALITY_P (t)))
> >      set = get_alias_set (TREE_TYPE (t));
> > @@ -1094,6 +1100,15 @@ get_alias_set (tree t)
> >  
> >    TYPE_ALIAS_SET (t) = set;
> >  
> > +  if (TREE_CODE (t) == ARRAY_TYPE
> > +      && TYPE_TYPELESS_STORAGE (t))
> 
> Shouldn't TYPE_TYPELESS_STORAGE apply even for non-array types?
> If somebody chooses to store anything in long long
> __attribute__((typeless_storage)), so be it.  Or what kind of complications
> do you see for that?

It's a new feature so I don't see why we should allow that.  Given that
people will have to do sth when the compiler doesn't support it the
only "reliable" way of using it is on an array of char anyway.

The complication starts when people use it on a type that currently
uses alias-set zero (because "zero" doesn't get an alias_set_entry).

Richard.

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Bernd Edlinger]

On 04/06/17 09:55, Richard Biener wrote:
> On Thu, 6 Apr 2017, Jakub Jelinek wrote:
>
>> On Thu, Apr 06, 2017 at 09:47:10AM +0200, Richard Biener wrote:
>>> @@ -955,6 +960,7 @@ get_alias_set (tree t)
>>>       Just be pragmatic here and make sure the array and its element
>>>       type get the same alias set assigned.  */
>>>    else if (TREE_CODE (t) == ARRAY_TYPE
>>> +	   && ! TYPE_TYPELESS_STORAGE (t)
>>>  	   && (!TYPE_NONALIASED_COMPONENT (t)
>>>  	       || TYPE_STRUCTURAL_EQUALITY_P (t)))
>>>      set = get_alias_set (TREE_TYPE (t));
>>> @@ -1094,6 +1100,15 @@ get_alias_set (tree t)
>>>
>>>    TYPE_ALIAS_SET (t) = set;
>>>
>>> +  if (TREE_CODE (t) == ARRAY_TYPE
>>> +      && TYPE_TYPELESS_STORAGE (t))
>>
>> Shouldn't TYPE_TYPELESS_STORAGE apply even for non-array types?
>> If somebody chooses to store anything in long long
>> __attribute__((typeless_storage)), so be it.  Or what kind of complications
>> do you see for that?
>
> It's a new feature so I don't see why we should allow that.  Given that
> people will have to do sth when the compiler doesn't support it the
> only "reliable" way of using it is on an array of char anyway.
>
> The complication starts when people use it on a type that currently
> uses alias-set zero (because "zero" doesn't get an alias_set_entry).
>

The typeless_storage does not need to implement all the C++ semantic
by itself.  It would be possible, but then it is not as generic as
it could be.  What I'd really like to have is make an arbitrary
type behave as if it were a char with respect to aliasing.

In my mind, the typeless_storage attribute has a value of its own,
but it can be used to implement the C++17 semantic of std::byte [N].

So I would not want to completely change the way TBAA is working
today.  I believe it is doing a fairly good job.

The TBAA machinery, does for instance not need to propagate this
attribute from the member to the enclosing struct that is
also not done for a struct that contains a char.

I think it is not too complicated to done in the C++ FE.
The FE looks for array of std::byte and unsigned char,
and sets the attribute when the final type is constructed.

What I am trying to do is just extend the semantic of may_alias
a bit, and then have the C++ FE use it in the way it has to.

Here is what I want to write in the doc:

@item typeless_storage
@cindex @code{typeless_storage} type attribute
A type declared with this attribute behaves like a character type
with respect to aliasing semantics.
This is attribute is similar to the @code{may_alias} attribute,
except that it is not restricted to pointers.

Example of use:

@smallexample
typedef int __attribute__((__typeless_storage__)) int_a;

int
main (void)
@{
   int_a a = 0x12345678;
   short *b = (short *) &a;

   b[1] = 0;

   if (a == 0x12345678)
     abort();

   exit(0);
@}
@end smallexample


we should first agree on that.

Bernd.

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Florian Weimer]

On 04/06/2017 04:11 PM, Bernd Edlinger wrote:

> I think it is not too complicated to done in the C++ FE.
> The FE looks for array of std::byte and unsigned char,
> and sets the attribute when the final type is constructed.
>
> What I am trying to do is just extend the semantic of may_alias
> a bit, and then have the C++ FE use it in the way it has to.

We also need this for some POSIX and Linux kernel interfaces.  A 
C++-only solution would not help with that.

> Here is what I want to write in the doc:
>
> @item typeless_storage
> @cindex @code{typeless_storage} type attribute
> A type declared with this attribute behaves like a character type
> with respect to aliasing semantics.
> This is attribute is similar to the @code{may_alias} attribute,
> except that it is not restricted to pointers.

As Jakub pointed out, this is not what we need here.  An object of type 
char does *not* have untyped storage.  Accessing it as a different type 
is still undefined.

The documentation says that the memory region is considered to by 
untyped, like a memory region returned by malloc (but obviously not with 
the implication that the memory region is separated from everything else).

Thanks,
Florian

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Richard Biener]

On Thu, 6 Apr 2017, Florian Weimer wrote:

> On 04/06/2017 04:11 PM, Bernd Edlinger wrote:
> 
> > I think it is not too complicated to done in the C++ FE.
> > The FE looks for array of std::byte and unsigned char,
> > and sets the attribute when the final type is constructed.
> > 
> > What I am trying to do is just extend the semantic of may_alias
> > a bit, and then have the C++ FE use it in the way it has to.
> 
> We also need this for some POSIX and Linux kernel interfaces.  A C++-only
> solution would not help with that.

Example(s)?

> > Here is what I want to write in the doc:
> > 
> > @item typeless_storage
> > @cindex @code{typeless_storage} type attribute
> > A type declared with this attribute behaves like a character type
> > with respect to aliasing semantics.
> > This is attribute is similar to the @code{may_alias} attribute,
> > except that it is not restricted to pointers.
> 
> As Jakub pointed out, this is not what we need here.  An object of type char
> does *not* have untyped storage.  Accessing it as a different type is still
> undefined.
> 
> The documentation says that the memory region is considered to by untyped,
> like a memory region returned by malloc (but obviously not with the
> implication that the memory region is separated from everything else).
> 
> Thanks,
> Florian
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Jonathan Wakely]

On 06/04/17 16:23 +0200, Richard Biener wrote:
>On Thu, 6 Apr 2017, Florian Weimer wrote:
>
>> On 04/06/2017 04:11 PM, Bernd Edlinger wrote:
>>
>> > I think it is not too complicated to done in the C++ FE.
>> > The FE looks for array of std::byte and unsigned char,
>> > and sets the attribute when the final type is constructed.
>> >
>> > What I am trying to do is just extend the semantic of may_alias
>> > a bit, and then have the C++ FE use it in the way it has to.
>>
>> We also need this for some POSIX and Linux kernel interfaces.  A C++-only
>> solution would not help with that.
>
>Example(s)?

sockaddr_storage comes to mind.

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Florian Weimer]

On 04/06/2017 04:43 PM, Jonathan Wakely wrote:
> On 06/04/17 16:23 +0200, Richard Biener wrote:
>> On Thu, 6 Apr 2017, Florian Weimer wrote:
>>
>>> On 04/06/2017 04:11 PM, Bernd Edlinger wrote:
>>>
>>> > I think it is not too complicated to done in the C++ FE.
>>> > The FE looks for array of std::byte and unsigned char,
>>> > and sets the attribute when the final type is constructed.
>>> >
>>> > What I am trying to do is just extend the semantic of may_alias
>>> > a bit, and then have the C++ FE use it in the way it has to.
>>>
>>> We also need this for some POSIX and Linux kernel interfaces.  A
>>> C++-only
>>> solution would not help with that.
>>
>> Example(s)?
>
> sockaddr_storage comes to mind.

Right.  The kernel also has many APIs which return multiple 
variable-length data blocks, such as getdents64, and many more 
interfaces in combination with read/recv system calls.  Variable length 
means that you cannot declare the appropriate type after the first data 
item, so you technically have to use malloc.

POSIX interfaces which exhibit a similar pattern are getpwnam_r and 
friends, but for them, you can probably use malloc without ill effect 
(although there are still performance concerns).

Thanks,
Florian

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Jakub Jelinek]

On Thu, Apr 06, 2017 at 04:51:01PM +0200, Florian Weimer wrote:
> On 04/06/2017 04:43 PM, Jonathan Wakely wrote:
> > On 06/04/17 16:23 +0200, Richard Biener wrote:
> > > On Thu, 6 Apr 2017, Florian Weimer wrote:
> > > 
> > > > On 04/06/2017 04:11 PM, Bernd Edlinger wrote:
> > > > 
> > > > > I think it is not too complicated to done in the C++ FE.
> > > > > The FE looks for array of std::byte and unsigned char,
> > > > > and sets the attribute when the final type is constructed.
> > > > >
> > > > > What I am trying to do is just extend the semantic of may_alias
> > > > > a bit, and then have the C++ FE use it in the way it has to.
> > > > 
> > > > We also need this for some POSIX and Linux kernel interfaces.  A
> > > > C++-only
> > > > solution would not help with that.
> > > 
> > > Example(s)?
> > 
> > sockaddr_storage comes to mind.
> 
> Right.  The kernel also has many APIs which return multiple variable-length
> data blocks, such as getdents64, and many more interfaces in combination

The kernel uses -fno-strict-aliasing I think, so it doesn't care.

	Jakub

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Florian Weimer]

On 04/06/2017 05:05 PM, Jakub Jelinek wrote:
> On Thu, Apr 06, 2017 at 04:51:01PM +0200, Florian Weimer wrote:
>> On 04/06/2017 04:43 PM, Jonathan Wakely wrote:
>>> On 06/04/17 16:23 +0200, Richard Biener wrote:
>>>> On Thu, 6 Apr 2017, Florian Weimer wrote:
>>>>
>>>>> On 04/06/2017 04:11 PM, Bernd Edlinger wrote:
>>>>>
>>>>>> I think it is not too complicated to done in the C++ FE.
>>>>>> The FE looks for array of std::byte and unsigned char,
>>>>>> and sets the attribute when the final type is constructed.
>>>>>>
>>>>>> What I am trying to do is just extend the semantic of may_alias
>>>>>> a bit, and then have the C++ FE use it in the way it has to.
>>>>>
>>>>> We also need this for some POSIX and Linux kernel interfaces.  A
>>>>> C++-only
>>>>> solution would not help with that.
>>>>
>>>> Example(s)?
>>>
>>> sockaddr_storage comes to mind.
>>
>> Right.  The kernel also has many APIs which return multiple variable-length
>> data blocks, such as getdents64, and many more interfaces in combination
>
> The kernel uses -fno-strict-aliasing I think, so it doesn't care.

These APIs (getdents64, inotify, lots of netlink stuff, probably more) 
extend to user space.

Thanks,
Florian

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Richard Biener]

On April 6, 2017 4:51:01 PM GMT+02:00, Florian Weimer <fweimer@redhat.com> wrote:
>On 04/06/2017 04:43 PM, Jonathan Wakely wrote:
>> On 06/04/17 16:23 +0200, Richard Biener wrote:
>>> On Thu, 6 Apr 2017, Florian Weimer wrote:
>>>
>>>> On 04/06/2017 04:11 PM, Bernd Edlinger wrote:
>>>>
>>>> > I think it is not too complicated to done in the C++ FE.
>>>> > The FE looks for array of std::byte and unsigned char,
>>>> > and sets the attribute when the final type is constructed.
>>>> >
>>>> > What I am trying to do is just extend the semantic of may_alias
>>>> > a bit, and then have the C++ FE use it in the way it has to.
>>>>
>>>> We also need this for some POSIX and Linux kernel interfaces.  A
>>>> C++-only
>>>> solution would not help with that.
>>>
>>> Example(s)?
>>
>> sockaddr_storage comes to mind.
>
>Right.  The kernel also has many APIs which return multiple 
>variable-length data blocks, such as getdents64, and many more 
>interfaces in combination with read/recv system calls.  Variable length
>
>means that you cannot declare the appropriate type after the first data
>
>item, so you technically have to use malloc.
>
>POSIX interfaces which exhibit a similar pattern are getpwnam_r and 
>friends, but for them, you can probably use malloc without ill effect 
>(although there are still performance concerns).

Can you give a concrete example which shows the issue and how typeless_storage helps?

Thanks,
Richard.

>Thanks,
>Florian

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Florian Weimer]

On 04/06/2017 09:12 PM, Richard Biener wrote:
>> Right.  The kernel also has many APIs which return multiple
>> variable-length data blocks, such as getdents64, and many more
>> interfaces in combination with read/recv system calls.  Variable length
>>
>> means that you cannot declare the appropriate type after the first data
>>
>> item, so you technically have to use malloc.
>>
>> POSIX interfaces which exhibit a similar pattern are getpwnam_r and
>> friends, but for them, you can probably use malloc without ill effect
>> (although there are still performance concerns).

> Can you give a concrete example which shows the issue and how typeless_storage helps?

An example is in libffi/src/closures.c:

       char buf[MAXPATHLEN * 3];

       if (getmntent_r (last_mntent, &mnt, buf, sizeof (buf)) == NULL)
         return -1;

The intent is that buf is untyped storage, from which the getmntent_r 
function can allocate objects as needed (instead of using malloc).

Based on your earlier comments, GCC already supports that without any 
further source code annotations.

Thanks,
Florian

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Richard Biener]

On Tue, 11 Apr 2017, Florian Weimer wrote:

> On 04/06/2017 09:12 PM, Richard Biener wrote:
> > > Right.  The kernel also has many APIs which return multiple
> > > variable-length data blocks, such as getdents64, and many more
> > > interfaces in combination with read/recv system calls.  Variable length
> > > 
> > > means that you cannot declare the appropriate type after the first data
> > > 
> > > item, so you technically have to use malloc.
> > > 
> > > POSIX interfaces which exhibit a similar pattern are getpwnam_r and
> > > friends, but for them, you can probably use malloc without ill effect
> > > (although there are still performance concerns).
> 
> > Can you give a concrete example which shows the issue and how
> > typeless_storage helps?
> 
> An example is in libffi/src/closures.c:
> 
>       char buf[MAXPATHLEN * 3];
> 
>       if (getmntent_r (last_mntent, &mnt, buf, sizeof (buf)) == NULL)
>         return -1;
> 
> The intent is that buf is untyped storage, from which the getmntent_r function
> can allocate objects as needed (instead of using malloc).
> 
> Based on your earlier comments, GCC already supports that without any further
> source code annotations.

Yes.

Richard.

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Bernd Edlinger]

On 04/06/17 16:17, Florian Weimer wrote:
>> Here is what I want to write in the doc:
>>
>> @item typeless_storage
>> @cindex @code{typeless_storage} type attribute
>> A type declared with this attribute behaves like a character type
>> with respect to aliasing semantics.
>> This is attribute is similar to the @code{may_alias} attribute,
>> except that it is not restricted to pointers.
>
> As Jakub pointed out, this is not what we need here.  An object of type
> char does *not* have untyped storage.  Accessing it as a different type
> is still undefined.
>

but, do you agree that this is valid in C11?

typedef char char_a[4];

int
main (void)
{
   char_a a = {1,2,3,4};
   short *b = (short *) &a;

   b[1] = 0;

   if (a[0] == 1 && a[1] == 2 && a[2] == 3 && a[3] == 4)
     abort();

   exit(0);
}


all I want to do is replace "char" with a different type.

Bernd.

> The documentation says that the memory region is considered to by
> untyped, like a memory region returned by malloc (but obviously not with
> the implication that the memory region is separated from everything else).
>
> Thanks,
> Florian

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Florian Weimer]

On 04/06/2017 07:39 PM, Bernd Edlinger wrote:
> On 04/06/17 16:17, Florian Weimer wrote:
>>> Here is what I want to write in the doc:
>>>
>>> @item typeless_storage
>>> @cindex @code{typeless_storage} type attribute
>>> A type declared with this attribute behaves like a character type
>>> with respect to aliasing semantics.
>>> This is attribute is similar to the @code{may_alias} attribute,
>>> except that it is not restricted to pointers.
>>
>> As Jakub pointed out, this is not what we need here.  An object of type
>> char does *not* have untyped storage.  Accessing it as a different type
>> is still undefined.
>>
>
> but, do you agree that this is valid in C11?
>
> typedef char char_a[4];
>
> int
> main (void)
> {
>    char_a a = {1,2,3,4};
>    short *b = (short *) &a;
>
>    b[1] = 0;
>
>    if (a[0] == 1 && a[1] == 2 && a[2] == 3 && a[3] == 4)
>      abort();
>
>    exit(0);
> }
>
>
> all I want to do is replace "char" with a different type.

Thanks a lot for posting a concrete example.

The effective type of a[2] and [3] is char.  The character type wildcard 
in 6.5(7) only applies to the type of the lvalue expression ysed for the 
access, not the effective type of the object being accessed.  The type 
of the LHS of the assignment expression is short.  So the access is 
undefined.

Florian

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Bernd Edlinger]

On 04/06/17 19:47, Florian Weimer wrote:
> On 04/06/2017 07:39 PM, Bernd Edlinger wrote:
>> On 04/06/17 16:17, Florian Weimer wrote:
>>>> Here is what I want to write in the doc:
>>>>
>>>> @item typeless_storage
>>>> @cindex @code{typeless_storage} type attribute
>>>> A type declared with this attribute behaves like a character type
>>>> with respect to aliasing semantics.
>>>> This is attribute is similar to the @code{may_alias} attribute,
>>>> except that it is not restricted to pointers.
>>>
>>> As Jakub pointed out, this is not what we need here.  An object of type
>>> char does *not* have untyped storage.  Accessing it as a different type
>>> is still undefined.
>>>
>>
>> but, do you agree that this is valid in C11?
>>
>> typedef char char_a[4];
>>
>> int
>> main (void)
>> {
>>    char_a a = {1,2,3,4};
>>    short *b = (short *) &a;
>>
>>    b[1] = 0;
>>
>>    if (a[0] == 1 && a[1] == 2 && a[2] == 3 && a[3] == 4)
>>      abort();
>>
>>    exit(0);
>> }
>>
>>
>> all I want to do is replace "char" with a different type.
>
> Thanks a lot for posting a concrete example.
>
> The effective type of a[2] and [3] is char.  The character type wildcard
> in 6.5(7) only applies to the type of the lvalue expression ysed for the
> access, not the effective type of the object being accessed.  The type
> of the LHS of the assignment expression is short.  So the access is
> undefined.
>

exactly *that* is what I want to make valid with that attribute, which
would be also useful in C and kernel code, IMHO.

But isn't the effective type changed by the assignment b[1] = 0;
as described in 6.5(6):
"If a value is stored into an object having no declared type through an
lvalue having a type that is not a character type, then the type of the
lvalue becomes the effective type of the object for that access and for
subsequent accesses that do not modify the stored value."



Bernd.

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Florian Weimer]

On 04/06/2017 08:12 PM, Bernd Edlinger wrote:
> On 04/06/17 19:47, Florian Weimer wrote:
>> On 04/06/2017 07:39 PM, Bernd Edlinger wrote:
>>> On 04/06/17 16:17, Florian Weimer wrote:
>>>>> Here is what I want to write in the doc:
>>>>>
>>>>> @item typeless_storage
>>>>> @cindex @code{typeless_storage} type attribute
>>>>> A type declared with this attribute behaves like a character type
>>>>> with respect to aliasing semantics.
>>>>> This is attribute is similar to the @code{may_alias} attribute,
>>>>> except that it is not restricted to pointers.
>>>>
>>>> As Jakub pointed out, this is not what we need here.  An object of type
>>>> char does *not* have untyped storage.  Accessing it as a different type
>>>> is still undefined.
>>>>
>>>
>>> but, do you agree that this is valid in C11?
>>>
>>> typedef char char_a[4];
>>>
>>> int
>>> main (void)
>>> {
>>>    char_a a = {1,2,3,4};
>>>    short *b = (short *) &a;
>>>
>>>    b[1] = 0;
>>>
>>>    if (a[0] == 1 && a[1] == 2 && a[2] == 3 && a[3] == 4)
>>>      abort();
>>>
>>>    exit(0);
>>> }
>>>
>>>
>>> all I want to do is replace "char" with a different type.
>>
>> Thanks a lot for posting a concrete example.
>>
>> The effective type of a[2] and [3] is char.  The character type wildcard
>> in 6.5(7) only applies to the type of the lvalue expression ysed for the
>> access, not the effective type of the object being accessed.  The type
>> of the LHS of the assignment expression is short.  So the access is
>> undefined.
>>
>
> exactly *that* is what I want to make valid with that attribute, which
> would be also useful in C and kernel code, IMHO.

And I think we all agree that this is a laudable goal.

> But isn't the effective type changed by the assignment b[1] = 0;
> as described in 6.5(6):
> "If a value is stored into an object having no declared type through an
> lvalue having a type that is not a character type, then the type of the
> lvalue becomes the effective type of the object for that access and for
> subsequent accesses that do not modify the stored value."

I don't know what your patch does, but your proposed documentation does 
not make this valid because “declared as char” is still not “having no 
declared type”.  Or put differently, “behaves like a character type” is 
not what we actually want here.

Let me repeat that I don't know if this is merely a documentation issue.

Thanks,
Florian

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Bernd Edlinger]

On 04/06/17 20:19, Florian Weimer wrote:
>
> I don't know what your patch does, but your proposed documentation does
> not make this valid because “declared as char” is still not “having no
> declared type”.  Or put differently, “behaves like a character type” is
> not what we actually want here.
>

What the patch does is just so simple but it is hard for me to find the
right words so that really everybody understands:

Technically, we already have the may_alias attribute, that forces all
access through pointers to have "alias set 0" that in turn makes all
other objects volatile, unless the compiler can prove that the address
is in fact different.  But it has no impact on DECLs, so if you
use may_alias on a type, and you declare an object with that type,
then directly accessing that object by name does NOT have "alias set 0".

When I noticed that in the context of PR79671 I initially thought that
was by accident, but Richi pointed out that this is a useful feature for
vector types, that are always declared as may_alias, and moreover
the may_alias is / has been always documented to have only meaning
on pointers, all that changed is that the TBAA aliasing oracle has
improved recently to follow the specified behavior more closely.

My patch simply duplicates the semantic of may_alias and adds
"alias set 0" for accesses through DECLs of that type.


However, I must confess I find it difficult to understand the
language in which the ISO standard is written.

For instance how do you "declare an object without a declared type"?


> Let me repeat that I don't know if this is merely a documentation issue.
>
> Thanks,
> Florian

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Florian Weimer]

On 04/06/2017 08:49 PM, Bernd Edlinger wrote:

> For instance how do you "declare an object without a declared type"?

malloc and other allocation functions return pointers to objects without 
a declared type.

Thanks,
Florian

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Bernd Edlinger]

On 04/06/17 21:05, Florian Weimer wrote:
> On 04/06/2017 08:49 PM, Bernd Edlinger wrote:
>
>> For instance how do you "declare an object without a declared type"?
>
> malloc and other allocation functions return pointers to objects without
> a declared type.
>

Thanks Florian,

this discussion is very helpful.

How about this for the documentation:

@item typeless_storage
@cindex @code{typeless_storage} type attribute
In the context of section 6.5 paragraph 6 of the C11 standard,
an object of this type behaves as if it has no declared type.
In the context of section 6.5 paragraph 7 of the C11 standard,
an object or a pointer if this type behaves as if it were a
character type.
This is attribute is similar to the @code{may_alias} attribute,
except that it is not restricted to pointers.

Example of use:

@smallexample
typedef int __attribute__((__typeless_storage__)) int_a;

int
main (void)
@{
   int_a a = 0x12345678;
   short *b = (short *) &a;

   b[1] = 0;

   if (a == 0x12345678)
     abort();

   exit(0);
@}
@end smallexample


Bernd.

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Richard Biener]

On Thu, 6 Apr 2017, Bernd Edlinger wrote:

> On 04/06/17 21:05, Florian Weimer wrote:
> > On 04/06/2017 08:49 PM, Bernd Edlinger wrote:
> >
> >> For instance how do you "declare an object without a declared type"?
> >
> > malloc and other allocation functions return pointers to objects without
> > a declared type.
> >
> 
> Thanks Florian,
> 
> this discussion is very helpful.
> 
> How about this for the documentation:
> 
> @item typeless_storage
> @cindex @code{typeless_storage} type attribute
> In the context of section 6.5 paragraph 6 of the C11 standard,
> an object of this type behaves as if it has no declared type.
> In the context of section 6.5 paragraph 7 of the C11 standard,
> an object or a pointer if this type behaves as if it were a
> character type.
> This is attribute is similar to the @code{may_alias} attribute,
> except that it is not restricted to pointers.
> 
> Example of use:
> 
> @smallexample
> typedef int __attribute__((__typeless_storage__)) int_a;
> 
> int
> main (void)
> @{
>    int_a a = 0x12345678;
>    short *b = (short *) &a;
> 
>    b[1] = 0;
> 
>    if (a == 0x12345678)
>      abort();
> 
>    exit(0);
> @}
> @end smallexample

Seriously, do not suggest such broken case.  There's a union to
do this example portably.

Richard.

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Bernd Edlinger]

On 04/07/17 08:47, Richard Biener wrote:
> On Thu, 6 Apr 2017, Bernd Edlinger wrote:
>
>> On 04/06/17 21:05, Florian Weimer wrote:
>>> On 04/06/2017 08:49 PM, Bernd Edlinger wrote:
>>>
>>>> For instance how do you "declare an object without a declared type"?
>>>
>>> malloc and other allocation functions return pointers to objects without
>>> a declared type.
>>>
>>
>> Thanks Florian,
>>
>> this discussion is very helpful.
>>
>> How about this for the documentation:
>>
>> @item typeless_storage
>> @cindex @code{typeless_storage} type attribute
>> In the context of section 6.5 paragraph 6 of the C11 standard,
>> an object of this type behaves as if it has no declared type.
>> In the context of section 6.5 paragraph 7 of the C11 standard,
>> an object or a pointer if this type behaves as if it were a
>> character type.
>> This is attribute is similar to the @code{may_alias} attribute,
>> except that it is not restricted to pointers.
>>
>> Example of use:
>>
>> @smallexample
>> typedef int __attribute__((__typeless_storage__)) int_a;
>>
>> int
>> main (void)
>> @{
>>    int_a a = 0x12345678;
>>    short *b = (short *) &a;
>>
>>    b[1] = 0;
>>
>>    if (a == 0x12345678)
>>      abort();
>>
>>    exit(0);
>> @}
>> @end smallexample
>
> Seriously, do not suggest such broken case.  There's a union to
> do this example portably.
>

Well, it is just a mod of the other example above in
the documentation of may_alias:

typedef short __attribute__((__may_alias__)) short_a;

int
main (void)
@{
   int a = 0x12345678;
   short_a *b = (short_a *) &a;

   b[1] = 0;

   if (a == 0x12345678)
     abort();

   exit(0);
@}
@end smallexample

I just moved the attribute from "b" to "a", and that
is what the C++ people want to do as well, just they
call it a "class int_a { std::byte x[4]; }".

I personally like to have the symmetry between the
two concepts here, because it helps to understand the
differences.


Bernd.

> Richard.
>

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Richard Biener]

On April 6, 2017 8:12:29 PM GMT+02:00, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote:
>On 04/06/17 19:47, Florian Weimer wrote:
>> On 04/06/2017 07:39 PM, Bernd Edlinger wrote:
>>> On 04/06/17 16:17, Florian Weimer wrote:
>>>>> Here is what I want to write in the doc:
>>>>>
>>>>> @item typeless_storage
>>>>> @cindex @code{typeless_storage} type attribute
>>>>> A type declared with this attribute behaves like a character type
>>>>> with respect to aliasing semantics.
>>>>> This is attribute is similar to the @code{may_alias} attribute,
>>>>> except that it is not restricted to pointers.
>>>>
>>>> As Jakub pointed out, this is not what we need here.  An object of
>type
>>>> char does *not* have untyped storage.  Accessing it as a different
>type
>>>> is still undefined.
>>>>
>>>
>>> but, do you agree that this is valid in C11?
>>>
>>> typedef char char_a[4];
>>>
>>> int
>>> main (void)
>>> {
>>>    char_a a = {1,2,3,4};
>>>    short *b = (short *) &a;
>>>
>>>    b[1] = 0;
>>>
>>>    if (a[0] == 1 && a[1] == 2 && a[2] == 3 && a[3] == 4)
>>>      abort();
>>>
>>>    exit(0);
>>> }
>>>
>>>
>>> all I want to do is replace "char" with a different type.
>>
>> Thanks a lot for posting a concrete example.
>>
>> The effective type of a[2] and [3] is char.  The character type
>wildcard
>> in 6.5(7) only applies to the type of the lvalue expression ysed for
>the
>> access, not the effective type of the object being accessed.  The
>type
>> of the LHS of the assignment expression is short.  So the access is
>> undefined.
>>
>
>exactly *that* is what I want to make valid with that attribute, which
>would be also useful in C and kernel code, IMHO.
>
>But isn't the effective type changed by the assignment b[1] = 0;
>as described in 6.5(6):
>"If a value is stored into an object having no declared type through an
>lvalue having a type that is not a character type, then the type of the
>lvalue becomes the effective type of the object for that access and for
>subsequent accesses that do not modify the stored value."

Yes.  I think the example is valid.  At least GCCs memory model makes it so.

Richard.

>
>
>Bernd.

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Florian Weimer]

On 04/06/2017 09:16 PM, Richard Biener wrote:
> On April 6, 2017 8:12:29 PM GMT+02:00, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote:
>> But isn't the effective type changed by the assignment b[1] = 0;
>> as described in 6.5(6):
>> "If a value is stored into an object having no declared type through an
>> lvalue having a type that is not a character type, then the type of the
>> lvalue becomes the effective type of the object for that access and for
>> subsequent accesses that do not modify the stored value."
>
> Yes.  I think the example is valid.  At least GCCs memory model makes it so.

As far as I understand the standard, C does not permit changing the 
effective type of an object if it has a declared type (at least not 
without a union).  If GCC supports it, that's an undocumented GCC extension.

Thanks,
Florian

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Richard Biener]

On Fri, 7 Apr 2017, Florian Weimer wrote:

> On 04/06/2017 09:16 PM, Richard Biener wrote:
> > On April 6, 2017 8:12:29 PM GMT+02:00, Bernd Edlinger
> > <bernd.edlinger@hotmail.de> wrote:
> > > But isn't the effective type changed by the assignment b[1] = 0;
> > > as described in 6.5(6):
> > > "If a value is stored into an object having no declared type through an
> > > lvalue having a type that is not a character type, then the type of the
> > > lvalue becomes the effective type of the object for that access and for
> > > subsequent accesses that do not modify the stored value."
> > 
> > Yes.  I think the example is valid.  At least GCCs memory model makes it so.
> 
> As far as I understand the standard, C does not permit changing the effective
> type of an object if it has a declared type (at least not without a union).
> If GCC supports it, that's an undocumented GCC extension.

The GCC middle-end supports it because C++ supports it and there is no
way for the C FE to tell the middle-end that this is not valid.

Richard.

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Richard Biener]

On April 6, 2017 7:39:14 PM GMT+02:00, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote:
>On 04/06/17 16:17, Florian Weimer wrote:
>>> Here is what I want to write in the doc:
>>>
>>> @item typeless_storage
>>> @cindex @code{typeless_storage} type attribute
>>> A type declared with this attribute behaves like a character type
>>> with respect to aliasing semantics.
>>> This is attribute is similar to the @code{may_alias} attribute,
>>> except that it is not restricted to pointers.
>>
>> As Jakub pointed out, this is not what we need here.  An object of
>type
>> char does *not* have untyped storage.  Accessing it as a different
>type
>> is still undefined.
>>
>
>but, do you agree that this is valid in C11?
>
>typedef char char_a[4];
>
>int
>main (void)
>{
>   char_a a = {1,2,3,4};
>   short *b = (short *) &a;
>
>   b[1] = 0;
>
>   if (a[0] == 1 && a[1] == 2 && a[2] == 3 && a[3] == 4)
>     abort();
>
>   exit(0);
>}
>
>
>all I want to do is replace "char" with a different type.

Why?

Richard.

>Bernd.
>
>> The documentation says that the memory region is considered to by
>> untyped, like a memory region returned by malloc (but obviously not
>with
>> the implication that the memory region is separated from everything
>else).
>>
>> Thanks,
>> Florian

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Bernd Edlinger]

On 04/06/17 21:14, Richard Biener wrote:
> On April 6, 2017 7:39:14 PM GMT+02:00, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote:
>> On 04/06/17 16:17, Florian Weimer wrote:
>>>> Here is what I want to write in the doc:
>>>>
>>>> @item typeless_storage
>>>> @cindex @code{typeless_storage} type attribute
>>>> A type declared with this attribute behaves like a character type
>>>> with respect to aliasing semantics.
>>>> This is attribute is similar to the @code{may_alias} attribute,
>>>> except that it is not restricted to pointers.
>>>
>>> As Jakub pointed out, this is not what we need here.  An object of
>> type
>>> char does *not* have untyped storage.  Accessing it as a different
>> type
>>> is still undefined.
>>>
>>
>> but, do you agree that this is valid in C11?
>>
>> typedef char char_a[4];
>>
>> int
>> main (void)
>> {
>>   char_a a = {1,2,3,4};
>>   short *b = (short *) &a;
>>
>>   b[1] = 0;
>>
>>   if (a[0] == 1 && a[1] == 2 && a[2] == 3 && a[3] == 4)
>>     abort();
>>
>>   exit(0);
>> }
>>
>>
>> all I want to do is replace "char" with a different type.
>
> Why?

- It feels more othogonal this way.
- Otherwise malloc would have magic power, in creating objects with no
   declared type.
- And implementing something like malloc in plain C would actually be
   forbidden, which is ridiculous, because I already have done it.
- It was easy to implement in the middle end.
- It feels useful in C and C++.
- Jason says :)


Bernd.

>
> Richard.

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Richard Biener]

On Thu, 6 Apr 2017, Bernd Edlinger wrote:

> On 04/06/17 09:55, Richard Biener wrote:
> > On Thu, 6 Apr 2017, Jakub Jelinek wrote:
> >
> >> On Thu, Apr 06, 2017 at 09:47:10AM +0200, Richard Biener wrote:
> >>> @@ -955,6 +960,7 @@ get_alias_set (tree t)
> >>>       Just be pragmatic here and make sure the array and its element
> >>>       type get the same alias set assigned.  */
> >>>    else if (TREE_CODE (t) == ARRAY_TYPE
> >>> +	   && ! TYPE_TYPELESS_STORAGE (t)
> >>>  	   && (!TYPE_NONALIASED_COMPONENT (t)
> >>>  	       || TYPE_STRUCTURAL_EQUALITY_P (t)))
> >>>      set = get_alias_set (TREE_TYPE (t));
> >>> @@ -1094,6 +1100,15 @@ get_alias_set (tree t)
> >>>
> >>>    TYPE_ALIAS_SET (t) = set;
> >>>
> >>> +  if (TREE_CODE (t) == ARRAY_TYPE
> >>> +      && TYPE_TYPELESS_STORAGE (t))
> >>
> >> Shouldn't TYPE_TYPELESS_STORAGE apply even for non-array types?
> >> If somebody chooses to store anything in long long
> >> __attribute__((typeless_storage)), so be it.  Or what kind of complications
> >> do you see for that?
> >
> > It's a new feature so I don't see why we should allow that.  Given that
> > people will have to do sth when the compiler doesn't support it the
> > only "reliable" way of using it is on an array of char anyway.
> >
> > The complication starts when people use it on a type that currently
> > uses alias-set zero (because "zero" doesn't get an alias_set_entry).
> >
> 
> The typeless_storage does not need to implement all the C++ semantic
> by itself.  It would be possible, but then it is not as generic as
> it could be.  What I'd really like to have is make an arbitrary
> type behave as if it were a char with respect to aliasing.
> 
> In my mind, the typeless_storage attribute has a value of its own,
> but it can be used to implement the C++17 semantic of std::byte [N].
> 
> So I would not want to completely change the way TBAA is working
> today.  I believe it is doing a fairly good job.
> 
> The TBAA machinery, does for instance not need to propagate this
> attribute from the member to the enclosing struct that is
> also not done for a struct that contains a char.
> 
> I think it is not too complicated to done in the C++ FE.
> The FE looks for array of std::byte and unsigned char,
> and sets the attribute when the final type is constructed.
> 
> What I am trying to do is just extend the semantic of may_alias
> a bit, and then have the C++ FE use it in the way it has to.
> 
> Here is what I want to write in the doc:
> 
> @item typeless_storage
> @cindex @code{typeless_storage} type attribute
> A type declared with this attribute behaves like a character type
> with respect to aliasing semantics.
> This is attribute is similar to the @code{may_alias} attribute,
> except that it is not restricted to pointers.
> 
> Example of use:
> 
> @smallexample
> typedef int __attribute__((__typeless_storage__)) int_a;
> 
> int
> main (void)
> @{
>    int_a a = 0x12345678;
>    short *b = (short *) &a;
> 
>    b[1] = 0;
> 
>    if (a == 0x12345678)
>      abort();
> 
>    exit(0);
> @}
> @end smallexample
> 
> 
> we should first agree on that.

I don't see anyone needing the above example, it's not going to be
portable in any way.

Please don't invent sth that invites users to write bad code.
I'd even restrict it to work on arrays of chars only!
(arrays of byte-size integer types)

Richard.

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Bernd Edlinger]

On 04/06/17 09:47, Richard Biener wrote:
> On Wed, 5 Apr 2017, Bernd Edlinger wrote:
>
>> On 04/05/17 19:22, Bernd Edlinger wrote:
>>> On 04/05/17 18:08, Jakub Jelinek wrote:
>>>
>>> Yes, exactly.  I really want to reach the deadline for gcc-7.
>>> Fixing the name is certainly the most important first step,
>>> and if everybody agrees on "typeless_storage", for the name
>>> I can start with adjusting the name, and look into how
>>> to use a spare type-flag that should be a mechanical change.
>>>
>>
>> Jakub, I just renamed the attribute and reworked the patch
>> as you suggested, reg-testing is not yet completed, but
>> it looks good so far.  I also added a few more tests.
>>
>> I have changed the documentation as Richi suggested, but
>> I am not too sure what to say here.
>
> The alias.c changes are not sufficient.  I think what you want is
> sth like
>
> Index: gcc/alias.c
> ===================================================================
> --- gcc/alias.c	(revision 246678)
> +++ gcc/alias.c	(working copy)
> @@ -136,6 +136,9 @@ struct GTY(()) alias_set_entry {
>    bool is_pointer;
>    /* Nonzero if is_pointer or if one of childs have has_pointer set.  */
>    bool has_pointer;
> +  /* Nonzero if we have a child serving as typeless storage (or are
> +     such storage ourselves).  */
> +  bool has_typeless_storage;
>
>    /* The children of the alias set.  These are not just the immediate
>       children, but, in fact, all descendants.  So, if we have:
> @@ -419,7 +422,8 @@ alias_set_subset_of (alias_set_type set1
>    /* Check if set1 is a subset of set2.  */
>    ase2 = get_alias_set_entry (set2);
>    if (ase2 != 0
> -      && (ase2->has_zero_child
> +      && (ase2->has_typeless_storage
> +	  || ase2->has_zero_child
>  	  || (ase2->children && ase2->children->get (set1))))
>      return true;
>

I think get_alias_set(t) will return 0 for typeless_storage
types, and therefore has_zero_child will be set anyway.
I think both mean the same thing in the end, but it depends on
what typeless_storage should actually mean, and we have
not yet the same idea about it.

> @@ -825,6 +829,7 @@ init_alias_set_entry (alias_set_type set
>    ase->has_zero_child = false;
>    ase->is_pointer = false;
>    ase->has_pointer = false;
> +  ase->has_typeless_storage = false;
>    gcc_checking_assert (!get_alias_set_entry (set));
>    (*alias_sets)[set] = ase;
>    return ase;
> @@ -955,6 +960,7 @@ get_alias_set (tree t)
>       Just be pragmatic here and make sure the array and its element
>       type get the same alias set assigned.  */
>    else if (TREE_CODE (t) == ARRAY_TYPE
> +	   && ! TYPE_TYPELESS_STORAGE (t)
>  	   && (!TYPE_NONALIASED_COMPONENT (t)
>  	       || TYPE_STRUCTURAL_EQUALITY_P (t)))
>      set = get_alias_set (TREE_TYPE (t));
> @@ -1094,6 +1100,15 @@ get_alias_set (tree t)
>
>    TYPE_ALIAS_SET (t) = set;
>
> +  if (TREE_CODE (t) == ARRAY_TYPE
> +      && TYPE_TYPELESS_STORAGE (t))
> +    {
> +      alias_set_entry *ase = get_alias_set_entry (set);
> +      if (!ase)
> +	ase = init_alias_set_entry (set);
> +      ase->has_typeless_storage = true;
> +    }
> +
>    /* If this is an aggregate type or a complex type, we must record any
>       component aliasing information.  */
>    if (AGGREGATE_TYPE_P (t) || TREE_CODE (t) == COMPLEX_TYPE)
> @@ -1173,6 +1188,8 @@ record_alias_subset (alias_set_type supe
>  	    superset_entry->has_zero_child = true;
>            if (subset_entry->has_pointer)
>  	    superset_entry->has_pointer = true;
> +	  if (subset_entry->has_typeless_storage)
> +	    superset_entry->has_typeless_storage = true;
>
>  	  if (subset_entry->children)
>  	    {
>
>
> please also restrict TYPE_TYPELESS_STORAGE to ARRAY_TYPEs (otherwise
> more complications will arise).
>
> Index: gcc/cp/class.c
> ===================================================================
> --- gcc/cp/class.c      (revision 246678)
> +++ gcc/cp/class.c      (working copy)
> @@ -2083,7 +2083,8 @@ fixup_attribute_variants (tree t)
>    tree attrs = TYPE_ATTRIBUTES (t);
>    unsigned align = TYPE_ALIGN (t);
>    bool user_align = TYPE_USER_ALIGN (t);
> -  bool may_alias = lookup_attribute ("may_alias", attrs);
> +  bool may_alias = TYPE_TYPELESS_STORAGE (t)
> +                  || lookup_attribute ("may_alias", attrs);
>
>    if (may_alias)
>      fixup_may_alias (t);
> @@ -7345,6 +7348,12 @@ finish_struct_1 (tree t)
>       the class or perform any other required target modifications.  */
>    targetm.cxx.adjust_class_at_definition (t);
>
> +  if (cxx_dialect >= cxx1z && cxx_type_contains_byte_buffer (t))
> +    {
> +      TYPE_TYPELESS_STORAGE (t) = 1;
> +      fixup_attribute_variants (t);
> ...
>
> I don't think you need all this given alias.c only looks at
> TYPE_MAIN_VARIANTs.

I wanted to be able to declare a int __attribute__((typeless_storage))
as in the test case, and the sample in the spec.  And that
information is not in the TYPE_MAIN_VARIANT.  Therefore I look for
typeless_storage before "t = TYPE_MAIN_VARIANT (t)".

>
> Index: gcc/cp/decl.c
> ===================================================================
> --- gcc/cp/decl.c       (revision 246678)
> +++ gcc/cp/decl.c       (working copy)
> @@ -14081,10 +14081,11 @@ start_enum (tree name, tree enumtype, tree
> underly
>           enumtype = pushtag (name, enumtype, /*tag_scope=*/ts_current);
>
>           /* std::byte aliases anything.  */
> -         if (enumtype != error_mark_node
> +         if (cxx_dialect >= cxx1z
> +             && enumtype != error_mark_node
>               && TYPE_CONTEXT (enumtype) == std_node
>               && !strcmp ("byte", TYPE_NAME_STRING (enumtype)))
> -           TYPE_ALIAS_SET (enumtype) = 0;
> +           TYPE_TYPELESS_STORAGE (enumtype) = 1;
>         }
>        else
>
> not needed (but also not sufficient - you need to handle arrays of
> byte somewhere).

See cxx_type_contains_byte_buffer: this function looks recursively into
structures and unions, and returns the information if the beast
contains an array of unsigned char or std::byte.

>
> @@ -1491,6 +1491,7 @@ struct GTY(()) tree_type_common {
>    unsigned needs_constructing_flag : 1;
>    unsigned transparent_aggr_flag : 1;
>    unsigned restrict_flag : 1;
> +  unsigned typeless_storage_flag : 1;
>    unsigned contains_placeholder_bits : 2;
>
>    ENUM_BITFIELD(machine_mode) mode : 8;
>
> bits are grouped in groups of 8 bits, this breaks it.
>

Oh..., does this explain the problems that I had with this version???

> @@ -8041,7 +8041,8 @@ build_pointer_type_for_mode (tree to_type, machine
>
>    /* If the pointed-to type has the may_alias attribute set, force
>       a TYPE_REF_CAN_ALIAS_ALL pointer to be generated.  */
> -  if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
> +  if (TYPE_TYPELESS_STORAGE (to_type)
> +      || lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
>      can_alias_all = true;
>
>    /* In some cases, languages will have things that aren't a POINTER_TYPE
> @@ -8110,7 +8111,8 @@ build_reference_type_for_mode (tree to_type, machi
>
>    /* If the pointed-to type has the may_alias attribute set, force
>       a TYPE_REF_CAN_ALIAS_ALL pointer to be generated.  */
> -  if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
> +  if (TYPE_TYPELESS_STORAGE (to_type)
> +      || lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
>      can_alias_all = true;
>
>    /* In some cases, languages will have things that aren't a
>
> not needed.
>

You mean, because the get_alias_set (to_type) will be 0 anyways,
and can_alias_all wont change the semantic?


Bernd.

> +/* Nonzero if the type should behave like a character type
> +   with respect to aliasing sementics.  */
> +#define TYPE_TYPELESS_STORAGE(NODE) \
> +  (TYPE_CHECK (NODE)->type_common.typeless_storage_flag)
>
> ARRAY_TYPE_CHECK (NODE)->
>
> Richard.
>

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Richard Biener]

On Thu, 6 Apr 2017, Bernd Edlinger wrote:

> On 04/06/17 09:47, Richard Biener wrote:
> > On Wed, 5 Apr 2017, Bernd Edlinger wrote:
> >
> >> On 04/05/17 19:22, Bernd Edlinger wrote:
> >>> On 04/05/17 18:08, Jakub Jelinek wrote:
> >>>
> >>> Yes, exactly.  I really want to reach the deadline for gcc-7.
> >>> Fixing the name is certainly the most important first step,
> >>> and if everybody agrees on "typeless_storage", for the name
> >>> I can start with adjusting the name, and look into how
> >>> to use a spare type-flag that should be a mechanical change.
> >>>
> >>
> >> Jakub, I just renamed the attribute and reworked the patch
> >> as you suggested, reg-testing is not yet completed, but
> >> it looks good so far.  I also added a few more tests.
> >>
> >> I have changed the documentation as Richi suggested, but
> >> I am not too sure what to say here.
> >
> > The alias.c changes are not sufficient.  I think what you want is
> > sth like
> >
> > Index: gcc/alias.c
> > ===================================================================
> > --- gcc/alias.c	(revision 246678)
> > +++ gcc/alias.c	(working copy)
> > @@ -136,6 +136,9 @@ struct GTY(()) alias_set_entry {
> >    bool is_pointer;
> >    /* Nonzero if is_pointer or if one of childs have has_pointer set.  */
> >    bool has_pointer;
> > +  /* Nonzero if we have a child serving as typeless storage (or are
> > +     such storage ourselves).  */
> > +  bool has_typeless_storage;
> >
> >    /* The children of the alias set.  These are not just the immediate
> >       children, but, in fact, all descendants.  So, if we have:
> > @@ -419,7 +422,8 @@ alias_set_subset_of (alias_set_type set1
> >    /* Check if set1 is a subset of set2.  */
> >    ase2 = get_alias_set_entry (set2);
> >    if (ase2 != 0
> > -      && (ase2->has_zero_child
> > +      && (ase2->has_typeless_storage
> > +	  || ase2->has_zero_child
> >  	  || (ase2->children && ase2->children->get (set1))))
> >      return true;
> >
> 
> I think get_alias_set(t) will return 0 for typeless_storage
> types, and therefore has_zero_child will be set anyway.
> I think both mean the same thing in the end, but it depends on
> what typeless_storage should actually mean, and we have
> not yet the same idea about it.

But has_zero_child does not do what we like it to because otherwise
in the PR using the char[] array member would have worked!

has_zero_child doesn't do that on purpose of course, but this means
returing alias-set zero for the typeless storage _member_ doesn't
suffice.

> > @@ -825,6 +829,7 @@ init_alias_set_entry (alias_set_type set
> >    ase->has_zero_child = false;
> >    ase->is_pointer = false;
> >    ase->has_pointer = false;
> > +  ase->has_typeless_storage = false;
> >    gcc_checking_assert (!get_alias_set_entry (set));
> >    (*alias_sets)[set] = ase;
> >    return ase;
> > @@ -955,6 +960,7 @@ get_alias_set (tree t)
> >       Just be pragmatic here and make sure the array and its element
> >       type get the same alias set assigned.  */
> >    else if (TREE_CODE (t) == ARRAY_TYPE
> > +	   && ! TYPE_TYPELESS_STORAGE (t)
> >  	   && (!TYPE_NONALIASED_COMPONENT (t)
> >  	       || TYPE_STRUCTURAL_EQUALITY_P (t)))
> >      set = get_alias_set (TREE_TYPE (t));
> > @@ -1094,6 +1100,15 @@ get_alias_set (tree t)
> >
> >    TYPE_ALIAS_SET (t) = set;
> >
> > +  if (TREE_CODE (t) == ARRAY_TYPE
> > +      && TYPE_TYPELESS_STORAGE (t))
> > +    {
> > +      alias_set_entry *ase = get_alias_set_entry (set);
> > +      if (!ase)
> > +	ase = init_alias_set_entry (set);
> > +      ase->has_typeless_storage = true;
> > +    }
> > +
> >    /* If this is an aggregate type or a complex type, we must record any
> >       component aliasing information.  */
> >    if (AGGREGATE_TYPE_P (t) || TREE_CODE (t) == COMPLEX_TYPE)
> > @@ -1173,6 +1188,8 @@ record_alias_subset (alias_set_type supe
> >  	    superset_entry->has_zero_child = true;
> >            if (subset_entry->has_pointer)
> >  	    superset_entry->has_pointer = true;
> > +	  if (subset_entry->has_typeless_storage)
> > +	    superset_entry->has_typeless_storage = true;
> >
> >  	  if (subset_entry->children)
> >  	    {
> >
> >
> > please also restrict TYPE_TYPELESS_STORAGE to ARRAY_TYPEs (otherwise
> > more complications will arise).
> >
> > Index: gcc/cp/class.c
> > ===================================================================
> > --- gcc/cp/class.c      (revision 246678)
> > +++ gcc/cp/class.c      (working copy)
> > @@ -2083,7 +2083,8 @@ fixup_attribute_variants (tree t)
> >    tree attrs = TYPE_ATTRIBUTES (t);
> >    unsigned align = TYPE_ALIGN (t);
> >    bool user_align = TYPE_USER_ALIGN (t);
> > -  bool may_alias = lookup_attribute ("may_alias", attrs);
> > +  bool may_alias = TYPE_TYPELESS_STORAGE (t)
> > +                  || lookup_attribute ("may_alias", attrs);
> >
> >    if (may_alias)
> >      fixup_may_alias (t);
> > @@ -7345,6 +7348,12 @@ finish_struct_1 (tree t)
> >       the class or perform any other required target modifications.  */
> >    targetm.cxx.adjust_class_at_definition (t);
> >
> > +  if (cxx_dialect >= cxx1z && cxx_type_contains_byte_buffer (t))
> > +    {
> > +      TYPE_TYPELESS_STORAGE (t) = 1;
> > +      fixup_attribute_variants (t);
> > ...
> >
> > I don't think you need all this given alias.c only looks at
> > TYPE_MAIN_VARIANTs.
> 
> I wanted to be able to declare a int __attribute__((typeless_storage))
> as in the test case, and the sample in the spec.  And that
> information is not in the TYPE_MAIN_VARIANT.  Therefore I look for
> typeless_storage before "t = TYPE_MAIN_VARIANT (t)".

As I said I believe this is a useless feature.  If you want something
typeless then the underlying type doesn't matter so we can as well
force it to be an array of char.  Makes our live simpler.  And
even makes the code portable to compilers that treat arrays of char
conservatively.

> >
> > Index: gcc/cp/decl.c
> > ===================================================================
> > --- gcc/cp/decl.c       (revision 246678)
> > +++ gcc/cp/decl.c       (working copy)
> > @@ -14081,10 +14081,11 @@ start_enum (tree name, tree enumtype, tree
> > underly
> >           enumtype = pushtag (name, enumtype, /*tag_scope=*/ts_current);
> >
> >           /* std::byte aliases anything.  */
> > -         if (enumtype != error_mark_node
> > +         if (cxx_dialect >= cxx1z
> > +             && enumtype != error_mark_node
> >               && TYPE_CONTEXT (enumtype) == std_node
> >               && !strcmp ("byte", TYPE_NAME_STRING (enumtype)))
> > -           TYPE_ALIAS_SET (enumtype) = 0;
> > +           TYPE_TYPELESS_STORAGE (enumtype) = 1;
> >         }
> >        else
> >
> > not needed (but also not sufficient - you need to handle arrays of
> > byte somewhere).
> 
> See cxx_type_contains_byte_buffer: this function looks recursively into
> structures and unions, and returns the information if the beast
> contains an array of unsigned char or std::byte.

But with a properly designed middle-end feature that's not needed.

There's technically no reason to pessimize TBAA for anything but
the typeless storage member of a structure.

> >
> > @@ -1491,6 +1491,7 @@ struct GTY(()) tree_type_common {
> >    unsigned needs_constructing_flag : 1;
> >    unsigned transparent_aggr_flag : 1;
> >    unsigned restrict_flag : 1;
> > +  unsigned typeless_storage_flag : 1;
> >    unsigned contains_placeholder_bits : 2;
> >
> >    ENUM_BITFIELD(machine_mode) mode : 8;
> >
> > bits are grouped in groups of 8 bits, this breaks it.
> >
> 
> Oh..., does this explain the problems that I had with this version???

No, just "cosmetics".

> > @@ -8041,7 +8041,8 @@ build_pointer_type_for_mode (tree to_type, machine
> >
> >    /* If the pointed-to type has the may_alias attribute set, force
> >       a TYPE_REF_CAN_ALIAS_ALL pointer to be generated.  */
> > -  if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
> > +  if (TYPE_TYPELESS_STORAGE (to_type)
> > +      || lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
> >      can_alias_all = true;
> >
> >    /* In some cases, languages will have things that aren't a POINTER_TYPE
> > @@ -8110,7 +8111,8 @@ build_reference_type_for_mode (tree to_type, machi
> >
> >    /* If the pointed-to type has the may_alias attribute set, force
> >       a TYPE_REF_CAN_ALIAS_ALL pointer to be generated.  */
> > -  if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
> > +  if (TYPE_TYPELESS_STORAGE (to_type)
> > +      || lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
> >      can_alias_all = true;
> >
> >    /* In some cases, languages will have things that aren't a
> >
> > not needed.
> >
> 
> You mean, because the get_alias_set (to_type) will be 0 anyways,
> and can_alias_all wont change the semantic?

Well, typeless_storage and may_alias are something different.  If
you require the above then your implementation of typeless_storage
is broken.

Richard.

> 
> Bernd.
> 
> > +/* Nonzero if the type should behave like a character type
> > +   with respect to aliasing sementics.  */
> > +#define TYPE_TYPELESS_STORAGE(NODE) \
> > +  (TYPE_CHECK (NODE)->type_common.typeless_storage_flag)
> >
> > ARRAY_TYPE_CHECK (NODE)->
> >
> > Richard.
> >
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Bernd Edlinger]

On 04/07/17 08:54, Richard Biener wrote:
> On Thu, 6 Apr 2017, Bernd Edlinger wrote:
>> I think get_alias_set(t) will return 0 for typeless_storage
>> types, and therefore has_zero_child will be set anyway.
>> I think both mean the same thing in the end, but it depends on
>> what typeless_storage should actually mean, and we have
>> not yet the same idea about it.
>
> But has_zero_child does not do what we like it to because otherwise
> in the PR using the char[] array member would have worked!
>
> has_zero_child doesn't do that on purpose of course, but this means
> returing alias-set zero for the typeless storage _member_ doesn't
> suffice.
>

I see you have a certain idea how to solve the C++17 issue.
And yes, I apologize, if I tried to pee on your tree :)

What you propose is I think the following:
The C++ FE sets TYPE_TYPELESS_STORAGE a std::byte
and on "unsigned char" if the language dialect is cxx17
and the TBAA makes all the rest.

What I propose is as follows:
The TYPE_TYPELESS_STORAGE is a generic attribute, it
can be set on any type, and in the TBAA the attribute
does not squirrel around at all.  If it is on a type,
then all DECLs with this type get the alias set 0.
If it is on a member of a struct that does not mean
more than if the struct has a char member this it
sets has_zero_child, which I do not want to mean
anything else than before.

The C++ FE does the business logic here, in deciding
where to distribute the TYPE_TYPELESS_STORAGE flags.

in this example
class A {
   class B {
     std::byte x[5];
   } b;
};

std::byte, class B, and class A would get the
TYPE_TYPELESS_STORAGE flag set by the C++FE if
the language dialect is cxx17 or above,
so that you can place anything into any object
of class A and class B, and of type std::byte.

but in this example
class B {
   std::byte x;
};

only std::byte would get the TYPE_TYPELESS_STORAGE
flag, so you can not put anyting into an object
of class B, just on an object of std::byte.



>>
>> I wanted to be able to declare a int __attribute__((typeless_storage))
>> as in the test case, and the sample in the spec.  And that
>> information is not in the TYPE_MAIN_VARIANT.  Therefore I look for
>> typeless_storage before "t = TYPE_MAIN_VARIANT (t)".
>
> As I said I believe this is a useless feature.  If you want something
> typeless then the underlying type doesn't matter so we can as well
> force it to be an array of char.  Makes our live simpler.  And
> even makes the code portable to compilers that treat arrays of char
> conservatively.
>

I just learned that the C11 standard does not guarantee that, and also
an array of char does not provide the necessary alignment per se, at
least without alignment attributes.

>>
>> See cxx_type_contains_byte_buffer: this function looks recursively into
>> structures and unions, and returns the information if the beast
>> contains an array of unsigned char or std::byte.
>
> But with a properly designed middle-end feature that's not needed.
>
> There's technically no reason to pessimize TBAA for anything but
> the typeless storage member of a structure.
>

Yes, it is just a matter of taste.  And if you want the middle
end to be flexible here or if everything should work without user
intervention.


>>>
>>> @@ -1491,6 +1491,7 @@ struct GTY(()) tree_type_common {
>>>    unsigned needs_constructing_flag : 1;
>>>    unsigned transparent_aggr_flag : 1;
>>>    unsigned restrict_flag : 1;
>>> +  unsigned typeless_storage_flag : 1;
>>>    unsigned contains_placeholder_bits : 2;
>>>
>>>    ENUM_BITFIELD(machine_mode) mode : 8;
>>>
>>> bits are grouped in groups of 8 bits, this breaks it.
>>>
>>
>> Oh..., does this explain the problems that I had with this version???
>
> No, just "cosmetics".
>
>>> @@ -8041,7 +8041,8 @@ build_pointer_type_for_mode (tree to_type, machine
>>>
>>>    /* If the pointed-to type has the may_alias attribute set, force
>>>       a TYPE_REF_CAN_ALIAS_ALL pointer to be generated.  */
>>> -  if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
>>> +  if (TYPE_TYPELESS_STORAGE (to_type)
>>> +      || lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
>>>      can_alias_all = true;
>>>
>>>    /* In some cases, languages will have things that aren't a POINTER_TYPE
>>> @@ -8110,7 +8111,8 @@ build_reference_type_for_mode (tree to_type, machi
>>>
>>>    /* If the pointed-to type has the may_alias attribute set, force
>>>       a TYPE_REF_CAN_ALIAS_ALL pointer to be generated.  */
>>> -  if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
>>> +  if (TYPE_TYPELESS_STORAGE (to_type)
>>> +      || lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
>>>      can_alias_all = true;
>>>
>>>    /* In some cases, languages will have things that aren't a
>>>
>>> not needed.
>>>
>>
>> You mean, because the get_alias_set (to_type) will be 0 anyways,
>> and can_alias_all wont change the semantic?
>
> Well, typeless_storage and may_alias are something different.  If
> you require the above then your implementation of typeless_storage
> is broken.
>

You are right, the hunk above is actually unnecessary.

> Richard.
>
>>
>> Bernd.
>>
>>> +/* Nonzero if the type should behave like a character type
>>> +   with respect to aliasing sementics.  */
>>> +#define TYPE_TYPELESS_STORAGE(NODE) \
>>> +  (TYPE_CHECK (NODE)->type_common.typeless_storage_flag)
>>>
>>> ARRAY_TYPE_CHECK (NODE)->
>>>
>>> Richard.
>>>
>>
>>
>

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Richard Biener]

On April 7, 2017 3:37:30 PM GMT+02:00, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote:
>On 04/07/17 08:54, Richard Biener wrote:
>> On Thu, 6 Apr 2017, Bernd Edlinger wrote:
>>> I think get_alias_set(t) will return 0 for typeless_storage
>>> types, and therefore has_zero_child will be set anyway.
>>> I think both mean the same thing in the end, but it depends on
>>> what typeless_storage should actually mean, and we have
>>> not yet the same idea about it.
>>
>> But has_zero_child does not do what we like it to because otherwise
>> in the PR using the char[] array member would have worked!
>>
>> has_zero_child doesn't do that on purpose of course, but this means
>> returing alias-set zero for the typeless storage _member_ doesn't
>> suffice.
>>
>
>I see you have a certain idea how to solve the C++17 issue.
>And yes, I apologize, if I tried to pee on your tree :)

We do have the need to support this part of the C++ standard.  For other user code may_alias suffices and I see no reason to haste inventing sth new without a single convincing testcase.  GCC/Language extensions should not be added without a good reason.

I didn't propose to expose the type flag to users at all.

Richard.

>What you propose is I think the following:
>The C++ FE sets TYPE_TYPELESS_STORAGE a std::byte
>and on "unsigned char" if the language dialect is cxx17
>and the TBAA makes all the rest.
>
>What I propose is as follows:
>The TYPE_TYPELESS_STORAGE is a generic attribute, it
>can be set on any type, and in the TBAA the attribute
>does not squirrel around at all.  If it is on a type,
>then all DECLs with this type get the alias set 0.
>If it is on a member of a struct that does not mean
>more than if the struct has a char member this it
>sets has_zero_child, which I do not want to mean
>anything else than before.
>
>The C++ FE does the business logic here, in deciding
>where to distribute the TYPE_TYPELESS_STORAGE flags.
>
>in this example
>class A {
>   class B {
>     std::byte x[5];
>   } b;
>};
>
>std::byte, class B, and class A would get the
>TYPE_TYPELESS_STORAGE flag set by the C++FE if
>the language dialect is cxx17 or above,
>so that you can place anything into any object
>of class A and class B, and of type std::byte.
>
>but in this example
>class B {
>   std::byte x;
>};
>
>only std::byte would get the TYPE_TYPELESS_STORAGE
>flag, so you can not put anyting into an object
>of class B, just on an object of std::byte.
>
>
>
>>>
>>> I wanted to be able to declare a int
>__attribute__((typeless_storage))
>>> as in the test case, and the sample in the spec.  And that
>>> information is not in the TYPE_MAIN_VARIANT.  Therefore I look for
>>> typeless_storage before "t = TYPE_MAIN_VARIANT (t)".
>>
>> As I said I believe this is a useless feature.  If you want something
>> typeless then the underlying type doesn't matter so we can as well
>> force it to be an array of char.  Makes our live simpler.  And
>> even makes the code portable to compilers that treat arrays of char
>> conservatively.
>>
>
>I just learned that the C11 standard does not guarantee that, and also
>an array of char does not provide the necessary alignment per se, at
>least without alignment attributes.
>
>>>
>>> See cxx_type_contains_byte_buffer: this function looks recursively
>into
>>> structures and unions, and returns the information if the beast
>>> contains an array of unsigned char or std::byte.
>>
>> But with a properly designed middle-end feature that's not needed.
>>
>> There's technically no reason to pessimize TBAA for anything but
>> the typeless storage member of a structure.
>>
>
>Yes, it is just a matter of taste.  And if you want the middle
>end to be flexible here or if everything should work without user
>intervention.
>
>
>>>>
>>>> @@ -1491,6 +1491,7 @@ struct GTY(()) tree_type_common {
>>>>    unsigned needs_constructing_flag : 1;
>>>>    unsigned transparent_aggr_flag : 1;
>>>>    unsigned restrict_flag : 1;
>>>> +  unsigned typeless_storage_flag : 1;
>>>>    unsigned contains_placeholder_bits : 2;
>>>>
>>>>    ENUM_BITFIELD(machine_mode) mode : 8;
>>>>
>>>> bits are grouped in groups of 8 bits, this breaks it.
>>>>
>>>
>>> Oh..., does this explain the problems that I had with this
>version???
>>
>> No, just "cosmetics".
>>
>>>> @@ -8041,7 +8041,8 @@ build_pointer_type_for_mode (tree to_type,
>machine
>>>>
>>>>    /* If the pointed-to type has the may_alias attribute set, force
>>>>       a TYPE_REF_CAN_ALIAS_ALL pointer to be generated.  */
>>>> -  if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
>>>> +  if (TYPE_TYPELESS_STORAGE (to_type)
>>>> +      || lookup_attribute ("may_alias", TYPE_ATTRIBUTES
>(to_type)))
>>>>      can_alias_all = true;
>>>>
>>>>    /* In some cases, languages will have things that aren't a
>POINTER_TYPE
>>>> @@ -8110,7 +8111,8 @@ build_reference_type_for_mode (tree to_type,
>machi
>>>>
>>>>    /* If the pointed-to type has the may_alias attribute set, force
>>>>       a TYPE_REF_CAN_ALIAS_ALL pointer to be generated.  */
>>>> -  if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
>>>> +  if (TYPE_TYPELESS_STORAGE (to_type)
>>>> +      || lookup_attribute ("may_alias", TYPE_ATTRIBUTES
>(to_type)))
>>>>      can_alias_all = true;
>>>>
>>>>    /* In some cases, languages will have things that aren't a
>>>>
>>>> not needed.
>>>>
>>>
>>> You mean, because the get_alias_set (to_type) will be 0 anyways,
>>> and can_alias_all wont change the semantic?
>>
>> Well, typeless_storage and may_alias are something different.  If
>> you require the above then your implementation of typeless_storage
>> is broken.
>>
>
>You are right, the hunk above is actually unnecessary.
>
>> Richard.
>>
>>>
>>> Bernd.
>>>
>>>> +/* Nonzero if the type should behave like a character type
>>>> +   with respect to aliasing sementics.  */
>>>> +#define TYPE_TYPELESS_STORAGE(NODE) \
>>>> +  (TYPE_CHECK (NODE)->type_common.typeless_storage_flag)
>>>>
>>>> ARRAY_TYPE_CHECK (NODE)->
>>>>
>>>> Richard.
>>>>
>>>
>>>
>>

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 1646 bytes --]

On 04/07/17 17:10, Richard Biener wrote:
> On April 7, 2017 3:37:30 PM GMT+02:00, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote:
>> On 04/07/17 08:54, Richard Biener wrote:
>>> On Thu, 6 Apr 2017, Bernd Edlinger wrote:
>>>> I think get_alias_set(t) will return 0 for typeless_storage
>>>> types, and therefore has_zero_child will be set anyway.
>>>> I think both mean the same thing in the end, but it depends on
>>>> what typeless_storage should actually mean, and we have
>>>> not yet the same idea about it.
>>>
>>> But has_zero_child does not do what we like it to because otherwise
>>> in the PR using the char[] array member would have worked!
>>>
>>> has_zero_child doesn't do that on purpose of course, but this means
>>> returing alias-set zero for the typeless storage _member_ doesn't
>>> suffice.
>>>
>>
>> I see you have a certain idea how to solve the C++17 issue.
>> And yes, I apologize, if I tried to pee on your tree :)
>
> We do have the need to support this part of the C++ standard.  For other user code may_alias suffices and I see no reason to haste inventing sth new without a single convincing testcase.  GCC/Language extensions should not be added without a good reason.
>
> I didn't propose to expose the type flag to users at all.
>
> Richard.
>

Well, actually you did:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671#c100

But I won't argue if you prefer to do it as you like,
after all TBAA is your area.  So that is fine for me.

Attached is the latest version of my patch, I fixed
the issue with the type-flag conversion, and it is
already fully functional.


Bernd.

[-- Attachment #2: changelog-typeless-storage.txt --]
[-- Type: text/plain, Size: 1488 bytes --]

gcc
2017-04-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* doc/extend.texi: Document the typeless_storage type attribute.
	* alias.c (get_alias_set): Handle the typeless_storage attribute.
	(record_component_aliases): Likewise.
	* tree-streamer-in.c (unpack_ts_type_common_value_fields): Likewise.
	* tree-streamer-out.c (pack_ts_type_common_value_fields): Likewise.
	* tree.h (TYPE_TYPELESS_STORAGE): New access macro.
	* tree-core.h: Update comment.

gcc/c-family
2017-04-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-attribs.c (c_common_attribute_tab): Add the typeless_storage
	attribute.
	(handle_typeless_storage_attribute): New function.

gcc/cp
2017-04-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* class.c (fixup_attribute_variants): Handle the typeless_storage
	attribute.
	(finish_struct_1): Set the typeless_storage attribute if required
	by C++17.
	* decl.c (start_enum): Likewise.
	* pt.c (lookup_template_class_1): Handle the typeless_storage
	attribute.
	* typeck2.c (cxx_type_contains_byte_buffer): New function.
	* cp-tree.h (cxx_type_contains_byte_buffer): Declare.

gcc/lto
2017-04-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* lto.c (compare_tree_sccs_1, hash_tree): Handle the typeless_storage
	attribute.

gcc/testsuite
2017-04-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-c++-common/attr-typeless-storage-1.c: New test.
	* c-c++-common/attr-typeless-storage-2.c: New test.
	* gcc.c-torture/execute/typeless-storage-1.c: New test.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: patch-typeless-storage.diff --]
[-- Type: text/x-patch; name="patch-typeless-storage.diff", Size: 13609 bytes --]

Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 246678)
+++ gcc/doc/extend.texi	(working copy)
@@ -6656,6 +6656,36 @@
 @option{-fstrict-aliasing}, which is on by default at @option{-O2} or
 above.
 
+@item typeless_storage
+@cindex @code{typeless_storage} type attribute
+In the context of section 6.5 paragraph 6 of the C11 standard,
+an object of this type behaves as if it has no declared type.
+In the context of section 6.5 paragraph 7 of the C11 standard,
+an object or a pointer if this type behaves as if it were a
+character type.
+This attribute is similar to the @code{may_alias} attribute,
+except that it is not restricted to pointers.
+
+Example of use:
+
+@smallexample
+typedef int __attribute__((__typeless_storage__)) int_a;
+
+int
+main (void)
+@{
+  int_a a = 0x12345678;
+  short *b = (short *) &a;
+
+  b[1] = 0;
+
+  if (a == 0x12345678)
+    abort();
+
+  exit(0);
+@}
+@end smallexample
+
 @item packed
 @cindex @code{packed} type attribute
 This attribute, attached to @code{struct} or @code{union} type
Index: gcc/alias.c
===================================================================
--- gcc/alias.c	(revision 246678)
+++ gcc/alias.c	(working copy)
@@ -879,6 +879,10 @@ get_alias_set (tree t)
       t = TREE_TYPE (t);
     }
 
+  /* Honor the typeless_storage type attribute.  */
+  if (TYPE_TYPELESS_STORAGE (t))
+    return 0;
+
   /* Variant qualifiers don't affect the alias set, so get the main
      variant.  */
   t = TYPE_MAIN_VARIANT (t);
@@ -1234,7 +1238,8 @@ record_component_aliases (tree type)
 		/* VECTOR_TYPE and ARRAY_TYPE share the alias set with their
 		   element type and that type has to be normalized to void *,
 		   too, in the case it is a pointer. */
-		while (!canonical_type_used_p (t) && !POINTER_TYPE_P (t))
+		while (!canonical_type_used_p (t) && !POINTER_TYPE_P (t)
+		       && !TYPE_TYPELESS_STORAGE (t))
 		  {
 		    gcc_checking_assert (TYPE_STRUCTURAL_EQUALITY_P (t));
 		    t = TREE_TYPE (t);
Index: gcc/c-family/c-attribs.c
===================================================================
--- gcc/c-family/c-attribs.c	(revision 246678)
+++ gcc/c-family/c-attribs.c	(working copy)
@@ -113,6 +113,7 @@ static tree handle_vector_size_attribute (tree *,
 					  bool *);
 static tree handle_nonnull_attribute (tree *, tree, tree, int, bool *);
 static tree handle_nothrow_attribute (tree *, tree, tree, int, bool *);
+static tree handle_typeless_storage_attribute (tree *, tree, tree, int, bool *);
 static tree handle_cleanup_attribute (tree *, tree, tree, int, bool *);
 static tree handle_warn_unused_result_attribute (tree *, tree, tree, int,
 						 bool *);
@@ -265,6 +266,8 @@ const struct attribute_spec c_common_attribute_tab
   { "nothrow",                0, 0, true,  false, false,
 			      handle_nothrow_attribute, false },
   { "may_alias",	      0, 0, false, true, false, NULL, false },
+  { "typeless_storage",       0, 0, false, true, false,
+			      handle_typeless_storage_attribute, false },
   { "cleanup",		      1, 1, true, false, false,
 			      handle_cleanup_attribute, false },
   { "warn_unused_result",     0, 0, false, true, true,
@@ -2879,6 +2882,24 @@ handle_nothrow_attribute (tree *node, tree name, t
   return NULL_TREE;
 }
 
+/* Handle a "typeless_storage" attribute; arguments as in
+   struct attribute_spec.handler.  */
+
+static tree
+handle_typeless_storage_attribute (tree *node, tree name, tree ARG_UNUSED (args),
+				   int ARG_UNUSED (flags), bool *no_add_attrs)
+{
+  if (TYPE_P (*node))
+    TYPE_TYPELESS_STORAGE (*node) = 1;
+  else
+    {
+      warning (OPT_Wattributes, "%qE attribute ignored", name);
+      *no_add_attrs = true;
+    }
+
+  return NULL_TREE;
+}
+
 /* Handle a "cleanup" attribute; arguments as in
    struct attribute_spec.handler.  */
 
Index: gcc/cp/class.c
===================================================================
--- gcc/cp/class.c	(revision 246678)
+++ gcc/cp/class.c	(working copy)
@@ -2097,6 +2097,7 @@ fixup_attribute_variants (tree t)
       /* These are the two fields that check_qualified_type looks at and
 	 are affected by attributes.  */
       TYPE_ATTRIBUTES (variants) = attrs;
+      TYPE_TYPELESS_STORAGE (variants) = TYPE_TYPELESS_STORAGE (t);
       unsigned valign = align;
       if (TYPE_USER_ALIGN (variants))
 	valign = MAX (valign, TYPE_ALIGN (variants));
@@ -7345,6 +7346,12 @@ finish_struct_1 (tree t)
      the class or perform any other required target modifications.  */
   targetm.cxx.adjust_class_at_definition (t);
 
+  if (cxx_dialect >= cxx1z && cxx_type_contains_byte_buffer (t))
+    {
+      TYPE_TYPELESS_STORAGE (t) = 1;
+      fixup_attribute_variants (t);
+    }
+
   maybe_suppress_debug_info (t);
 
   if (flag_vtable_verify)
Index: gcc/cp/cp-tree.h
===================================================================
--- gcc/cp/cp-tree.h	(revision 246678)
+++ gcc/cp/cp-tree.h	(working copy)
@@ -6858,6 +6858,7 @@ extern tree finish_binary_fold_expr          (tree
 extern void require_complete_eh_spec_types	(tree, tree);
 extern void cxx_incomplete_type_diagnostic	(location_t, const_tree,
 						 const_tree, diagnostic_t);
+extern bool cxx_type_contains_byte_buffer	(tree);
 inline void
 cxx_incomplete_type_diagnostic (const_tree value, const_tree type,
 				diagnostic_t diag_kind)
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 246678)
+++ gcc/cp/decl.c	(working copy)
@@ -14081,10 +14081,11 @@ start_enum (tree name, tree enumtype, tree underly
 	  enumtype = pushtag (name, enumtype, /*tag_scope=*/ts_current);
 
 	  /* std::byte aliases anything.  */
-	  if (enumtype != error_mark_node
+	  if (cxx_dialect >= cxx1z
+	      && enumtype != error_mark_node
 	      && TYPE_CONTEXT (enumtype) == std_node
 	      && !strcmp ("byte", TYPE_NAME_STRING (enumtype)))
-	    TYPE_ALIAS_SET (enumtype) = 0;
+	    TYPE_TYPELESS_STORAGE (enumtype) = 1;
 	}
       else
 	  enumtype = xref_tag (enum_type, name, /*tag_scope=*/ts_current,
Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c	(revision 246678)
+++ gcc/cp/pt.c	(working copy)
@@ -8853,7 +8853,8 @@ lookup_template_class_1 (tree d1, tree arglist, tr
 	{
 	  static const char *tags[] = {"abi_tag", "may_alias"};
 
-	  for (unsigned ix = 0; ix != 2; ix++)
+	  TYPE_TYPELESS_STORAGE (t) |= TYPE_TYPELESS_STORAGE (template_type);
+	  for (unsigned ix = 0; ix < sizeof (tags) / sizeof (tags[0]); ix++)
 	    {
 	      tree attributes
 		= lookup_attribute (tags[ix], TYPE_ATTRIBUTES (template_type));
Index: gcc/cp/typeck2.c
===================================================================
--- gcc/cp/typeck2.c	(revision 246678)
+++ gcc/cp/typeck2.c	(working copy)
@@ -2234,5 +2234,29 @@ require_complete_eh_spec_types (tree fntype, tree
     }
 }
 
+/* True iff type either is or contains a byte buffer (which can be used for
+   storing any trivially copyable type).  */
+
+bool
+cxx_type_contains_byte_buffer (tree type)
+{
+  if (TREE_CODE (type) == ARRAY_TYPE
+      && (cxx_type_contains_byte_buffer (TREE_TYPE (type))
+	  || TREE_TYPE (type) == unsigned_char_type_node
+	  || (TREE_CODE (TREE_TYPE (type)) == ENUMERAL_TYPE
+	      && TYPE_CONTEXT (TREE_TYPE (type)) == std_node
+	      && !strcmp ("byte", TYPE_NAME_STRING (TREE_TYPE (type))))))
+    return true;
+
+  if (CLASS_TYPE_P (type))
+    for (tree field = next_initializable_field (TYPE_FIELDS (type));
+	 field;
+	 field = next_initializable_field (DECL_CHAIN (field)))
+      if (cxx_type_contains_byte_buffer (TREE_TYPE (field)))
+	return true;
+
+  return false;
+}
+
 \f
 #include "gt-cp-typeck2.h"
Index: gcc/lto/lto.c
===================================================================
--- gcc/lto/lto.c	(revision 246678)
+++ gcc/lto/lto.c	(working copy)
@@ -1164,6 +1164,7 @@ compare_tree_sccs_1 (tree t1, tree t2, tree **map)
 	compare_values (TYPE_NONALIASED_COMPONENT);
       compare_values (TYPE_PACKED);
       compare_values (TYPE_RESTRICT);
+      compare_values (TYPE_TYPELESS_STORAGE);
       compare_values (TYPE_USER_ALIGN);
       compare_values (TYPE_READONLY);
       compare_values (TYPE_PRECISION);
Index: gcc/lto-streamer-out.c
===================================================================
--- gcc/lto-streamer-out.c	(revision 246678)
+++ gcc/lto-streamer-out.c	(working copy)
@@ -1134,6 +1134,7 @@ hash_tree (struct streamer_tree_cache_d *cache, ha
       hstate.add_flag (TYPE_NEEDS_CONSTRUCTING (t));
       hstate.add_flag (TYPE_PACKED (t));
       hstate.add_flag (TYPE_RESTRICT (t));
+      hstate.add_flag (TYPE_TYPELESS_STORAGE (t));
       hstate.add_flag (TYPE_USER_ALIGN (t));
       hstate.add_flag (TYPE_READONLY (t));
       if (RECORD_OR_UNION_TYPE_P (t))
Index: gcc/tree-core.h
===================================================================
--- gcc/tree-core.h	(revision 246678)
+++ gcc/tree-core.h	(working copy)
@@ -1216,6 +1216,9 @@ struct GTY(()) tree_base {
        DECL_NONALIASED in
 	  VAR_DECL
 
+       TYPE_TYPELESS_STORAGE in
+	  all types
+
    deprecated_flag:
 
        TREE_DEPRECATED in
Index: gcc/tree-streamer-in.c
===================================================================
--- gcc/tree-streamer-in.c	(revision 246678)
+++ gcc/tree-streamer-in.c	(working copy)
@@ -367,6 +367,7 @@ unpack_ts_type_common_value_fields (struct bitpack
   TYPE_NEEDS_CONSTRUCTING (expr) = (unsigned) bp_unpack_value (bp, 1);
   TYPE_PACKED (expr) = (unsigned) bp_unpack_value (bp, 1);
   TYPE_RESTRICT (expr) = (unsigned) bp_unpack_value (bp, 1);
+  TYPE_TYPELESS_STORAGE (expr) = (unsigned) bp_unpack_value (bp, 1);
   TYPE_USER_ALIGN (expr) = (unsigned) bp_unpack_value (bp, 1);
   TYPE_READONLY (expr) = (unsigned) bp_unpack_value (bp, 1);
   if (RECORD_OR_UNION_TYPE_P (expr))
Index: gcc/tree-streamer-out.c
===================================================================
--- gcc/tree-streamer-out.c	(revision 246678)
+++ gcc/tree-streamer-out.c	(working copy)
@@ -316,6 +316,7 @@ pack_ts_type_common_value_fields (struct bitpack_d
   bp_pack_value (bp, TYPE_NEEDS_CONSTRUCTING (expr), 1);
   bp_pack_value (bp, TYPE_PACKED (expr), 1);
   bp_pack_value (bp, TYPE_RESTRICT (expr), 1);
+  bp_pack_value (bp, TYPE_TYPELESS_STORAGE (expr), 1);
   bp_pack_value (bp, TYPE_USER_ALIGN (expr), 1);
   bp_pack_value (bp, TYPE_READONLY (expr), 1);
   /* We used to stream TYPE_ALIAS_SET == 0 information to let frontends mark
Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 246678)
+++ gcc/tree.h	(working copy)
@@ -1944,6 +1944,11 @@ extern machine_mode element_mode (const_tree t);
    the term.  */
 #define TYPE_RESTRICT(NODE) (TYPE_CHECK (NODE)->type_common.restrict_flag)
 
+/* Nonzero if the type should behave like a character type
+   with respect to aliasing sementics.  */
+#define TYPE_TYPELESS_STORAGE(NODE) \
+  (TYPE_CHECK (NODE)->base.nothrow_flag)
+
 /* If nonzero, type's name shouldn't be emitted into debug info.  */
 #define TYPE_NAMELESS(NODE) (TYPE_CHECK (NODE)->base.u.bits.nameless_flag)
 
Index: gcc/testsuite/c-c++-common/attr-typeless-storage-1.c
===================================================================
--- gcc/testsuite/c-c++-common/attr-typeless-storage-1.c	(revision 0)
+++ gcc/testsuite/c-c++-common/attr-typeless-storage-1.c	(working copy)
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wall" } */
+
+typedef int T __attribute__((typeless_storage));
+
+extern T t, v;
+extern T *p;
+extern int *p;
+
+extern int *p2;
+extern T *p2;
+
+void fn1 (T);
+void fn1 (int);
+
+void fn2 (int);
+void fn2 (T);
+
+/* Ensure that the composite types have typeless_storage.  */
+void
+f (long *i)
+{
+  *i = *(__typeof (*p) *) &p;
+  asm ("" : : "r" (*p));
+  *i = *(__typeof (*p2) *) &p2;
+  asm ("" : : "r" (*p2));
+  t = v;
+  asm ("" : : "r" (t));
+}
Index: gcc/testsuite/c-c++-common/attr-typeless-storage-2.c
===================================================================
--- gcc/testsuite/c-c++-common/attr-typeless-storage-2.c	(revision 0)
+++ gcc/testsuite/c-c++-common/attr-typeless-storage-2.c	(working copy)
@@ -0,0 +1,17 @@
+/* We used to reject this because types differentiating only in
+   TYPE_REF_CAN_ALIAS_ALL were deemed incompatible.  */
+/* { dg-do compile } */
+
+struct sockaddr;
+struct sockaddr *f (void);
+
+struct __attribute__((typeless_storage)) sockaddr { int j; };
+struct sockaddr *
+f (void)
+{
+  return
+#ifndef __cplusplus
+    (void *)
+#endif
+    0;
+}
Index: gcc/testsuite/gcc.c-torture/execute/typeless-storage-1.c
===================================================================
--- gcc/testsuite/gcc.c-torture/execute/typeless-storage-1.c	(revision 0)
+++ gcc/testsuite/gcc.c-torture/execute/typeless-storage-1.c	(working copy)
@@ -0,0 +1,20 @@
+/* Tests that the typeless_storage attribute works as expected.  */
+ 
+extern void abort(void);
+extern void exit(int);
+
+typedef int __attribute__((__typeless_storage__)) int_a;
+
+int
+main (void)
+{
+  int_a a = 0x12345678;
+  short *b = (short*) &a;
+
+  b[1] = 0;
+
+  if (a == 0x12345678)
+    abort();
+
+  exit(0);
+}

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Jason Merrill]

On Fri, Apr 7, 2017 at 11:32 AM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
> On 04/07/17 17:10, Richard Biener wrote:
>> On April 7, 2017 3:37:30 PM GMT+02:00, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote:
>>> On 04/07/17 08:54, Richard Biener wrote:
>>>> On Thu, 6 Apr 2017, Bernd Edlinger wrote:
>>>>> I think get_alias_set(t) will return 0 for typeless_storage
>>>>> types, and therefore has_zero_child will be set anyway.
>>>>> I think both mean the same thing in the end, but it depends on
>>>>> what typeless_storage should actually mean, and we have
>>>>> not yet the same idea about it.
>>>>
>>>> But has_zero_child does not do what we like it to because otherwise
>>>> in the PR using the char[] array member would have worked!
>>>>
>>>> has_zero_child doesn't do that on purpose of course, but this means
>>>> returing alias-set zero for the typeless storage _member_ doesn't
>>>> suffice.
>>>>
>>>
>>> I see you have a certain idea how to solve the C++17 issue.
>>> And yes, I apologize, if I tried to pee on your tree :)
>>
>> We do have the need to support this part of the C++ standard.  For other user code may_alias suffices and I see no reason to haste inventing sth new without a single convincing testcase.  GCC/Language extensions should not be added without a good reason.
>>
>> I didn't propose to expose the type flag to users at all.
>>
>> Richard.
>>
>
> Well, actually you did:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671#c100
>
> But I won't argue if you prefer to do it as you like,
> after all TBAA is your area.  So that is fine for me.
>
> Attached is the latest version of my patch, I fixed
> the issue with the type-flag conversion, and it is
> already fully functional.

I agree that we don't want a new attribute.

This should not be limited to C++17 mode; std::aligned_storage is in
C++11 and we might as well have the same behavior in C++98 mode.

Jason

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Richard Biener]

On Fri, 7 Apr 2017, Jason Merrill wrote:

> On Fri, Apr 7, 2017 at 11:32 AM, Bernd Edlinger
> <bernd.edlinger@hotmail.de> wrote:
> > On 04/07/17 17:10, Richard Biener wrote:
> >> On April 7, 2017 3:37:30 PM GMT+02:00, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote:
> >>> On 04/07/17 08:54, Richard Biener wrote:
> >>>> On Thu, 6 Apr 2017, Bernd Edlinger wrote:
> >>>>> I think get_alias_set(t) will return 0 for typeless_storage
> >>>>> types, and therefore has_zero_child will be set anyway.
> >>>>> I think both mean the same thing in the end, but it depends on
> >>>>> what typeless_storage should actually mean, and we have
> >>>>> not yet the same idea about it.
> >>>>
> >>>> But has_zero_child does not do what we like it to because otherwise
> >>>> in the PR using the char[] array member would have worked!
> >>>>
> >>>> has_zero_child doesn't do that on purpose of course, but this means
> >>>> returing alias-set zero for the typeless storage _member_ doesn't
> >>>> suffice.
> >>>>
> >>>
> >>> I see you have a certain idea how to solve the C++17 issue.
> >>> And yes, I apologize, if I tried to pee on your tree :)
> >>
> >> We do have the need to support this part of the C++ standard.  For other user code may_alias suffices and I see no reason to haste inventing sth new without a single convincing testcase.  GCC/Language extensions should not be added without a good reason.
> >>
> >> I didn't propose to expose the type flag to users at all.
> >>
> >> Richard.
> >>
> >
> > Well, actually you did:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671#c100
> >
> > But I won't argue if you prefer to do it as you like,
> > after all TBAA is your area.  So that is fine for me.
> >
> > Attached is the latest version of my patch, I fixed
> > the issue with the type-flag conversion, and it is
> > already fully functional.
> 
> I agree that we don't want a new attribute.
> 
> This should not be limited to C++17 mode; std::aligned_storage is in
> C++11 and we might as well have the same behavior in C++98 mode.

So here's my variant of a fix.  I constrained the new flag
TYPE_TYPELESS_STORAGE to arrays and thus hope my localized fix in
build_cplus_array_type will suffice (if I have time I'll play with
template instantiation).

I've created a nice small testcase that exposes the issue but it
needs a PTA fix to work (I'll commit the tree-ssa-structalias.c
fix separately).  Some more obfuscation might remove the requirement
of it (don't use an asm but a noinline const function maybe).

Bootstrap and regtest is underway on x86_64-unknown-linux-gnu.

There may be some issues with LTO / cross-language support remaining,
I need to think more about this (TYPE_CANONICAL merging plus alias.c
punning down to TYPE_CANONICAL).

Richard.

2017-04-10  Richard Biener  <rguenther@suse.de>

	PR c++/79671
	cp/
	* tree.c (build_cplus_array_type): Set TYPE_TYPELESS_STORAGE
	for arrays of unsigned char or std::byte.

	* tree-core.h (tree_type_common): Add typeless_storage flag.
	* tree.h (TYPE_TYPELESS_STORAGE): New macro.
	* alias.c (alias_set_entry): Add has_typeless_storage member.
	(alias_set_subset_of): Handle it.
	(alias_sets_conflict_p): Likewise.
	(init_alias_set_entry): Initialize it.
	(get_alias_set): For ARRAY_TYPEs handle TYPE_TYPELESS_STORAGE.
	(record_alias_subset): Likewise.
	* lto-streamer-out.c (hash_tree): Hash TYPE_TYPELESS_STORAGE.
	* tree-streamer-in.c (unpack_ts_type_common_value_fields): Stream
	TYPE_TYPELESS_STORAGE.
	* tree-streamer-out.c (pack_ts_type_common_value_fields): Likewise.
	* tree-ssa-structalias.c (find_func_aliases): Properly handle
	asm inputs.

	lto/
	* lto.c (compare_tree_sccs_1): Compare TYPE_TYPELESS_STORAGE.

	* g++.dg/torture/pr79671.C: New testcase.


Index: gcc/alias.c
===================================================================
--- gcc/alias.c	(revision 246803)
+++ gcc/alias.c	(working copy)
@@ -136,6 +136,9 @@ struct GTY(()) alias_set_entry {
   bool is_pointer;
   /* Nonzero if is_pointer or if one of childs have has_pointer set.  */
   bool has_pointer;
+  /* Nonzero if we have a child serving as typeless storage (or are
+     such storage ourselves).  */
+  bool has_typeless_storage;
 
   /* The children of the alias set.  These are not just the immediate
      children, but, in fact, all descendants.  So, if we have:
@@ -419,7 +422,8 @@ alias_set_subset_of (alias_set_type set1
   /* Check if set1 is a subset of set2.  */
   ase2 = get_alias_set_entry (set2);
   if (ase2 != 0
-      && (ase2->has_zero_child
+      && (ase2->has_typeless_storage
+	  || ase2->has_zero_child
 	  || (ase2->children && ase2->children->get (set1))))
     return true;
 
@@ -480,7 +484,8 @@ alias_sets_conflict_p (alias_set_type se
   /* See if the first alias set is a subset of the second.  */
   ase1 = get_alias_set_entry (set1);
   if (ase1 != 0
-      && ase1->children && ase1->children->get (set2))
+      && (ase1->has_typeless_storage
+	  || (ase1->children && ase1->children->get (set2))))
     {
       ++alias_stats.num_dag;
       return 1;
@@ -489,7 +494,8 @@ alias_sets_conflict_p (alias_set_type se
   /* Now do the same, but with the alias sets reversed.  */
   ase2 = get_alias_set_entry (set2);
   if (ase2 != 0
-      && ase2->children && ase2->children->get (set1))
+      && (ase2->has_typeless_storage
+	  || (ase2->children && ase2->children->get (set1))))
     {
       ++alias_stats.num_dag;
       return 1;
@@ -825,6 +831,7 @@ init_alias_set_entry (alias_set_type set
   ase->has_zero_child = false;
   ase->is_pointer = false;
   ase->has_pointer = false;
+  ase->has_typeless_storage = false;
   gcc_checking_assert (!get_alias_set_entry (set));
   (*alias_sets)[set] = ase;
   return ase;
@@ -955,6 +962,7 @@ get_alias_set (tree t)
      Just be pragmatic here and make sure the array and its element
      type get the same alias set assigned.  */
   else if (TREE_CODE (t) == ARRAY_TYPE
+	   && ! TYPE_TYPELESS_STORAGE (t)
 	   && (!TYPE_NONALIASED_COMPONENT (t)
 	       || TYPE_STRUCTURAL_EQUALITY_P (t)))
     set = get_alias_set (TREE_TYPE (t));
@@ -1094,6 +1102,15 @@ get_alias_set (tree t)
 
   TYPE_ALIAS_SET (t) = set;
 
+  if (TREE_CODE (t) == ARRAY_TYPE
+      && TYPE_TYPELESS_STORAGE (t))
+    {
+      alias_set_entry *ase = get_alias_set_entry (set);
+      if (!ase)
+	ase = init_alias_set_entry (set);
+      ase->has_typeless_storage = true;
+    }
+
   /* If this is an aggregate type or a complex type, we must record any
      component aliasing information.  */
   if (AGGREGATE_TYPE_P (t) || TREE_CODE (t) == COMPLEX_TYPE)
@@ -1173,6 +1190,8 @@ record_alias_subset (alias_set_type supe
 	    superset_entry->has_zero_child = true;
           if (subset_entry->has_pointer)
 	    superset_entry->has_pointer = true;
+	  if (subset_entry->has_typeless_storage)
+	    superset_entry->has_typeless_storage = true;
 
 	  if (subset_entry->children)
 	    {
Index: gcc/cp/tree.c
===================================================================
--- gcc/cp/tree.c	(revision 246803)
+++ gcc/cp/tree.c	(working copy)
@@ -938,6 +938,11 @@ build_cplus_array_type (tree elt_type, t
   else
     {
       t = build_array_type (elt_type, index_type);
+      if (elt_type == unsigned_char_type_node
+	  || (TREE_CODE (elt_type) == ENUMERAL_TYPE
+	      && TYPE_CONTEXT (elt_type) == std_node
+	      && !strcmp ("byte", TYPE_NAME_STRING (elt_type))))
+	TYPE_TYPELESS_STORAGE (t) = 1;
     }
 
   /* Now check whether we already have this array variant.  */
Index: gcc/lto/lto.c
===================================================================
--- gcc/lto/lto.c	(revision 246803)
+++ gcc/lto/lto.c	(working copy)
@@ -1161,7 +1161,10 @@ compare_tree_sccs_1 (tree t1, tree t2, t
 	  compare_values (TYPE_FINAL_P);
 	}
       else if (code == ARRAY_TYPE)
-	compare_values (TYPE_NONALIASED_COMPONENT);
+	{
+	  compare_values (TYPE_NONALIASED_COMPONENT);
+	  compare_values (TYPE_TYPELESS_STORAGE);
+	}
       compare_values (TYPE_PACKED);
       compare_values (TYPE_RESTRICT);
       compare_values (TYPE_USER_ALIGN);
Index: gcc/lto-streamer-out.c
===================================================================
--- gcc/lto-streamer-out.c	(revision 246803)
+++ gcc/lto-streamer-out.c	(working copy)
@@ -1142,7 +1142,10 @@ hash_tree (struct streamer_tree_cache_d
 	  hstate.add_flag (TYPE_FINAL_P (t));
 	}
       else if (code == ARRAY_TYPE)
-	hstate.add_flag (TYPE_NONALIASED_COMPONENT (t));
+	{
+	  hstate.add_flag (TYPE_NONALIASED_COMPONENT (t));
+	  hstate.add_flag (TYPE_TYPELESS_STORAGE (t));
+	}
       hstate.commit_flag ();
       hstate.add_int (TYPE_PRECISION (t));
       hstate.add_int (TYPE_ALIGN (t));
Index: gcc/testsuite/g++.dg/torture/pr79671.C
===================================================================
--- gcc/testsuite/g++.dg/torture/pr79671.C	(nonexistent)
+++ gcc/testsuite/g++.dg/torture/pr79671.C	(working copy)
@@ -0,0 +1,25 @@
+// { dg-do run }
+
+void *operator new(__SIZE_TYPE__, void *p2) { return p2; }
+struct B { B(int i_) : i(i_) {} int i; };
+struct X
+{
+  unsigned char buf[sizeof (B)];
+};
+
+int __attribute__((noinline)) foo()
+{
+  X x, y;
+  new (&x) B (0);
+  y = x;
+  B *q = reinterpret_cast <B *>(&y);
+  asm volatile ("" : "=r" (q) : "r" (q));
+  return q->i;
+}
+
+int main()
+{
+  if (foo() != 0)
+    __builtin_abort ();
+  return 0;
+}
Index: gcc/tree-core.h
===================================================================
--- gcc/tree-core.h	(revision 246803)
+++ gcc/tree-core.h	(working copy)
@@ -1511,7 +1511,9 @@ struct GTY(()) tree_type_common {
      so we need to store the value 32 (not 31, as we need the zero
      as well), hence six bits.  */
   unsigned align : 6;
-  unsigned spare : 25;
+  unsigned typeless_storage : 1;
+  unsigned spare : 24;
+
   alias_set_type alias_set;
   tree pointer_to;
   tree reference_to;
Index: gcc/tree-ssa-structalias.c
===================================================================
--- gcc/tree-ssa-structalias.c	(revision 246803)
+++ gcc/tree-ssa-structalias.c	(working copy)
@@ -4944,14 +4944,14 @@ find_func_aliases (struct function *fn,
 	    make_escape_constraint (build_fold_addr_expr (op));
 
 	  /* The asm may read global memory, so outputs may point to
-	     any global memory.  */
+	     any global or escaped memory.  */
 	  if (op)
 	    {
 	      auto_vec<ce_s, 2> lhsc;
 	      struct constraint_expr rhsc, *lhsp;
 	      unsigned j;
 	      get_constraint_for (op, &lhsc);
-	      rhsc.var = nonlocal_id;
+	      rhsc.var = escaped_id;
 	      rhsc.offset = 0;
 	      rhsc.type = SCALAR;
 	      FOR_EACH_VEC_ELT (lhsc, j, lhsp)
Index: gcc/tree-streamer-in.c
===================================================================
--- gcc/tree-streamer-in.c	(revision 246803)
+++ gcc/tree-streamer-in.c	(working copy)
@@ -375,7 +375,10 @@ unpack_ts_type_common_value_fields (stru
       TYPE_FINAL_P (expr) = (unsigned) bp_unpack_value (bp, 1);
     }
   else if (TREE_CODE (expr) == ARRAY_TYPE)
-    TYPE_NONALIASED_COMPONENT (expr) = (unsigned) bp_unpack_value (bp, 1);
+    {
+      TYPE_NONALIASED_COMPONENT (expr) = (unsigned) bp_unpack_value (bp, 1);
+      TYPE_TYPELESS_STORAGE (expr) = (unsigned) bp_unpack_value (bp, 1);
+    }
   TYPE_PRECISION (expr) = bp_unpack_var_len_unsigned (bp);
   SET_TYPE_ALIGN (expr, bp_unpack_var_len_unsigned (bp));
 #ifdef ACCEL_COMPILER
Index: gcc/tree-streamer-out.c
===================================================================
--- gcc/tree-streamer-out.c	(revision 246803)
+++ gcc/tree-streamer-out.c	(working copy)
@@ -327,7 +327,10 @@ pack_ts_type_common_value_fields (struct
       bp_pack_value (bp, TYPE_FINAL_P (expr), 1);
     }
   else if (TREE_CODE (expr) == ARRAY_TYPE)
-    bp_pack_value (bp, TYPE_NONALIASED_COMPONENT (expr), 1);
+    {
+      bp_pack_value (bp, TYPE_NONALIASED_COMPONENT (expr), 1);
+      bp_pack_value (bp, TYPE_TYPELESS_STORAGE (expr), 1);
+    }
   bp_pack_var_len_unsigned (bp, TYPE_PRECISION (expr));
   bp_pack_var_len_unsigned (bp, TYPE_ALIGN (expr));
 }
Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 246803)
+++ gcc/tree.h	(working copy)
@@ -2035,6 +2035,9 @@ extern machine_mode element_mode (const_
 #define TYPE_NONALIASED_COMPONENT(NODE) \
   (ARRAY_TYPE_CHECK (NODE)->type_common.transparent_aggr_flag)
 
+#define TYPE_TYPELESS_STORAGE(NODE) \
+  (ARRAY_TYPE_CHECK (NODE)->type_common.typeless_storage)
+
 /* Indicated that objects of this type should be laid out in as
    compact a way as possible.  */
 #define TYPE_PACKED(NODE) (TYPE_CHECK (NODE)->base.u.bits.packed_flag)

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Jason Merrill]

On Mon, Apr 10, 2017 at 8:50 AM, Richard Biener <rguenther@suse.de> wrote:
> So here's my variant of a fix.  I constrained the new flag
> TYPE_TYPELESS_STORAGE to arrays and thus hope my localized fix in
> build_cplus_array_type will suffice (if I have time I'll play with
> template instantiation).

Looks good.  I would expect this to just work with templates, but it
should of course be tested.  In particular, std::aligned_storage needs
to work.

>         * tree.c (build_cplus_array_type): Set TYPE_TYPELESS_STORAGE
>         for arrays of unsigned char or std::byte.

I think it would be good to have a flag to select whether these
semantics apply to any char variant and std::byte, only unsigned char
and std::byte, or only std::byte.

Jason

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Richard Biener]

On Mon, 10 Apr 2017, Jason Merrill wrote:

> On Mon, Apr 10, 2017 at 8:50 AM, Richard Biener <rguenther@suse.de> wrote:
> > So here's my variant of a fix.  I constrained the new flag
> > TYPE_TYPELESS_STORAGE to arrays and thus hope my localized fix in
> > build_cplus_array_type will suffice (if I have time I'll play with
> > template instantiation).
> 
> Looks good.  I would expect this to just work with templates, but it
> should of course be tested.  In particular, std::aligned_storage needs
> to work.

The libstdc++ testsuite passes... (of course it did before as well).
I'll experiment with some trivial bits tomorrow.

> >         * tree.c (build_cplus_array_type): Set TYPE_TYPELESS_STORAGE
> >         for arrays of unsigned char or std::byte.
> 
> I think it would be good to have a flag to select whether these
> semantics apply to any char variant and std::byte, only unsigned char
> and std::byte, or only std::byte.

Any suggestion?  Not sure we really need it (I'm hesitant to write
all the testcases to verify it actually works).

Meanwhile re-testing with the following hunk to fix some LTO fallout.

Richard.

Index: gcc/tree.h
===================================================================
--- gcc/tree.h  (revision 246808)
+++ gcc/tree.h  (working copy)
@@ -4914,7 +4917,7 @@ inline bool
 canonical_type_used_p (const_tree t)
 {
   return !(POINTER_TYPE_P (t)
-          || TREE_CODE (t) == ARRAY_TYPE
+          || (TREE_CODE (t) == ARRAY_TYPE && ! TYPE_TYPELESS_STORAGE (t))
           || TREE_CODE (t) == VECTOR_TYPE);
 }
 

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Jason Merrill]

On Mon, Apr 10, 2017 at 11:30 AM, Richard Biener <rguenther@suse.de> wrote:
> On Mon, 10 Apr 2017, Jason Merrill wrote:
>> On Mon, Apr 10, 2017 at 8:50 AM, Richard Biener <rguenther@suse.de> wrote:
>> >         * tree.c (build_cplus_array_type): Set TYPE_TYPELESS_STORAGE
>> >         for arrays of unsigned char or std::byte.
>>
>> I think it would be good to have a flag to select whether these
>> semantics apply to any char variant and std::byte, only unsigned char
>> and std::byte, or only std::byte.
>
> Any suggestion?  Not sure we really need it (I'm hesitant to write
> all the testcases to verify it actually works).

Well, there's existing code that uses plain char (e.g. boost) that I
want to support.  If there's a significant optimization penalty for
allowing that, we probably also want to be able to limit the impact.
If there isn't much penalty, let's just support all char variants.

Jason

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Richard Biener]

On Mon, 10 Apr 2017, Jason Merrill wrote:

> On Mon, Apr 10, 2017 at 11:30 AM, Richard Biener <rguenther@suse.de> wrote:
> > On Mon, 10 Apr 2017, Jason Merrill wrote:
> >> On Mon, Apr 10, 2017 at 8:50 AM, Richard Biener <rguenther@suse.de> wrote:
> >> >         * tree.c (build_cplus_array_type): Set TYPE_TYPELESS_STORAGE
> >> >         for arrays of unsigned char or std::byte.
> >>
> >> I think it would be good to have a flag to select whether these
> >> semantics apply to any char variant and std::byte, only unsigned char
> >> and std::byte, or only std::byte.
> >
> > Any suggestion?  Not sure we really need it (I'm hesitant to write
> > all the testcases to verify it actually works).
> 
> Well, there's existing code that uses plain char (e.g. boost) that I
> want to support.  If there's a significant optimization penalty for
> allowing that, we probably also want to be able to limit the impact.
> If there isn't much penalty, let's just support all char variants.

I haven't assessed the penalty involved but it can't be worse than
the situation we had in GCC 6.  So I think it's reasonable to support
all char variants for now.  One could add some instrumenting to
alias_set_subset_of / alias_sets_conflict_p but it would only yield
an upper bound on the number of failed queries (TBAA is a quite early
out before PTA info is used for example).

The following variant -- added missing

Index: gcc/cp/tree.c
===================================================================
--- gcc/cp/tree.c       (revision 246832)
+++ gcc/cp/tree.c       (working copy)
@@ -972,6 +979,7 @@ build_cplus_array_type (tree elt_type, t
                 as it will overwrite alignment etc. of all variants.  */
              TYPE_SIZE (t) = TYPE_SIZE (m);
              TYPE_SIZE_UNIT (t) = TYPE_SIZE_UNIT (m);
+             TYPE_TYPELESS_STORAGE (t) = TYPE_TYPELESS_STORAGE (m);
            }
 
          TYPE_MAIN_VARIANT (t) = m;

that caused LTO bootstrap to fail and removed the tree-ssa-structalias.c
change (committed separately) [LTO] bootstrapped and tested ok on 
x86_64-unknown-linux-gnu.

I've tested some template examples and they seem to work fine.

Ok for trunk?

Disclaimer: there might still be an issue with cross-language LTO
support, but it might at most result in TYPE_TYPELESS_STORAGE
getting lost.  Trying to craft a testcase to verify my theory.

Thanks,
Richard.

2017-04-11  Richard Biener  <rguenther@suse.de>

	PR c++/79671
	cp/
	* tree.c (build_cplus_array_type): Set TYPE_TYPELESS_STORAGE
	for arrays of unsigned char or std::byte.

	* tree-core.h (tree_type_common): Add typeless_storage flag.
	* tree.h (TYPE_TYPELESS_STORAGE): New macro.
	(canonical_type_used_p): Add arrays with TYPE_TYPELESS_STORAGE.
	* alias.c (alias_set_entry): Add has_typeless_storage member.
	(alias_set_subset_of): Handle it.
	(alias_sets_conflict_p): Likewise.
	(init_alias_set_entry): Initialize it.
	(get_alias_set): For ARRAY_TYPEs handle TYPE_TYPELESS_STORAGE.
	(record_alias_subset): Likewise.
	* lto-streamer-out.c (hash_tree): Hash TYPE_TYPELESS_STORAGE.
	* tree-streamer-in.c (unpack_ts_type_common_value_fields): Stream
	TYPE_TYPELESS_STORAGE.
	* tree-streamer-out.c (pack_ts_type_common_value_fields): Likewise.

	lto/
	* lto.c (compare_tree_sccs_1): Compare TYPE_TYPELESS_STORAGE.

	* g++.dg/torture/pr79671.C: New testcase.

Index: gcc/alias.c
===================================================================
--- gcc/alias.c	(revision 246832)
+++ gcc/alias.c	(working copy)
@@ -136,6 +136,9 @@ struct GTY(()) alias_set_entry {
   bool is_pointer;
   /* Nonzero if is_pointer or if one of childs have has_pointer set.  */
   bool has_pointer;
+  /* Nonzero if we have a child serving as typeless storage (or are
+     such storage ourselves).  */
+  bool has_typeless_storage;
 
   /* The children of the alias set.  These are not just the immediate
      children, but, in fact, all descendants.  So, if we have:
@@ -419,7 +422,8 @@ alias_set_subset_of (alias_set_type set1
   /* Check if set1 is a subset of set2.  */
   ase2 = get_alias_set_entry (set2);
   if (ase2 != 0
-      && (ase2->has_zero_child
+      && (ase2->has_typeless_storage
+	  || ase2->has_zero_child
 	  || (ase2->children && ase2->children->get (set1))))
     return true;
 
@@ -480,7 +484,8 @@ alias_sets_conflict_p (alias_set_type se
   /* See if the first alias set is a subset of the second.  */
   ase1 = get_alias_set_entry (set1);
   if (ase1 != 0
-      && ase1->children && ase1->children->get (set2))
+      && (ase1->has_typeless_storage
+	  || (ase1->children && ase1->children->get (set2))))
     {
       ++alias_stats.num_dag;
       return 1;
@@ -489,7 +494,8 @@ alias_sets_conflict_p (alias_set_type se
   /* Now do the same, but with the alias sets reversed.  */
   ase2 = get_alias_set_entry (set2);
   if (ase2 != 0
-      && ase2->children && ase2->children->get (set1))
+      && (ase2->has_typeless_storage
+	  || (ase2->children && ase2->children->get (set1))))
     {
       ++alias_stats.num_dag;
       return 1;
@@ -825,6 +831,7 @@ init_alias_set_entry (alias_set_type set
   ase->has_zero_child = false;
   ase->is_pointer = false;
   ase->has_pointer = false;
+  ase->has_typeless_storage = false;
   gcc_checking_assert (!get_alias_set_entry (set));
   (*alias_sets)[set] = ase;
   return ase;
@@ -955,6 +962,7 @@ get_alias_set (tree t)
      Just be pragmatic here and make sure the array and its element
      type get the same alias set assigned.  */
   else if (TREE_CODE (t) == ARRAY_TYPE
+	   && ! TYPE_TYPELESS_STORAGE (t)
 	   && (!TYPE_NONALIASED_COMPONENT (t)
 	       || TYPE_STRUCTURAL_EQUALITY_P (t)))
     set = get_alias_set (TREE_TYPE (t));
@@ -1094,6 +1102,15 @@ get_alias_set (tree t)
 
   TYPE_ALIAS_SET (t) = set;
 
+  if (TREE_CODE (t) == ARRAY_TYPE
+      && TYPE_TYPELESS_STORAGE (t))
+    {
+      alias_set_entry *ase = get_alias_set_entry (set);
+      if (!ase)
+	ase = init_alias_set_entry (set);
+      ase->has_typeless_storage = true;
+    }
+
   /* If this is an aggregate type or a complex type, we must record any
      component aliasing information.  */
   if (AGGREGATE_TYPE_P (t) || TREE_CODE (t) == COMPLEX_TYPE)
@@ -1173,6 +1190,8 @@ record_alias_subset (alias_set_type supe
 	    superset_entry->has_zero_child = true;
           if (subset_entry->has_pointer)
 	    superset_entry->has_pointer = true;
+	  if (subset_entry->has_typeless_storage)
+	    superset_entry->has_typeless_storage = true;
 
 	  if (subset_entry->children)
 	    {
Index: gcc/cp/tree.c
===================================================================
--- gcc/cp/tree.c	(revision 246832)
+++ gcc/cp/tree.c	(working copy)
@@ -949,6 +949,13 @@ build_cplus_array_type (tree elt_type, t
   else
     {
       t = build_array_type (elt_type, index_type);
+      if (elt_type == unsigned_char_type_node
+	  || elt_type == signed_char_type_node
+	  || elt_type == char_type_node
+	  || (TREE_CODE (elt_type) == ENUMERAL_TYPE
+	      && TYPE_CONTEXT (elt_type) == std_node
+	      && !strcmp ("byte", TYPE_NAME_STRING (elt_type))))
+	TYPE_TYPELESS_STORAGE (t) = 1;
     }
 
   /* Now check whether we already have this array variant.  */
@@ -972,6 +979,7 @@ build_cplus_array_type (tree elt_type, t
 		 as it will overwrite alignment etc. of all variants.  */
 	      TYPE_SIZE (t) = TYPE_SIZE (m);
 	      TYPE_SIZE_UNIT (t) = TYPE_SIZE_UNIT (m);
+	      TYPE_TYPELESS_STORAGE (t) = TYPE_TYPELESS_STORAGE (m);
 	    }
 
 	  TYPE_MAIN_VARIANT (t) = m;
Index: gcc/lto/lto.c
===================================================================
--- gcc/lto/lto.c	(revision 246832)
+++ gcc/lto/lto.c	(working copy)
@@ -1161,7 +1161,10 @@ compare_tree_sccs_1 (tree t1, tree t2, t
 	  compare_values (TYPE_FINAL_P);
 	}
       else if (code == ARRAY_TYPE)
-	compare_values (TYPE_NONALIASED_COMPONENT);
+	{
+	  compare_values (TYPE_NONALIASED_COMPONENT);
+	  compare_values (TYPE_TYPELESS_STORAGE);
+	}
       compare_values (TYPE_PACKED);
       compare_values (TYPE_RESTRICT);
       compare_values (TYPE_USER_ALIGN);
Index: gcc/lto-streamer-out.c
===================================================================
--- gcc/lto-streamer-out.c	(revision 246832)
+++ gcc/lto-streamer-out.c	(working copy)
@@ -1142,7 +1142,10 @@ hash_tree (struct streamer_tree_cache_d
 	  hstate.add_flag (TYPE_FINAL_P (t));
 	}
       else if (code == ARRAY_TYPE)
-	hstate.add_flag (TYPE_NONALIASED_COMPONENT (t));
+	{
+	  hstate.add_flag (TYPE_NONALIASED_COMPONENT (t));
+	  hstate.add_flag (TYPE_TYPELESS_STORAGE (t));
+	}
       hstate.commit_flag ();
       hstate.add_int (TYPE_PRECISION (t));
       hstate.add_int (TYPE_ALIGN (t));
Index: gcc/testsuite/g++.dg/torture/pr79671.C
===================================================================
--- gcc/testsuite/g++.dg/torture/pr79671.C	(nonexistent)
+++ gcc/testsuite/g++.dg/torture/pr79671.C	(working copy)
@@ -0,0 +1,25 @@
+// { dg-do run }
+
+void *operator new(__SIZE_TYPE__, void *p2) { return p2; }
+struct B { B(int i_) : i(i_) {} int i; };
+struct X
+{
+  unsigned char buf[sizeof (B)];
+};
+
+int __attribute__((noinline)) foo()
+{
+  X x alignas(B), y alignas(B);
+  new (&x) B (0);
+  y = x;
+  B *q = reinterpret_cast <B *>(&y);
+  asm volatile ("" : "=r" (q) : "r" (q));
+  return q->i;
+}
+
+int main()
+{
+  if (foo() != 0)
+    __builtin_abort ();
+  return 0;
+}
Index: gcc/tree-core.h
===================================================================
--- gcc/tree-core.h	(revision 246832)
+++ gcc/tree-core.h	(working copy)
@@ -1511,7 +1511,9 @@ struct GTY(()) tree_type_common {
      so we need to store the value 32 (not 31, as we need the zero
      as well), hence six bits.  */
   unsigned align : 6;
-  unsigned spare : 25;
+  unsigned typeless_storage : 1;
+  unsigned spare : 24;
+
   alias_set_type alias_set;
   tree pointer_to;
   tree reference_to;
Index: gcc/tree-streamer-in.c
===================================================================
--- gcc/tree-streamer-in.c	(revision 246832)
+++ gcc/tree-streamer-in.c	(working copy)
@@ -375,7 +375,10 @@ unpack_ts_type_common_value_fields (stru
       TYPE_FINAL_P (expr) = (unsigned) bp_unpack_value (bp, 1);
     }
   else if (TREE_CODE (expr) == ARRAY_TYPE)
-    TYPE_NONALIASED_COMPONENT (expr) = (unsigned) bp_unpack_value (bp, 1);
+    {
+      TYPE_NONALIASED_COMPONENT (expr) = (unsigned) bp_unpack_value (bp, 1);
+      TYPE_TYPELESS_STORAGE (expr) = (unsigned) bp_unpack_value (bp, 1);
+    }
   TYPE_PRECISION (expr) = bp_unpack_var_len_unsigned (bp);
   SET_TYPE_ALIGN (expr, bp_unpack_var_len_unsigned (bp));
 #ifdef ACCEL_COMPILER
Index: gcc/tree-streamer-out.c
===================================================================
--- gcc/tree-streamer-out.c	(revision 246832)
+++ gcc/tree-streamer-out.c	(working copy)
@@ -327,7 +327,10 @@ pack_ts_type_common_value_fields (struct
       bp_pack_value (bp, TYPE_FINAL_P (expr), 1);
     }
   else if (TREE_CODE (expr) == ARRAY_TYPE)
-    bp_pack_value (bp, TYPE_NONALIASED_COMPONENT (expr), 1);
+    {
+      bp_pack_value (bp, TYPE_NONALIASED_COMPONENT (expr), 1);
+      bp_pack_value (bp, TYPE_TYPELESS_STORAGE (expr), 1);
+    }
   bp_pack_var_len_unsigned (bp, TYPE_PRECISION (expr));
   bp_pack_var_len_unsigned (bp, TYPE_ALIGN (expr));
 }
Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 246832)
+++ gcc/tree.h	(working copy)
@@ -2035,6 +2035,9 @@ extern machine_mode element_mode (const_
 #define TYPE_NONALIASED_COMPONENT(NODE) \
   (ARRAY_TYPE_CHECK (NODE)->type_common.transparent_aggr_flag)
 
+#define TYPE_TYPELESS_STORAGE(NODE) \
+  (ARRAY_TYPE_CHECK (NODE)->type_common.typeless_storage)
+
 /* Indicated that objects of this type should be laid out in as
    compact a way as possible.  */
 #define TYPE_PACKED(NODE) (TYPE_CHECK (NODE)->base.u.bits.packed_flag)
@@ -4914,7 +4917,7 @@ inline bool
 canonical_type_used_p (const_tree t)
 {
   return !(POINTER_TYPE_P (t)
-	   || TREE_CODE (t) == ARRAY_TYPE
+	   || (TREE_CODE (t) == ARRAY_TYPE && ! TYPE_TYPELESS_STORAGE (t))
 	   || TREE_CODE (t) == VECTOR_TYPE);
 }
 

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Richard Biener]

[-- Attachment #1: Type: text/plain, Size: 5857 bytes --]

On Tue, 11 Apr 2017, Richard Biener wrote:

> On Mon, 10 Apr 2017, Jason Merrill wrote:
> 
> > On Mon, Apr 10, 2017 at 11:30 AM, Richard Biener <rguenther@suse.de> wrote:
> > > On Mon, 10 Apr 2017, Jason Merrill wrote:
> > >> On Mon, Apr 10, 2017 at 8:50 AM, Richard Biener <rguenther@suse.de> wrote:
> > >> >         * tree.c (build_cplus_array_type): Set TYPE_TYPELESS_STORAGE
> > >> >         for arrays of unsigned char or std::byte.
> > >>
> > >> I think it would be good to have a flag to select whether these
> > >> semantics apply to any char variant and std::byte, only unsigned char
> > >> and std::byte, or only std::byte.
> > >
> > > Any suggestion?  Not sure we really need it (I'm hesitant to write
> > > all the testcases to verify it actually works).
> > 
> > Well, there's existing code that uses plain char (e.g. boost) that I
> > want to support.  If there's a significant optimization penalty for
> > allowing that, we probably also want to be able to limit the impact.
> > If there isn't much penalty, let's just support all char variants.
> 
> I haven't assessed the penalty involved but it can't be worse than
> the situation we had in GCC 6.  So I think it's reasonable to support
> all char variants for now.  One could add some instrumenting to
> alias_set_subset_of / alias_sets_conflict_p but it would only yield
> an upper bound on the number of failed queries (TBAA is a quite early
> out before PTA info is used for example).
> 
> The following variant -- added missing
> 
> Index: gcc/cp/tree.c
> ===================================================================
> --- gcc/cp/tree.c       (revision 246832)
> +++ gcc/cp/tree.c       (working copy)
> @@ -972,6 +979,7 @@ build_cplus_array_type (tree elt_type, t
>                  as it will overwrite alignment etc. of all variants.  */
>               TYPE_SIZE (t) = TYPE_SIZE (m);
>               TYPE_SIZE_UNIT (t) = TYPE_SIZE_UNIT (m);
> +             TYPE_TYPELESS_STORAGE (t) = TYPE_TYPELESS_STORAGE (m);
>             }
>  
>           TYPE_MAIN_VARIANT (t) = m;
> 
> that caused LTO bootstrap to fail and removed the tree-ssa-structalias.c
> change (committed separately) [LTO] bootstrapped and tested ok on 
> x86_64-unknown-linux-gnu.
> 
> I've tested some template examples and they seem to work fine.
> 
> Ok for trunk?
> 
> Disclaimer: there might still be an issue with cross-language LTO
> support, but it might at most result in TYPE_TYPELESS_STORAGE
> getting lost.  Trying to craft a testcase to verify my theory.

Not too difficult in the end (see below).  A fix (below) is to
not treat arrays with differing TYPE_TYPELESS_STORAGE as
compatible for the purpose of computing TYPE_CANONICAL (and
thus recursively structures containing them).  While they'd
still alias each other (because currently a TYPE_TYPELESS_STORAGE
member makes the whole thing effectively alias anything) this
results in warnings in case such a type is used in the interface
between C and C++ (it's also considered a ODR type).

t.C:17:17: warning: type of Β‘barΒ’ does not match original declaration 
[-Wlto-type-mismatch]
 extern "C" void bar (X *);
                 ^
t2.c:5:6: note: Β‘barΒ’ was previously declared here
 void bar (struct X *p) {}
      ^

if you add a struct X * parameter to bar().

So it's not the optimal solution here.  Another fix would be to
somehow make TYPE_TYPELESS_STORAGE "sticky" when merging canonical
types but there's not precedent in doing this kind of thing and
I'm not sure we'll get everything merged before computing alias
sets.

CCing Honza.

Richard.

2017-04-11  Richard Biener  <rguenther@suse.de>

	PR middle-end/79671
	* tree.c (gimple_canonical_types_compatible_p): Do not treat
	arrays with differing TYPE_TYPELESS_STORAGE as compatible.

	* g++.dg/lto/pr79671_0.C: New testcase.
	* g++.dg/lto/pr79671_1.c: Likewise.

Index: gcc/tree.c
===================================================================
*** gcc/tree.c	(revision 246835)
--- gcc/tree.c	(working copy)
*************** gimple_canonical_types_compatible_p (con
*** 13709,13715 ****
  						trust_type_canonical)
  	  || TYPE_STRING_FLAG (t1) != TYPE_STRING_FLAG (t2)
  	  || TYPE_REVERSE_STORAGE_ORDER (t1) != TYPE_REVERSE_STORAGE_ORDER (t2)
! 	  || TYPE_NONALIASED_COMPONENT (t1) != TYPE_NONALIASED_COMPONENT (t2))
  	return false;
        else
  	{
--- 13709,13716 ----
  						trust_type_canonical)
  	  || TYPE_STRING_FLAG (t1) != TYPE_STRING_FLAG (t2)
  	  || TYPE_REVERSE_STORAGE_ORDER (t1) != TYPE_REVERSE_STORAGE_ORDER (t2)
! 	  || TYPE_NONALIASED_COMPONENT (t1) != TYPE_NONALIASED_COMPONENT (t2)
! 	  || TYPE_TYPELESS_STORAGE (t1) != TYPE_TYPELESS_STORAGE (t2))
  	return false;
        else
  	{
Index: gcc/testsuite/g++.dg/lto/pr79671_0.C
===================================================================
*** gcc/testsuite/g++.dg/lto/pr79671_0.C	(nonexistent)
--- gcc/testsuite/g++.dg/lto/pr79671_0.C	(working copy)
***************
*** 0 ****
--- 1,26 ----
+ // { dg-lto-do run }
+ 
+ void *operator new(__SIZE_TYPE__, void *p2) { return p2; }
+ struct B { B(int i_) : i(i_) {} int i; };
+ struct X
+ {
+   unsigned char buf[sizeof (B)];
+ };
+ 
+ int __attribute__((noinline)) foo()
+ {
+   X x alignas (B), y alignas (B);
+   new (&x) B (0);
+   y = x;
+   B *q = reinterpret_cast <B *>(&y);
+   asm volatile ("" : "=r" (q) : "r" (q));
+   return q->i;
+ }
+ extern "C" void bar ();
+ int main()
+ {
+   if (foo() != 0)
+     __builtin_abort ();
+   bar ();
+   return 0;
+ }
Index: gcc/testsuite/g++.dg/lto/pr79671_1.c
===================================================================
*** gcc/testsuite/g++.dg/lto/pr79671_1.c	(nonexistent)
--- gcc/testsuite/g++.dg/lto/pr79671_1.c	(working copy)
***************
*** 0 ****
--- 1,5 ----
+ struct X
+ {
+   unsigned char buf[sizeof (int)];
+ };
+ void bar () { struct X x; *(volatile char *)x.buf = 1; }

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Richard Biener]

[-- Attachment #1: Type: text/plain, Size: 13870 bytes --]

On Tue, 11 Apr 2017, Richard Biener wrote:

> On Tue, 11 Apr 2017, Richard Biener wrote:
> 
> > On Mon, 10 Apr 2017, Jason Merrill wrote:
> > 
> > > On Mon, Apr 10, 2017 at 11:30 AM, Richard Biener <rguenther@suse.de> wrote:
> > > > On Mon, 10 Apr 2017, Jason Merrill wrote:
> > > >> On Mon, Apr 10, 2017 at 8:50 AM, Richard Biener <rguenther@suse.de> wrote:
> > > >> >         * tree.c (build_cplus_array_type): Set TYPE_TYPELESS_STORAGE
> > > >> >         for arrays of unsigned char or std::byte.
> > > >>
> > > >> I think it would be good to have a flag to select whether these
> > > >> semantics apply to any char variant and std::byte, only unsigned char
> > > >> and std::byte, or only std::byte.
> > > >
> > > > Any suggestion?  Not sure we really need it (I'm hesitant to write
> > > > all the testcases to verify it actually works).
> > > 
> > > Well, there's existing code that uses plain char (e.g. boost) that I
> > > want to support.  If there's a significant optimization penalty for
> > > allowing that, we probably also want to be able to limit the impact.
> > > If there isn't much penalty, let's just support all char variants.
> > 
> > I haven't assessed the penalty involved but it can't be worse than
> > the situation we had in GCC 6.  So I think it's reasonable to support
> > all char variants for now.  One could add some instrumenting to
> > alias_set_subset_of / alias_sets_conflict_p but it would only yield
> > an upper bound on the number of failed queries (TBAA is a quite early
> > out before PTA info is used for example).
> > 
> > The following variant -- added missing
> > 
> > Index: gcc/cp/tree.c
> > ===================================================================
> > --- gcc/cp/tree.c       (revision 246832)
> > +++ gcc/cp/tree.c       (working copy)
> > @@ -972,6 +979,7 @@ build_cplus_array_type (tree elt_type, t
> >                  as it will overwrite alignment etc. of all variants.  */
> >               TYPE_SIZE (t) = TYPE_SIZE (m);
> >               TYPE_SIZE_UNIT (t) = TYPE_SIZE_UNIT (m);
> > +             TYPE_TYPELESS_STORAGE (t) = TYPE_TYPELESS_STORAGE (m);
> >             }
> >  
> >           TYPE_MAIN_VARIANT (t) = m;
> > 
> > that caused LTO bootstrap to fail and removed the tree-ssa-structalias.c
> > change (committed separately) [LTO] bootstrapped and tested ok on 
> > x86_64-unknown-linux-gnu.
> > 
> > I've tested some template examples and they seem to work fine.
> > 
> > Ok for trunk?
> > 
> > Disclaimer: there might still be an issue with cross-language LTO
> > support, but it might at most result in TYPE_TYPELESS_STORAGE
> > getting lost.  Trying to craft a testcase to verify my theory.
> 
> Not too difficult in the end (see below).  A fix (below) is to
> not treat arrays with differing TYPE_TYPELESS_STORAGE as
> compatible for the purpose of computing TYPE_CANONICAL (and
> thus recursively structures containing them).  While they'd
> still alias each other (because currently a TYPE_TYPELESS_STORAGE
> member makes the whole thing effectively alias anything) this
> results in warnings in case such a type is used in the interface
> between C and C++ (it's also considered a ODR type).
> 
> t.C:17:17: warning: type of Β‘barΒ’ does not match original declaration 
> [-Wlto-type-mismatch]
>  extern "C" void bar (X *);
>                  ^
> t2.c:5:6: note: Β‘barΒ’ was previously declared here
>  void bar (struct X *p) {}
>       ^
> 
> if you add a struct X * parameter to bar().
> 
> So it's not the optimal solution here.  Another fix would be to
> somehow make TYPE_TYPELESS_STORAGE "sticky" when merging canonical
> types but there's not precedent in doing this kind of thing and
> I'm not sure we'll get everything merged before computing alias
> sets.
> 
> CCing Honza.

So after discussion and some more thinking we don't really benefit
(and really can't) from having different aggregates with such
members distignuishable via their alias set.  So the following
simplifies the approach and makes the above LTO bits trivial
by more following Bernds approach of assigning the types alias
set zero and instead of in the alias machinery propagate the
flag on the types.

It should also make reference_alias_ptr_type correct and it
does the flag propagation in type layout.

[LTO] bootstrap and regtest running on x86_64-unknown-linux-gnu.

The C++ bits are unchanged.

Richard.

2017-04-11  Richard Biener  <rguenther@suse.de>
	Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR middle-end/79671
	* alias.c (component_uses_parent_alias_set_from): Handle
	TYPE_TYPELESS_STORAGE.
	(get_alias_set): Likewise.
	* tree-core.h (tree_type_common): Add typeless_storage flag.
	* tree.h (TYPE_TYPELESS_STORAGE): New macro.
	* stor-layout.c (place_union_field): Set TYPE_TYPELESS_STORAGE
	for types containing members with TYPE_TYPELESS_STORAGE.
	(place_field): Likewise.
	(layout_type): Likewise for ARRAY_TYPE.
	* lto-streamer-out.c (hash_tree): Hash TYPE_TYPELESS_STORAGE.
	* tree-streamer-in.c (unpack_ts_type_common_value_fields): Stream
	TYPE_TYPELESS_STORAGE.
	* tree-streamer-out.c (pack_ts_type_common_value_fields): Likewise.

	lto/
	* lto.c (compare_tree_sccs_1): Compare TYPE_TYPELESS_STORAGE.

	cp/
	* tree.c (build_cplus_array_type): Set TYPE_TYPELESS_STORAGE
	for arrays of character or std::byte type.

	* g++.dg/torture/pr79671.C: New testcase.
	* g++.dg/lto/pr79671_0.C: Likewise.
	* g++.dg/lto/pr79671_1.c: Likewise.

Index: gcc/alias.c
===================================================================
--- gcc/alias.c	(revision 246835)
+++ gcc/alias.c	(working copy)
@@ -613,6 +619,10 @@ component_uses_parent_alias_set_from (co
 {
   const_tree found = NULL_TREE;
 
+  if (AGGREGATE_TYPE_P (TREE_TYPE (t))
+      && TYPE_TYPELESS_STORAGE (TREE_TYPE (t)))
+    return const_cast <tree> (t);
+
   while (handled_component_p (t))
     {
       switch (TREE_CODE (t))
@@ -883,6 +894,10 @@ get_alias_set (tree t)
      variant.  */
   t = TYPE_MAIN_VARIANT (t);
 
+  if (AGGREGATE_TYPE_P (t)
+      && TYPE_TYPELESS_STORAGE (t))
+    return 0;
+
   /* Always use the canonical type as well.  If this is a type that
      requires structural comparisons to identify compatible types
      use alias set zero.  */
Index: gcc/cp/tree.c
===================================================================
--- gcc/cp/tree.c	(revision 246835)
+++ gcc/cp/tree.c	(working copy)
@@ -949,6 +949,13 @@ build_cplus_array_type (tree elt_type, t
   else
     {
       t = build_array_type (elt_type, index_type);
+      if (elt_type == unsigned_char_type_node
+	  || elt_type == signed_char_type_node
+	  || elt_type == char_type_node
+	  || (TREE_CODE (elt_type) == ENUMERAL_TYPE
+	      && TYPE_CONTEXT (elt_type) == std_node
+	      && !strcmp ("byte", TYPE_NAME_STRING (elt_type))))
+	TYPE_TYPELESS_STORAGE (t) = 1;
     }
 
   /* Now check whether we already have this array variant.  */
@@ -972,6 +979,7 @@ build_cplus_array_type (tree elt_type, t
 		 as it will overwrite alignment etc. of all variants.  */
 	      TYPE_SIZE (t) = TYPE_SIZE (m);
 	      TYPE_SIZE_UNIT (t) = TYPE_SIZE_UNIT (m);
+	      TYPE_TYPELESS_STORAGE (t) = TYPE_TYPELESS_STORAGE (m);
 	    }
 
 	  TYPE_MAIN_VARIANT (t) = m;
Index: gcc/lto/lto.c
===================================================================
--- gcc/lto/lto.c	(revision 246835)
+++ gcc/lto/lto.c	(working copy)
@@ -1162,6 +1162,8 @@ compare_tree_sccs_1 (tree t1, tree t2, t
 	}
       else if (code == ARRAY_TYPE)
 	compare_values (TYPE_NONALIASED_COMPONENT);
+      if (AGGREGATE_TYPE_P (t1))
+	compare_values (TYPE_TYPELESS_STORAGE);
       compare_values (TYPE_PACKED);
       compare_values (TYPE_RESTRICT);
       compare_values (TYPE_USER_ALIGN);
Index: gcc/lto-streamer-out.c
===================================================================
--- gcc/lto-streamer-out.c	(revision 246835)
+++ gcc/lto-streamer-out.c	(working copy)
@@ -1143,6 +1143,8 @@ hash_tree (struct streamer_tree_cache_d
 	}
       else if (code == ARRAY_TYPE)
 	hstate.add_flag (TYPE_NONALIASED_COMPONENT (t));
+      if (AGGREGATE_TYPE_P (t))
+	hstate.add_flag (TYPE_TYPELESS_STORAGE (t));
       hstate.commit_flag ();
       hstate.add_int (TYPE_PRECISION (t));
       hstate.add_int (TYPE_ALIGN (t));
Index: gcc/stor-layout.c
===================================================================
--- gcc/stor-layout.c	(revision 246835)
+++ gcc/stor-layout.c	(working copy)
@@ -1091,6 +1091,10 @@ place_union_field (record_layout_info rl
   if (TREE_CODE (TREE_TYPE (field)) == ERROR_MARK)
     return;
 
+  if (AGGREGATE_TYPE_P (TREE_TYPE (field))
+      && TYPE_TYPELESS_STORAGE (TREE_TYPE (field)))
+    TYPE_TYPELESS_STORAGE (rli->t) = 1;
+
   /* We assume the union's size will be a multiple of a byte so we don't
      bother with BITPOS.  */
   if (TREE_CODE (rli->t) == UNION_TYPE)
@@ -1168,6 +1172,10 @@ place_field (record_layout_info rli, tre
       return;
     }
 
+  if (AGGREGATE_TYPE_P (type)
+      && TYPE_TYPELESS_STORAGE (type))
+    TYPE_TYPELESS_STORAGE (rli->t) = 1;
+
   /* Work out the known alignment so far.  Note that A & (-A) is the
      value of the least-significant bit in A that is one.  */
   if (! integer_zerop (rli->bitpos))
@@ -2340,6 +2348,8 @@ layout_type (tree type)
 		SET_TYPE_MODE (type, BLKmode);
 	      }
 	  }
+	if (AGGREGATE_TYPE_P (element))
+	  TYPE_TYPELESS_STORAGE (type) = TYPE_TYPELESS_STORAGE (element);
 	/* When the element size is constant, check that it is at least as
 	   large as the element alignment.  */
 	if (TYPE_SIZE_UNIT (element)
Index: gcc/testsuite/g++.dg/lto/pr79671_0.C
===================================================================
--- gcc/testsuite/g++.dg/lto/pr79671_0.C	(nonexistent)
+++ gcc/testsuite/g++.dg/lto/pr79671_0.C	(working copy)
@@ -0,0 +1,26 @@
+// { dg-lto-do run }
+
+void *operator new(__SIZE_TYPE__, void *p2) { return p2; }
+struct B { B(int i_) : i(i_) {} int i; };
+struct X
+{
+  unsigned char buf[sizeof (B)];
+};
+
+int __attribute__((noinline)) foo()
+{
+  X x alignas (B), y alignas (B);
+  new (&x) B (0);
+  y = x;
+  B *q = reinterpret_cast <B *>(&y);
+  asm volatile ("" : "=r" (q) : "r" (q));
+  return q->i;
+}
+extern "C" void bar ();
+int main()
+{
+  if (foo() != 0)
+    __builtin_abort ();
+  bar ();
+  return 0;
+}
Index: gcc/testsuite/g++.dg/lto/pr79671_1.c
===================================================================
--- gcc/testsuite/g++.dg/lto/pr79671_1.c	(nonexistent)
+++ gcc/testsuite/g++.dg/lto/pr79671_1.c	(working copy)
@@ -0,0 +1,5 @@
+struct X
+{
+  unsigned char buf[sizeof (int)];
+};
+void bar () { struct X x; *(volatile char *)x.buf = 1; }
Index: gcc/testsuite/g++.dg/torture/pr79671.C
===================================================================
--- gcc/testsuite/g++.dg/torture/pr79671.C	(nonexistent)
+++ gcc/testsuite/g++.dg/torture/pr79671.C	(working copy)
@@ -0,0 +1,25 @@
+// { dg-do run }
+
+void *operator new(__SIZE_TYPE__, void *p2) { return p2; }
+struct B { B(int i_) : i(i_) {} int i; };
+struct X
+{
+  unsigned char buf[sizeof (B)];
+};
+
+int __attribute__((noinline)) foo()
+{
+  X x alignas(B), y alignas(B);
+  new (&x) B (0);
+  y = x;
+  B *q = reinterpret_cast <B *>(&y);
+  asm volatile ("" : "=r" (q) : "r" (q));
+  return q->i;
+}
+
+int main()
+{
+  if (foo() != 0)
+    __builtin_abort ();
+  return 0;
+}
Index: gcc/tree-core.h
===================================================================
--- gcc/tree-core.h	(revision 246835)
+++ gcc/tree-core.h	(working copy)
@@ -1511,7 +1511,9 @@ struct GTY(()) tree_type_common {
      so we need to store the value 32 (not 31, as we need the zero
      as well), hence six bits.  */
   unsigned align : 6;
-  unsigned spare : 25;
+  unsigned typeless_storage : 1;
+  unsigned spare : 24;
+
   alias_set_type alias_set;
   tree pointer_to;
   tree reference_to;
Index: gcc/tree-streamer-in.c
===================================================================
--- gcc/tree-streamer-in.c	(revision 246835)
+++ gcc/tree-streamer-in.c	(working copy)
@@ -376,6 +376,8 @@ unpack_ts_type_common_value_fields (stru
     }
   else if (TREE_CODE (expr) == ARRAY_TYPE)
     TYPE_NONALIASED_COMPONENT (expr) = (unsigned) bp_unpack_value (bp, 1);
+  if (AGGREGATE_TYPE_P (expr))
+    TYPE_TYPELESS_STORAGE (expr) = (unsigned) bp_unpack_value (bp, 1);
   TYPE_PRECISION (expr) = bp_unpack_var_len_unsigned (bp);
   SET_TYPE_ALIGN (expr, bp_unpack_var_len_unsigned (bp));
 #ifdef ACCEL_COMPILER
Index: gcc/tree-streamer-out.c
===================================================================
--- gcc/tree-streamer-out.c	(revision 246835)
+++ gcc/tree-streamer-out.c	(working copy)
@@ -328,6 +328,8 @@ pack_ts_type_common_value_fields (struct
     }
   else if (TREE_CODE (expr) == ARRAY_TYPE)
     bp_pack_value (bp, TYPE_NONALIASED_COMPONENT (expr), 1);
+  if (AGGREGATE_TYPE_P (expr))
+    bp_pack_value (bp, TYPE_TYPELESS_STORAGE (expr), 1);
   bp_pack_var_len_unsigned (bp, TYPE_PRECISION (expr));
   bp_pack_var_len_unsigned (bp, TYPE_ALIGN (expr));
 }
Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 246835)
+++ gcc/tree.h	(working copy)
@@ -2035,6 +2035,13 @@ extern machine_mode element_mode (const_
 #define TYPE_NONALIASED_COMPONENT(NODE) \
   (ARRAY_TYPE_CHECK (NODE)->type_common.transparent_aggr_flag)
 
+/* For an ARRAY_TYPE, a RECORD_TYPE, a UNION_TYPE or a QUAL_UNION_TYPE
+   whether the array is typeless storage or the type contains a member
+   with this flag set.  Such types are excempt from type-based alias
+   analysis.  */
+#define TYPE_TYPELESS_STORAGE(NODE) \
+  (TREE_CHECK4 (NODE, RECORD_TYPE, UNION_TYPE, QUAL_UNION_TYPE, ARRAY_TYPE)->type_common.typeless_storage)
+
 /* Indicated that objects of this type should be laid out in as
    compact a way as possible.  */
 #define TYPE_PACKED(NODE) (TYPE_CHECK (NODE)->base.u.bits.packed_flag)

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Jason Merrill]

On Tue, Apr 11, 2017 at 9:35 AM, Richard Biener <rguenther@suse.de> wrote:
> On Tue, 11 Apr 2017, Richard Biener wrote:
>
>> On Tue, 11 Apr 2017, Richard Biener wrote:
>>
>> > On Mon, 10 Apr 2017, Jason Merrill wrote:
>> >
>> > > On Mon, Apr 10, 2017 at 11:30 AM, Richard Biener <rguenther@suse.de> wrote:
>> > > > On Mon, 10 Apr 2017, Jason Merrill wrote:
>> > > >> On Mon, Apr 10, 2017 at 8:50 AM, Richard Biener <rguenther@suse.de> wrote:
>> > > >> >         * tree.c (build_cplus_array_type): Set TYPE_TYPELESS_STORAGE
>> > > >> >         for arrays of unsigned char or std::byte.
>> > > >>
>> > > >> I think it would be good to have a flag to select whether these
>> > > >> semantics apply to any char variant and std::byte, only unsigned char
>> > > >> and std::byte, or only std::byte.
>> > > >
>> > > > Any suggestion?  Not sure we really need it (I'm hesitant to write
>> > > > all the testcases to verify it actually works).
>> > >
>> > > Well, there's existing code that uses plain char (e.g. boost) that I
>> > > want to support.  If there's a significant optimization penalty for
>> > > allowing that, we probably also want to be able to limit the impact.
>> > > If there isn't much penalty, let's just support all char variants.
>> >
>> > I haven't assessed the penalty involved but it can't be worse than
>> > the situation we had in GCC 6.  So I think it's reasonable to support
>> > all char variants for now.  One could add some instrumenting to
>> > alias_set_subset_of / alias_sets_conflict_p but it would only yield
>> > an upper bound on the number of failed queries (TBAA is a quite early
>> > out before PTA info is used for example).
>> >
>> > The following variant -- added missing
>> >
>> > Index: gcc/cp/tree.c
>> > ===================================================================
>> > --- gcc/cp/tree.c       (revision 246832)
>> > +++ gcc/cp/tree.c       (working copy)
>> > @@ -972,6 +979,7 @@ build_cplus_array_type (tree elt_type, t
>> >                  as it will overwrite alignment etc. of all variants.  */
>> >               TYPE_SIZE (t) = TYPE_SIZE (m);
>> >               TYPE_SIZE_UNIT (t) = TYPE_SIZE_UNIT (m);
>> > +             TYPE_TYPELESS_STORAGE (t) = TYPE_TYPELESS_STORAGE (m);
>> >             }
>> >
>> >           TYPE_MAIN_VARIANT (t) = m;
>> >
>> > that caused LTO bootstrap to fail and removed the tree-ssa-structalias.c
>> > change (committed separately) [LTO] bootstrapped and tested ok on
>> > x86_64-unknown-linux-gnu.
>> >
>> > I've tested some template examples and they seem to work fine.
>> >
>> > Ok for trunk?
>> >
>> > Disclaimer: there might still be an issue with cross-language LTO
>> > support, but it might at most result in TYPE_TYPELESS_STORAGE
>> > getting lost.  Trying to craft a testcase to verify my theory.
>>
>> Not too difficult in the end (see below).  A fix (below) is to
>> not treat arrays with differing TYPE_TYPELESS_STORAGE as
>> compatible for the purpose of computing TYPE_CANONICAL (and
>> thus recursively structures containing them).  While they'd
>> still alias each other (because currently a TYPE_TYPELESS_STORAGE
>> member makes the whole thing effectively alias anything) this
>> results in warnings in case such a type is used in the interface
>> between C and C++ (it's also considered a ODR type).
>>
>> t.C:17:17: warning: type of ‘bar’ does not match original declaration
>> [-Wlto-type-mismatch]
>>  extern "C" void bar (X *);
>>                  ^
>> t2.c:5:6: note: ‘bar’ was previously declared here
>>  void bar (struct X *p) {}
>>       ^
>>
>> if you add a struct X * parameter to bar().
>>
>> So it's not the optimal solution here.  Another fix would be to
>> somehow make TYPE_TYPELESS_STORAGE "sticky" when merging canonical
>> types but there's not precedent in doing this kind of thing and
>> I'm not sure we'll get everything merged before computing alias
>> sets.
>>
>> CCing Honza.
>
> So after discussion and some more thinking we don't really benefit
> (and really can't) from having different aggregates with such
> members distignuishable via their alias set.  So the following
> simplifies the approach and makes the above LTO bits trivial
> by more following Bernds approach of assigning the types alias
> set zero and instead of in the alias machinery propagate the
> flag on the types.
>
> It should also make reference_alias_ptr_type correct and it
> does the flag propagation in type layout.
>
> [LTO] bootstrap and regtest running on x86_64-unknown-linux-gnu.
>
> The C++ bits are unchanged.

The C++ bits are OK.

Jason

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Pedro Alves]

On 04/10/2017 01:50 PM, Richard Biener wrote:

> +void *operator new(__SIZE_TYPE__, void *p2) { return p2; }
> +struct B { B(int i_) : i(i_) {} int i; };
> +struct X
> +{
> +  unsigned char buf[sizeof (B)];
> +};

Pedantically, shouldn't there be something here to enforce
X's alignment to be at least the same as B's ?

> +
> +int __attribute__((noinline)) foo()
> +{
> +  X x, y;
> +  new (&x) B (0);

Thanks,
Pedro Alves

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Richard Biener]

On Mon, 10 Apr 2017, Pedro Alves wrote:

> On 04/10/2017 01:50 PM, Richard Biener wrote:
> 
> > +void *operator new(__SIZE_TYPE__, void *p2) { return p2; }
> > +struct B { B(int i_) : i(i_) {} int i; };
> > +struct X
> > +{
> > +  unsigned char buf[sizeof (B)];
> > +};
> 
> Pedantically, shouldn't there be something here to enforce
> X's alignment to be at least the same as B's ?
> 
> > +
> > +int __attribute__((noinline)) foo()
> > +{
> > +  X x, y;
> > +  new (&x) B (0);

Right.  I'll do

  X x alignas(B), y alignas(B);

Meanwhile need to debug a LTO bootstrap failure caused by the patch.

Richard.

Re: [PATCH] Add a new type attribute always_alias (PR79671)

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 1387 bytes --]

On 04/05/17 23:02, Bernd Edlinger wrote:
> On 04/05/17 19:22, Bernd Edlinger wrote:
>> On 04/05/17 18:08, Jakub Jelinek wrote:
>>
>> Yes, exactly.  I really want to reach the deadline for gcc-7.
>> Fixing the name is certainly the most important first step,
>> and if everybody agrees on "typeless_storage", for the name
>> I can start with adjusting the name, and look into how
>> to use a spare type-flag that should be a mechanical change.
>>
>
> Jakub, I just renamed the attribute and reworked the patch
> as you suggested, reg-testing is not yet completed, but
> it looks good so far.  I also added a few more tests.
>

Aehm, sorry, actually I ran into a problem with the latest
patch version, where I tried to convert the TYPE_ATTRIBUTE
into a TYPE_FLAG here:
https://gcc.gnu.org/ml/gcc-patches/2017-04/msg00254.html

That is for instance with g++.dg/cpp1z/init-statement6.C an
internal error: "same canonical type node for different types"
happened, and I was not able to fix it immediately.

Although I would have liked it better this way, I think this
can be fixed separately, unless someone sees an obvious thinko
in the previous version.

So in the moment I restored the typeless_storage as an
ordinary TYPE_ATTRIBUTE, but at least it bootstraps and
causes no test regressions, as always with
languages=all,ada,go,obj-c++


Thanks
Bernd.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-typeless-storage-1.diff --]
[-- Type: text/x-patch; name="patch-typeless-storage-1.diff", Size: 10668 bytes --]

Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 246678)
+++ gcc/doc/extend.texi	(working copy)
@@ -6656,6 +6656,36 @@
 @option{-fstrict-aliasing}, which is on by default at @option{-O2} or
 above.
 
+@item typeless_storage
+@cindex @code{typeless_storage} type attribute
+In the context of section 6.5 paragraph 6 of the C11 standard,
+an object of this type behaves as if it has no declared type.
+In the context of section 6.5 paragraph 7 of the C11 standard,
+an object or a pointer if this type behaves as if it were a
+character type.
+This attribute is similar to the @code{may_alias} attribute,
+except that it is not restricted to pointers.
+
+Example of use:
+
+@smallexample
+typedef int __attribute__((__typeless_storage__)) int_a;
+
+int
+main (void)
+@{
+  int_a a = 0x12345678;
+  short *b = (short *) &a;
+
+  b[1] = 0;
+
+  if (a == 0x12345678)
+    abort();
+
+  exit(0);
+@}
+@end smallexample
+
 @item packed
 @cindex @code{packed} type attribute
 This attribute, attached to @code{struct} or @code{union} type
Index: gcc/alias.c
===================================================================
--- gcc/alias.c	(revision 246678)
+++ gcc/alias.c	(working copy)
@@ -879,6 +879,10 @@ get_alias_set (tree t)
       t = TREE_TYPE (t);
     }
 
+  /* Honor the typeless_storage type attribute.  */
+  if (lookup_attribute ("typeless_storage", TYPE_ATTRIBUTES (t)))
+    return 0;
+
   /* Variant qualifiers don't affect the alias set, so get the main
      variant.  */
   t = TYPE_MAIN_VARIANT (t);
@@ -1234,7 +1238,9 @@ record_component_aliases (tree type)
 		/* VECTOR_TYPE and ARRAY_TYPE share the alias set with their
 		   element type and that type has to be normalized to void *,
 		   too, in the case it is a pointer. */
-		while (!canonical_type_used_p (t) && !POINTER_TYPE_P (t))
+		while (!canonical_type_used_p (t) && !POINTER_TYPE_P (t)
+		       && !lookup_attribute ("typeless_storage",
+					     TYPE_ATTRIBUTES (t)))
 		  {
 		    gcc_checking_assert (TYPE_STRUCTURAL_EQUALITY_P (t));
 		    t = TREE_TYPE (t);
Index: gcc/tree.c
===================================================================
--- gcc/tree.c	(revision 246678)
+++ gcc/tree.c	(working copy)
@@ -8041,7 +8041,8 @@ build_pointer_type_for_mode (tree to_type, machine
 
   /* If the pointed-to type has the may_alias attribute set, force
      a TYPE_REF_CAN_ALIAS_ALL pointer to be generated.  */
-  if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
+  if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type))
+      || lookup_attribute ("typeless_storage", TYPE_ATTRIBUTES (to_type)))
     can_alias_all = true;
 
   /* In some cases, languages will have things that aren't a POINTER_TYPE
@@ -8110,7 +8111,8 @@ build_reference_type_for_mode (tree to_type, machi
 
   /* If the pointed-to type has the may_alias attribute set, force
      a TYPE_REF_CAN_ALIAS_ALL pointer to be generated.  */
-  if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type)))
+  if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (to_type))
+      || lookup_attribute ("typeless_storage", TYPE_ATTRIBUTES (to_type)))
     can_alias_all = true;
 
   /* In some cases, languages will have things that aren't a REFERENCE_TYPE
Index: gcc/c-family/c-attribs.c
===================================================================
--- gcc/c-family/c-attribs.c	(revision 246678)
+++ gcc/c-family/c-attribs.c	(working copy)
@@ -265,6 +265,7 @@ const struct attribute_spec c_common_attribute_tab
   { "nothrow",                0, 0, true,  false, false,
 			      handle_nothrow_attribute, false },
   { "may_alias",	      0, 0, false, true, false, NULL, false },
+  { "typeless_storage",       0, 0, false, true, false, NULL, false },
   { "cleanup",		      1, 1, true, false, false,
 			      handle_cleanup_attribute, false },
   { "warn_unused_result",     0, 0, false, true, true,
Index: gcc/cp/class.c
===================================================================
--- gcc/cp/class.c	(revision 246678)
+++ gcc/cp/class.c	(working copy)
@@ -2083,7 +2082,8 @@ fixup_attribute_variants (tree t)
   tree attrs = TYPE_ATTRIBUTES (t);
   unsigned align = TYPE_ALIGN (t);
   bool user_align = TYPE_USER_ALIGN (t);
-  bool may_alias = lookup_attribute ("may_alias", attrs);
+  bool may_alias = lookup_attribute ("may_alias", attrs)
+		   || lookup_attribute ("typeless_storage", attrs);
 
   if (may_alias)
     fixup_may_alias (t);
@@ -7345,6 +7346,15 @@ finish_struct_1 (tree t)
      the class or perform any other required target modifications.  */
   targetm.cxx.adjust_class_at_definition (t);
 
+  if (cxx_dialect >= cxx1z && cxx_type_contains_byte_buffer (t)
+      && !lookup_attribute ("typeless_storage", TYPE_ATTRIBUTES (t)))
+    {
+      TYPE_ATTRIBUTES (t)
+	= tree_cons (get_identifier ("typeless_storage"),
+		     NULL_TREE, TYPE_ATTRIBUTES (t));
+      fixup_attribute_variants (t);
+    }
+
   maybe_suppress_debug_info (t);
 
   if (flag_vtable_verify)
Index: gcc/cp/cp-tree.h
===================================================================
--- gcc/cp/cp-tree.h	(revision 246678)
+++ gcc/cp/cp-tree.h	(working copy)
@@ -6858,6 +6858,7 @@ extern tree finish_binary_fold_expr          (tree
 extern void require_complete_eh_spec_types	(tree, tree);
 extern void cxx_incomplete_type_diagnostic	(location_t, const_tree,
 						 const_tree, diagnostic_t);
+extern bool cxx_type_contains_byte_buffer	(tree);
 inline void
 cxx_incomplete_type_diagnostic (const_tree value, const_tree type,
 				diagnostic_t diag_kind)
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 246678)
+++ gcc/cp/decl.c	(working copy)
@@ -14081,10 +14081,15 @@ start_enum (tree name, tree enumtype, tree underly
 	  enumtype = pushtag (name, enumtype, /*tag_scope=*/ts_current);
 
 	  /* std::byte aliases anything.  */
-	  if (enumtype != error_mark_node
+	  if (cxx_dialect >= cxx1z
+	      && enumtype != error_mark_node
 	      && TYPE_CONTEXT (enumtype) == std_node
-	      && !strcmp ("byte", TYPE_NAME_STRING (enumtype)))
-	    TYPE_ALIAS_SET (enumtype) = 0;
+	      && !strcmp ("byte", TYPE_NAME_STRING (enumtype))
+	      && !lookup_attribute ("typeless_storage",
+				    TYPE_ATTRIBUTES (enumtype)))
+	    TYPE_ATTRIBUTES (enumtype)
+	      = tree_cons (get_identifier ("typeless_storage"),
+			   NULL_TREE, TYPE_ATTRIBUTES (enumtype));
 	}
       else
 	  enumtype = xref_tag (enum_type, name, /*tag_scope=*/ts_current,
Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c	(revision 246678)
+++ gcc/cp/pt.c	(working copy)
@@ -8851,9 +8851,10 @@ lookup_template_class_1 (tree d1, tree arglist, tr
       if (OVERLOAD_TYPE_P (t)
 	  && !DECL_ALIAS_TEMPLATE_P (gen_tmpl))
 	{
-	  static const char *tags[] = {"abi_tag", "may_alias"};
+	  static const char *tags[] = {"abi_tag", "may_alias",
+				       "typeless_storage"};
 
-	  for (unsigned ix = 0; ix != 2; ix++)
+	  for (unsigned ix = 0; ix < sizeof (tags) / sizeof (tags[0]); ix++)
 	    {
 	      tree attributes
 		= lookup_attribute (tags[ix], TYPE_ATTRIBUTES (template_type));
Index: gcc/cp/typeck2.c
===================================================================
--- gcc/cp/typeck2.c	(revision 246678)
+++ gcc/cp/typeck2.c	(working copy)
@@ -2234,5 +2234,29 @@ require_complete_eh_spec_types (tree fntype, tree
     }
 }
 
+/* True iff type either is or contains a byte buffer (which can be used for
+   storing any trivially copyable type).  */
+
+bool
+cxx_type_contains_byte_buffer (tree type)
+{
+  if (TREE_CODE (type) == ARRAY_TYPE
+      && (cxx_type_contains_byte_buffer (TREE_TYPE (type))
+	  || TREE_TYPE (type) == unsigned_char_type_node
+	  || (TREE_CODE (TREE_TYPE (type)) == ENUMERAL_TYPE
+	      && TYPE_CONTEXT (TREE_TYPE (type)) == std_node
+	      && !strcmp ("byte", TYPE_NAME_STRING (TREE_TYPE (type))))))
+    return true;
+
+  if (CLASS_TYPE_P (type))
+    for (tree field = next_initializable_field (TYPE_FIELDS (type));
+	 field;
+	 field = next_initializable_field (DECL_CHAIN (field)))
+      if (cxx_type_contains_byte_buffer (TREE_TYPE (field)))
+	return true;
+
+  return false;
+}
+
 \f
 #include "gt-cp-typeck2.h"
Index: gcc/testsuite/c-c++-common/attr-typeless-storage-1.c
===================================================================
--- gcc/testsuite/c-c++-common/attr-typeless-storage-1.c	(revision 0)
+++ gcc/testsuite/c-c++-common/attr-typeless-storage-1.c	(working copy)
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wall" } */
+
+typedef int T __attribute__((typeless_storage));
+
+extern T t, v;
+extern T *p;
+extern int *p;
+
+extern int *p2;
+extern T *p2;
+
+void fn1 (T);
+void fn1 (int);
+
+void fn2 (int);
+void fn2 (T);
+
+/* Ensure that the composite types have typeless_storage.  */
+void
+f (long *i)
+{
+  *i = *(__typeof (*p) *) &p;
+  asm ("" : : "r" (*p));
+  *i = *(__typeof (*p2) *) &p2;
+  asm ("" : : "r" (*p2));
+  t = v;
+  asm ("" : : "r" (t));
+}
Index: gcc/testsuite/c-c++-common/attr-typeless-storage-2.c
===================================================================
--- gcc/testsuite/c-c++-common/attr-typeless-storage-2.c	(revision 0)
+++ gcc/testsuite/c-c++-common/attr-typeless-storage-2.c	(working copy)
@@ -0,0 +1,17 @@
+/* We used to reject this because types differentiating only in
+   TYPE_REF_CAN_ALIAS_ALL were deemed incompatible.  */
+/* { dg-do compile } */
+
+struct sockaddr;
+struct sockaddr *f (void);
+
+struct __attribute__((typeless_storage)) sockaddr { int j; };
+struct sockaddr *
+f (void)
+{
+  return
+#ifndef __cplusplus
+    (void *)
+#endif
+    0;
+}
Index: gcc/testsuite/gcc.c-torture/execute/typeless-storage-1.c
===================================================================
--- gcc/testsuite/gcc.c-torture/execute/typeless-storage-1.c	(revision 0)
+++ gcc/testsuite/gcc.c-torture/execute/typeless-storage-1.c	(working copy)
@@ -0,0 +1,20 @@
+/* Tests that the typeless_storage attribute works as expected.  */
+ 
+extern void abort(void);
+extern void exit(int);
+
+typedef int __attribute__((__typeless_storage__)) int_a;
+
+int
+main (void)
+{
+  int_a a = 0x12345678;
+  short *b = (short *) &a;
+
+  b[1] = 0;
+
+  if (a == 0x12345678)
+    abort();
+
+  exit(0);
+}