[PATCH] Do not attach a debug location to code inserted by ARC / API for disabling DebugLocations

[Eric Christopher]

Looks like it's working around the return value load/alloca eliding in
the code generator? I definitely like a more general way of
approaching the problem.

I wonder if we can either localize this fix into EmitFunctionEpilog
(couldn't come up with one) or if this has applicability in the > 1
return statement condition. Open questions, this is a bit of a finicky
and complicated area as you've no doubt realized. :)

One other comment on the patch:

+  if (CGDebugInfo *DI = getDebugInfo()) {
     DI->EmitLocation(Builder, EndLoc);
+    EndLocL = Builder.getCurrentDebugLocation();
+    if ((NStopPoints == 1) && !FnRetTy->isVoidType()) {
+      DI->EmitLocation(Builder, FirstStopPoint);
+      EmitRetDbgLoc = false;

Do we need the two locations emitted here? or the check for VoidType?
(I realize based on the return value load/alloca eliding it would need
to be void type - but hence me asking).

-eric


On Fri, Apr 26, 2013 at 2:15 AM, Adrian Prantl <aprantl at apple.com> wrote:
>
> On Apr 23, 2013, at 4:00 PM, Eric Christopher <echristo at gmail.com> wrote:
>
>> Not sure what you mean, I'm curious why the arc code is getting line
>> information associated with the end of the block even at the
>> beginning? Same with blocks. (To be fair I didn't implement that front
>> end part so I'm relying on you to know since you're adjusting it :)
>
> With blocks the problem is that we insert an extra alloca and some setup code into the prologue, but unless we set the DebugLoc to an empty location that code will have line info attached to it. This in turn will trigger the heuristic in AsmPrinter/DwarfDebug to believe that it should emit the prologue_end flag when in fact we are still in the middle of the frame setup. Which will cause some debugger tests to fail. I think that that is fairly uncontroversial.
>
> Lets move over to the interesting part:
>
> With ARC the problem is closer to the C++ example you provided the other day. After reconsidering the problem you got me convinced (thanks!) that removing the debug info for the ARC cleanup code goes a bit too far. The problem is that if we have only a single return statement in the function -- especially one where the return expression does not use anything that needs to be cleaned up -- the (ARC) cleanup code will be the first non-prologue (linetable-)stmt in the function. That means we *need* to provide a breakpoint there, otherwise the user can’t inspect the values before they get cleaned up.
>
>     1   - (int)testFoo:(NSString *)foo {
>     2       return 1;
>     3   } // Cleanup code, comes before ret 1, but has a higher line number
>
> My new patch ensures that the line table is always sequential. If there is cleanup code and a single return statement it essentially flips their debug locations so the first breakpoint in the function is before the cleanup code and in a location that does not confuse the user.
> In any function, if there is cleanup code, it will get the debug location of the ‘}’ *and* the ret instruction in the epilog will have no debug location, so the debugger doesn’t jump back into the function body again. There is nothing to see there anyway :-)
>
> I tried it on your C++ example and it seemed reasonable, too.
> It also should be very close to the behavior of GCC that David mentioned the other day where the return expression is at the line of the return statement and the cleanup code and actual return are t the closing brace of the scope.
>
> thanks!
> Adrian