[lldb] Add omitted abstract formal parameters in DWARF symbol files

[jarin]

Summary

This change fixes a problem introduced by clang change
described by https://reviews.llvm.org/D95617 and described by
https://bugs.llvm.org/show_bug.cgi?id=50076#c6, where inlined
functions omit the unused parameters both in the stack trace
and in frame var command. With this change, the parameters
are listed correctly in the stack trace and in frame var
command (the included test tests frame var).

This change adds parsing of formal parameters from the abstract
version of inlined functions and use those formal parameters if
they are missing from the concrete version.

[jarin]

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?
• 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).
• 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.
• 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?
• 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?

[labath]

I haven't looked at the actual code yet, so I could be off, but here are some thoughts.

In 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?

Yeah, what's the deal with that? Why wouldn't a regular function be sufficient? You can just pass things in arguments instead of closures or classes..

• 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).

Judging by your description, I take it you parse these variables only once, regardless of how many functions they are inlined in. Could we fix that my creating a fresh variable object for each inlined instance? Then it could maybe be correctly made to point to the actual block and function it is inlined into(?)

• 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.
• 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?
• 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?

I think you could get quite far by just testing the output of the "image lookup" command. That should give you list variables that are in scope for any particular address, and a bunch of details about each var, including the expression used to compute its value (not the value itself, obviously). The main advantage is that you wouldn't need a fully functional program, as you wouldn't be running anything. That would remove a lot of bloat, and also allow the test to run on non-x86-pc-linux hosts. Then, maybe it wouldn't be too messy to add the additional test cases you mention.

You can look at (e.g.) DW_AT_loclists_base.s for an example of a test case with image lookup and local variables.

After that, we could think about adding a c++ test case. Although tests with optimized code are tricky, it is often possible (with judicious use of noinline, always_inline and optnone attributes) to constrain the optimizer in a way that it has no choice but to do exactly what we want.

lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test

33–35

Including the actual message would make it clearer what is going on.

[labath]

(It would also make it easier to understand the code if you could paste some dwarfdump output which illustrates the kind of debug info we're dealing with.)

[jarin]

Just a few replies below; I am hoping to put the relevant code changes together tomorrow.

In D110571#3027173, @labath wrote:

I haven't looked at the actual code yet, so I could be off, but here are some thoughts.

In 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?

Yeah, what's the deal with that? Why wouldn't a regular function be sufficient? You can just pass things in arguments instead of closures or classes..

Right, I worked on a codebase where they used local classes for such things and in lldb I have seen lambdas. I actually do not have a strong preference, will rewrite this to use plain methods.

• 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).

Judging by your description, I take it you parse these variables only once, regardless of how many functions they are inlined in. Could we fix that my creating a fresh variable object for each inlined instance? Then it could maybe be correctly made to point to the actual block and function it is inlined into(?)

Yes, they are parsed only once. This is because there is a DIE->Variable map (see SymbolFileDWARF::GetDIEToVariable) that makes sure no DIE gets parsed twice. Are you suggesting to index the map with a pair <concrete-function-die, variable-die>? That would indeed create healthier structure (even though I could not spot any problems even with my current somewhat flawed approach).

• 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.
• 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?
• 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?

I think you could get quite far by just testing the output of the "image lookup" command. That should give you list variables that are in scope for any particular address, and a bunch of details about each var, including the expression used to compute its value (not the value itself, obviously). The main advantage is that you wouldn't need a fully functional program, as you wouldn't be running anything. That would remove a lot of bloat, and also allow the test to run on non-x86-pc-linux hosts. Then, maybe it wouldn't be too messy to add the additional test cases you mention.

You can look at (e.g.) DW_AT_loclists_base.s for an example of a test case with image lookup and local variables.

Makes sense, I really like that approach. Let me try to get that working.

After that, we could think about adding a c++ test case. Although tests with optimized code are tricky, it is often possible (with judicious use of noinline, always_inline and optnone attributes) to constrain the optimizer in a way that it has no choice but to do exactly what we want.

I have actually use the attributes when experimenting with the patch - if you think this is useful, I can certainly provide those tests.

[labath]

In D110571#3027283, @jarin wrote:

In D110571#3027173, @labath wrote:

Judging by your description, I take it you parse these variables only once, regardless of how many functions they are inlined in. Could we fix that my creating a fresh variable object for each inlined instance? Then it could maybe be correctly made to point to the actual block and function it is inlined into(?)

Yes, they are parsed only once. This is because there is a DIE->Variable map (see SymbolFileDWARF::GetDIEToVariable) that makes sure no DIE gets parsed twice. Are you suggesting to index the map with a pair <concrete-function-die, variable-die>? That would indeed create healthier structure (even though I could not spot any problems even with my current somewhat flawed approach).

I am not really suggesting anything (at least not yet). I'm just trying to map out the problem space. Having separate variables could be nice, but it could also be a needless complication. I've added some people to see what they make of this.

After that, we could think about adding a c++ test case. Although tests with optimized code are tricky, it is often possible (with judicious use of noinline, always_inline and optnone attributes) to constrain the optimizer in a way that it has no choice but to do exactly what we want.

I have actually use the attributes when experimenting with the patch - if you think this is useful, I can certainly provide those tests.

If you have something ready, feel free to include it and we can see what to do then. If you don't have them, maybe wait and see how the other approach pans out first...

[jarin]

Rewrote the recursive parser to use a plain method.

Pruned and annotated the test.

Added other test cases:

• all parameters unused,
• inlining from two different functions,
• stack trace.

This still uses frame variable rather than image lookup, mostly because frame variable tests better the user experience and the cognitive overhead for making the code runnable does not seem to be too high.

[labath]

In D110571#3030192, @jarin wrote:

This still uses frame variable rather than image lookup, mostly because frame variable tests better the user experience and the cognitive overhead for making the code runnable does not seem to be too high.

This is not really about "cognitive overhead", but "who can run this test". With a running process the answer is "a person with a linux x86 machine". With image lookup it's "everyone". It's also easier to debug failures in the test, as less code gets run before you get to the interesting part.

[jarin]

Added a C test (I have also verified that the C test fails without the SymbolFileDWARF patch).

[jarin]

In D110571#3030222, @labath wrote:

In D110571#3030192, @jarin wrote:

This still uses frame variable rather than image lookup, mostly because frame variable tests better the user experience and the cognitive overhead for making the code runnable does not seem to be too high.

This is not really about "cognitive overhead", but "who can run this test". With a running process the answer is "a person with a linux x86 machine". With image lookup it's "everyone". It's also easier to debug failures in the test, as less code gets run before you get to the interesting part.

Good point. Would you prefer to recast the test in terms of image lookup and get rid of checking the stack trace?

[jarin]

Changed the test to avoid running the process and use image lookup instead of frame variable.

I think I would still slightly prefer frame variable, mostly because there seem to be differences between what image lookup and frame variable show (image lookup omit variables that have DW_AT_location disjoint from the inspected address). As opposed to image lookup, frame variable tests more directly what the users would actually use.

[clayborg]

In 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.

[clayborg]

This looks good to me. Pavel, are you ok with the testing strategy with the updated tests?

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

3699–3700

Maybe expand this comment a bit. If I understand the problem correctly it might read something like:

DW_TAG_inline_subroutine objects may omit DW_TAG_formal_parameter in instances of the function when they are unused or ... . The current DW_TAG_inline_subroutine may refer to another DW_TAG_inline_subroutine or DW_TAG_subprogram that might actually have the definitions of the parameters and we need to include these so they show up in the variables for this function.

[jarin]

First of all, thank you, Greg and Pavel, for all the great feedback and discussion. I have followed all your suggestions (use plain method, use image lookup, added the additional tests). I have also packaged the C test, but as Greg notes I am not convinced it will keep testing what it's supposed to.

Now, let me answer the question regarding the context:

In D110571#3030913, @clayborg wrote:

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?

With this change, the symbol context for unused parameters points to one of the concrete inlined function block (DW_TAG_inlined_subroutine). That concrete inlined function will not contain that formal parameter because after the clang change https://reviews.llvm.org/D95617, unused formal parameters are deleted from their concrete inlined functions (i.e., from their DW_TAG_inlined_subroutine).

The point here is that if a function is inlined into two places, i.e., there are two corresponding DW_TAG_inlined_subroutines for the inlined function, we still create just one instance of Variable and its symbol context will be one randomly chosen DW_TAG_inlined_subroutine.

