diff mbox

RFA: PATCH to handle_transparent_union_attribute for c++/51228 (ICE on bogus use of transparent_union)

Message ID 4EE91FBA.1070204@redhat.com
State New
Headers show

Commit Message

Jason Merrill Dec. 14, 2011, 10:14 p.m. UTC 
It doesn't make sense to mark a type with no fields as a transparent 
union, and indeed handle_transparent_union checks for that.  But this 
testcase was slipping past the checks because we're applying the 
attribute in place but after the type has been completed.  This patch 
adjusts the check logic so that we check for a suitable field even when 
applying the attribute in place if we've already done finish_struct.

Tested x86_64-pc-linux-gnu.  OK for trunk?  This is apparently a 
regression back to 4.5.0, but doesn't seem worth fixing in older 
releases since this isn't a valid use of the attribute.

Comments

Jakub Jelinek Dec. 19, 2011, 12:42 p.m. UTC | #1 
On Wed, Dec 14, 2011 at 05:14:18PM -0500, Jason Merrill wrote:
> --- a/gcc/c-family/c-common.c
> +++ b/gcc/c-family/c-common.c
> @@ -6286,13 +6286,21 @@ handle_transparent_union_attribute (tree *node, tree name,
>  
>    if (TREE_CODE (type) == UNION_TYPE)
>      {
> -      /* When IN_PLACE is set, leave the check for FIELDS and MODE to
> -	 the code in finish_struct.  */
> +      /* Make sure that the first field will work for a transparent union.
> +	 If the type isn't complete yet, leave the check to the code in
> +	 finish_struct.  */
> +      if (TYPE_SIZE (type))
> +	{
> +	  tree first = first_field (type);
> +	  if (first == NULL_TREE
> +	      || TYPE_MODE (type) != DECL_MODE (first))
> +	    goto ignored;
> +	}

I'd think it would be nicer to emit the same diagnostic from this
spot as we do if handle_transparent_union_attribute is called on
an incomplete type and finish_struct is called later.
Unfortunately that varies between C and C++ FEs, we'd need a langhook
for that, which is perhaps an overkill for such rarely used attribute.

So, I'm fine with this patch for the trunk too, though perhaps you
want also
	|| first == error_mark_node
	|| DECL_ARTIFICIAL (first)
for the condition to catch what finish_struct_1 does for C++?

	Jakub
diff mbox

Patch

commit f3b9daf04363159ed8707df50ed77989e1ee31be
Author: Jason Merrill <jason@redhat.com>
Date:   Wed Dec 14 14:05:57 2011 -0500

    	PR c++/51228
    	* c-common.c (handle_transparent_union_attribute): Check the first
    	field if the type is complete.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index fbbcb38..87eee79 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -6286,13 +6286,21 @@  handle_transparent_union_attribute (tree *node, tree name,
 
   if (TREE_CODE (type) == UNION_TYPE)
     {
-      /* When IN_PLACE is set, leave the check for FIELDS and MODE to
-	 the code in finish_struct.  */
+      /* Make sure that the first field will work for a transparent union.
+	 If the type isn't complete yet, leave the check to the code in
+	 finish_struct.  */
+      if (TYPE_SIZE (type))
+	{
+	  tree first = first_field (type);
+	  if (first == NULL_TREE
+	      || TYPE_MODE (type) != DECL_MODE (first))
+	    goto ignored;
+	}
+
       if (!(flags & (int) ATTR_FLAG_TYPE_IN_PLACE))
 	{
-	  if (TYPE_FIELDS (type) == NULL_TREE
-	      || c_dialect_cxx ()
-	      || TYPE_MODE (type) != DECL_MODE (TYPE_FIELDS (type)))
+	  /* build_duplicate_type doesn't work for C++.  */
+	  if (c_dialect_cxx ())
 	    goto ignored;
 
 	  /* A type variant isn't good enough, since we don't a cast
diff --git a/gcc/testsuite/c-c++-common/transparent-union-1.c b/gcc/testsuite/c-c++-common/transparent-union-1.c
new file mode 100644
index 0000000..3fb6e78
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/transparent-union-1.c
@@ -0,0 +1,5 @@ 
+/* PR c++/51228 */
+
+typedef union {} U __attribute__((transparent_union)); /* { dg-warning "ignored" } */
+
+void foo(U u) {}