[sdcc-devel] [ sdcc-Bugs-608752 ] Fix for OP_ASSIGN bug

[SourceForge.net <no...@so...>]

Bugs item #608752, was opened at 2002-09-13 06:58
Message generated for change (Settings changed) made by maartenbrock
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=100599&aid=608752&group_id=599

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
>Category: C-Front End
>Group: fixed
>Status: Closed
>Resolution: Fixed
Priority: 5
Submitted By: Wim Lewis (wiml)
Assigned to: Maarten Brock (maartenbrock)
Summary: Fix for OP_ASSIGN bug

Initial Comment:
SDCC generates incorrect code for some statements of
the form foo += bar. The current implementation of
those operators transforms the statement to foo = foo +
bar, and then strips out any increment/decrement
operators found in the second instance of foo. However
this does not work in more complex cases, e.g.
   getitem()->flags |= 42;
(the getitem() call will be invoked twice).

This patch makes SDCC create temporary variables if
needed in order to store side-effect-ful values. The
above statement now gets transformed to
   tmp = getitem(); tmp->flags = tmp->flags | 42;

The generated code could use some more attention from
the peephole optimizer, but at least it is correct.


----------------------------------------------------------------------

>Comment By: Maarten Brock (maartenbrock)
Date: 2006-06-10 22:21

Message:
Logged In: YES 
user_id=888171

And finally after almost 4 years this bug is fixed in SDCC 
2.5.6 #4212.

----------------------------------------------------------------------

Comment By: Frieder Ferlemann (frief)
Date: 2006-04-27 18:17

Message:
Logged In: YES 
user_id=589052

The bug still persists. I'm appending a file which
reproduces the bug.

Changed from a patch submission to a bug report.

//--------8<-----------------------------------
typedef struct
{ 
  char a; 
  char n;
} item_type;

item_type t;


char more_items_in_list(void)
{
   return 1;
}


near item_type* get_next_item(void)
{
  /* have a side effect */
  t.n++;

  /* keep things easy, not implementing a list.
     Using a true list would break things
     even more pointedly: 
     a) reading beyond end of the list and 
     b) intermixing list members */
  return &t;
}


void main(void)
{
  t.a = 0;
  t.n = 0;
  
  if( more_items_in_list() )
  {
    /* get_item() is called twice */
    get_next_item()->a |= 42;
  }
  
  /* 
   * Now t.n is 2 instead of 1.
   * Also the or'ed value of 42 and the 2nd list item
   * would be written to the 1st list item
   */
    
}

//-------->8-----------------------------------

// compiles to:

//--------8<-----------------------------------
#if 0

_main:
;bug-608752.c:32: t.a = 0;
        mov     _t,#0x00
;bug-608752.c:33: t.n = 0;
        mov     (_t + 0x0001),#0x00
;bug-608752.c:35: if( more_items_in_list() )
        lcall   _more_items_in_list
        mov     a,dpl
        jz      00103$
;bug-608752.c:38: get_next_item()->a |= 42;
        lcall   _get_next_item ; ------------#1
        mov     r0,dpl
        push    ar0
        lcall   _get_next_item ; ------------#2
        mov     r1,dpl
        pop     ar0
        mov     ar2,@r1        ; read from @r1
        mov     a,#0x2A
        orl     a,r2
        mov     @r0,acc        ; written to @r0
00103$:
        ret

#endif


----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=100599&aid=608752&group_id=599