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

Wed Sep 29 10:25:29 PDT 2021

clayborg added a comment.

In D110571#3025527 <https://reviews.llvm.org/D110571#3025527>, @jarin wrote:

> Hi, could you take a look at this change?
>
> Some discussion points:
>
> - In the ParseVariablesInFunctionContext method, we are using a lambda for the recursive parser. We could also use a function-local class or inner class of SymbolFileDWARF. Would any of these be preferable?

Real function is fine and preferable IMHO.

> - The variables created by ParseVariableDIE on abstract formal parameters are fairly strange, especially if a function gets inlined into two different functions. If that happens, then the parsed variable will refer to a symbol context that does not contain the variable DIE and a block can contain a variable that is not in the DIE of tree of the block. Is that a big problem? (Quick testing of this situation did not reveal any strange stack traces or `frame var` anomalies.) Unfortunately, there is no good way to provide the correct block and the correct function because LLDB does not parse functions and blocks for the abstract functions (i.e., for the DW_TAG_subroutines that are referenced by DW_AT_abstract_origin of concrete functions).

LLDB doesn't parse function definitions that have no low/high PC into lldb_private::Function with lldb_private::Block objects, it only does this for instances of functions with address ranges.

I would expect that the SymbolContextScope (one pointer that can identify the SymbolContext) for each variable parsed by ParseVariableDIE to point to the DW_TAG_variable that is in the function with the address range. Are you saying that the symbol context points to the definition?

> - The provided test only tests the case of an inlined function where some parameters are unused/omitted. Would it make sense to also provide tests for other interesting cases or would that be too much bloat? The particularly interesting cases are:
>   - Inlined function with all its parameters unused/omitted,
>   - Inlined function that is called from different top-level functions.
>   - Test correctness of the stack trace in the cases above.

Anything that tests what the compilers are emitting would be great to have if we can make them.

> - We could supply a test written in C, but it needs -O1 and is fairly sensitive to the meaning of -O1 (e.g., clang started inlining and omitting unsued inlined parameters only recently, so changes to -O1 could make a C test easily meaningless). Any concerns here?

It is really hard to make sure the compiler generates what you want for a test case as it will change over time and you might not end up testing what you think you are testing. The easiest way to avoid this is to emit the assembly from the compiler and then use that as the source for the test since that will guarantee the same output.

> - The provided test is a bit verbose, mostly because we wanted to mostly preserve the structure of the C compiler output. We could still cut the size of the test down by removing the main function in favour of _start and by removing all the file/line info. Would any of that make sense?

The image lookup as Pavel suggested is a good way to test info for various addresses without having to run the process or run to multiple locations.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110571