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

[Vedant Kumar via Phabricator via cfe-commits]

vsk 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)
----------------
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.




Repository:
  rC Clang

https://reviews.llvm.org/D47097