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

[Nikola Prica via Phabricator via llvm-commits]

NikolaPrica marked an inline comment as done.
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:
> 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?


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

https://reviews.llvm.org/D61600