[sdcc-devel] [ sdcc-Bugs-3440327 ] wrong code for pointer to functions in a loop, z80 port

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

Bugs item #3440327, was opened at 2011-11-19 13:03
Message generated for change (Comment added) made by spth
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=100599&aid=3440327&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: z80 port
Group: None
>Status: Closed
Resolution: Fixed
Priority: 7
Private: No
Submitted By: Martin Matyas (mz-fuzzy)
Assigned to: Philipp Klaus Krause (spth)
Summary: wrong code for pointer to functions in a loop, z80 port

Initial Comment:
Wrong code generated. Attached example is simplified as far as possible to reproduce the error; in such simplification, the code itself does not make a good sense, however problem appears also in meaningful cases.
See the loop "while (val < dt->id)". This compiled to asm code at rows 98 to 124. This loop utilize bc register, push/pop is done around __sdcc_call_hl call (row 120).
>>> The error: Row 110 increases value of BC register but the loop semantics obviously does not count with that. This leads to calling wrong pointer at row 120 in the 2nd pass of the loop.

command line:
sdcc -mz80 -c sdccerr.c

This bug does not occur when using --oldralloc as the code looks quite differently.
SDCC : mcs51/gbz80/z80/z180/r2k/ds390/pic16/pic14/TININative/ds400/hc08 3.1.0 #7037 (Nov 19 2011) (Linux)

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

>Comment By: Philipp Klaus Krause (spth)
Date: 2011-11-28 06:54

Message:
The regression test has been added in revision #7083.

Philipp


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

Comment By: Philipp Klaus Krause (spth)
Date: 2011-11-21 00:51

Message:
Fixed in revision #7066.

Keeping this bug report open, since
1) There's more problematic uses of isLastUse() in src/z80/gen.c
2) A regression test should be added
Which should be looked at after the 3.1.0 release.

Philipp


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

Comment By: Philipp Klaus Krause (spth)
Date: 2011-11-20 06:16

Message:
Increasing priority, since bad code is silently generated.

Code generation thinks that bc is no longer needed: The !isLastUse (ic,
left) in genPointerGet(), gen.c returns false. Otherwise code generation
would add generate an additional dec bc, as it should.

The problem is this: isLastUse(ic, left) checks if ic is the last use of
left. In this case this is true: The current ic is indeed the last use
(since any further use of bc will happen in the current ic again). Due to
this behaviour of isLastUse(), isLastUse() seems to be a very dangerous
function. Fortunately, in this case it can be repalced by a call to
isPairDead(), but all other uses of isLastUse() in gen.c should be checked
for the potential of similar problems and a warning comment added to
isLastUse(). 

Philipp


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

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