#2503 Miscompilation and wrong warning

[Alan Cox]

The attached code produces a wrong warning about c being unused. From the asm it appears that it also incorrectly eliminates the code so the generated code is also broken for this fairly common code pattern.

Forcing the char signed generates correct code, which makes it doubly nasty as this code pattern works with older sdcc due to the default signed nature of char.

[alvin]

The code is incorrect for unsigned char - the programmer should be using signed char. sdcc is producing a warning about this but it makes the output doubly wrong when it correctly eliminates the >0 conditional but incorrectly takes the assignment with it.

[Maarten Brock]

Am I missing something here? I only see signed char variables in z1.c. Which char are you forcing signed?

Btw. something this small can also be inlined in the message. Use ~~~ markers around it (or just use the button).

[Alan Cox]

The char is unsigned on Z80, forcing it signed avoids the warning and bug

Te code is correct in both case. The char is not marked as signed or unsigned so the source must cover it being either. In that sense the warning is wrong (but useful!). The warning is just an annoyance but the assignment vanishing is a bug

[alvin]

The code is definitely bugged when c is made unsigned and will produce incorrect output - I'm not sure if you're disagreeing.

The implementation is allowed to choose whether char is signed or unsigned. In this case, char is unsigned for the z80 target. That makes the comparison

(c = *cp++ - '0') >= 0

an unsigned comparison which will always be true so sdcc emits a warning and correctly decides to eliminate the comparison. This is definitely not what the programmer intended but that's what he's written.

The problem step is that in eliminating the comparison, sdcc is eliminating the entire expression including the assignment. And then we get other warnings like c may be used before it is assigned to (true!).

[Philipp Klaus Krause]

Smaller example to reproduce the issue:

unsigned f(void)
{
    unsigned d;
    (d = 1) >= 0;
    return d;
}

The problem can be found in line 4674 ost SDCCast.c. There, SDCC correctly replaces the condition by a constant. But it also completly omits generating any code for the comparison. I guess the correct thing would be also generate the comparison, and let dead code elimination sort out the parts without side-effects.

Philipp

[Ben Shi]

You solution would be performed only if the comparason tree had some side effects（assignment， increment）. Otherwise it would still be replaced by a constant.

I thought that would be better.

[Erik Petrich]

Fixed in [r9709]

I used the comma operator to return the always true/false result but yet still retain the side effects while discarding the unneeded comparison.