forced cast char to char (and may be other casts) problem
Posted: Wed Nov 23, 2016 10:13 pm
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
+ }
}
}
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
+ }
}
}