[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

Wed Sep 29 23:21:54 PDT 2021

jarin added a comment.

In D110571#3031846 <https://reviews.llvm.org/D110571#3031846>, @clayborg wrote:

> In D110571#3031140 <https://reviews.llvm.org/D110571#3031140>, @jarin wrote:
>
>> For illustration:
>>
>>   0x100  DW_TAG_subprogram
>>            DW_AT_name "inlined_function"
>>            ... no DW_AT_low_pc here ...
>>   0x110    DW_TAG_formal_parameter
>>              DW_AT_name "unused"
>>              ...
>>          ...
>>   0x200  DW_TAG_subprogram
>>            DW_AT_name    ("top_level_function_with_address"
>>            DW_AT_low_pc  (0x3000)
>>            DW_AT_high_pc  (0x3100)
>>            ...
>>   0x210    DW_TAG_inlined_subroutine
>>              DW_AT_abstract_origin (0x100 "inlined_function")
>>              DW_AT_low_pc  (0x3010)
>>              DW_AT_high_pc  (0x3020)
>>              # Note the missing DW_TAG_formal_parameter here!
>>            NULL
>>          ...
>>   0x400  DW_TAG_subprogram
>>            DW_AT_name    ("another_top_level_function_with_address"
>>            DW_AT_low_pc  (0x5000)
>>            DW_AT_high_pc  (0x5100)
>>            ...
>>   0x410    DW_TAG_inlined_subroutine
>>              DW_AT_abstract_origin (0x100 "inlined_function")
>>              DW_AT_low_pc  (0x5030)
>>              DW_AT_high_pc  (0x5040)
>>              # Note the missing DW_TAG_formal_parameter here!
>>            NULL
>>          ...
>>
>> Here, we will create just one variable for the formal parameter "unused" (DIE offset 0x110). That variable's symbol context will be randomly one of the DW_TAG_inline subroutine blocks (either 0x210 or 0x410), and the variable will be inserted into two variable lists, one for the Block associated with the DIE at 0x210 and one for DIE associated with 0x410.
>
> I hear what you are saying, but I am not sure this will be happening. Let me explain: for each concrete DW_TAG_subprogram (0x200 and 0x400 in your example above), we create a unique lldb_private::Function object whose UserID will be 0x200 for "top_level_function_with_address" and 0x400 for "another_top_level_function_with_address". Each of those functions might be asked for their lldb_private::Block objects at some point and we should create unique lldb_private::Block for each DW_TAG_lexical_block and DW_TAG_inlined_subroutine that is contained within these unique DIEs. Each of these should now have a variable within the block that is a parameter whose name is "unused" and whose symbol context should be 0x210 for the 0x200 DIE, and 0x410 for the 0x400 DIE. So it would be great to make sure this happens correctly. From looking at the code, it seems like this should be happening correctly, but you might know better since you made these new modifications.

Hi Greg, thanks for the detailed description! What you say is indeed happening until the point "Each of these [blocks] should now have a variable within the block that is a parameter whose name is "unused" and whose symbol context should be 0x210 for the 0x200 DIE, and 0x410 for the 0x400 DIE.".

With my patch, LLDB creates only one variable here, its symbol context will be whichever block was parsed first and that variable will be inserted into the variable lists of blocks corresponding to 0x210 and 0x410. The reason why LLDB creates only one variable is that there is a cache of variables indexed by DIEs. When we call ParseVariableDIE <https://github.com/llvm/llvm-project/blob/941191aae4abaf88b176bec8cfe1c841a5832642/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L3087> first time for the variable "unused" (DIE 0x110) and symbol context 0x210, the variable gets created <https://github.com/llvm/llvm-project/blob/941191aae4abaf88b176bec8cfe1c841a5832642/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L3417> and inserted <https://github.com/llvm/llvm-project/blob/941191aae4abaf88b176bec8cfe1c841a5832642/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L3431> under the key 0x110. When we call ParseVariableDIE second time for "unused" (still 0x110) and symbol context 0x410, we will find and return <https://github.com/llvm/llvm-project/blob/941191aae4abaf88b176bec8cfe1c841a5832642/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L3097> the originally created variable (with symbol context 0x210!) and happily insert it into the block for 0x410.

>From what you say, this is not the desired behavior? If we wanted two instances of the variable (one for each block), we could change the DIE-to-variable cache <https://github.com/llvm/llvm-project/blob/47d66355ef9039a15b7265945e3deb331d7f9e05/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h#L324> to be indexed by a pair <symbol-context-DIE, variable-DIE>.

I have validated this with a simple example below, after adding printing of the variable address (var_sp.get()) at its creation point <https://github.com/llvm/llvm-project/blob/941191aae4abaf88b176bec8cfe1c841a5832642/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L3417> and printing of variable address (var_sp.get()) and list address (this) at variable list addition <https://github.com/llvm/llvm-project/blob/1f0bc617bdd11a4f3b0d820603b0089f81aca79b/lldb/source/Symbol/VariableList.cpp#L29>. Below is the code and the session log of lldb (with this patch applied).

Code (compiled with a recent clang with `-O1 -g`):

  #include <stdio.h>

  __attribute__((always_inline))
  void f(int unused) {
    printf("Hello");
  }

  __attribute__((noinline))
  void other() {
    f(1);
  }

  int main() {
    f(2);
    other();
    return 0;
  }

The lldb session (some fluff replaced with `...`):

  $ bin/lldb a.out
  ...
  (lldb) b f
  ...
  (lldb) r
  ...
  Created var 'unused' 0x7f6c48004f60 from DIE 0x4f   ### 0x4f is the formal_parameter from the abstract |f|
  Adding variable 0x7f6c48004f60 to the list 0x7f6c48004910  ### Inserting into the inlined block in |main|
  Adding variable 0x7f6c48004f60 to the list 0x7f6c4f0a4a70  ### Ignore, this is the output list for formatting
  Process ... stopped
  * thread #1, name = 'a.out', stop reason = breakpoint 1.3
      frame #0: 0x0000000000401151 a.out`main [inlined] f(unused=<unavailable>) at a.cc:5:3
  ...
  (lldb) c
  ...
  Adding variable 0x7f6c48004f60 to the list 0x7f6c4804b250 ### Inserting the same var into the block for |other|
  Adding variable 0x7f6c48004f60 to the list 0x7f6c4f0a4a70
  Process ... stopped
  * thread #1, name = 'a.out', stop reason = breakpoint 1.2
      frame #0: 0x0000000000401141 a.out`other() [inlined] f(unused=<unavailable>) at a.cc:5:3
  ...

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110571