+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
