#2854 Z80 - squashed function return value

[Phillip Stevens]

I think I've found an issue present in the r10748 release.
Comparison of the r9958 version against r10748 with the optimiser turned off shows a problem.

This is the fragment of C.

bank_get_abs((int8_t)atoi(args[  ]))

This is an example of the correct assembly found in r9958

2212  073A              ;main.c:366: bankLockBase[ bank_get_abs((int8_t)atoi(args[1])) ] = 0x00;
2213  073A  C5              push    bc
2214  073B  6B              ld  l, e
2215  073C  62              ld  h, d
2216  073D  CD 00 00        call    _atoi_fastcall
2217  0740  5D              ld  e, l
2218  0741  54              ld  d, h
2219  0742  C1              pop bc
2220  0743  6B              ld  l, e
2221  0744  CD 00 00        call    _bank_get_abs_fastcall

This is an example of incorrect assembly in r10748

2177  0730              ;main.c:366: bankLockBase[ bank_get_abs((int8_t)atoi(args[1])) ] = 0x00;
2178  0730  C5              push    bc
2179  0731  6B              ld  l, e
2180  0732  62              ld  h, d
2181  0733  CD 00 00        call    _atoi_fastcall
2182  0736  5D              ld  e, l
2183  0737  54              ld  d, h
2184  0738  C1              pop bc
2185  0739  1E 00           ld  e,0x00
2186  073B  16 00           ld  d,0x00
2187  073D  6B              ld  l, e
2188  073E  26 00           ld  h,0x00
2189  0740  CD 00 00        call    _bank_get_abs_fastcall

And another piece of bad assembly from r10748

3030  0C5F              ;main.c:527: fprintf(output,"Saving Bank %01X to \"%s\"", bank_get_abs((int8_t)atoi(args[1])), args[2] );
3031  0C5F  DD 6E EF        ld  l,(ix-17)
3032  0C62  DD 66 F0        ld  h,(ix-16)
3033  0C65  4E              ld  c, (hl)
3034  0C66  23              inc hl
3035  0C67  46              ld  b, (hl)
3036  0C68  DD 6E ED        ld  l,(ix-19)
3037  0C6B  DD 66 EE        ld  h,(ix-18)
3038  0C6E  5E              ld  e, (hl)
3039  0C6F  23              inc hl
3040  0C70  66              ld  h, (hl)
3041  0C71  C5              push    bc
3042  0C72  6B              ld  l, e
3043  0C73  CD 00 00        call    _atoi_fastcall
3044  0C76  5D              ld  e, l
3045  0C77  54              ld  d, h
3046  0C78  C1              pop bc
3047  0C79  1E 00           ld  e,0x00
3048  0C7B  16 00           ld  d,0x00
3049  0C7D  6B              ld  l, e
3050  0C7E  26 00           ld  h,0x00
3051  0C80  CD 00 00        call    _bank_get_abs_fastcall

Note that this issue doesn't happen everywhere. The code fragment occurs 17 times in the main.c file, and the result is variable. The issue seems to be the insertion of the zero setting of e and d registers, without due consideration.

[Philipp Klaus Krause]

Can you provide a small, compileable example (i.e. also including the declaration of bank_get_abs()? asm output with --fverbose-asm could also be helpful.

Philipp

[Phillip Stevens]

Philipp,

attached an edited version of a shell, that uses the offending piece of code 12 times.
Most of the time the assembly is correct. Some of the time it is not.

Just search in the main.c.asm file for bank_get_abs((int8_t)atoi(args[ and you will find the example assembly.

The command line is noted at the top of main.c.

[Philipp Klaus Krause]

I can't reproduce this. But it looks to me like bug #2852, on which should be fixed since a few hours go. Can you check if the issue is fixed in current SDCC [r10756]?

Philipp

[Phillip Stevens]

I've recompiled with the latest SVN version.

SDCC : z80/z180/r2k/r3ka/gbz80/ez80_z80 3.8.4 #10757 (Linux)

and the issue seems to be now resolved.
Excellent.

Also, the spurious insertion of zero setting of d and e registers is gone too.

1E 00           ld  e,0x00
16 00           ld  d,0x00

another win.

[Maarten Brock]

The C source which is also used as the regression test throws in some arbitrary addresses that it starts to write to. How do you know it is safe to write to these addresses? Your code might be living there, or your data.

Of course the regression test for host fails with a segmentation fault. But that it doesn't fail for other targets is sheer luck.

The regression test for this bug is in such bad shape that I reopen this bug report. I will also disable the test completely for now.

[Philipp Klaus Krause]

I'll look into it on Sunday, Monday or next year.

Philipp

[Philipp Klaus Krause]

I looked through the test, but did not find such a write.
However, I found and fixed (in [r10779]) a read from a pointer that points one beyond the end of an array (by extending the array).

Philipp