From: SourceForge.net <no...@so...> - 2008-01-20 20:42:37
|
Bugs item #1875933, was opened at 2008-01-20 21:42 Message generated for change (Tracker Item Submitted) made by Item Submitter You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1875933&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Philipp Krause (spth) Assigned to: Nobody/Anonymous (nobody) Summary: Evelyn jumps into the void Initial Comment: 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 ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1875933&group_id=599 |
From: SourceForge.net <no...@so...> - 2008-03-08 19:10:00
|
Bugs item #1875933, was opened at 2008-01-20 21:42 Message generated for change (Comment added) made by spth You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1875933&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. >Category: Icode generator Group: None Status: Open Resolution: None >Priority: 6 Private: No Submitted By: Philipp Krause (spth) Assigned to: Nobody/Anonymous (nobody) Summary: Evelyn jumps into the void Initial Comment: 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 ---------------------------------------------------------------------- >Comment By: Philipp Krause (spth) Date: 2008-03-08 20:09 Message: 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 ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1875933&group_id=599 |
From: SourceForge.net <no...@so...> - 2008-03-17 19:10:46
|
Bugs item #1875933, was opened at 2008-01-20 21:42 Message generated for change (Comment added) made by rlar You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1875933&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: Icode generator Group: None Status: Open Resolution: None Priority: 6 Private: No Submitted By: Philipp Krause (spth) Assigned to: Nobody/Anonymous (nobody) Summary: Evelyn jumps into the void Initial Comment: 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 ---------------------------------------------------------------------- Comment By: Robert Larice (rlar) Date: 2008-03-17 20:10 Message: 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 ... ---------------------------------------------------------------------- Comment By: Philipp Krause (spth) Date: 2008-03-08 20:09 Message: 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 ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1875933&group_id=599 |
From: SourceForge.net <no...@so...> - 2009-02-07 13:39:53
|
Bugs item #1875933, was opened at 2008-01-20 21:42 Message generated for change (Settings changed) made by spth You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1875933&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: Icode generator Group: None Status: Open Resolution: None >Priority: 8 Private: No Submitted By: Philipp Krause (spth) Assigned to: Nobody/Anonymous (nobody) Summary: Evelyn jumps into the void Initial Comment: 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 ---------------------------------------------------------------------- >Comment By: Philipp Krause (spth) Date: 2009-02-07 14:39 Message: 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 ---------------------------------------------------------------------- Comment By: Robert Larice (rlar) Date: 2008-03-17 20:10 Message: 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 ... ---------------------------------------------------------------------- Comment By: Philipp Krause (spth) Date: 2008-03-08 20:09 Message: 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 ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1875933&group_id=599 |
From: SourceForge.net <no...@so...> - 2009-03-08 19:04:15
|
Bugs item #1875933, was opened at 2008-01-20 21:42 Message generated for change (Comment added) made by rlar You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1875933&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: Icode generator Group: None Status: Open Resolution: None Priority: 8 Private: No Submitted By: Philipp Krause (spth) Assigned to: Nobody/Anonymous (nobody) Summary: Evelyn jumps into the void Initial Comment: 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 ---------------------------------------------------------------------- Comment By: Robert Larice (rlar) Date: 2009-03-08 20:03 Message: 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----- ---------------------------------------------------------------------- Comment By: Philipp Krause (spth) Date: 2009-02-07 14:39 Message: 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 ---------------------------------------------------------------------- Comment By: Robert Larice (rlar) Date: 2008-03-17 20:10 Message: 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 ... ---------------------------------------------------------------------- Comment By: Philipp Krause (spth) Date: 2008-03-08 20:09 Message: 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 ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1875933&group_id=599 |
From: SourceForge.net <no...@so...> - 2009-03-10 00:51:19
|
Bugs item #1875933, was opened at 2008-01-20 21:42 Message generated for change (Comment added) made by borutr You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1875933&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: Icode generator Group: None Status: Open Resolution: None Priority: 8 Private: No Submitted By: Philipp Krause (spth) Assigned to: Nobody/Anonymous (nobody) Summary: Evelyn jumps into the void Initial Comment: 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 ---------------------------------------------------------------------- >Comment By: Borut Ražem (borutr) Date: 2009-03-10 01:51 Message: 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 ---------------------------------------------------------------------- Comment By: Robert Larice (rlar) Date: 2009-03-08 20:03 Message: 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----- ---------------------------------------------------------------------- Comment By: Philipp Krause (spth) Date: 2009-02-07 14:39 Message: 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 ---------------------------------------------------------------------- Comment By: Robert Larice (rlar) Date: 2008-03-17 20:10 Message: 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 ... ---------------------------------------------------------------------- Comment By: Philipp Krause (spth) Date: 2008-03-08 20:09 Message: 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 ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1875933&group_id=599 |
From: Maarten B. <sou...@ds...> - 2009-03-10 10:00:10
|
Borut, I'm sorry, but I really don't have time to check this. I leave this upto you guys and will check when I come back from Colombia. Since there are also several FIXME's in there for better optimizations, you may leave it open if you choose to apply the patch. Maarten > Bugs item #1875933, was opened at 2008-01-20 21:42 > Message generated for change (Comment added) made by borutr > You can respond by visiting: > https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1875933&group_id=599 > > Please note that this message will contain a full copy of the comment thread, > including the initial issue submission, for this request, > not just the latest update. > Category: Icode generator > Group: None > Status: Open > Resolution: None > Priority: 8 > Private: No > Submitted By: Philipp Krause (spth) > Assigned to: Nobody/Anonymous (nobody) > Summary: Evelyn jumps into the void > > Initial Comment: > 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 > > > ---------------------------------------------------------------------- > > >Comment By: Borut Razem (borutr) > Date: 2009-03-10 01:51 > > Message: > 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 > > ---------------------------------------------------------------------- > > Comment By: Robert Larice (rlar) > Date: 2009-03-08 20:03 > > Message: > 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----- > > > > ---------------------------------------------------------------------- > > Comment By: Philipp Krause (spth) > Date: 2009-02-07 14:39 > > Message: > 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 > > ---------------------------------------------------------------------- > > Comment By: Robert Larice (rlar) > Date: 2008-03-17 20:10 > > Message: > 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 ... > > > ---------------------------------------------------------------------- > > Comment By: Philipp Krause (spth) > Date: 2008-03-08 20:09 > > Message: > 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 > > > ---------------------------------------------------------------------- > > You can respond by visiting: > https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1875933&group_id=599 > > ------------------------------------------------------------------------------ > _______________________________________________ > sdcc-devel mailing list > sdc...@li... > https://lists.sourceforge.net/lists/listinfo/sdcc-devel |
From: SourceForge.net <no...@so...> - 2009-03-10 13:22:12
|
Bugs item #1875933, was opened at 2008-01-20 21:42 Message generated for change (Comment added) made by spth You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1875933&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: Icode generator Group: None Status: Open Resolution: None Priority: 8 Private: No Submitted By: Philipp Krause (spth) Assigned to: Nobody/Anonymous (nobody) Summary: Evelyn jumps into the void Initial Comment: 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 ---------------------------------------------------------------------- >Comment By: Philipp Krause (spth) Date: 2009-03-10 14:21 Message: 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 ---------------------------------------------------------------------- Comment By: Borut Ražem (borutr) Date: 2009-03-10 01:51 Message: 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 ---------------------------------------------------------------------- Comment By: Robert Larice (rlar) Date: 2009-03-08 20:03 Message: 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----- ---------------------------------------------------------------------- Comment By: Philipp Krause (spth) Date: 2009-02-07 14:39 Message: 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 ---------------------------------------------------------------------- Comment By: Robert Larice (rlar) Date: 2008-03-17 20:10 Message: 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 ... ---------------------------------------------------------------------- Comment By: Philipp Krause (spth) Date: 2008-03-08 20:09 Message: 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 ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1875933&group_id=599 |
From: SourceForge.net <no...@so...> - 2009-03-10 21:37:56
|
Bugs item #1875933, was opened at 2008-01-20 21:42 Message generated for change (Comment added) made by borutr You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1875933&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: Icode generator >Group: fixed >Status: Closed >Resolution: Fixed Priority: 8 Private: No Submitted By: Philipp Krause (spth) >Assigned to: Borut Ražem (borutr) Summary: Evelyn jumps into the void Initial Comment: 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 ---------------------------------------------------------------------- >Comment By: Borut Ražem (borutr) Date: 2009-03-10 22:37 Message: Fixed in svn revision #5408. Borut ---------------------------------------------------------------------- Comment By: Philipp Krause (spth) Date: 2009-03-10 14:21 Message: 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 ---------------------------------------------------------------------- Comment By: Borut Ražem (borutr) Date: 2009-03-10 01:51 Message: 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 ---------------------------------------------------------------------- Comment By: Robert Larice (rlar) Date: 2009-03-08 20:03 Message: 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----- ---------------------------------------------------------------------- Comment By: Philipp Krause (spth) Date: 2009-02-07 14:39 Message: 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 ---------------------------------------------------------------------- Comment By: Robert Larice (rlar) Date: 2008-03-17 20:10 Message: 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 ... ---------------------------------------------------------------------- Comment By: Philipp Krause (spth) Date: 2008-03-08 20:09 Message: 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 ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1875933&group_id=599 |
From: Larice R. <la...@vi...> - 2009-03-11 21:34:14
Attachments:
_diff3.diff
|
On Tue, 10 Mar 2009, SourceForge.net wrote: > Bugs item #1875933, was opened at 2008-01-20 21:42 > Message generated for change (Comment added) made by borutr > You can respond by visiting: > https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1875933&group_id=599 > > Please note that this message will contain a full copy of the comment thread, > including the initial issue submission, for this request, > not just the latest update. > Category: Icode generator > >Group: fixed > >Status: Closed ... > ---------------------------------------------------------------------- > > >Comment By: Borut Ra?em (borutr) > Date: 2009-03-10 22:37 > > Message: > Fixed in svn revision #5408. > > Borut Hello Borut, thanks for commiting this, but unfortunately gen.c(genXor) for various processors suffers from the same problem. i've attached another patch concerning this, including a testcase. the patch fixes z80 mc51 and ds390, but not hc08, i've left a comment there. Robert Larice |