From: SourceForge.net <no...@so...> - 2011-02-08 17:34:32
|
Bugs item #1653651, was opened at 2007-02-06 23:34 Message generated for change (Comment added) made by borutr You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1653651&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: Debugger Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Frieder Ferlemann (frief) Assigned to: Nobody/Anonymous (nobody) Summary: sdcdb incorrectly reports "Stack underflow" Initial Comment: As reported by arkady (larytet) in the Open Discussion forum: http://sourceforge.net/forum/forum.php?thread_id=1651244&forum_id=1864 sdcdb reports "Stack underflow" when debugging the straightforward code: --------8<------------------------------------------ static unsigned char idleTask(unsigned char context) { context++; return context; } static void mainLoop() { unsigned char context = 0; while (1) { context = idleTask(context); } } void main() { mainLoop(); } -------->8------------------------------------------ (More details in the above mentioned thread) ---------------------------------------------------------------------- >Comment By: Borut Ražem (borutr) Date: 2011-02-08 18:34 Message: When I wrote about "gcc compatibility" I didn't mean the number of dashes but the option name compatibility. sdcc is not gcc compatible regarding number of dashes by design: gcc (mostly) uses single dash for both short and long options whilst sdcc uses single dash for short and double dashes for long options. "fverbose-asm" and "fno-peep-return" are both long options, like for example "--preprocessonly", so double dashes ere required. Some sdcc options has both short and long versions, for example -c is equal to --compile-only. If "-fno-peep-return" is a short version, how would the long version look like? Borut ---------------------------------------------------------------------- Comment By: Maarten Brock (maartenbrock) Date: 2011-02-07 19:17 Message: I understand and even support the desire for gcc compatibility, but then it should be -fverbose-asm instead of --fverbose-asm. I'm going forward with this and will use -fpeep-return and -fno-peep-return to enable or disable this optimization and have it disabled by default when --debug is used and enabled otherwise. ---------------------------------------------------------------------- Comment By: Borut Ražem (borutr) Date: 2011-01-09 16:58 Message: sdcc command line parser doesn't follow the standard (neither does gcc): multiple short options can be merged in standard implementation, for example -a -b -c is equal to -abc. This is not the case for sdcc: short options has to be separated. This is why the argument has to be space separated from the short option in the standard implementation and it doesn't have to be separated in sdcc case: standard: -ab == -a -b -a b == -a with argument b sdcc: -ab == -a with argument b -a b == -a with argument b The 'f' in --fverbose-asm is just for gcc compatibility. My idea was to be as much as possible gcc compatible regarding command line options. And gcc doesn't accept -f verbose-asm... Borut ---------------------------------------------------------------------- Comment By: Philipp Klaus Krause (spth) Date: 2011-01-09 14:38 Message: > "Underscores in command line options? I guess you mean dashes." Yes. It seems thinking about register allocation for weeks takes its toll on all other brain functions. AFAIR there should be a whitespace between the option and the argument. To be sure you might want to have a look at the standard at http://www.unix.org/version3/ >Tail calls do not trigger this bug on their own but only when combined with JUMP to JUMP optimization These are the same conditions that trigger the bug I seem to remember, which made me disable tail call optimization in the Z80. However that bug made sdcc crash instead of just affecting the debugger. Philipp ---------------------------------------------------------------------- Comment By: Maarten Brock (maartenbrock) Date: 2011-01-09 14:04 Message: Underscores in command line options? I guess you mean dashes. But when is an option long or short? Should there be whitespace between a short option and its argument? Can it even have an argument? If it can have one and needs no whitespace then -fpeep-return complies and so does -mz80. If --fverbose-asm was not meant to be --f with argument verbose-asm then what does the f stand for? Why not use --verbose-asm? Whether we call it sibling calls or tail calls doesn't really matter as this issue is bigger than that. Tail calls do not trigger this bug on their own but only when combined with JUMP to JUMP optimization. I propose to leave the latter optimization in and disable tail call optimization along with dead code removal of RET instructions in the peephole optimizer. I only checked the mcs51 peeph.def file which does have tail call optimizations right now. I want to disable rules 251.x, 259.x and 400.x with this option. I'll try to explain what happens in the debugger: It assumes there is only one function entry-point and only one exit-point. It also assumes that the first asm instruction of a function is the entry-point and the last is the exit-point. It sets up a pseudostack (linked list) for housekeeping automatic breakpoints etc. When it passes the entry-point it pushes onto this stack and when it passes the exit-point it pops it. So when the last instruction is no longer the exit point it gets confused. In this bug report the last RET was removed and resulted in a JUMP becoming the last instruction. The debugger consequently decided this was the exit-point and popped. But in reality it was jumping back into the function and the next time around it popped the context of the caller. After three turns the stack was empty and resulted in stack underflow. Something similar can happen when a JUMP to RET is replaced by RET as the debugger will not pass the original RET instruction. It will not pop the stack and will eventually cause a stack overflow. The same can happen when first CALL + RET is replaced by JUMP and then a JUMP to this new JUMP is replaced by a single JUMP which is no longer the last instruction then. So why fix this by disabling tail call optimization instead of JUMP to JUMP? Because tail call affects the debugging experience. At the end of the function you single step and expect to go to the caller but instead end up in the last callee. You expect to go up but instead you go down the calling tree. Maarten ---------------------------------------------------------------------- Comment By: Borut Ražem (borutr) Date: 2011-01-09 13:47 Message: Referencing to the Philipp's comment no. 1: -mmcs51 is actually -m option with mcs51 argument, so it is single letter option. --fverbose-asm is a long option (it is not -f with verbose-asm argument, at least it was not meant to be ;-) I think sdcc --help explains this quite well. Borut ---------------------------------------------------------------------- Comment By: Philipp Klaus Krause (spth) Date: 2011-01-09 13:03 Message: 1) AFAIK POSIX says that long options should start with a double underscore, while short ones (i.e. one letter) should start with a single underscore. Many programs have long options that start with a sngle underscore for legacy reasons though. 2) Replacing call, ret by jump is commonly called tail call optimization. Maybe this shold be reflected in the name. 3) I seem to remeber taill call optimization being broken, but can't find the bug report. Still the tail call optimization peepholes 65 and 66 are commented out for the Z80. AFAIR it messed up the label table by using a label that previously was only used in a call statement to a jump statement, which made subsequent peepholes fail. ---------------------------------------------------------------------- Comment By: Maarten Brock (maartenbrock) Date: 2011-01-09 12:34 Message: Well, no. It uses -mmcs51 etc which in my view is not much different from -fverbose-asm if I consider -m to set my target and -f to set some flag. Or --i-code-in-asm which uses no --f at all. It all seems rather inconsistent. And gcc seems to accept both --verbose-asm and -fverbose-asm but not --fverbose-asm. So, thinking more about it maybe it should be either -fpeep-return or --peep-return. I think I like -fpeep-return best as it could be extended to check for -fno- for all flags automatically. (Let's not consider to use a postfix +/- like some pragmas do.) Maarten ---------------------------------------------------------------------- Comment By: Borut Ražem (borutr) Date: 2011-01-09 11:36 Message: > Btw. why do we use a double dash in --fverbose-asm? Doesn't sdcc use duble dash for all long (non-one-letter) options? ---------------------------------------------------------------------- Comment By: Maarten Brock (maartenbrock) Date: 2011-01-09 11:02 Message: In this case it was the second example about dead code elimination that triggered the bug as it jumps back to the start of the infinite loop. The following RET was removed and the debugger thinks the JUMP is the end of the function and pops its household stack. Also another optimization could cause it: JUMP to RET => RET And when sibling optimization as gcc calls it replaces the final RET with a JUMP another optimization can also cause it: JUMP to JUMP => JUMP as it does not reach the last generated instruction of the function. So disabling either of these could fix the problem. Since sibling optimization is usually surprising and sometimes annoying while debugging I considered this the best candidate to be disabled. >From the little I read about sibling optimization I think gcc tries to do a lot more than what our peephole optimizer can do or does. Ours is just one special case of a void returning function without stack frame for locals. All in all I think -foptimize-sibling-calls does not quite describe what's happening. I think I'll go for -foptimize-return and -fno-optimize-return. Btw. why do we use a double dash in --fverbose-asm? Maarten ---------------------------------------------------------------------- Comment By: Patryk (patryks) Date: 2011-01-09 02:33 Message: I suggest 3rd option in Borut's version - optimization enabled by default in non-debug, disabled by default in debug, but switchable. Disabled inreases stack usage, and some applications in 8051 small memory model are written using every possible RAM byte, with stack usage analyzed for deepest branch. ---------------------------------------------------------------------- Comment By: Borut Ražem (borutr) Date: 2011-01-08 18:07 Message: I think the 2nd option is the right one. Other compilers also disable (or at least discourage) some optimizations when debugging is enabled. The 3rd option is also not a bad idea, as an addition to the 2nd one: in non-debug mode the return optimization will be enabled by default, in debug mode it will be disabled, while the new command line option will give a possibility to toggle it. I took a quick look to the gcc optimization options and I found -foptimize-sibling-calls, which I think does the job. So in sdcc it would be --foptimize-sibling-calls and --fno-optimize-sibling-calls. Whatever will be implemented it should also be documented. Borut ---------------------------------------------------------------------- Comment By: Maarten Brock (maartenbrock) Date: 2011-01-08 11:31 Message: Hi, I want to fix this by disabling several return-optimizations in the peephole optimizer. Like: CALL + RET => JUMP JUMP + RET => JUMP When functions have only one exit-point and this RET is the last instruction then the debugger can keep track. Now the question is: how? 1. Just remove them 2. disable them when --debug is used 3. en/disable them with a new command line option Any suggestions anyone? Maarten ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1653651&group_id=599 |