This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

PATCH: PR 23171


PR c++/23171 is an ICE-on-valid.  This test cases uses the address of
a compound literal as an initializer for a variable with static
storage duration.  That's a C99 extension, which we try to support in
G++.  

G++ has represented this as an ADDR_EXPR around a CONSTRUCTOR since
forever, and the middle end used to be able to handle that.  However,
as Dan Berlin explains in the PR, the static initializer analysis
stuff can't handle this construct, and has no conservative fallback,
so it was effectively desupported.  However,
varasm.c:initializer_constant_valid was not updated, so G++ still
thinks it's OK to use such a construct as a static initializer.

So, the static initializer patch was incomplete, in this respect; it
changed the front-end/middle-end interface, but didn't update the
function that front ends use to figure out whether the middle end will
accept an initializer.  This patch does that.

As a result, the test case appears to work with that change -- because
G++ performs a dynamic initialization of the variable.  Although
suboptimal, that's OK -- but there's still a problem.

In particular, from the .t03.gimple dump:

  int D.2336[1];

  if (__initialize_p == 1)
    {
      if (__priority == 65535)
        {
          D.2336[0] = 23;
          p = &D.2336[0];
        }

So, the CONSTRUCTOR has become a local *non-static* array.  That's
death; the address must refer to an array in static storage.  There is
code in gimplify_init_constructor to generate a CONSTRUCTOR in static
storage, but it doesn't honor TREE_STATIC on a CONSTRUCTOR which says
that the constructor *must* go in static storage.

I checked in this patch (4.1 and mainline) before I realized that we
were getting a non-static initializer, after testing on
x86_64-unknown-linux-gnu.  Since this patch will fix the problem, once
the gimplifier is fixed, I've left it in -- for now.  (If I'm way off
base in this email, I'll back it out, since I'd rather have the ICE
than silent wrong code.)

I'm now testing a patch to make the gimplifier honor TREE_STATIC on
CONSTRUCTORs.
 
--
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com

2005-12-24  Mark Mitchell  <mark@codesourcery.com>

	PR c++/23171
	* varasm.c (initializer_constant_valid_p): An ADDR_EXPR of a
	CONSTRUCTOR is invalid.

2005-12-24  Mark Mitchell  <mark@codesourcery.com>

	PR c++/23171
	* g++.dg/opt/init1.C: New test.

Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c	(revision 109009)
+++ gcc/varasm.c	(working copy)
@@ -3477,18 +3477,24 @@ initializer_constant_valid_p (tree value
     case ADDR_EXPR:
     case FDESC_EXPR:
       value = staticp (TREE_OPERAND (value, 0));
-      /* "&(*a).f" is like unto pointer arithmetic.  If "a" turns out to
-	 be a constant, this is old-skool offsetof-like nonsense.  */
-      if (value
-	  && TREE_CODE (value) == INDIRECT_REF
-	  && TREE_CONSTANT (TREE_OPERAND (value, 0)))
-	return null_pointer_node;
-      /* Taking the address of a nested function involves a trampoline.  */
-      if (value
-	  && TREE_CODE (value) == FUNCTION_DECL
-	  && ((decl_function_context (value) && !DECL_NO_STATIC_CHAIN (value))
-	      || DECL_DLLIMPORT_P (value)))
-	return NULL_TREE;
+      if (value)
+	{
+	  /* "&(*a).f" is like unto pointer arithmetic.  If "a" turns out to
+	     be a constant, this is old-skool offsetof-like nonsense.  */
+	  if (TREE_CODE (value) == INDIRECT_REF
+	      && TREE_CONSTANT (TREE_OPERAND (value, 0)))
+	    return null_pointer_node;
+	  /* Taking the address of a nested function involves a trampoline.  */
+	  if (TREE_CODE (value) == FUNCTION_DECL
+	      && ((decl_function_context (value) 
+		   && !DECL_NO_STATIC_CHAIN (value))
+		  || DECL_DLLIMPORT_P (value)))
+	    return NULL_TREE;
+	  /* "&{...}" requires a temporary to hold the constructed
+	     object.  */
+	  if (TREE_CODE (value) == CONSTRUCTOR)
+	    return NULL_TREE;
+	}
       return value;
 
     case VIEW_CONVERT_EXPR:
Index: gcc/testsuite/g++.dg/opt/init1.C
===================================================================
--- gcc/testsuite/g++.dg/opt/init1.C	(revision 0)
+++ gcc/testsuite/g++.dg/opt/init1.C	(revision 0)
@@ -0,0 +1,4 @@
+// PR c++/23171
+// { dg-options "-O" }
+
+int *p = (int*)(int[1]){0};




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]