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:
> > 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.
Thanks for explaining, I didn't realize that's how the end of the function
prologue is computed! Should we leave out the any debug location changes for
allocas in this patch, then?
Repository:
rC Clang
https://reviews.llvm.org/D47097
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
