[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.


================
Comment at: lib/CodeGen/CGExpr.cpp:72
+  // Set debug location in order to preserve the scope
+  Alloca->setDebugLoc(Builder.getCurrentDebugLocation());
   if (AllocaAddr)
----------------
vsk wrote:
> aprantl wrote:
> > vsk wrote:
> > > I think we need to be a bit more careful here. The current debug location stored in the builder may not be an artificial 0-location. This may cause non-linear single-stepping behavior. Consider this example:
> > > 
> > > ```
> > > void foo() {
> > >   bar();
> > >   if (...) {
> > >     int var = ...; //< Clang emits an alloca for "var".
> > >   }
> > > ...
> > > ```
> > > 
> > > The current debug location at the line "int var = ..." would be at line 4. But the alloca is emitted in the entry block of the function. In the debugger, this may result in strange single-stepping behavior when stepping into foo(). You could step to line 4, then line 2, then line 3, then line 4 again.
> > > 
> > > I think we can avoid that by setting an artificial location on allocas.
> > > I think we can avoid that by setting an artificial location on allocas.
> > An alloca doesn't really generate any code (or rather.. the code it generates is in the function prologue). In what situation would the debug location on an alloca influence stepping? Are you thinking about the alloca() function?
> An alloca instruction can lower to a subtraction (off the stack pointer) though: https://godbolt.org/g/U4nGzJ.
> 
> `dwarfdump` shows that this subtraction instruction is actually assigned a location -- it currently happens to be the first location in the body of the function. I thought that assigning an artificial location to the alloca would be a first step towards fixing this.
> 
> Also, using an artificial location could mitigate possible bad interactions between code motion passes and IRBuilder. If, say, we were to set the insertion point right after an alloca, we might infer some arbitrary debug location. So long as this inference happens, it seems safer to infer an artificial location.
> 
> 
This may have unintended side-effects: By assigning a debug location to an alloca you are moving the end of the function prolog to before the alloca instructions, since LLVM computes the end of the function prologue as the first instruction with a non-empty debug location. Moving the end of the function prologue to before that stack pointer is adjusted is wrong, since that's the whole point of the prologue_end marker.

To me it looks more like a bug in a much later stage. With the exception of shrink-wrapped code, the prologue_end should always be after the stack pointer adjustment, I think.


Repository:
  rC Clang

https://reviews.llvm.org/D47097