As Pavel suggested, an alternative would be creating one Variable instance per DW_TAG_inlined_subroutine. That would require some changes to other data structures because the existing code assumes there is just one Variable for each DIE (see SymbolFileDWARF::GetDIEToVariable).

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.

[clayborg]

In D110571#3031140, @jarin wrote:

First of all, thank you, Greg and Pavel, for all the great feedback and discussion. I have followed all your suggestions (use plain method, use image lookup, added the additional tests). I have also packaged the C test, but as Greg notes I am not convinced it will keep testing what it's supposed to.

Now, let me answer the question regarding the context:

In D110571#3030913, @clayborg wrote:

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?

With this change, the symbol context for unused parameters points to one of the concrete inlined function block (DW_TAG_inlined_subroutine). That concrete inlined function will not contain that formal parameter because after the clang change https://reviews.llvm.org/D95617, unused formal parameters are deleted from their concrete inlined functions (i.e., from their DW_TAG_inlined_subroutine).

The point here is that if a function is inlined into two places, i.e., there are two corresponding DW_TAG_inlined_subroutines for the inlined function, we still create just one instance of Variable and its symbol context will be one randomly chosen DW_TAG_inlined_subroutine.

As Pavel suggested, an alternative would be creating one Variable instance per DW_TAG_inlined_subroutine. That would require some changes to other data structures because the existing code assumes there is just one Variable for each DIE (see SymbolFileDWARF::GetDIEToVariable).

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.

[jarin]

In D110571#3031846, @clayborg wrote:

In 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 first time for the variable "unused" (DIE 0x110) and symbol context 0x210, the variable gets created and inserted under the key 0x110. When we call ParseVariableDIE second time for "unused" (still 0x110) and symbol context 0x410, we will find and return 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 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 and printing of variable address (var_sp.get()) and list address (this) at variable list addition. 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
...

[dblaikie]

• 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.

If anyone ever needs a hand constructing a stable debug info test case using clang (or other compilers for that matter) - I'm totally happy to help. It's quite possible to constrain the compiler enough and give it easy enough things to inline to make it pretty reliable - for instance for this sort of issue, I'd expect something like this is what I'd use to demonstrate a missing parameter:

__attribute__((optnone)) __attribute__((nodebug)) void use(int*) { }
inline void f1(int a, int b) {
  use(&b);
}
int main() {
  f1(5,  6);
}

This compiled with some optimizations (-O1 or above should be adequate) should result in a single concrete subprogram for main, a single inlined subroutine with a single formal parameter in the inlined subroutine (for 'b') and the abstract origin will have both 'a' and 'b'.

[jarin]

Improved the comment, as Greg suggested.

[labath]

Here's one more question. AIUI, lldb relies on the order of formal parameter declarations in dwarf to establish the the function signature (dwarf doesn't leave us much choice. This then affects how the function is printed in the backtrace, for instance. What will be the resulting order of arguments for these functions? I'm wondering if we don't need a two-pass algorithm, which first parses the arguments in the function declaration (to establish their order), and then do another pass over the concrete instance to fill in the missing information. (I'm sorry if you're doing this already, but I'm still too scared of the code to figure it out myself :P ).

In D110571#3031140, @jarin wrote:

I have also packaged the C test, but as Greg notes I am not convinced it will keep testing what it's supposed to.

Given that we have targeted asm tests, I am not particularly worried about that. In fact, one could consider that a feature, as it means we will be able to catch the cases where the compiler output for unused variables changes into something we do not support (that is one of the goals of API tests).

In D110571#3032599, @jarin wrote:

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 to be indexed by a pair <symbol-context-DIE, variable-DIE>.

Given that was Greg's (and yours, kinda) reaction as well. I guess we should do something like that. Even if it does not cause problems now, it could certainly cause them in the future, if something starts relying on the symbol_context_scope link making sense.

I am wondering about the best way to implement in though. Having a pair as a key seems very redundant to me. As we already know the block its going to end up in maybe we could somehow check if its already present there? Since blocks/functions don't generally have that many variables [citation needed], maybe even a simple iteration would suffice? (The situation is probably different for global variables, but those don't need the extra key.)

[jarin]

In D110571#3033078, @labath wrote:

Here's one more question. AIUI, lldb relies on the order of formal parameter declarations in dwarf to establish the the function signature (dwarf doesn't leave us much choice. This then affects how the function is printed in the backtrace, for instance. What will be the resulting order of arguments for these functions? I'm wondering if we don't need a two-pass algorithm, which first parses the arguments in the function declaration (to establish their order), and then do another pass over the concrete instance to fill in the missing information. (I'm sorry if you're doing this already, but I'm still too scared of the code to figure it out myself :P ).

The code already does the merging. For DW_TAG_inlined_subroutine, it first collects the formal_parameter list from from its abstract_origin and then it will parse/insert all the missing one while parsing the concrete instance. This will preserve the order of formal parameters. Now that I think about this, it might add some formal parameters after local variables, but I hope this is not a real problem for LLDB. If this is a problem, we could perhaps flush the abstract formal parameters whenever we encounter DW_TAG_variable.

In D110571#3032599, @jarin wrote:

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 to be indexed by a pair <symbol-context-DIE, variable-DIE>.

Given that was Greg's (and yours, kinda) reaction as well. I guess we should do something like that. Even if it does not cause problems now, it could certainly cause them in the future, if something starts relying on the symbol_context_scope link making sense.

I am wondering about the best way to implement in though. Having a pair as a key seems very redundant to me. As we already know the block its going to end up in maybe we could somehow check if its already present there? Since blocks/functions don't generally have that many variables [citation needed], maybe even a simple iteration would suffice? (The situation is probably different for global variables, but those don't need the extra key.)

Are you worried about code redundancy or memory redundancy? I do not think a pair would be much extra code. If you are worried about memory, we could also have a separate map for the abstract parameters - we always know whether we are inserting an abstract parameter or a concrete one. (I did not quite understand the idea with block lookup/iteration.)

An interesting question is whether the caching is needed at all in the context of functions - even without the cache, we should not parse block variables multiple times because the variables are already cached in their block's variable list. I actually verified that the cache never hits for function scoped variables on the LLDB test suite (with and without this patch). It does hit for global variables, but they take a different path now. So how would you feel about bypassing the cache when parsing in the function context? (I would basically move the caching code from SymbolFileDWARF::ParseVariableDIE to SymbolFileDWARF::ParseAndAppendGlobalVariable.)

[labath]

In D110571#3033282, @jarin wrote:

In D110571#3033078, @labath wrote:

Here's one more question. AIUI, lldb relies on the order of formal parameter declarations in dwarf to establish the the function signature (dwarf doesn't leave us much choice. This then affects how the function is printed in the backtrace, for instance. What will be the resulting order of arguments for these functions? I'm wondering if we don't need a two-pass algorithm, which first parses the arguments in the function declaration (to establish their order), and then do another pass over the concrete instance to fill in the missing information. (I'm sorry if you're doing this already, but I'm still too scared of the code to figure it out myself :P ).

The code already does the merging. For DW_TAG_inlined_subroutine, it first collects the formal_parameter list from from its abstract_origin and then it will parse/insert all the missing one while parsing the concrete instance. This will preserve the order of formal parameters. Now that I think about this, it might add some formal parameters after local variables, but I hope this is not a real problem for LLDB. If this is a problem, we could perhaps flush the abstract formal parameters whenever we encounter DW_TAG_variable.

Cool. I see you're way ahead of me. If you're not careful you may end up as the dwarf maintainer. :P

In D110571#3032599, @jarin wrote:

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 to be indexed by a pair <symbol-context-DIE, variable-DIE>.

Given that was Greg's (and yours, kinda) reaction as well. I guess we should do something like that. Even if it does not cause problems now, it could certainly cause them in the future, if something starts relying on the symbol_context_scope link making sense.

I am wondering about the best way to implement in though. Having a pair as a key seems very redundant to me. As we already know the block its going to end up in maybe we could somehow check if its already present there? Since blocks/functions don't generally have that many variables [citation needed], maybe even a simple iteration would suffice? (The situation is probably different for global variables, but those don't need the extra key.)

Are you worried about code redundancy or memory redundancy?

A little bit of both, but I would mostly say its because "it doesn't feel right". However,

I do not think a pair would be much extra code. If you are worried about memory, we could also have a separate map for the abstract parameters - we always know whether we are inserting an abstract parameter or a concrete one. (I did not quite understand the idea with block lookup/iteration.)

