I'm generally in favor of plumbing is_stmt through the front end for another reason as well: in general, looking at the front end we'll know what's likely a "better" place to put a breakpoint. We can then use the plumbing to make the experience good in general.
On Mon Feb 09 2015 at 9:39:12 AM David Blaikie <[email protected]> wrote: > +Eric - this was the thread about is_stmt, scope locations, etc, I was > talking about last week. > > On Fri, Feb 6, 2015 at 2:52 PM, David Blaikie <[email protected]> wrote: > >> >> >> On Tue, Aug 26, 2014 at 1:45 PM, David Blaikie <[email protected]> >> wrote: >> >>> >>> >>> On Mon, Aug 18, 2014 at 2:33 PM, Adrian Prantl <[email protected]> >>> wrote: >>> > IMO, the best we can do is to use the end of the CompoundStmt as the >>> cleanup location if there is one, and otherwise give up and use line 0. >>> >>> OK. The "let's do better when there's braces" isn't something I'm >>> worrying about for now - it's a possible incremental improvement after we >>> figure out the "worst case" behavior. >>> >>> > >>> > ``` >>> > if (c) >>> > foo(); // cleanups = line 0 >>> > >>> > if (c) { >>> > } // cleanups >>> > >>> > if (c) >>> > foo(); >>> > else { >>> > bar(); >>> > } // cleanups >>> >>> In this case, I'm not sure the close } to the else is any better than >>> the last line of the else, it still feels like part of the else, not the >>> end of the if scope. >>> >>> Anyway, using line zero (did this by hand and used no-integrated-as >>> because Clang doesn't cope well with emitting line table entries for line >>> 0) here's GDB's behavior in some examples: >>> >>> GCC is GCC 4.8 >>> ToT is Clang ToT >>> End is this patch applied to Clang ToT to use the end of the if as the >>> location >>> Zer is the end patch applied, and then manually editing the assembly >>> (and assembling with the system assembler, not the integrated assembler) to >>> have the non-exceptional dtor call be on line 0. >>> >>> So we get the totally right behavior from GDB the same as we would by >>> assigning it to the end. In the cases where we get different behavior, it's >>> better - we don't step to lines that don't execute, but we still get a bad >>> location walking up from the dtor call (end up wherever the preceeding code >>> was - which means sometimes we end up on the last line of the function >>> where the EH cleanup is ascribed to). Seems better, but not perfect. (but >>> we have no way to get perfect) >>> >>> 1: for >>> 2: if >>> 3: func(); >>> 4: } >>> >>> GCC: 1 2 3 1 2 1 2 3 1 4 >>> ToT: 1 2 3 2 1 2 2 1 2 3 2 1 4 >>> End: 1 2 3 3 1 2 3 1 2 3 3 1 4 >>> Zer: 1 2 3 1 2 1 2 3 1 4, dtor skipped and ascribed to 4 >>> >>> 1: for >>> 2: if >>> 3: func(); >>> 4: else >>> 5: func(); >>> 6: } >>> >>> GCC: 1 2 3 1 2 5 1 2 3 1 6 >>> ToT: 1 2 3 2 1 2 5 2 2 3 2 1 6 >>> End: 1 2 3 1 2 5 1 2 3 1 6 >>> Zer: 1 2 3 1 2 5 1 2 3 1 6, dtor skipped and ascribed to 5 >>> >>> 1: for >>> 2: if >>> 3: ; >>> 4: else >>> 5: ; >>> 6: } >>> >>> GCC: 1 2 5 1 2 5 1 2 5 1 6 >>> ToT: 1 2 2 1 2 2 1 2 2 1 6 >>> End: 1 2 5 1 2 5 1 2 5 1 6 >>> Zer: 1 2 1 2 1 2 1 6, dtor skipped and ascribed to 6 >>> >>> 1: for >>> 2: if >>> 3: ; >>> 4: } >>> >>> GCC: 1 2 3 1 2 3 1 2 3 1 4 >>> ToT: 1 2 2 1 2 2 1 2 2 1 4 >>> End: 1 2 3 1 2 3 1 2 3 1 4 >>> Zer: 1 2 1 2 1 2 1 4, dtor skipped and ascribed to 4 >>> >> >> Updating these tables with some more ideas... >> >> TL;DR; We should put cleanup code on the right line (whatever that may >> be, I think the end of the scope is best) with is_stmt 0. GDB essentially >> ignores is_stmt 0 line table entries and attributes those lines to the >> previous line entry which is wrong but fairly beningly so. (it'll just mean >> the backtrace is at the last line we codegen'd which is /probably/ the if >> block instead of the else block, if the else block is empty) ASan can do >> the right thing and we can file a bug on GDB in case they care. But this >> gets all the /stepping/ behavior as right as we can get it, really (empty >> blocks like the first two cases will still step right past them and get a 1 >> 2 1 2 1 2 sort of behavior, but that seems OK) >> >> That said, implementing this means plumbing is_stmt through from the >> frontend, and that seems like more work than I'm willing to put in right >> now - but at least it gives us a plan for if/when this bug becomes a >> priority for anyone to implement. >> > > One other thing in favor of the is_stmt solution is that it's fairly > robust - almost all the other solutions depend on block ordering for their > GDB behavior (zero doesn't - but it seems to have some rather bad behavior, > perhaps that's just GDB bugs, though). The GDB behavior of not breaking > when stepping into the middle of a line doesn't seem entirely unreasonable, > and that's all that's 'saving' any of the current solutions (including > GCC's) and requires specific block ordering for it to do so. Using is_stmt > 0 is block ordering independent with some minor GDB bugs (GDB just > completely ignores the is_stmt 0 line entries, as though they weren't there > at all - causing them to be wherever the previous line entry is, getting > that GDB "don't break in the middle of a line" behavior). > > >> >> Sound plausible to everyone? >> >> >> 1: for // loop twice >> 2: if // true on the first call, false on the second >> 3: CALL; >> 4: else >> 5: CALL; >> >> CALL=, exceptions >> >> GCC: 1 2 5 1 2 5 1 (dtor: 5) >> ToT: 1 2 1 2 1 (dtor: 2) >> End: 1 2 1 2 1 (dtor: 5) >> Zero: 1 2 1 2 1 (dtor: 2) >> no stmt: <same as ToT, already has is_stmt 0 there by coincidence> >> >> CALL=, no-exceptions >> >> GCC: 1 2 5 1 2 5 1 (dtor: 5) >> ToT: 1 2 1 2 1 (dtor: 2) >> End: 1 2 5 1 2 5 1 (dtor: 5) >> Zero: 1 2 0x400603 (dtor: 0x400608) ?? >> no stmt: <same as ToT, already has is_stmt 0 there by coincidence> >> >> CALL=func(), exceptions >> >> GCC: 1 2 3 1 2 5 1 (dtor: 5) >> ToT: 1 2 3 2 1 2 5 2 1 (dtor: 2) >> End: 1 2 3 1 2 1 (dtor: 5)* >> Zero: 1 2 3 0x4007ed (dtor: 0x4007f2) ?? >> no stmt: 1 2 3 1 2 5 1 (dtor: 5) >> >> CALL=func(), no-exceptions >> >> GCC: 1 2 3 1 2 5 1 (dtor: 5) >> ToT: 1 2 3 2 1 2 5 2 1 (dtor: 2) >> End: 1 2 3 1 2 5 1 (dtor: 5) >> Zero: 1 2 3 0x40060b (dtor: 0x400610) >> no stmt: 1 2 3 1 2 5 1 (dtor: 5) >> >> * because the EH cleanup code appears before the else block, the else >> block is a line continuation and stepping to it skips over :/ is_stmt might >> help? Let's see. Yep. Once I make the EH cleanup block is_stmt 0 and the >> else block is_stmt 1 (on the same source line) I get the obvious 1 2 3 1 2 >> 5 1 (dtor: 5) behavior. >> >> >>> >>> > ``` >>> > >>> > http://reviews.llvm.org/D4956 >>> > >>> > >>> >> >>
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
