From: bwilson@shasta.stanford.edu (Bob Wilson)
Subject: possible fix for structure copying bug
Organization: GNUs Not Usenet
Distribution: gnu
Date: Thu, 5 Nov 1992 17:07:16 GMT

Several weeks ago I reported a bug in g++ 2.2.2 that occurred when copying
structures.  Basically, the problem was that for an assignment like
"*p = *p->next" the value of "p->next" gets recalculated for each member
of the structure.  This breaks when the "next" field gets overwritten.
Since this bug was not fixed in 2.3.1, I had to try to fix it myself.
Here's what I came up with (along with a test program).  I have tested it
on several programs and it seems to work, but I'm hardly a gcc expert and
I wouldn't be too surprised if it's not quite correct.  Please let me know
if you make some more changes to this patch.



struct foo {
    int m;
    foo(int a) { m = a; }
    operator int() { return m; }
    void operator=(int a) { m = a; }
};

struct node {
    int z;
    struct node *next[2];
    foo n;

    node(int a) : n(a) { }
};

main()
{
    struct node x(11), y(12), *p;

    x.next[0] = &y;
    y.next[0] = 0;

    p = &x;
    *p = *p->next[0];
    printf("%d\n", p->n.m);
}



*** ../gcc-bugs/cp-typeck.c.orig	Tue Nov  3 11:08:01 1992
--- cp-typeck.c	Wed Nov  4 15:02:07 1992
***************
*** 5063,5068 ****
--- 5063,5069 ----
        tree vbases = CLASSTYPE_VBASECLASSES (lhstype);
        tree lhs_addr = build_unary_op (ADDR_EXPR, lhs, 0);
        tree rhs_addr;
+       tree rhs_addr_rtl;
  
        /* Memberwise assignment would cause NEWRHS to be
  	 evaluated for every member that gets assigned.
***************
*** 5080,5085 ****
--- 5081,5087 ----
  	newrhs = save_expr (break_out_cleanups (newrhs));
  
        rhs_addr = build_unary_op (ADDR_EXPR, newrhs, 0);
+       rhs_addr_rtl = NULL_TREE;
        result = NULL_TREE;
  
        if (! comptypes (TREE_TYPE (lhs_addr), TREE_TYPE (rhs_addr), 1))
***************
*** 5088,5095 ****
  	 to work from there.  Otherwise we can get multiple evaluations
  	 of NEWRHS.  */
        if (TREE_CODE (newrhs) != SAVE_EXPR)
- 	newrhs = build_indirect_ref (rhs_addr, 0);
  
        while (vbases)
  	{
  	  tree elt_lhs = convert_pointer_to (vbases, lhs_addr);
--- 5090,5128 ----
  	 to work from there.  Otherwise we can get multiple evaluations
  	 of NEWRHS.  */
        if (TREE_CODE (newrhs) != SAVE_EXPR)
  
+ 	/* I'm not sure what the previous comment means, but the original
+ 	   code in gcc-2.3.1 didn't work.  I've changed it to wrap a SAVE_EXPR
+ 	   around the rhs_addr and to build an RTL_EXPR to evaluate that
+ 	   SAVE_EXPR before building the trees to copy the structure fields.
+ 	   The second part is needed because when an array is included in the
+ 	   structure a recursive invocation of this routine creates an
+ 	   RTL_EXPR to copy the array, and the SAVE_EXPR gets evaluated
+ 	   in that code instead of in the code to copy the first field
+ 	   (i.e. the SAVE_EXPR is used before it's defined.) --rpw 11/4/92 */
+ 	{
+ 	  rhs_addr = save_expr (break_out_cleanups (rhs_addr));
+ 	  newrhs = build_indirect_ref (rhs_addr, 0);
+ 
+ 	  rhs_addr_rtl = make_node (RTL_EXPR);
+ 	  TREE_TYPE (rhs_addr_rtl) = void_type_node;
+ 	  do_pending_stack_adjust ();
+ 	  start_sequence ();
+ 
+ 	  /* As a matter of principle, `start_sequence' should do this.  */
+ 	  emit_note (0, -1);
+ 
+ 	  expand_expr(rhs_addr, const0_rtx, VOIDmode, 0);
+ 	  emit_queue ();
+ 
+ 	  do_pending_stack_adjust ();
+ 
+ 	  TREE_SIDE_EFFECTS (rhs_addr_rtl) = 1;
+ 	  RTL_EXPR_SEQUENCE (rhs_addr_rtl) = get_insns ();
+ 	  RTL_EXPR_RTL (rhs_addr_rtl) = const0_rtx;
+ 	  end_sequence ();
+ 	}
+ 
        while (vbases)
  	{
  	  tree elt_lhs = convert_pointer_to (vbases, lhs_addr);
***************
*** 5102,5107 ****
--- 5135,5142 ----
        result = tree_cons (NULL_TREE,
  			  build_modify_expr_1 (lhs, modifycode, newrhs, TYPE_BINFO (lhstype)),
  			  result);
+       if (rhs_addr_rtl)
+ 	result = tree_cons (NULL_TREE, rhs_addr_rtl, result);
        return build_compound_expr (result);
      }
  

Newsgroups: gnu.g++.bug