Re: c/9853: [3.2/3.3/3.4 regression] miscompilation of non-constant structure initializer

Re: c/9853: [3.2/3.3/3.4 regression] miscompilation of non-constant structure initializer

[Glen Nakamura]

The following reply was made to PR c/9853; it has been noted by GNATS.

From: Glen Nakamura <glen@imodulo.com>
To: gcc-gnats@gcc.gnu.org, gcc-bugs@gcc.gnu.org,
	mas@systems.caltech.edu
Cc:  
Subject: Re: c/9853: [3.2/3.3/3.4 regression] miscompilation of non-constant structure initializer
Date: Tue, 11 Mar 2003 21:27:44 +0000

 --yrj/dFKFPuw6o+aM
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 
 http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=9853
 
 After looking at the test case for PR c/9853, I believe the problem deals
 with assigning and freeing temp slots while emitting the initializer.
 In this case, the temp slot created by assign_temp in expand_expr (expr.c:7087)
 is freed early by free_temp_slots in expand_decl_init (stmt.c:4061) and then
 overwritten when a new temp slot is allocated.
 
 I don't really understand how the different temp slot functions are intended
 to be used, but the call to free_temp_slots at the end of expand_decl_init
 didn't make sense to me because expand_assignment manages its own temp slots
 using the push/free/pop functions.  Anyway, the following patch fixes the
 problem (bootstrapped and regtested on i686-pc-linux-gnu 3.3 branch):
 
 	* stmt.c (expand_decl_init): Remove redundant call to free_temp_slots.
 
 diff -Nru3p gcc-3.3.orig/gcc/stmt.c gcc-3.3/gcc/stmt.c
 --- gcc-3.3.orig/gcc/stmt.c	2003-02-22 10:04:10.000000000 +0000
 +++ gcc-3.3/gcc/stmt.c	2003-02-22 10:04:10.000000000 +0000
 @@ -4055,10 +4055,6 @@ expand_decl_init (decl)
  
    /* Don't let the initialization count as "using" the variable.  */
    TREE_USED (decl) = was_used;
 -
 -  /* Free any temporaries we made while initializing the decl.  */
 -  preserve_temp_slots (NULL_RTX);
 -  free_temp_slots ();
  }
  
  /* CLEANUP is an expression to be executed at exit from this binding contour;
 
 
 
 Although the above patch fixes the problem, perhaps changing the keep argument
 to the assign_temp in expand_expr would be safer?  I'll let someone who
 understands the temp slot functions better decide.
 
 diff -Nru10p gcc-3.3.orig/gcc/expr.c gcc-3.3/gcc/expr.c
 --- gcc-3.3.orig/gcc/expr.c	2003-03-05 00:14:33.000000000 +0000
 +++ gcc-3.3/gcc/expr.c	2003-03-05 00:14:33.000000000 +0000
 @@ -7082,21 +7082,21 @@ expand_expr (exp, target, tmode, modifie
  	  /* Handle calls that pass values in multiple non-contiguous
  	     locations.  The Irix 6 ABI has examples of this.  */
  	  if (target == 0 || ! safe_from_p (target, exp, 1)
  	      || GET_CODE (target) == PARALLEL
  	      || modifier == EXPAND_STACK_PARM)
  	    target
  	      = assign_temp (build_qualified_type (type,
  						   (TYPE_QUALS (type)
  						    | (TREE_READONLY (exp)
  						       * TYPE_QUAL_CONST))),
 -			     0, TREE_ADDRESSABLE (exp), 1);
 +			     1, TREE_ADDRESSABLE (exp), 1);
  
  	  store_constructor (exp, target, 0, int_expr_size (exp));
  	  return target;
  	}
  
      case INDIRECT_REF:
        {
  	tree exp1 = TREE_OPERAND (exp, 0);
  	tree index;
  	tree string = string_constant (exp1, &index);
 
 --yrj/dFKFPuw6o+aM
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: attachment; filename="testcase.c"
 
 /* PR c/9853 */
 /* Contributed by Srinivas Aji <mas@systems.caltech.edu> */
 
 extern void abort (void);
 
 struct box_group
 {
   int a, b, c, d;
 };
 
 struct test_struct
 {
   int id;
   struct box_group elem1;
   struct box_group elem2;
 };
 
 int my_zero (void)
 {
   return 0;
 }
 
 int main ()
 {
   struct test_struct x = {
     .id = my_zero (),
     .elem1 = (struct box_group) {1, 2, 3, 4},
     .elem2 = (struct box_group) {5, 6, 7, 8},
   };
   if ((x.id != 0) || (x.elem1.a != 1) || (x.elem1.b != 2)
       || (x.elem1.c != 3) || (x.elem1.d != 4) || (x.elem2.a != 5)
       || (x.elem2.b != 6) || (x.elem2.c != 7) || (x.elem2.d != 8))
     abort ();
   return 0;
 }
 
 --yrj/dFKFPuw6o+aM--

Re: c/9853: [3.2/3.3/3.4 regression] miscompilation of non-constant structure initializer

[Glen Nakamura]

The following reply was made to PR c/9853; it has been noted by GNATS.

From: Glen Nakamura <glen@imodulo.com>
To: gcc-gnats@gcc.gnu.org
Cc:  
Subject: Re: c/9853: [3.2/3.3/3.4 regression] miscompilation of non-constant structure initializer
Date: Tue, 11 Mar 2003 12:47:31 -1000

 http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=9853
 
 Also, as a note to anyone looking at this PR, notice how many unnecessary
 temporaries are used in the test case.  The generated code is quite awful.
 A quick search returned PR opt/9540 relating to this issue:
 http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=9540

Re: c/9853: [3.2/3.3/3.4 regression] miscompilation of non-constant structure initializer

[bangerth]

Old Synopsis: gcc 3.2 miscompiles non constant structure initializer
New Synopsis: [3.2/3.3/3.4 regression] miscompilation of non-constant structure initializer

State-Changed-From-To: open->analyzed
State-Changed-By: bangerth
State-Changed-When: Wed Feb 26 19:26:00 2003
State-Changed-Why:
    Behavior confirmed. If the code is legal, then this is
    a regression from 3.0.4, which yielded the expected
    result. 3.2, 3.3 and mainline all print something
    unexpected.
    
    Someone with standard knowledge should verify what is
    expected behavior here.
    
    W.

http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=9853