[PATCH] D61600: [DebugInfo] More precise variable range for stack locations

Nikola Prica via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 7 05:32:08 PDT 2019


NikolaPrica added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1359
+    // TODO: Add support for single value fragment locations.
+    if (Entries.size() == 1 && !MInsn->getDebugExpression()->isFragment()) {
+      const auto *End = HistoryMapEntries.back().isClobber()
----------------
dstenb wrote:
> NikolaPrica wrote:
> > dstenb wrote:
> > > It seems that this can give incorrect single value locations for cases where we have produced location list entries with empty ranges.
> > > 
> > > For example:
> > > 
> > > ```
> > >     frame-setup PUSH64r undef $rax, implicit-def $rsp, implicit $rsp
> > >     DBG_VALUE 123, $noreg, !12, !DIExpression(), debug-location !13 
> > >     DBG_VALUE 456, $noreg, !12, !DIExpression(), debug-location !13 
> > >     CALL64pcrel32 @foo, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, debug-location !14 
> > >     $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, debug-location !15                                                                   
> > >     $rcx = frame-destroy POP64r implicit-def $rsp, implicit $rsp, debug-location !15 
> > >     RETQ killed $eax, debug-location !15 
> > > ```
> > > 
> > > Here, the first debug value will result in a location list entry with an empty range, since it is immediately followed by the second, and such entries are omitted from the location list, resulting in a single entry in `Entries` for the constant value 456. However, `MInsn` is the first debug value, so we will create a single location description with the wrong value:
> > > 
> > > 
> > > ```
> > > 0x00000043:     DW_TAG_variable
> > >                   DW_AT_const_value	(123)
> > > ```
> > > 
> > > I'm not sure what the check should be to avoid these sort of cases. Perhaps check that all debug value entries in `HistoryMapEntries` are described by the same location, and have the same expression?
> > Such check could be return value of `buildLocationList`? Or we can just take the last `DebugInstr` from `HistoryMapEntries` which could be the last entry or the entry before the last one?
> > Or we can just take the last DebugInstr from HistoryMapEntries which could be the last entry or the entry before the last one?
> 
> That is perhaps possible. I tried to think if we can land in a situation where the last `DebugInstr` in `HistoryMapEntries` produces a location list entry with an empty range. For example something like this:
> 
> ```
> DBG_VALUE 123, $noreg, !12, !DIExpression(), debug-location !13 
> [...]
> DBG_VALUE 123, $noreg, !12, !DIExpression(), debug-location !13
> DBG_VALUE 456, $noreg, !12, !DIExpression(), debug-location !13 
> [end of function]
> ```
> 
> However, AFAIK we can't insert debug values after terminators currently (the machine verifier will complain about that), and with a presence of a terminator in the last block then the range will not be empty, so perhaps this is guaranteed? I'm not completely sure.
I'm not sure either.  But I suppose that such situation can appear for local variable that ends at some basic block? Maybe it is safer perform such verification in `buildLocationList` and return it as result?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61600/new/

https://reviews.llvm.org/D61600





More information about the llvm-commits mailing list