<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Aug 26, 2014 at 1:45 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><span><br><br>On Mon, Aug 18, 2014 at 2:33 PM, Adrian Prantl <<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>> wrote:<br>> 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.<br>
<br></span>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.<span><br>
<br>><br>> ```<br>> if (c)<br>> foo(); // cleanups = line 0<br>><br>> if (c) {<br>> } // cleanups<br>><br>> if (c)<br>> foo();<br>> else {<br>> bar();<br>> } // cleanups<br>
<br></span>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.<br><br>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:<br>
<br>GCC is GCC 4.8<br>ToT is Clang ToT<br>End is this patch applied to Clang ToT to use the end of the if as the location<br>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.<br>
<br>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)<br>
<br><font face="courier new, monospace">1: for<br>2: if<br>3: func();<br>4: }<br><br>GCC: 1 2 3 1 2 1 2 3 1 4<br>ToT: 1 2 3 2 1 2 2 1 2 3 2 1 4<br>End: 1 2 3 3 1 2 3 1 2 3 3 1 4<br>Zer: 1 2 3 1 2 1 2 3 1 4, dtor skipped and ascribed to 4<br>
<br>1: for<br>2: if<br>3: func();<br>4: else<br>5: func();<br>6: }<br><br>GCC: 1 2 3 1 2 5 1 2 3 1 6<br>ToT: 1 2 3 2 1 2 5 2 2 3 2 1 6<br>End: 1 2 3 1 2 5 1 2 3 1 6<br>Zer: 1 2 3 1 2 5 1 2 3 1 6, dtor skipped and ascribed to 5<br>
<br>1: for<br>2: if<br>3: ;<br>4: else<br>5: ;<br>6: }<br><br>GCC: 1 2 5 1 2 5 1 2 5 1 6<br>ToT: 1 2 2 1 2 2 1 2 2 1 6<br>End: 1 2 5 1 2 5 1 2 5 1 6<br>Zer: 1 2 1 2 1 2 1 6, dtor skipped and ascribed to 6<br>
<br>1: for<br>2: if<br>3: ;<br>4: }<br><br>GCC: 1 2 3 1 2 3 1 2 3 1 4<br>ToT: 1 2 2 1 2 2 1 2 2 1 4<br>End: 1 2 3 1 2 3 1 2 3 1 4<br>Zer: 1 2 1 2 1 2 1 4, dtor skipped and ascribed to 4</font></div></blockquote><div><br>Updating these tables with some more ideas... <br><br>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)<br><br>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.<br><br>Sound plausible to everyone?<br><br><br><font face="monospace, monospace">1: for // loop twice<br>2: if // true on the first call, false on the second<br>3: CALL;<br>4: else<br>5: CALL;<br><br>CALL=, exceptions<br><br> GCC: 1 2 5 1 2 5 1 (dtor: 5)<br> ToT: 1 2 1 2 1 (dtor: 2)<br> End: 1 2 1 2 1 (dtor: 5)<br> Zero: 1 2 1 2 1 (dtor: 2)<br>no stmt: <same as ToT, already has is_stmt 0 there by coincidence><br></font><br><span style="font-family:monospace">CALL=, no-exceptions</span><br style="font-family:monospace"><br style="font-family:monospace"><span style="font-family:monospace"> GCC: 1 2 5 1 2 5 1 (dtor: 5)</span><br style="font-family:monospace"><span style="font-family:monospace"> ToT: 1 2 1 2 1 (dtor: 2)</span><br style="font-family:monospace"><span style="font-family:monospace"> End: 1 2 5 1 2 5 1 (dtor: 5)</span><br style="font-family:monospace"><span style="font-family:monospace"> Zero: 1 2 0x400603 (dtor: 0x400608) ??</span><br style="font-family:monospace"><span style="font-family:monospace">no stmt: <same as ToT, already has is_stmt 0 there by coincidence></span><br style="font-family:monospace"> <br><font face="monospace">CALL=func(), exceptions</font><br style="font-family:monospace"><br style="font-family:monospace"><span style="font-family:monospace"> GCC: 1 2 3 1 2 5 1 (dtor: 5)</span><br style="font-family:monospace"><span style="font-family:monospace"> ToT: 1 2 3 2 1 2 5 2 1 (dtor: 2)</span><br style="font-family:monospace"><span style="font-family:monospace"> End: 1 2 3 1 2 1 (dtor: 5)*</span><br style="font-family:monospace"><span style="font-family:monospace"> Zero: 1 2 3 0x4007ed (dtor: 0x4007f2) ??</span><br style="font-family:monospace"><span style="font-family:monospace">no stmt: 1 2 3 1 2 5 1 (dtor: 5)<br></span><br><font face="monospace">CALL=func(), no-exceptions</font><br style="font-family:monospace"><br style="font-family:monospace"><span style="font-family:monospace"> GCC: 1 2 3 1 2 5 1 (dtor: 5)</span><br style="font-family:monospace"><span style="font-family:monospace"> ToT: 1 2 3 2 1 2 5 2 1 (dtor: 2)</span><br style="font-family:monospace"><span style="font-family:monospace"> End: 1 2 3 1 2 5 1 (dtor: 5)</span><br style="font-family:monospace"><span style="font-family:monospace"> Zero: 1 2 3 0x40060b (dtor: 0x400610)</span><br style="font-family:monospace"><span style="font-family:monospace">no stmt: 1 2 3 1 2 5 1 (dtor: 5)<br><br>* 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.<br></span><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><br><br>> ```<br>
><br>> <a href="http://reviews.llvm.org/D4956" target="_blank">http://reviews.llvm.org/D4956</a><br>><br>><br></div>
</blockquote></div><br></div></div>