An interesting question is whether the caching is needed at all in the context of functions - even without the cache, we should not parse block variables multiple times because the variables are already cached in their block's variable list. I actually verified that the cache never hits for function scoped variables on the LLDB test suite (with and without this patch). It does hit for global variables, but they take a different path now. So how would you feel about bypassing the cache when parsing in the function context? (I would basically move the caching code from SymbolFileDWARF::ParseVariableDIE to SymbolFileDWARF::ParseAndAppendGlobalVariable.)

this sounds like an excellent idea. Make the code correct by deleting it, and saving some memory in the process. :)

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

397

The correct thing to do (despite the weird-looking name) is for the function to take a SmallVectorImpl<DWARFDIE> & argument.

[jarin]

Cache only global variables.

[clayborg]

In D110571#3033685, @jarin wrote:

Cache only global variables.

I concur that we should only cache globals. So now we have unique variables for each block and they have the right symbol context? LGTM if so and if the test suite is happy. Thanks for digging into this.

[jarin]

In D110571#3033481, @labath wrote:

In D110571#3033282, @jarin wrote:

In D110571#3033078, @labath wrote:

Here's one more question. AIUI, lldb relies on the order of formal parameter declarations in dwarf to establish the the function signature (dwarf doesn't leave us much choice. This then affects how the function is printed in the backtrace, for instance. What will be the resulting order of arguments for these functions? I'm wondering if we don't need a two-pass algorithm, which first parses the arguments in the function declaration (to establish their order), and then do another pass over the concrete instance to fill in the missing information. (I'm sorry if you're doing this already, but I'm still too scared of the code to figure it out myself :P ).

The code already does the merging. For DW_TAG_inlined_subroutine, it first collects the formal_parameter list from from its abstract_origin and then it will parse/insert all the missing one while parsing the concrete instance. This will preserve the order of formal parameters. Now that I think about this, it might add some formal parameters after local variables, but I hope this is not a real problem for LLDB. If this is a problem, we could perhaps flush the abstract formal parameters whenever we encounter DW_TAG_variable.

Cool. I see you're way ahead of me. If you're not careful you may end up as the dwarf maintainer. :P

Pavel, it is unfortunately really the case that with the current patch, the parameters might get interleaved with locals:

#include <stdio.h>

void f(int used, int unused) {
  int local = 1 + used;
  printf("Hello %i", local); // break here
}

int main() {
  f(4, 3);
  return 0;
}

Here is the LLDB session:

$ bin/lldb a.out
...
(lldb) b f
Breakpoint 1: 2 locations.
(lldb) r
...
* thread #1, name = 'a.out', stop reason = breakpoint 1.2
    frame #0: 0x0000000000401151 a.out`main [inlined] f(used=4, unused=<unavailable>) at a.cc:5:3
(lldb) frame var
(int) used = 4
(int) local = 5      <--- HERE, a local variables got between the parameters because we append unused parameters at the end.
(int) unused = <no location, value may have been optimized out>

Let me try to rewrite the code so that the trailing unused parameters are inserted after the last concrete parameter (or at the beginning of variable list if there are no concrete parameters). Let me know if you think it is unnecessary.

[labath]

Just a couple of questions inline.

In D110571#3035814, @jarin wrote:

• thread #1, name = 'a.out', stop reason = breakpoint 1.2 frame #0: 0x0000000000401151 a.out`main [inlined] f(used=4, unused=<unavailable>) at a.cc:5:3

(lldb) frame var
(int) used = 4
(int) local = 5 <--- HERE, a local variables got between the parameters because we append unused parameters at the end.
(int) unused = <no location, value may have been optimized out>

Let me try to rewrite the code so that the trailing unused parameters are inserted after the last concrete parameter (or at the beginning of variable list if there are no concrete parameters). Let me know if you think it is unnecessary.

TBH, I have no idea. The function description comes out right, so it seems at least some parts of lldb are prepared to handle this. I suppose it would be nicer if they were grouped together, but if it makes the code significantly more complex, then I probably wouldn't bother.

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

3659–3660

I'm wondering if one could pass a single ArrayRef<DWARFDIE> & argument instead of the array+index pair. FWICS, you're processing the list in a left-to-right fashion, which seems ideal for ArrayRef::drop_front. WDYT?

3671–3672

All of these abstract origin loops make me uneasy. They make it very easy to hang (whether deliberately or not) the debugger with a bit of incorrect dwarf (self-referencing DIEs). Do we actually know of any abstract_origin chains?
It's not really clear to me what would be the right interpretation of that, so I can't even say whether this algorithm would be correct there.

Maybe just stick to a single "dereference" ?

lldb/test/API/functionalities/unused-inlined-parameters/TestUnusedInlinedParameters.py

18

Maybe we could check something else as well... Do GetDescription or str(value) return something reasonable here?

lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test

2

You should be able to drop this now.

34

You could add CHECK-NOT: partial here

[jarin]

Addressed reviewer comments, separated merging of the abstract parameters into a function, prevented interleaving of parameters with locals.

[jarin]

Thank you for the great comments, Pavel. I took a stab at merging the parameters without interleaving them with the locals. Let me know what you think; I can certainly put this back to the original state if you think this is a change for the worse.

(I am sorry for the churn, but I feel the code is fairly subtle and would like to leave it in a state we are all happy with.)

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

3659–3660

I have replaced the in place-merging with a separate merging function, so this should not be relevant anymore.

lldb/test/API/functionalities/unused-inlined-parameters/TestUnusedInlinedParameters.py

18

Actually, the most important thing is the type here, so this was quite deliberate.

GetDescription returns (void *) unused1 = <no location, value may have been optimized out>\n\n, but I am not sure if we can safely require that all future versions/platforms optimize the parameter out.

[labath]

Looks good, apart from some stylistic comments inline. Thank you for taking the time to do this right.

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

3566

In llvm, we prefer static functions over anonymous namespaces. Theoretically, you could keep the anonymous namespace around the using declaration (per https://llvm.org/docs/CodingStandards.html#anonymous-namespaces, as those can't use static), though I would actually probably prefer DIEArray type defined in DIERef.h over a custom type.

3609

I would assume this is redundant, as an invalid DIE will never match abstract_child.

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

401

When you're not mutating the vector, the usual argument type is ArrayRef<T>

402

We generally do not put a const qualifier on by-value arguments (it's pretty useless).

(I see it's present on other functions too, but I don't know if they were introduced by you or you're just propagating them.)

lldb/test/API/functionalities/unused-inlined-parameters/TestUnusedInlinedParameters.py

18

I don't feel strongly about it, but I would say that this function is so simple than any optimizer worthy of that name should be able to optimize those arguments away. I might replace printf with a noinline/optnone function though, to avoid any libc shenanigans.

[jarin]

Addressed Pavel's comments.

[jarin]

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

3566

Changed to static function, DIEArray (interestingly, this file actually starts with anonymous namespace, see line 121).

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

402

Copy pasta, unfortunately.

lldb/test/API/functionalities/unused-inlined-parameters/TestUnusedInlinedParameters.py

18

I do not feel too strongly about this eiher.

[labath]

Let's ship this.

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

3566

We're not always very good at following llvm policies, although I would say that this particular namespace is mostly ok-ish -- it mostly contains type declarations (classes, enums), where static does not work.

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

protected:
   lldb_private::Type *ResolveTypeUID(const DWARFDIE &die,
                                      bool assert_not_being_parsed);
 
   lldb_private::Type *ResolveTypeUID(const DIERef &die_ref);
 
   lldb::VariableSP ParseVariableDIE(const lldb_private::SymbolContext &sc,
                                     const DWARFDIE &die,
                                     const lldb::addr_t func_low_pc);
+  lldb::VariableSP ParseVariableDIECached(const lldb_private::SymbolContext &sc,
+                                          const DWARFDIE &die);
 
   void
   ParseAndAppendGlobalVariable(const lldb_private::SymbolContext &sc,
                                const DWARFDIE &die,
                                lldb_private::VariableList &cc_variable_list);
 
   size_t ParseVariablesInFunctionContext(const lldb_private::SymbolContext &sc,
                                          const DWARFDIE &die,
                                          const lldb::addr_t func_low_pc);
 
   size_t ParseVariablesInFunctionContextRecursive(
       const lldb_private::SymbolContext &sc, const DWARFDIE &die,
-      const lldb::addr_t func_low_pc,
-      lldb_private::VariableList &variable_list);
+      lldb::addr_t func_low_pc, DIEArray &accumulator);
+
+  size_t PopulateBlockVariableList(lldb_private::VariableList &variable_list,
+                                   const lldb_private::SymbolContext &sc,
+                                   llvm::ArrayRef<DIERef> variable_dies,
+                                   lldb::addr_t func_low_pc);
+
+  DIEArray MergeBlockAbstractParameters(const DWARFDIE &block_die,
+                                        DIEArray &&variable_dies);
 
   bool ClassOrStructIsVirtual(const DWARFDIE &die);
 
   // Given a die_offset, figure out the symbol context representing that die.
   bool ResolveFunction(const DWARFDIE &die, bool include_inlines,
                        lldb_private::SymbolContextList &sc_list);
 
   /// Resolve functions and (possibly) blocks for the given file address and a

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

if (sc.function) {
       uint32_t vars_added = 0;
       VariableListSP variables(sc.comp_unit->GetVariableList(false));
 
       if (variables.get() == nullptr) {
         variables = std::make_shared<VariableList>();
         sc.comp_unit->SetVariableList(variables);
 
         m_index->GetGlobalVariables(*dwarf_cu, [&](DWARFDIE die) {
-          VariableSP var_sp(ParseVariableDIE(sc, die, LLDB_INVALID_ADDRESS));
+          VariableSP var_sp(ParseVariableDIECached(sc, die));
           if (var_sp) {
             variables->AddVariableIfUnique(var_sp);
             ++vars_added;
           }
           return true;
         });
       }
       return vars_added;
     }
   }
   return 0;
 }
 
+VariableSP SymbolFileDWARF::ParseVariableDIECached(const SymbolContext &sc,
+                                                   const DWARFDIE &die) {
+  if (!die)
+    return nullptr;
+
+  DIEToVariableSP &die_to_variable = die.GetDWARF()->GetDIEToVariable();
+
+  VariableSP var_sp = die_to_variable[die.GetDIE()];
+  if (var_sp)
+    return var_sp;
+
+  var_sp = ParseVariableDIE(sc, die, LLDB_INVALID_ADDRESS);
+  if (var_sp) {
+    die_to_variable[die.GetDIE()] = var_sp;
+    if (DWARFDIE spec_die = die.GetReferencedDIE(DW_AT_specification))
+      die_to_variable[spec_die.GetDIE()] = var_sp;
+  }
+  return var_sp;
+}
+
 VariableSP SymbolFileDWARF::ParseVariableDIE(const SymbolContext &sc,
                                              const DWARFDIE &die,
                                              const lldb::addr_t func_low_pc) {
   if (die.GetDWARF() != this)
     return die.GetDWARF()->ParseVariableDIE(sc, die, func_low_pc);
 
   if (!die)
     return nullptr;
 
-  if (VariableSP var_sp = GetDIEToVariable()[die.GetDIE()])
-    return var_sp; // Already been parsed!
-
   const dw_tag_t tag = die.Tag();
   ModuleSP module = GetObjectFile()->GetModule();
 
   if (tag != DW_TAG_variable && tag != DW_TAG_constant &&
       (tag != DW_TAG_formal_parameter || !sc.function))
     return nullptr;
 
   DWARFAttributes attributes;
   const size_t num_attributes = die.GetAttributes(attributes);
-  DWARFDIE spec_die;
-  VariableSP var_sp;
   const char *name = nullptr;
   const char *mangled = nullptr;
   Declaration decl;
   DWARFFormValue type_die_form;
   DWARFExpression location;
   bool is_external = false;
   bool is_artificial = false;
   DWARFFormValue const_value_form, location_form;
case DW_AT_external:
       is_external = form_value.Boolean();
       break;
     case DW_AT_const_value:
       const_value_form = form_value;
       break;
     case DW_AT_location:
       location_form = form_value;
       break;
-    case DW_AT_specification:
-      spec_die = form_value.Reference();
-      break;
     case DW_AT_start_scope:
       // TODO: Implement this.
       break;
     case DW_AT_artificial:
       is_artificial = form_value.Boolean();
       break;
     case DW_AT_declaration:
     case DW_AT_description:
     case DW_AT_endianity:
     case DW_AT_segment:
+    case DW_AT_specification:
     case DW_AT_visibility:
     default:
     case DW_AT_abstract_origin:
     case DW_AT_sibling:
       break;
     }
   }
 
