[all-commits] [llvm/llvm-project] 98c3dc: [lldb] Make GetDIENamesAndRanges() allow 0-valued ...

[David via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 98c3dc3fa748072fc42054fb90b295c2d5190bfe
      https://github.com/llvm/llvm-project/commit/98c3dc3fa748072fc42054fb90b295c2d5190bfe
  Author: David Stenberg <david.stenberg at ericsson.com>
  Date:   2023-03-06 (Mon, 06 Mar 2023)

  Changed paths:
    M lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
    M lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
    M lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
    M lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
    M lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
    M lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
    A lldb/test/Shell/SymbolFile/DWARF/Inputs/inlined-file0-line0-col0.yaml
    A lldb/test/Shell/SymbolFile/DWARF/inlined-file0-line0-col0.test

  Log Message:
  -----------
  [lldb] Make GetDIENamesAndRanges() allow 0-valued decl and call lines

In an upcoming patch, D142556, Clang is proposed to be changed to emit
line locations that are inlined at line 0. This clashed with the behavior of
GetDIENamesAndRanges() which used 0 as a default value to determine if
file, line or column numbers had been set. Users of that function then
checked for any non-0 values when setting up the call site:

  if (call_file != 0 || call_line != 0 || call_column != 0)
    [...]

which did not work with the Clang change since all three values then
could be 0.

This changes the function to use std::optional to catch non-set values
instead.

Reviewed By: clayborg

Differential Revision: https://reviews.llvm.org/D142552


  Commit: 12a7aea6b02288000ba6f5c477284b26df8dca01
      https://github.com/llvm/llvm-project/commit/12a7aea6b02288000ba6f5c477284b26df8dca01
  Author: David Stenberg <david.stenberg at ericsson.com>
  Date:   2023-03-06 (Mon, 06 Mar 2023)

  Changed paths:
    M llvm/lib/IR/DebugInfoMetadata.cpp
    A llvm/test/DebugInfo/MIR/X86/merge-inline-loc1.mir
    A llvm/test/DebugInfo/MIR/X86/merge-inline-loc2.mir
    A llvm/test/DebugInfo/MIR/X86/merge-inline-loc3.mir
    A llvm/test/DebugInfo/MIR/X86/merge-inline-loc4.mir
    M llvm/test/DebugInfo/X86/merge_inlined_loc.ll
    M llvm/unittests/IR/MetadataTest.cpp

  Log Message:
  -----------
  [DebugInfo] Merge partially matching chains of inlined locations

For example, if you have a chain of inlined funtions like this:

   1 #include <stdlib.h>
   2 int g1 = 4, g2 = 6;
   3
   4 static inline void bar(int q) {
   5   if (q > 5)
   6     abort();
   7 }
   8
   9 static inline void foo(int q) {
  10   bar(q);
  11 }
  12
  13 int main() {
  14   foo(g1);
  15   foo(g2);
  16   return 0;
  17 }

with optimizations you could end up with a single abort call for the two
inlined instances of foo(). When merging the locations for those inlined
instances you would previously end up with a 0:0 location in main().
Leaving out that inlined chain from the location for the abort call
could make troubleshooting difficult in some cases.

This patch changes DILocation::getMergedLocation() to try to handle such
cases. The function is rewritten to first find a common starting point
for the two locations (same subprogram and inlined-at location), and
then in reverse traverses the inlined-at chain looking for matches in
each subprogram. For each subprogram, the merge function will find the
nearest common scope for the two locations, and matching line and
column (or set them to 0 if not matching).

In the example above, you will for the abort call get a location in
bar() at 6:5, inlined in foo() at 10:3, inlined in main() at 0:0 (since
the two inlined functions are on different lines, but in the same
scope).

I have not seen anything in the DWARF standard that would disallow
inlining a non-zero location at 0:0 in the inlined-at function, and both
LLDB and GDB seem to accept these locations (with D142552 needed for
LLDB to handle cases where the file, line and column number are all 0).
One incompatibility with GDB is that it seems to ignore 0-line locations
in some cases, but I am not aware of any specific issue that this patch
produces related to that.

With x86-64 LLDB (trunk) you previously got:

  frame #0: 0x00007ffff7a44930 libc.so.6`abort
  frame #1: 0x00005555555546ec a.out`main at merge.c:0

and will now get:

  frame #0: 0x[...] libc.so.6`abort
  frame #1: 0x[...] a.out`main [inlined] bar(q=<unavailable>) at merge.c:6:5
  frame #2: 0x[...] a.out`main [inlined] foo(q=<unavailable>) at merge.c:10:3
  frame #3: 0x[...] a.out`main at merge.c:0

and with x86-64 GDB (11.1) you will get:

  (gdb) bt
  #0  0x00007ffff7a44930 in abort () from /lib64/libc.so.6
  #1  0x00005555555546ec in bar (q=<optimized out>) at merge.c:6
  #2  foo (q=<optimized out>) at merge.c:10
  #3  0x00005555555546ec in main ()

Reviewed By: aprantl, dblaikie

Differential Revision: https://reviews.llvm.org/D142556


Compare: https://github.com/llvm/llvm-project/compare/33a13f300bd6...12a7aea6b022