forced cast char to char (and may be other casts) problem

Bug reports (if you don't/won't have a Github account)
Post Reply
anton_sh
New member
Posts: 1
Joined: Wed Nov 23, 2016 9:52 pm

forced cast char to char (and may be other casts) problem

Post by anton_sh »

Discovered bug of (at least) redundant casting of (unsigned char)
to (unsigned char) which and can be demonstrated using following
testcase:

#include <stdio.h>

unsigned char ex_var = 240;

#define _WITH_ASM_LABLES
//----------------------------------------------------------------
int main(int argc, char** argv) {
unsigned char loc_var = 240;
printf("Hello from main.");
#ifdef _WITH_ASM_LABLES
#asm
_dummy_before_if_ext:
ld a, (_ex_var)
#endasm
#endif
if (((unsigned char)ex_var) >= ((unsigned char)200)) {
printf(" ex_var is >= 200;");
} else {
printf(" ex_var is < 200;");
}
#ifdef _WITH_ASM_LABLES
#asm
_dummy_before_if_aut:
ld a, (_ex_var)
#endasm
#endif
if (((unsigned char)loc_var) >= ((unsigned char)200)) {
printf(" loc_var is >= 200;");
} else {
printf(" loc_var is < 200;");
}
printf(" Bye!\n");
return 0;
}


----------------------------------------------
This will surprisingly give with the second "if" the message "loc_var is < 200".

The reason behind is the code generated to cast (unsigned char)
to (unsigned char) again:


._main
ld hl,240 ;const
ld a,l
call l_sxt
dec sp
ld a,l
pop hl
ld l,a
push hl
ld hl,i_1+0
push hl
ld a,1
call printf
pop bc
_dummy_before_if_ext:
ld a, (_ex_var)
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
ld h,0 ; <- this is added by cast
; here it does not yet create
; big problem, just corrupt h
; register
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
ld a,(_ex_var)
cp #(200 % 256 % 256)
jr z,i_4_uge
jp c,i_4
.i_4_uge
ld hl,i_1+17
push hl
ld a,1
call printf
pop bc
jp i_5
.i_4
ld hl,i_1+36
push hl
ld a,1
call printf
pop bc
.i_5
_dummy_before_if_aut:
ld a, (_ex_var)
ld hl,0 ;const
add hl,sp
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
ld h,0 ; <- this is added by cast
; here it is worse - instead of
; reading the variable value from
; stack to register "A" we read
; some random value from ROM
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
ld a,(hl)
cp #(200 % 256 % 256)
...

Following is my naive attempt to fix the problem. It helps to
make my testcase working properly ("ld h, 0" disappers). I cannot
judge, however, whether such cast to single byte ("ld h, 0") can
be dangerous in other situations - can imagine cases when for
example value to be casted is not in HL register pair - so the
fix need anyway to be reviewed by somebody who is "deeply inside"
the sccz80 compiler code.

dev@allu:/data/sonstiges/spectrum/z88dk/z88dk > diff -u src/sccz80/primary.c.orig src/sccz80/primary.c
--- src/sccz80/primary.c.orig 2016-11-23 20:13:12.488705037 +0100
+++ src/sccz80/primary.c 2016-11-23 23:05:30.446320023 +0100
@@ -276,13 +276,36 @@
if ( t1 == CCHAR && sign2 == NO && !lconst) {
if ( sign1 == NO )
convSint2char();
- else
+ else {
+
+// cast must be disabled to avoid a problem, keep enabled to
+// reproduce the problem only !
+//#define _WITH_CHAR2CHAR_CAST
+
+#ifndef _WITH_CHAR2CHAR_CAST
+ if (t1 == CCHAR && t2 == CCHAR) {
+ warning(W_HARON1) ;
+ } else {
+#endif // _WITH_CHAR2CHAR_CAST
convUint2char();
+#ifndef _WITH_CHAR2CHAR_CAST
+ }
+#endif // _WITH_CHAR2CHAR_CAST
+ }
} else if ( t1 == CCHAR && sign2 == YES && !lconst ) {
if ( sign1 == NO )
convSint2char();
- else
+ else {
+#ifndef _WITH_CHAR2CHAR_CAST
+ if (t1 == CCHAR && t2 == CCHAR) {
+ warning(W_HARON2) ;
+ } else {
+#endif // _WITH_CHAR2CHAR_CAST
convUint2char();
+#ifndef _WITH_CHAR2CHAR_CAST
+ }
+#endif // _WITH_CHAR2CHAR_CAST
+ }
}

}
User avatar
dom
Well known member
Posts: 1195
Joined: Sun Jul 15, 2007 10:01 pm

Post by dom »

Thanks, this is the weird cast issue that I've never had the time to track down properly.

The issue is the extra level of parenthesis you've got - if you remove those then the code works as you'd expect.

It appears that the extra parenthesis delay the memory dereference and so the cast takes place on the address of rather than the value.
User avatar
dom
Well known member
Posts: 1195
Joined: Sun Jul 15, 2007 10:01 pm

Post by dom »

This issue was relocated to GitHub as https://github.com/z88dk/z88dk/issues/26

I've rewritten the casting so it works correctly and this issue (and others in the same area) should work as expected. It's now merged in, so the nightly of 20170423 should contain the fix.
Post Reply