if (is_static_lifetime) {
           // which needs to be linked up correctly.
           const lldb::addr_t exe_file_addr =
               debug_map_symfile->LinkOSOFileAddress(this, location_DW_OP_addr);
           if (exe_file_addr != LLDB_INVALID_ADDRESS) {
             // Update the file address for this variable
             location.Update_DW_OP_addr(exe_file_addr);
           } else {
             // Variable didn't make it into the final executable
-            return var_sp;
+            return nullptr;
           }
         }
       }
     } else {
       if (location_is_const_value_data &&
           die.GetDIE()->IsGlobalOrStaticScopeVariable())
         scope = eValueTypeVariableStatic;
       else {
case DW_TAG_lexical_block:
       break;
 
     default:
       symbol_context_scope = sc.comp_unit;
       break;
     }
   }
 
-  if (symbol_context_scope) {
-    auto type_sp = std::make_shared<SymbolFileType>(
-        *this, GetUID(type_die_form.Reference()));
+  if (!symbol_context_scope) {
+    // Not ready to parse this variable yet. It might be a global or static
+    // variable that is in a function scope and the function in the symbol
+    // context wasn't filled in yet
+    return nullptr;
+  }
+
 
-    if (use_type_size_for_value && type_sp->GetType())
-      location.UpdateValue(
-          const_value_form.Unsigned(),
+    location.UpdateValue(const_value_form.Unsigned(),
                          type_sp->GetType()->GetByteSize(nullptr).getValueOr(0),
                          die.GetCU()->GetAddressByteSize());
 
-    var_sp = std::make_shared<Variable>(
-        die.GetID(), name, mangled, type_sp, scope, symbol_context_scope,
-        scope_ranges, &decl, location, is_external, is_artificial,
-        location_is_const_value_data, is_static_member);
-  } else {
-    // Not ready to parse this variable yet. It might be a global or static
-    // variable that is in a function scope and the function in the symbol
-    // context wasn't filled in yet
-    return var_sp;
-  }
-  // Cache var_sp even if NULL (the variable was just a specification or was
-  // missing vital information to be able to be displayed in the debugger
-  // (missing location due to optimization, etc)) so we don't re-parse this
-  // DIE over and over later...
-  GetDIEToVariable()[die.GetDIE()] = var_sp;
-  if (spec_die)
-    GetDIEToVariable()[spec_die.GetDIE()] = var_sp;
-
-  return var_sp;
+  return std::make_shared<Variable>(
 }
 
 DWARFDIE
 SymbolFileDWARF::FindBlockContainingSpecification(
     const DIERef &func_die_ref, dw_offset_t spec_block_die_offset) {
   // Give the concrete function die specified by "func_die_offset", find the
   // concrete block whose DW_AT_specification or DW_AT_abstract_origin points
   // to "spec_block_die_offset"
void SymbolFileDWARF::ParseAndAppendGlobalVariable(
 
   // Check to see if we have already parsed this variable or constant?
   VariableSP var_sp = GetDIEToVariable()[die.GetDIE()];
   if (var_sp) {
     cc_variable_list.AddVariableIfUnique(var_sp);
     return;
   }
 
-  // We haven't already parsed it, lets do that now.
+  // We haven't parsed the variable yet, lets do that now. Also, let us include
+  // the variable in the relevant compilation unit's variable list, if it
+  // exists.
   VariableListSP variable_list_sp;
   DWARFDIE sc_parent_die = GetParentSymbolContextDIE(die);
   dw_tag_t parent_tag = sc_parent_die.Tag();
   switch (parent_tag) {
   case DW_TAG_compile_unit:
   case DW_TAG_partial_unit:
     if (sc.comp_unit != nullptr) {
       variable_list_sp = sc.comp_unit->GetVariableList(false);
void SymbolFileDWARF::ParseAndAppendGlobalVariable(
   default:
     GetObjectFile()->GetModule()->ReportError(
         "didn't find appropriate parent DIE for variable list for "
         "0x%8.8" PRIx64 " %s.\n",
         die.GetID(), die.GetTagAsCString());
     return;
   }
 
-  var_sp = ParseVariableDIE(sc, die, LLDB_INVALID_ADDRESS);
+  var_sp = ParseVariableDIECached(sc, die);
   if (!var_sp)
     return;
 
   cc_variable_list.AddVariableIfUnique(var_sp);
   if (variable_list_sp)
     variable_list_sp->AddVariableIfUnique(var_sp);
 }
 
+DIEArray
+SymbolFileDWARF::MergeBlockAbstractParameters(const DWARFDIE &block_die,
+                                              DIEArray &&variable_dies) {
+  // DW_TAG_inline_subroutine objects may omit DW_TAG_formal_parameter in
+  // instances of the function when they are unused (i.e., the parameter's
+  // location list would be empty). The current DW_TAG_inline_subroutine may
+  // refer to another DW_TAG_subprogram that might actually have the definitions
+  // of the parameters and we need to include these so they show up in the
+  // variables for this function (for example, in a stack trace). Let us try to
+  // find the abstract subprogram that might contain the parameter definitions
+  // and merge with the concrete parameters.
+
+  // Nothing to merge if the block is not an inlined function.
+  if (block_die.Tag() != DW_TAG_inlined_subroutine) {
+    return std::move(variable_dies);
+  }
+
+  // Nothing to merge if the block does not have abstract parameters.
+  DWARFDIE abs_die = block_die.GetReferencedDIE(DW_AT_abstract_origin);
+  if (!abs_die || abs_die.Tag() != DW_TAG_subprogram ||
+      !abs_die.HasChildren()) {
+    return std::move(variable_dies);
+  }
+
+  // For each abstract parameter, if we have its concrete counterpart, insert
+  // it. Otherwise, insert the abstract parameter.
+  DIEArray::iterator concrete_it = variable_dies.begin();
+  DWARFDIE abstract_child = abs_die.GetFirstChild();
+  DIEArray merged;
+  bool did_merge_abstract = false;
+  for (; abstract_child; abstract_child = abstract_child.GetSibling()) {
+    if (abstract_child.Tag() == DW_TAG_formal_parameter) {
+      if (concrete_it == variable_dies.end() ||
+          GetDIE(*concrete_it).Tag() != DW_TAG_formal_parameter) {
+        // We arrived at the end of the concrete parameter list, so all
+        // the remaining abstract parameters must have been omitted.
+        // Let us insert them to the merged list here.
+        merged.push_back(*abstract_child.GetDIERef());
+        did_merge_abstract = true;
+        continue;
+      }
+
+      DWARFDIE origin_of_concrete =
+          GetDIE(*concrete_it).GetReferencedDIE(DW_AT_abstract_origin);
+      if (origin_of_concrete == abstract_child) {
+        // The current abstract paramater is the origin of the current
+        // concrete parameter, just push the concrete parameter.
+        merged.push_back(*concrete_it);
+        ++concrete_it;
+      } else {
+        // Otherwise, the parameter must have been omitted from the concrete
+        // function, so insert the abstract one.
+        merged.push_back(*abstract_child.GetDIERef());
+        did_merge_abstract = true;
+      }
+    }
+  }
+
+  // Shortcut if no merging happened.
+  if (!did_merge_abstract)
+    return std::move(variable_dies);
+
+  // We inserted all the abstract parameters (or their concrete counterparts).
+  // Let us insert all the remaining concrete variables to the merged list.
+  // During the insertion, let us check there are no remaining concrete
+  // formal parameters. If that's the case, then just bailout from the merge -
+  // the variable list is malformed.
+  for (; concrete_it != variable_dies.end(); ++concrete_it) {
+    if (GetDIE(*concrete_it).Tag() == DW_TAG_formal_parameter) {
+      return std::move(variable_dies);
+    }
+    merged.push_back(*concrete_it);
+  }
+  return std::move(merged);
+}
+
 size_t SymbolFileDWARF::ParseVariablesInFunctionContext(
     const SymbolContext &sc, const DWARFDIE &die,
     const lldb::addr_t func_low_pc) {
   if (!die || !sc.function)
     return 0;
 
-  VariableList empty_variable_list;
-  // Since |die| corresponds to a Block instance, the recursive call will get
-  // a variable list from the block. |empty_variable_list| should remain empty.
+  DIEArray dummy_block_variables; // The recursive call should not add anything
+                                  // to this vector because |die| should be a
+                                  // subprogram, so all variables will be added
+                                  // to the subprogram's list.
   return ParseVariablesInFunctionContextRecursive(sc, die, func_low_pc,
-                                                  empty_variable_list);
+                                                  dummy_block_variables);
 }
 
+// This method parses all the variables in the blocks in the subtree of |die|,
+// and inserts them to the variable list for all the nested blocks.
+// The uninserted variables for the current block are accumulated in
+// |accumulator|.
 size_t SymbolFileDWARF::ParseVariablesInFunctionContextRecursive(
     const lldb_private::SymbolContext &sc, const DWARFDIE &die,
-    const lldb::addr_t func_low_pc, VariableList &variable_list) {
+    lldb::addr_t func_low_pc, DIEArray &accumulator) {
   size_t vars_added = 0;
   dw_tag_t tag = die.Tag();
 
   if ((tag == DW_TAG_variable) || (tag == DW_TAG_constant) ||
       (tag == DW_TAG_formal_parameter)) {
-    VariableSP var_sp(ParseVariableDIE(sc, die, func_low_pc));
-    if (var_sp) {
-      variable_list.AddVariableIfUnique(var_sp);
-      ++vars_added;
-    }
+    accumulator.push_back(*die.GetDIERef());
   }
 
   switch (tag) {
   case DW_TAG_subprogram:
   case DW_TAG_inlined_subroutine:
   case DW_TAG_lexical_block: {
     // If we start a new block, compute a new block variable list and recurse.
     Block *block =
         sc.function->GetBlock(/*can_create=*/true).FindBlockByID(die.GetID());
     if (block == nullptr) {
       // This must be a specification or abstract origin with a
       // concrete block counterpart in the current function. We need
       // to find the concrete block so we can correctly add the
       // variable to it.
       const DWARFDIE concrete_block_die = FindBlockContainingSpecification(
           GetDIE(sc.function->GetID()), die.GetOffset());
       if (concrete_block_die)
         block = sc.function->GetBlock(/*can_create=*/true)
                     .FindBlockByID(concrete_block_die.GetID());
     }
 
     if (block == nullptr)
       return 0;
 
     const bool can_create = false;
     VariableListSP block_variable_list_sp =
         block->GetBlockVariableList(can_create);
     if (block_variable_list_sp.get() == nullptr) {
       block_variable_list_sp = std::make_shared<VariableList>();
       block->SetVariableList(block_variable_list_sp);
     }
+
+    DIEArray block_variables;
     for (DWARFDIE child = die.GetFirstChild(); child;
          child = child.GetSibling()) {
       vars_added += ParseVariablesInFunctionContextRecursive(
-          sc, child, func_low_pc, *block_variable_list_sp);
+          sc, child, func_low_pc, block_variables);
     }
-
+    block_variables =
+        MergeBlockAbstractParameters(die, std::move(block_variables));
+    vars_added += PopulateBlockVariableList(*block_variable_list_sp, sc,
+                                            block_variables, func_low_pc);
     break;
   }
 
   default:
-    // Recurse to children with the same variable list.
+    // Recurse to children with the same variable accumulator.
     for (DWARFDIE child = die.GetFirstChild(); child;
          child = child.GetSibling()) {
       vars_added += ParseVariablesInFunctionContextRecursive(
-          sc, child, func_low_pc, variable_list);
+          sc, child, func_low_pc, accumulator);
     }
-
     break;
   }
 
   return vars_added;
 }
 
+size_t SymbolFileDWARF::PopulateBlockVariableList(
+    VariableList &variable_list, const lldb_private::SymbolContext &sc,
+    llvm::ArrayRef<DIERef> variable_dies, lldb::addr_t func_low_pc) {
+  // Parse the variable DIEs and insert them to the list.
+  for (auto &die : variable_dies) {
+    if (VariableSP var_sp = ParseVariableDIE(sc, GetDIE(die), func_low_pc)) {
+      variable_list.AddVariableIfUnique(var_sp);
+    }
+  }
+  return variable_dies.size();
+}
+
 /// Collect call site parameters in a DW_TAG_call_site DIE.
 static CallSiteParameterArray
 CollectCallSiteParameters(ModuleSP module, DWARFDIE call_site_die) {
   CallSiteParameterArray parameters;
   for (DWARFDIE child : call_site_die.children()) {
     if (child.Tag() != DW_TAG_call_site_parameter &&
         child.Tag() != DW_TAG_GNU_call_site_parameter)
       continue;

lldb/test/API/functionalities/unused-inlined-parameters/Makefile

+C_SOURCES := main.c
+CFLAGS_EXTRAS := -O1
+
+include Makefile.rules

lldb/test/API/functionalities/unused-inlined-parameters/TestUnusedInlinedParameters.py

+"""
+Test that unused inlined parameters are displayed.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestUnusedInlinedParameters(TestBase):
+    mydir = TestBase.compute_mydir(__file__)
+
+    def test_unused_inlined_parameters(self):
+        self.build()
+        lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.c"))
+
+        # For the unused parameters, only check the types.
+        self.assertIn("(void *) unused1 = <no location, value may have been optimized out>",
+                      lldbutil.get_description(self.frame().FindVariable("unused1")))
+        self.assertEqual(42, self.frame().FindVariable("used").GetValueAsUnsigned())
+        self.assertIn("(int) unused2 = <no location, value may have been optimized out>",
+                      lldbutil.get_description(self.frame().FindVariable("unused2")))

lldb/test/API/functionalities/unused-inlined-parameters/main.c

+#include <stdio.h>
+
+__attribute__((optnone)) __attribute__((nodebug)) void use(int used) {}
+
+__attribute__((always_inline)) void f(void *unused1, int used, int unused2) {
+  use(used); // break here
+}
+
+int main(int argc, char **argv) {
+  f(argv, 42, 1);
+  return 0;
+}
 No newline at end of file

lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/unused-inlined-params.s

+# The below program is roughly derived from the following C program.
+# To see the annotated debug info, look for the section 
+# '.section    .debug_info' below.
+#
+# __attribute__((always_inline))
+# void f(void* unused1, int used, int unused2, int partial, int unused3) {
+#   used += partial;
+#   printf("f %i", partial);
+#   printf("f %i", used);   // |partial| is not live at this line.
+# }
+#
+# void g(int unused) {
+#   printf("Hello");
+# }
+#
+# __attribute__((noinline))
+# void other() {
+#   f(nullptr, 1, 0, 2, 0);
+# }
+#
+# int main(int argc, char** argv) {
+#   f(argv, 42, 1, argc, 2);
+#   g(1);
+#   other();
+#   return 0;
+# }
+
+    .text
+    .file    "unused-inlined-params.c"
+
+.Lcu_begin:
+
+    .globl    other
+other:
+    nop
+.Linlined_f_in_other:
+break_at_inlined_f_in_other:
+    callq    printf        # Omitted the setup of arguments.
+.Linlined_f_in_other_between_printfs:
+    callq    printf        # Omitted the setup of arguments.
+.Linlined_f_in_other_end:
+    retq
+.Lother_end:
+    .size    other, .Lother_end-other
+
+    .globl    main
+main:
+    .file    1 "/example" "unused-inlined-params.c"
+    movl    $1, %esi
+.Linlined_f:
+break_at_inlined_f_in_main:
+    leal    42(%rsi), %ebx
+.Linlined_f_before_printf:
+    callq    printf        # Omitted the setup of arguments.
+.Linlined_f_between_printfs:
+break_at_inlined_f_in_main_between_printfs:
+    callq    printf        # Omitted the setup of arguments.
+.Linlined_f_end:
+.Linlined_g:
+break_at_inlined_g_in_main:
+    callq    printf        # Omitted the setup of arguments.
+.Linlined_g_end:
+    callq    other
+    retq
+.Lmain_end:
+    .size    main, .Lmain_end-main
+
+# Dummy printf implementation.
+printf:
+    retq
+
+# Simple entry point to make the linker happy.
+    .globl  _start
+_start:
+    jmp     main
+
+.Lcu_end:
+
+
+    .section    .debug_loc,"",@progbits
+.Ldebug_loc_partial:
+    .quad    .Linlined_f-.Lcu_begin
+    .quad    .Linlined_f_between_printfs-.Lcu_begin
+    .short   1                               # Loc expr size
+    .byte    84                              # super-register DW_OP_reg4
+    .quad    0
+    .quad    0
+.Ldebug_loc_used:
+    .quad    .Linlined_f-.Lcu_begin
+    .quad    .Linlined_f_before_printf-.Lcu_begin
+    .short   3                               # Loc expr size
+    .byte    17                              # DW_OP_consts
+    .byte    42                              # value
+    .byte    159                             # DW_OP_stack_value
+    .quad    .Linlined_f_before_printf-.Lcu_begin
+    .quad    .Linlined_f_end-.Lcu_begin
+    .short   1                               # Loc expr size
+    .byte    83                              # super-register DW_OP_reg3
+    .quad    0
+    .quad    0
+.Ldebug_loc_partial_in_other:
+    .quad    .Linlined_f_in_other-.Lcu_begin
+    .quad    .Linlined_f_in_other_between_printfs-.Lcu_begin
+    .short   3                               # Loc expr size
+    .byte    17                              # DW_OP_consts
+    .byte    2                               # value
+    .byte    159                             # DW_OP_stack_value
+    .quad    0
+    .quad    0
+.Ldebug_loc_used_in_other:
+    .quad    .Linlined_f_in_other-.Lcu_begin
+    .quad    .Linlined_f_in_other_end-.Lcu_begin
+    .short   3                               # Loc expr size
+    .byte    17                              # DW_OP_consts
+    .byte    1                               # value
+    .byte    159                             # DW_OP_stack_value
+    .quad    0
+    .quad    0
+
+    .section    .debug_abbrev,"",@progbits
+    .byte    1                               # Abbreviation Code
+    .byte    17                              # DW_TAG_compile_unit
+    .byte    1                               # DW_CHILDREN_yes
+    .byte    3                               # DW_AT_name
+    .byte    14                              # DW_FORM_strp
+    .byte    16                              # DW_AT_stmt_list
+    .byte    23                              # DW_FORM_sec_offset
+    .byte    17                              # DW_AT_low_pc
+    .byte    1                               # DW_FORM_addr
+    .byte    18                              # DW_AT_high_pc
+    .byte    6                               # DW_FORM_data4
+    .byte    0                               # EOM(1)
+    .byte    0                               # EOM(2)
+
+    .byte    4                               # Abbreviation Code
+    .byte    5                               # DW_TAG_formal_parameter
+    .byte    0                               # DW_CHILDREN_no
+    .byte    2                               # DW_AT_location
+    .byte    23                              # DW_FORM_sec_offset
+    .byte    49                              # DW_AT_abstract_origin
+    .byte    19                              # DW_FORM_ref4
+    .byte    0                               # EOM(1)
+    .byte    0                               # EOM(2)
+
+    .byte    5                               # Abbreviation Code
+    .byte    46                              # DW_TAG_subprogram
+    .byte    1                               # DW_CHILDREN_yes
+    .byte    3                               # DW_AT_name
+    .byte    14                              # DW_FORM_strp
+    .byte    58                              # DW_AT_decl_file
+    .byte    11                              # DW_FORM_data1
+    .byte    59                              # DW_AT_decl_line
+    .byte    11                              # DW_FORM_data1
+    .byte    39                              # DW_AT_prototyped
+    .byte    25                              # DW_FORM_flag_present
+    .byte    63                              # DW_AT_external
+    .byte    25                              # DW_FORM_flag_present
+    .byte    32                              # DW_AT_inline
+    .byte    11                              # DW_FORM_data1
+    .byte    0                               # EOM(1)
+    .byte    0                               # EOM(2)
+
+    .byte    6                               # Abbreviation Code
+    .byte    5                               # DW_TAG_formal_parameter
+    .byte    0                               # DW_CHILDREN_no
+    .byte    3                               # DW_AT_name
+    .byte    14                              # DW_FORM_strp
+    .byte    58                              # DW_AT_decl_file
+    .byte    11                              # DW_FORM_data1
+    .byte    59                              # DW_AT_decl_line
+    .byte    11                              # DW_FORM_data1
+    .byte    73                              # DW_AT_type
+    .byte    19                              # DW_FORM_ref4
+    .byte    0                               # EOM(1)
+    .byte    0                               # EOM(2)
+
+    .byte    7                               # Abbreviation Code
+    .byte    15                              # DW_TAG_pointer_type
+    .byte    0                               # DW_CHILDREN_no
+    .byte    0                               # EOM(1)
+    .byte    0                               # EOM(2)
+
+    .byte    8                               # Abbreviation Code
+    .byte    36                              # DW_TAG_base_type
+    .byte    0                               # DW_CHILDREN_no
+    .byte    3                               # DW_AT_name
+    .byte    14                              # DW_FORM_strp
+    .byte    62                              # DW_AT_encoding
+    .byte    11                              # DW_FORM_data1
+    .byte    11                              # DW_AT_byte_size
+    .byte    11                              # DW_FORM_data1
+    .byte    0                               # EOM(1)
+    .byte    0                               # EOM(2)
+
+    .byte    9                               # Abbreviation Code
+    .byte    46                              # DW_TAG_subprogram
+    .byte    1                               # DW_CHILDREN_yes
+    .byte    17                              # DW_AT_low_pc
+    .byte    1                               # DW_FORM_addr
+    .byte    18                              # DW_AT_high_pc
+    .byte    6                               # DW_FORM_data4
+    .byte    64                              # DW_AT_frame_base
+    .byte    24                              # DW_FORM_exprloc
+    .byte    3                               # DW_AT_name
+    .byte    14                              # DW_FORM_strp
+    .byte    58                              # DW_AT_decl_file
+    .byte    11                              # DW_FORM_data1
+    .byte    59                              # DW_AT_decl_line
+    .byte    11                              # DW_FORM_data1
+    .byte    39                              # DW_AT_prototyped
+    .byte    25                              # DW_FORM_flag_present
+    .byte    73                              # DW_AT_type
+    .byte    19                              # DW_FORM_ref4
+    .byte    63                              # DW_AT_external
+    .byte    25                              # DW_FORM_flag_present
+    .byte    0                               # EOM(1)
+    .byte    0                               # EOM(2)
+
+    .byte    10                              # Abbreviation Code
+    .byte    5                               # DW_TAG_formal_parameter
+    .byte    0                               # DW_CHILDREN_no
+    .byte    2                               # DW_AT_location
+    .byte    23                              # DW_FORM_sec_offset
+    .byte    3                               # DW_AT_name
+    .byte    14                              # DW_FORM_strp
+    .byte    58                              # DW_AT_decl_file
+    .byte    11                              # DW_FORM_data1
+    .byte    59                              # DW_AT_decl_line
+    .byte    11                              # DW_FORM_data1
+    .byte    73                              # DW_AT_type
+    .byte    19                              # DW_FORM_ref4
+    .byte    0                               # EOM(1)
+    .byte    0                               # EOM(2)
+
+    .byte    11                              # Abbreviation Code
+    .byte    29                              # DW_TAG_inlined_subroutine
+    .byte    1                               # DW_CHILDREN_yes
+    .byte    49                              # DW_AT_abstract_origin
+    .byte    19                              # DW_FORM_ref4
+    .byte    17                              # DW_AT_low_pc
+    .byte    1                               # DW_FORM_addr
+    .byte    18                              # DW_AT_high_pc
+    .byte    6                               # DW_FORM_data4
+    .byte    88                              # DW_AT_call_file
+    .byte    11                              # DW_FORM_data1
+    .byte    89                              # DW_AT_call_line
+    .byte    11                              # DW_FORM_data1
+    .byte    87                              # DW_AT_call_column
+    .byte    11                              # DW_FORM_data1
+    .byte    0                               # EOM(1)
+    .byte    0                               # EOM(2)
+
+    .byte    12                              # Abbreviation Code
+    .byte    15                              # DW_TAG_pointer_type
+    .byte    0                               # DW_CHILDREN_no
+    .byte    73                              # DW_AT_type
+    .byte    19                              # DW_FORM_ref4
+    .byte    0                               # EOM(1)
+    .byte    0                               # EOM(2)
+    .byte    0                               # EOM(3)
+
+    .section    .debug_info,"",@progbits
+.Ldi_cu_begin:
+    .long    .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit
+.Ldebug_info_start0:
+    .short    4                              # DWARF version number
+    .long    .debug_abbrev                   # Offset Into Abbrev. Section
+    .byte    8                               # Address Size (in bytes)
+    .byte    1                               # Abbrev [1] DW_TAG_compile_unit
+    .long    .Linfo_string_fname             #   DW_AT_name
+    .long    .Lline_table_start0             #   DW_AT_stmt_list
+    .quad    .Lcu_begin                      #   DW_AT_low_pc
+    .long    .Lcu_end-.Lcu_begin             #   DW_AT_high_pc
+
+# Debug info for |f| (abstract version with all parameters).
+
+.Ldebug_info_f:
+    .byte    5                               #   Abbrev [5] DW_TAG_subprogram
+    .long    .Linfo_string_f                 #     DW_AT_name
+    .byte    1                               #     DW_AT_decl_file
+    .byte    4                               #     DW_AT_decl_line
+                                             #     DW_AT_prototyped
+                                             #     DW_AT_external
+    .byte    1                               #     DW_AT_inline
+.Ldebug_info_param1:
+    .byte    6                               #     Abbrev [6] DW_TAG_formal_parameter
+    .long    .Linfo_string_unused1           #       DW_AT_name
+    .byte    1                               #       DW_AT_decl_file
+    .byte    4                               #       DW_AT_decl_line
+    .long    .Ldebug_info_void_ptr-.Ldi_cu_begin
+                                             #       DW_AT_type
+.Ldebug_info_param2:
+    .byte    6                               #     Abbrev [6] DW_TAG_formal_parameter
+    .long    .Linfo_string_used              #       DW_AT_name
+    .byte    1                               #       DW_AT_decl_file
+    .byte    4                               #       DW_AT_decl_line
+    .long    .Ldebug_info_int-.Ldi_cu_begin  #       DW_AT_type
+.Ldebug_info_param3:
+    .byte    6                               #     Abbrev [6] DW_TAG_formal_parameter
+    .long    .Linfo_string_unused2           #       DW_AT_name
+    .byte    1                               #       DW_AT_decl_file
+    .byte    4                               #       DW_AT_decl_line
+    .long    .Ldebug_info_int-.Ldi_cu_begin  #       DW_AT_type
+.Ldebug_info_param4:
+    .byte    6                               #     Abbrev [6] DW_TAG_formal_parameter
+    .long    .Linfo_string_partial           #       DW_AT_name
+    .byte    1                               #       DW_AT_decl_file
+    .byte    4                               #       DW_AT_decl_line
+    .long    .Ldebug_info_int-.Ldi_cu_begin  #       DW_AT_type
+.Ldebug_info_param5:
+    .byte    6                               #     Abbrev [6] DW_TAG_formal_parameter
+    .long    .Linfo_string_unused3           #       DW_AT_name
+    .byte    1                               #       DW_AT_decl_file
+    .byte    4                               #       DW_AT_decl_line
+    .long    .Ldebug_info_int-.Ldi_cu_begin  #       DW_AT_type
+    .byte    0                               #   End Of Children Mark (DW_TAG_subprogram)
+
+# Debug info for |g| (abstract version with all parameters).
+
+.Ldebug_info_g:
+    .byte    5                               #   Abbrev [5] DW_TAG_subprogram
+    .long    .Linfo_string_g                 #     DW_AT_name
+    .byte    1                               #     DW_AT_decl_file
+    .byte    4                               #     DW_AT_decl_line
+                                             #     DW_AT_prototyped
+                                             #     DW_AT_external
+    .byte    1                               #     DW_AT_inline
+.Ldebug_info_g_param1:
+    .byte    6                               #     Abbrev [6] DW_TAG_formal_parameter
+    .long    .Linfo_string_unused            #       DW_AT_name
+    .byte    1                               #       DW_AT_decl_file
+    .byte    10                              #       DW_AT_decl_line
+    .long    .Ldebug_info_int-.Ldi_cu_begin
+    .byte    0                               #   End Of Children Mark (DW_TAG_subprogram)
+
+# Debug info for |main|.
+
+    .byte    9                               #   Abbrev [9] DW_TAG_subprogram
+    .quad    main                            #     DW_AT_low_pc
+    .long    .Lmain_end-main                 #     DW_AT_high_pc
+    .byte    1                               #     DW_AT_frame_base
+    .byte    87
+    .long    .Linfo_string_main              #     DW_AT_name
+    .byte    1                               #     DW_AT_decl_file
+    .byte    18                              #     DW_AT_decl_line
+                                             #     DW_AT_prototyped
+    .long    .Ldebug_info_int-.Ldi_cu_begin  #     DW_AT_type
+                                             #     DW_AT_external
+
+#   Debug info for concrete |f| inlined into |main|.
+
+    .byte    11                              #     Abbrev [11] DW_TAG_inlined_subroutine
+    .long    .Ldebug_info_f-.Ldi_cu_begin
+                                             #       DW_AT_abstract_origin
+    .quad    .Linlined_f                     #       DW_AT_low_pc
+    .long    .Linlined_f_end-.Linlined_f     #       DW_AT_high_pc
+    .byte    1                               #       DW_AT_call_file
+    .byte    20                              #       DW_AT_call_line
+    .byte    3                               #       DW_AT_call_column
+    .byte    4                               #       Abbrev [4] DW_TAG_formal_parameter
+    .long    .Ldebug_loc_used                #         DW_AT_location
+    .long    .Ldebug_info_param2-.Ldi_cu_begin
+                                             #         DW_AT_abstract_origin
+    .byte    4                               #       Abbrev [4] DW_TAG_formal_parameter
+    .long    .Ldebug_loc_partial             #         DW_AT_location
+    .long    .Ldebug_info_param4-.Ldi_cu_begin
+                                             #         DW_AT_abstract_origin
+    .byte    0                               #     End Of Children Mark (DW_TAG_inlined_subroutine)
+
+#   Debug info for concrete |g| inlined into |main|.
+
+    .byte    11                              #     Abbrev [11] DW_TAG_inlined_subroutine
+    .long    .Ldebug_info_g-.Ldi_cu_begin
+                                             #       DW_AT_abstract_origin
+    .quad    .Linlined_g                     #       DW_AT_low_pc
+    .long    .Linlined_g_end-.Linlined_g     #       DW_AT_high_pc
+    .byte    1                               #       DW_AT_call_file
+    .byte    21                              #       DW_AT_call_line
+    .byte    3                               #       DW_AT_call_column
+    .byte    0                               #     End Of Children Mark (DW_TAG_inlined_subroutine)
+
+    .byte    0                               #   End Of Children Mark (DW_TAG_subprogram)
+
+# Debug info for |other|.
+
+    .byte    9                               #   Abbrev [9] DW_TAG_subprogram
+    .quad    other                           #     DW_AT_low_pc
+    .long    .Lother_end-other               #     DW_AT_high_pc
+    .byte    1                               #     DW_AT_frame_base
+    .byte    87
+    .long    .Linfo_string_other             #     DW_AT_name
+    .byte    1                               #     DW_AT_decl_file
+    .byte    15                              #     DW_AT_decl_line
+                                             #     DW_AT_prototyped
+    .long    .Ldebug_info_int-.Ldi_cu_begin  #     DW_AT_type
+                                             #     DW_AT_external
+
+#   Debug info for concrete |f| inlined into |other|.
+
+    .byte    11                              #     Abbrev [11] DW_TAG_inlined_subroutine
+    .long    .Ldebug_info_f-.Ldi_cu_begin
+                                             #       DW_AT_abstract_origin
+    .quad    .Linlined_f_in_other            #       DW_AT_low_pc
+    .long    .Linlined_f_in_other_end-.Linlined_f_in_other
+                                             #       DW_AT_high_pc
+    .byte    1                               #       DW_AT_call_file
+    .byte    16                              #       DW_AT_call_line
+    .byte    3                               #       DW_AT_call_column
+    .byte    4                               #       Abbrev [4] DW_TAG_formal_parameter
+    .long    .Ldebug_loc_used_in_other       #         DW_AT_location
+    .long    .Ldebug_info_param2-.Ldi_cu_begin
+                                             #         DW_AT_abstract_origin
+    .byte    4                               #       Abbrev [4] DW_TAG_formal_parameter
+    .long    .Ldebug_loc_partial_in_other    #         DW_AT_location
+    .long    .Ldebug_info_param4-.Ldi_cu_begin
+                                             #         DW_AT_abstract_origin
+    .byte    0                               #     End Of Children Mark (DW_TAG_inlined_subroutine)
+    .byte    0                               #   End Of Children Mark (DW_TAG_subprogram)
+
+.Ldebug_info_void_ptr:
+    .byte    7                               #   Abbrev [7] DW_TAG_pointer_type
+.Ldebug_info_int:
+    .byte    8                               #   Abbrev [8] DW_TAG_base_type
+    .long    .Linfo_string_int               #     DW_AT_name
+    .byte    5                               #     DW_AT_encoding
+    .byte    4                               #     DW_AT_byte_size
+
+    .byte    0                               # End Of Children Mark (DW_TAG_compile_unit)
+.Ldebug_info_end0:
+    .section    .debug_str,"MS",@progbits,1
+.Linfo_string_fname:
+    .asciz    "unused-inlined-params.c"
+.Linfo_string_f:
+    .asciz    "f"
+.Linfo_string_unused1:
+    .asciz    "unused1"
+.Linfo_string_used:
+    .asciz    "used"
+.Linfo_string_int:
+    .asciz    "int"
+.Linfo_string_unused2:
+    .asciz    "unused2"
+.Linfo_string_partial:
+    .asciz    "partial"
+.Linfo_string_unused3:
+    .asciz    "unused3"
+.Linfo_string_main:
+    .asciz    "main"
+.Linfo_string_g:
+    .asciz    "g"
+.Linfo_string_unused:
+    .asciz    "unused"
+.Linfo_string_other:
+    .asciz    "other"
+    .section    ".note.GNU-stack","",@progbits
+    .addrsig
+    .section    .debug_line,"",@progbits
+.Lline_table_start0:

lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test

+# RUN: llvm-mc -filetype=obj %S/Inputs/unused-inlined-params.s \
+# RUN:         -triple x86_64-pc-linux -o %t.o
+# RUN: %lldb %t.o -s %s -o exit | FileCheck %s
+
+
+# In this test we verify that inlined functions still mention
+# all their parameters in `frame variable`, even when those
+# parameters were completely optimized away from the concrete
+# instance of the inlined function in the debug info.
+# The debugger should look up the parameters in the abstract
+# origin of the concrete instance.
+
+# Let us check that unused parameters of an inlined function are listed
+# at the inlined function entry.
+image lookup -v -s break_at_inlined_f_in_main
+# CHECK-LABEL: image lookup -v -s break_at_inlined_f_in_main
+# CHECK: name = "unused1", type = "void *", location = <empty>
+# CHECK: name = "used", type = "int", location = DW_OP_consts +42
+# CHECK: name = "unused2", type = "int", location = <empty>
+# CHECK: name = "partial", type = "int", location = DW_OP_reg4 RSI
+# CHECK: name = "unused3", type = "int", location = <empty>
+
+# Show variables outsid of the live range of the 'partial' parameter
+# and verify that the output is as expected.
+image lookup -v -s break_at_inlined_f_in_main_between_printfs
+# CHECK-LABEL: image lookup -v -s break_at_inlined_f_in_main_between_printfs
+# CHECK: name = "unused1", type = "void *", location = <empty>
+# CHECK: name = "used", type = "int", location = DW_OP_reg3 RBX
+# CHECK: name = "unused2", type = "int", location = <empty>
+# Note: image lookup does not show variables outside of their
+#       location, so |partial| is missing here.
+# CHECK-NOT: partial
+# CHECK: name = "unused3", type = "int", location = <empty>
+
+# Check that we show parameters even if all of them are compiled away.
+image lookup -v -s  break_at_inlined_g_in_main
+# CHECK-LABEL: image lookup -v -s  break_at_inlined_g_in_main
+# CHECK: name = "unused", type = "int", location = <empty>
+
+# Check that even the other inlined instance of f displays the correct
+# parameters.
+image lookup -v -s  break_at_inlined_f_in_other
+# CHECK-LABEL: image lookup -v -s  break_at_inlined_f_in_other
+# CHECK: name = "unused1", type = "void *", location = <empty>
+# CHECK: name = "used", type = "int", location = DW_OP_consts +1
+# CHECK: name = "unused2", type = "int", location = <empty>
+# CHECK: name = "partial", type = "int", location =  DW_OP_consts +2
+# CHECK: name = "unused3", type = "int", location = <empty>