Menu
▾
▴
#1422 Evelyn jumps into the void
This is a problem I have often seen in complicated code; it just went away when doing small changes to the code.
Now I finally managed to create a small testcase triggering the problem.
When I compile the attached file using
sdcc -mz80 evelyn.c
sdcc generates assembler code that contains a jump to a nonexisting label. In the attached example a jump to $00115 is generated (it should probably go to $00106 instead)
I always found the nonexisting jump target to have a number somewhat higher than all existing labels.
I use sdcc 2.7.2 #4861.
Philipp
sdcc generates a jump to a nonexisting label
Logged In: YES
user_id=564030
Originator: YES
This is a bug in the label optimizations on iCode.
sdcc tries to remove a conditional jump, but removes it only partially. Somehow the jump target is deleted, but the test of the jump condition remains. This results in test of the condition and a conditional jump to a nonexisting label on z80 and gbz80. hc08 has an assertion that catches this. On other ports, the condition is still tested (compile evelyn.c and you'll see an and instruction generated), but there's no jump.
It seems the bug is in the file SDCClabel.c, in the labelIfx function. The bug occours when deleteIfx is called in line 160.
Philipp
Logged In: YES
user_id=1840151
Originator: NO
the offending code can be reduced a bit.
-------<<8-----
char *goo;
void foo(void) {
for(;;)
if(*goo & 0x40)
if(*goo)
continue;
}
------->>8-----
compilation with
sdcc -mz80 -c thisfile.c
still exposes the missing branch target.
whats funny, with this code you get a double EVELYN message...
further remark:
1) --nolabelopt avoids the problem.
2) with various modfication of the offending code, i experianced a
considerable correlation with reoccurance of a sub expression.
for example i always needed two times *goo, or two times something+1, or
two times struct_thing.element ...
A very simple example that reproduces the bug.
Bug encountered by others. Changing priority. A simpler example for bug reproduction could be condensed from their code. Attached as cch.c.
File Added: cch.c
functions genAnd (and as it turns out genOr as well) in z80/gen.c are responsible for this bug.
they are not prepared to handle a special case where parameter ifx == 0.
mcs51/gen.c got this right in genAnd, but not in genOr.
the nice "evelyn message ..." is generated far far ahead, and has only remotely to to with the
actual bug.
i've created three support/regression/tests files which trigger those failures, and a patch
for mcs51 ds390 z80 and hc08 which should fix this.
here the regression system seems not to work all that good for the other processors,
and so i've simply neglected them.
it seems i'm too stupid to find an attach button somewhere, thus i'm including the patch
here.
Robert Larice
-------<<8-----
Index: src/mcs51/gen.c
===================================================================
--- src/mcs51/gen.c (Revision 5406)
+++ src/mcs51/gen.c (Arbeitskopie)
@@ -7210,7 +7210,7 @@
// result = 1
if (size)
emitcode ("setb", "%s", AOP (result)->aopu.aop_dir);
- else
+ else if(ifx)
continueIfTrue (ifx);
goto release;
}
@@ -7228,7 +7228,7 @@
emitLabel (tlbl);
}
else
- {
+ { /* FIXME, thats pretty fishy, check for ifx!=0, testcase .. */
genIfxJump (ifx, "a", left, right, result, ic->next);
goto release;
}
Index: src/z80/gen.c
===================================================================
--- src/z80/gen.c (Revision 5406)
+++ src/z80/gen.c (Arbeitskopie)
@@ -5386,6 +5386,7 @@
/* For the flags */
emit2 ("or a,a");
}
+ if (size || ifx) /* emit jmp only, if it is actually used */
emit2 ("!shortjp NZ,!tlabel", tlbl->key + 100);
}
offset++;
@@ -5569,10 +5570,20 @@
bytelit = (lit >> (offset * 8)) & 0x0FFL;
_moveA (aopGet (AOP (left), offset, FALSE));
- /* OR with any literal is the same as OR with itself. */
- emit2 ("or a,a");
- emit2 ("!shortjp NZ,!tlabel", tlbl->key + 100);
+ if (bytelit != 0)
+ { /* FIXME, allways true, shortcut possible */
+ emit2 ("or a,%s", aopGet (AOP (right), offset, FALSE));
+ }
+ else
+ {
+ /* For the flags */
+ emit2 ("or a,a");
+ }
+
+ if (ifx) /* emit jmp only, if it is actually used */
+ emit2 ("!shortjp NZ,!tlabel", tlbl->key + 100);
+
offset++;
}
if (ifx)
Index: src/ds390/gen.c
===================================================================
--- src/ds390/gen.c (Revision 5406)
+++ src/ds390/gen.c (Arbeitskopie)
@@ -7829,7 +7829,7 @@
// result = 1
if (size)
emitcode ("setb", "%s", AOP (result)->aopu.aop_dir);
- else
+ else if(ifx)
continueIfTrue (ifx);
goto release;
}
@@ -7847,7 +7847,7 @@
emitLabel (tlbl);
}
else
- {
+ { /* FIXME, thats pretty fishy, check for ifx!=0, testcase .. */
genIfxJump (ifx, "a", ic->next);
goto release;
}
Index: src/hc08/gen.c
===================================================================
--- src/hc08/gen.c (Revision 5406)
+++ src/hc08/gen.c (Arbeitskopie)
@@ -5038,7 +5038,6 @@
if (AOP_TYPE (result) == AOP_CRY)
{
symbol *tlbl = NULL;
- wassertl (ifx, "AOP_CRY result without ifx");
offset = 0;
while (size--)
@@ -5075,7 +5074,8 @@
}
if (tlbl)
emitLabel (tlbl);
- genIfxJump (ifx, "a");
+ if(ifx)
+ genIfxJump (ifx, "a");
goto release;
}
@@ -5202,7 +5202,6 @@
if (AOP_TYPE (result) == AOP_CRY)
{
symbol *tlbl = NULL;
- wassertl (ifx, "AOP_CRY result without ifx");
offset = 0;
while (size--)
@@ -5235,7 +5234,8 @@
}
if (tlbl)
emitLabel (tlbl);
- genIfxJump (ifx, "a");
+ if(ifx)
+ genIfxJump (ifx, "a");
}
if (AOP_TYPE (right) == AOP_LIT)
Index: support/regression/tests/bug1875933-3.c
===================================================================
--- support/regression/tests/bug1875933-3.c (Revision 0)
+++ support/regression/tests/bug1875933-3.c (Revision 0)
@@ -0,0 +1,49 @@
+/*
+ */
+
+/*
+ * mcs51 segmentation fault
+ *
+ * function genOr() in mcs51/gen.c
+ * was not prepeared for ifx==0
+ */
+
+#include <testfwk.h>
+#include <stdint.h>
+
+char identity(char x) {
+ return x;
+}
+
+void void_tor1(char x) {
+ char y = (identity(x) | 1) ? 42 : 43;
+}
+
+void void_tor0(char x) {
+ char y = (identity(x) | 0) ? 42 : 43;
+}
+
+void void_tor(char x) {
+ char y = (identity(x) | x) ? 42 : 43;
+}
+
+
+void
+testBug(void)
+{
+ void_tor1(1);
+ void_tor1(0);
+ void_tor0(1);
+ void_tor0(0);
+
+ void_tor(0);
+
+ ASSERT(1 == 1);
+}
+
+
+/*
+ * Local Variables:
+ * compile-command: "make -C .. ALL_TESTS='./tests/bug1875933-3.c'"
+ * End:
+ */
Index: support/regression/tests/bug1875933-1.c
===================================================================
--- support/regression/tests/bug1875933-1.c (Revision 0)
+++ support/regression/tests/bug1875933-1.c (Revision 0)
@@ -0,0 +1,43 @@
+/*
+ */
+
+/*
+ * [ 1875933 ] Evelyn jumps into the void
+ * http://sourceforge.net/tracker/index.php?func=detail&aid=1875933&group_id=599&atid=100599
+ *
+ * function genAnd() and genOr() in z80/gen.c
+ * were not prepared to handle the special case where ifx == 0
+ */
+
+#include <testfwk.h>
+#include <stdint.h>
+
+char identity(char x) {
+ return x;
+}
+
+void void_tand1(char x) {
+ char y = (identity(x) & 1) ? 42 : 43;
+}
+
+void void_tand0(char x) {
+ char y = (identity(x) & 0) ? 42 : 43;
+}
+
+void
+testBug(void)
+{
+ void_tand1(1);
+ void_tand1(0);
+ void_tand0(1);
+ void_tand0(0);
+
+ ASSERT(1 == 1);
+}
+
+
+/*
+ * Local Variables:
+ * compile-command: "make -C .. ALL_TESTS='./tests/bug1875933-1.c'"
+ * End:
+ */
Index: support/regression/tests/bug1875933-2.c
===================================================================
--- support/regression/tests/bug1875933-2.c (Revision 0)
+++ support/regression/tests/bug1875933-2.c (Revision 0)
@@ -0,0 +1,55 @@
+/*
+ */
+
+/*
+ * function genOr() in z80/gen.c
+ * assumed identity of "or a, literal" and "or a,a"
+ * thats definitly not so
+ */
+
+#include <testfwk.h>
+#include <stdint.h>
+
+char identity(char x) {
+ return x;
+}
+
+char tor1(char x) {
+ char y = (identity(x) | 1) ? 42 : 43;
+ return y;
+}
+
+char tor0(char x) {
+ char y = (identity(x) | 0) ? 42 : 43;
+ return y;
+}
+
+char tand1(char x) {
+ char y = (identity(x) & 1) ? 42 : 43;
+ return y;
+}
+
+char tand0(char x) {
+ char y = (identity(x) & 0) ? 42 : 43;
+ return y;
+}
+
+void
+testBug(void)
+{
+ ASSERT(tor1(1) == 42);
+ ASSERT(tor1(0) == 42);
+ ASSERT(tor0(1) == 42);
+ ASSERT(tor0(0) == 43);
+ ASSERT(tand1(1) == 42);
+ ASSERT(tand1(0) == 43);
+ ASSERT(tand0(1) == 43);
+ ASSERT(tand0(0) == 43);
+}
+
+
+/*
+ * Local Variables:
+ * compile-command: "make -C .. ALL_TESTS='./tests/bug1875933-2.c'"
+ * End:
+ */
------->>8-----
I applied the patch and run the regression tests without errors on my snapshot.
Philipp, Maarten and other sdcc developers: do you think that the patch should be committed before 2.9.0 release?
P.S.: I merged all 3 regression tests into one bug1875933.c file.
Borut
At least the Z80 part of the patch seems okay to me. I suggest committing it before the 2.9.0 release. IMo this is currently the worst sdcc bug, affecting multiple ports and being encountered frequently. Especially to newbies this bug can make sdcc in a bad light. I'd rather delay the 2.9.0 release a bit by making one more release candidate than not fix thisbug now.
Philipp