<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Apr 28, 2020 at 1:57 PM Adrian Prantl <<a href="mailto:aprantl@apple.com">aprantl@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Getting source location information right is tricky and all about finding a balance.<br>
Recently, I was wondering why stepping through this contrived example<br>
<br>
   1       struct Foo {<br>
   2         Foo *getFoo() { return this; }<br>
   3       };<br>
   4       <br>
   5       int main(int argc, char **argv) {<br>
   6         Foo *foo = new Foo();<br>
   7         foo->getFoo()->getFoo();<br>
   8         return 0;<br>
   9       }<br>
<br>
LLDB was showing the column marker as<br>
<br>
   7         foo->getFoo()->getFoo();<br>
             ^^^<br>
<br>
focussing on foo instead of at the method call getFoo() that I was expecting.<br>
<br>
In LLVM IR, this code looks like<br>
<br>
  %1 = load %struct.Foo*, %struct.Foo** %foo, align 8, !dbg !30<br>
  %call1 = call %struct.Foo* @_ZN3Foo6getFooEv(%struct.Foo* %1), !dbg !31 <br>
  %call2 = call %struct.Foo* @_ZN3Foo6getFooEv(%struct.Foo* %call1), !dbg !32 <br>
<br>
or, in x86_64 assembler:<br>
<br>
  .loc 1 7 3 is_stmt 1 ## column_info.cpp:7:3<br>
  movq -24(%rbp), %rdi<br>
  .loc 1 7 8 is_stmt 0 ## column_info.cpp:7:8<br>
  callq __ZN3Foo6getFooEv<br>
  .loc 1 7 18 ## column_info.cpp:7:18<br>
<br>
The spurious (7:3) location is attached to an instruction that is the load of the variable from the stack slot, fused with moving that value into the register the ABI defines for $arg0.<br>
<br>
I’m postulating that the source location of the LLVM IR load is uninteresting and perhaps even harmful. It is uninteresting, because at -O0, the location does not refer to explicit code that the user wrote and thus causes unintuitive stepping, and with optimizations enabled, there is a high likelihood that the entire instruction is going to be eliminated because of mem2reg, so the effect on profiling should be minimal. Since the load is from an alloca, it also cannot crash under normal operation. The location is harmful, because loads (at least on a CISC instruction set) are often fused with other instructions and having conflicting locations will cause both locations to be dropped when merged.<br>
<br>
Based on all this I would most like to assign a form of “weak” source location to loads from allocas generated by the Clang frontend, that looses against any other source location when merged. The closest thing we have to this in LLVM IR is attaching no debug location. An instruction without a debug location either belongs to the function prologue, or will inherit whatever debug location the instruction before it has. In this particular case I think that no debug location is preferable over line 0 (which is how we usually denote compiler-generated code) because we don’t want the load instruction’s source location to erase any source location it may get merged with. One thing I need to check is what happens when an instruction without a location is the first instruction in a basic block. We may need to make an exception for that case.<br>
<br>
To summarize, I’m proposing to delete all debug locations from instructions generated in the Clang frontend for loads from allocas that are holding source variables to improve the debug experience at -O0. This will have little effect on optimized code.<br>
<br>
Let me know what you think!<br></blockquote><div><br></div><div>In general, pretty apprehensive - in terms of practical effects that come to mind, seems like this might break sanitizers that want to describe specific loads/stores (even for local memory allocations - wonder how it looks for something like MSan). There is some work on value based profiling, I think, too? But I don't know much about it & whether this would impact that.<br><br>Could you show a more motivating example? I assume when you're actually inside the getFoo call, going up one frame would give you a line/column that points to the specific getFoo function call, yes? So this is only about stepping, but that step doesn't look particularly problematic to me - perhaps it gets worse/confusing/misleading in other places?</div></div></div>