[llvm-dwarfdump][locstats] Unify handling of inlined vars with no loc

[djtodoro]

Summary

The presence or absence of an inline variable (as well as formal parameter) with only an abstract_origin ref (without DW_AT_location) should not change the location coverage.
It means, for both:

DW_TAG_inlined_subroutine
              DW_AT_abstract_origin (0x0000004e "f")
              DW_AT_low_pc  (0x0000000000000010)
              DW_AT_high_pc (0x0000000000000013)
  DW_TAG_formal_parameter
              DW_AT_abstract_origin       (0x0000005a "b")
NULL

and

DW_TAG_inlined_subroutine
            DW_AT_abstract_origin (0x0000004e "f")
            DW_AT_low_pc  (0x0000000000000010)
            DW_AT_high_pc (0x0000000000000013)
NULL

we should report 0% location coverage. If we add DW_AT_location, for both cases the coverage should be improved.

[djtodoro]

Discussed in: https://lists.llvm.org/pipermail/llvm-dev/2021-January/148139.html

[jmorse]

Thanks for the patch -- do I understand correctly that any coverage of an inlined variable will lead to it being removed from the "InlinedVariables" vector? I'm wondering what happens if we have two inlining sites: one where an inlined variable is optimised out, another where the variable has a location. If we're keeping global data and deleting elements from it when coverage is observed, I think that'll make the output dependent on which the inlining sites are encountered.

If a site without coverage is seen before a site with coverage, it'll get a zero-coverage record from InlinedVariables successfully. But if you flip the order, the site with coverage will erase from InlinedVariables, and the site without coverage won't know about the un-covered variable.

I think this patch also relies on the abstract DW_AT_inline copy of the subprogram being encountered before any call sites, I don't know that that's guaranteed. (Fix this is probably hard, hence I uh, avoided looking at this).

llvm/tools/llvm-dwarfdump/Statistics.cpp

30

Nit, this can probably be a SmallVector, right?

549–550

[djtodoro]

In D96045#2542581, @jmorse wrote:

Thanks for the patch -- do I understand correctly that any coverage of an inlined variable will lead to it being removed from the "InlinedVariables" vector? I'm wondering what happens if we have two inlining sites: one where an inlined variable is optimised out, another where the variable has a location. If we're keeping global data and deleting elements from it when coverage is observed, I think that'll make the output dependent on which the inlining sites are encountered.

If a site without coverage is seen before a site with coverage, it'll get a zero-coverage record from InlinedVariables successfully. But if you flip the order, the site with coverage will erase from InlinedVariables, and the site without coverage won't know about the un-covered variable.

Oh, I've forgotten to replace the vector with a map for concrete inlined instances.

I think this patch also relies on the abstract DW_AT_inline copy of the subprogram being encountered before any call sites, I don't know that that's guaranteed. (Fix this is probably hard, hence I uh, avoided looking at this).

Hm....yes... it could be a problem if it hasn't guaranteed... I'll try to think about it.

[djtodoro]

• Replace the vector representing inline vars with a map
• Replace std::vector with SmallVector
• Improve the test with multiple concrete inlined instacies

[jmorse]

Looking good with a couple of comments and the harder case of the DW_AT_inline copy being out-of-order wanting to be solved.

llvm/test/tools/llvm-dwarfdump/X86/locstats-for-inlined-vars.yaml

36–37

Could I request a third inlining site with no DIE at all for this variable -- that'll ensure we test the behaviour that D94976 is trying to create.

llvm/tools/llvm-dwarfdump/Statistics.cpp

439

This doesn't need to be in the outer scope, does it? IMO better to keep the declaration where it was, as it's only used in that block.

556–558

I get the feeling modifying this vector while iterating over it is vulnerable to re-ordering / re-allocating.

[jmorse]

Thinking out loud, and motivated by memory pressure: for the per-inlining-site record, that doesn't need be be maintained before or after we visit the inlining site, right? We might be able to get away with keeping a "Current scopes inlined variables" vector on the stack and passing it down through collectStatsRecursive. That'll automagically release the vector when we're done with the scope.

llvm-dwarfdump --statistics already takes a while to examine a stage2reldeb clang binary, it'd be best to avoid allocating more global data than we need.

[djtodoro]

@jmorse Thanks for the review!

In D96045#2544740, @jmorse wrote:

Thinking out loud, and motivated by memory pressure: for the per-inlining-site record, that doesn't need be be maintained before or after we visit the inlining site, right? We might be able to get away with keeping a "Current scopes inlined variables" vector on the stack and passing it down through collectStatsRecursive. That'll automagically release the vector when we're done with the scope.

llvm-dwarfdump --statistics already takes a while to examine a stage2reldeb clang binary, it'd be best to avoid allocating more global data than we need.

It makes sense. I tried that approach, but I had encountered an issue, and suddenly just dropped that way of the implementation. I'll try to implement this that way now, since we have a prototype working.

llvm/test/tools/llvm-dwarfdump/X86/locstats-for-inlined-vars.yaml

36–37

Sure. I'll add it in the next revision.

llvm/tools/llvm-dwarfdump/Statistics.cpp

439

It is being used at the end of the function again.

556–558

Right... It will be removed completely if we keep the vector on the stack.

[djtodoro]

• Maintain one array of inlined vars per inlined instance by creating it on the stack
• Improve the test

TODO: Handle the case then DW_TAG_subprogram with DW_AT_inline comes after inlined instancies

[djtodoro]

• Code refactor
• Handle the case when a DW_AT_inline copy of subprogram is out of order
  • Add test for that

[jmorse]

Looking good; a few comments inline, thanks for pushing forwards.

llvm/tools/llvm-dwarfdump/Statistics.cpp

451–453

Should this be setting InlinedVarsPtr to nullptr? Inlined variables from a higher-up-the-stack inlining site might still be present and pointed to by the InlinedVarsPtr argument.

674–676

Isn't this "removing" twice, once with std::remove, the other with the erase method?

679

Hmmmmm, this isn't going to collect variables that are nested within lexical_block's I guess, and fixing that probably requires more recursion. Ouch.

681

I reckon we'll need to bump the version number for this.

[dblaikie]

llvm/tools/llvm-dwarfdump/Statistics.cpp

674–676

Ah the joys of C++. This code is correct and an application of the classic "erase remove" idiom ( https://en.wikipedia.org/wiki/Erase%E2%80%93remove_idiom ).

Though we do have an llmv wrapper that tidies this up a bit, llvm::erase_if and llvm::erase_value

[djtodoro]

llvm/tools/llvm-dwarfdump/Statistics.cpp

451–453

it makes sense

468–480

This piece of code needs some changes in order to handle lexical scopes...

674–676

Yeah, it is an idiom for eliminating elements from stl containers.

@dblaikie I think that using of the std:: ones is recommended approach, right? Or do you proposing using of the llvm:: ones?

679

O yeah... inlined functions can also have lexical scopes... I will add dedicated test case and handle it.

681

I thought that only in the case of new metrics we should update this...
Thanks! I will update it.

[dblaikie]

llvm/tools/llvm-dwarfdump/Statistics.cpp

674–676

generally we use the llvm ones, that's why we have them - they simplify the API (make it range based instead of iterator pair based, and in this case wrap up the common erase+remove idiom into a single more legible function call (avoiding the confusion @jmorse ran into when reading this code, for instance))

[djtodoro]

llvm/tools/llvm-dwarfdump/Statistics.cpp

674–676

OK, it sounds reasonable to me. Thanks.

[djtodoro]

• Handle nested nodes (added a test for that)
• Use llvm::erase_value instead of std::erase-remove idiom

[djtodoro]

In D96045#2544740, @jmorse wrote:

llvm-dwarfdump --statistics already takes a while to examine a stage2reldeb clang binary, it'd be best to avoid allocating more global data than we need.

(Out-of-this-concrete-topic) The D96871 is a small optimization of the llvm-dwarfdump --statistics.

[jmorse]

Ship it!

[djtodoro]

Thanks!

[rdhindsa]

This patch seems to be causing a slowdown while running dwarfdump --statistics. It looks like more than 50% of the time is spent in collectZeroCovInlinedVars function.

In function collectStatsForObjectFile, variable InlinedFnsToBeProcessed is being created outside the for loop. It looks like the inline functions to be processed are appended and processed multiple times. I verified that by moving creation of that variable into the for loop, the slowdown goes away.

Can you please take a look!

[djtodoro]

In D96045#2672748, @rdhindsa wrote:

This patch seems to be causing a slowdown while running dwarfdump --statistics. It looks like more than 50% of the time is spent in collectZeroCovInlinedVars function.

In function collectStatsForObjectFile, variable InlinedFnsToBeProcessed is being created outside the for loop. It looks like the inline functions to be processed are appended and processed multiple times. I verified that by moving creation of that variable into the for loop, the slowdown goes away.

Can you please take a look!

Thanks for finding this! It does solve the problem -- do you want me to patch that or you are willing to post it?

[rdhindsa]

After moving the variable InlinedFnsToBeProcessed to inside the for loop, I noticed a difference in two dwarfdump statistics for a quite large binary with fission=yes:

#variables processed by location statistics
#variables with 0% of parent scope covered by DW_AT_location

Since I am not very familiar with this code, I would request if you could investigate and apply the patch.

[dblaikie]

In D96045#2678117, @rdhindsa wrote:

After moving the variable InlinedFnsToBeProcessed to inside the for loop, I noticed a difference in two dwarfdump statistics for a quite large binary with fission=yes:

#variables processed by location statistics
#variables with 0% of parent scope covered by DW_AT_location

Since I am not very familiar with this code, I would request if you could investigate and apply the patch.

Hmm, looking at this trying to reproduce the behavior makes me think this wouldn't be the right fix & the change in numbers is probably reflective of a bug in the fix - specifically in LTO situations (where the abstract origin of the function may be in a different CU than the one it's referenced from/inlined into).

But there might be a reasonable alternative fix/improvement - pruning the contents of the list once it has been handled. Possibly sorting the list so it's easy to just visit the elements that need to.

[djtodoro]

In D96045#2684570, @dblaikie wrote:

In D96045#2678117, @rdhindsa wrote:

After moving the variable InlinedFnsToBeProcessed to inside the for loop, I noticed a difference in two dwarfdump statistics for a quite large binary with fission=yes:

#variables processed by location statistics
#variables with 0% of parent scope covered by DW_AT_location

Since I am not very familiar with this code, I would request if you could investigate and apply the patch.

Hmm, looking at this trying to reproduce the behavior makes me think this wouldn't be the right fix & the change in numbers is probably reflective of a bug in the fix - specifically in LTO situations (where the abstract origin of the function may be in a different CU than the one it's referenced from/inlined into).

But there might be a reasonable alternative fix/improvement - pruning the contents of the list once it has been handled. Possibly sorting the list so it's easy to just visit the elements that need to.

I have taken a look into this and come up with the similar conclusion.
This could be reproduced with the Split DWARF -- when building e.g. GDB project with -gsplit-dwarf option enabled, it triggers the issue. I was struggling to make a simple test case (from a high level) that meets all the requirements, but it is very hard. That is why I've sent the email: https://lists.llvm.org/pipermail/llvm-dev/2021-April/149735.html.

[dblaikie]

In D96045#2685000, @djtodoro wrote:

In D96045#2684570, @dblaikie wrote:

In D96045#2678117, @rdhindsa wrote:

After moving the variable InlinedFnsToBeProcessed to inside the for loop, I noticed a difference in two dwarfdump statistics for a quite large binary with fission=yes:

#variables processed by location statistics
#variables with 0% of parent scope covered by DW_AT_location

Since I am not very familiar with this code, I would request if you could investigate and apply the patch.

Hmm, looking at this trying to reproduce the behavior makes me think this wouldn't be the right fix & the change in numbers is probably reflective of a bug in the fix - specifically in LTO situations (where the abstract origin of the function may be in a different CU than the one it's referenced from/inlined into).

But there might be a reasonable alternative fix/improvement - pruning the contents of the list once it has been handled. Possibly sorting the list so it's easy to just visit the elements that need to.

I have taken a look into this and come up with the similar conclusion.
This could be reproduced with the Split DWARF -- when building e.g. GDB project with -gsplit-dwarf option enabled, it triggers the issue. I was struggling to make a simple test case (from a high level) that meets all the requirements, but it is very hard. That is why I've sent the email: https://lists.llvm.org/pipermail/llvm-dev/2021-April/149735.html.

Ah, hmm - why would this come up in Split DWARF, do you think? Split DWARF doesn't have any cross-CU references (the format has no way to encode them) so I'd expect no DIEs to be added to the InlinedFnsToBeProcessed list?

[djtodoro]

In D96045#2688025, @dblaikie wrote:

In D96045#2685000, @djtodoro wrote:

In D96045#2684570, @dblaikie wrote:

In D96045#2678117, @rdhindsa wrote:

After moving the variable InlinedFnsToBeProcessed to inside the for loop, I noticed a difference in two dwarfdump statistics for a quite large binary with fission=yes:

#variables processed by location statistics
#variables with 0% of parent scope covered by DW_AT_location

Since I am not very familiar with this code, I would request if you could investigate and apply the patch.

Hmm, looking at this trying to reproduce the behavior makes me think this wouldn't be the right fix & the change in numbers is probably reflective of a bug in the fix - specifically in LTO situations (where the abstract origin of the function may be in a different CU than the one it's referenced from/inlined into).

But there might be a reasonable alternative fix/improvement - pruning the contents of the list once it has been handled. Possibly sorting the list so it's easy to just visit the elements that need to.

I have taken a look into this and come up with the similar conclusion.
This could be reproduced with the Split DWARF -- when building e.g. GDB project with -gsplit-dwarf option enabled, it triggers the issue. I was struggling to make a simple test case (from a high level) that meets all the requirements, but it is very hard. That is why I've sent the email: https://lists.llvm.org/pipermail/llvm-dev/2021-April/149735.html.

Ah, hmm - why would this come up in Split DWARF, do you think? Split DWARF doesn't have any cross-CU references (the format has no way to encode them) so I'd expect no DIEs to be added to the InlinedFnsToBeProcessed list?

The example I had in my mind is a bit different (I haven't looked into cross-cu support yet, but we should have covered it).
Let me first explain two main variables here:

GlobalInlinedFnInfo -- keeps track of DW_TAG_subprogram copies with DW_AT_inline, so when we face a DW_TAG_inlined_subroutine we know for how many variables we have zero % location coverage
InlinedFnsToBeProcessed -- doing recursive approach if we can firsly meet a DW_TAG_inlined_subroutine referencing a DW_TAG_subprogram copy with DW_AT_inline (since DWARF standard does not propose the order), this variable keeps the DW_TAG_inlined_subroutine DIEs that should be processed after we are finished with the dwarf tree

Let us know consider a big project (let it be GDB) compiled with -gsplit-dwarf. Main executable will have debug_info section:

$ llvm-dwarfdump gdb
0x0000000b: DW_TAG_compile_unit
           DW_AT_stmt_list   (0x00000000)
           DW_AT_comp_dir    ("/gdb")
           DW_AT_GNU_pubnames        (true)
           DW_AT_GNU_dwo_name        ("gdb.dwo")
           DW_AT_GNU_dwo_id  (0xb8ed71b1bb823706)
           DW_AT_low_pc      (0x0000000000482370)
           DW_AT_high_pc     (0x000000000048239a)
           DW_AT_GNU_addr_base       (0x00000000)
0x00000030: Compile Unit: length = 0x00000030, format = DWARF32, version = 0x0004, abbr_offset = 0x001a, addr_size = 0x08 (next unit at 0x00000064)

0x0000003b: DW_TAG_compile_unit
           DW_AT_stmt_list   (0x00000069)
           DW_AT_comp_dir    ("/gdb")
           DW_AT_GNU_pubnames        (true)
           DW_AT_GNU_dwo_name        ("amd64-tdep.dwo")
           DW_AT_GNU_dwo_id  (0x97c5a9e4ce1a43bf)
           DW_AT_GNU_ranges_base     (0x00000000)
           DW_AT_low_pc      (0x00000000004823a0)
           DW_AT_high_pc     (0x000000000048e982)
           DW_AT_GNU_addr_base       (0x00000018)
0x00000064: Compile Unit: length = 0x0000002c, format = DWARF32, version = 0x0004, abbr_offset = 0x0037, addr_size = 0x08 (next unit at 0x00000094)

0x0000006f: DW_TAG_compile_unit
....

We will keep filling both of these variables (GlobalInlinedFnInfo and InlinedFnsToBeProcessed) for all the CUs, but there could be a situation where we have inlined_subroutine referencing a subprogram, but we accidently use variables info from subprogram from previous CU with the same DIE offset -- that is why we have different numbers for this case.

[dblaikie]

In D96045#2688579, @djtodoro wrote:

In D96045#2688025, @dblaikie wrote:

In D96045#2685000, @djtodoro wrote:

In D96045#2684570, @dblaikie wrote:

In D96045#2678117, @rdhindsa wrote:

After moving the variable InlinedFnsToBeProcessed to inside the for loop, I noticed a difference in two dwarfdump statistics for a quite large binary with fission=yes:

#variables processed by location statistics
#variables with 0% of parent scope covered by DW_AT_location

Since I am not very familiar with this code, I would request if you could investigate and apply the patch.

Hmm, looking at this trying to reproduce the behavior makes me think this wouldn't be the right fix & the change in numbers is probably reflective of a bug in the fix - specifically in LTO situations (where the abstract origin of the function may be in a different CU than the one it's referenced from/inlined into).

But there might be a reasonable alternative fix/improvement - pruning the contents of the list once it has been handled. Possibly sorting the list so it's easy to just visit the elements that need to.

I have taken a look into this and come up with the similar conclusion.
This could be reproduced with the Split DWARF -- when building e.g. GDB project with -gsplit-dwarf option enabled, it triggers the issue. I was struggling to make a simple test case (from a high level) that meets all the requirements, but it is very hard. That is why I've sent the email: https://lists.llvm.org/pipermail/llvm-dev/2021-April/149735.html.

Ah, hmm - why would this come up in Split DWARF, do you think? Split DWARF doesn't have any cross-CU references (the format has no way to encode them) so I'd expect no DIEs to be added to the InlinedFnsToBeProcessed list?

The example I had in my mind is a bit different (I haven't looked into cross-cu support yet, but we should have covered it).

Hmm, OK. If the intent is that this change isn't meant to work for/fix the LTO case - then changing the scope sounds like something worth doing immediately. (& the discussion below about Split DWARF sounds like it'd be a place where the current buggy code would exhibit different (incorrect) behavior). I can probably try to come up with a test case that demonstrates that buggy behavior.

Let me first explain two main variables here:

GlobalInlinedFnInfo -- keeps track of DW_TAG_subprogram copies with DW_AT_inline, so when we face a DW_TAG_inlined_subroutine we know for how many variables we have zero % location coverage

As an aside: This issue can come up for the concrete DW_TAG_subprogram that refers to the abstract origin, not only for the DW_TAG_inlined_subroutines that refer to the abstract origin. ie:

$ cat test.cpp
void f1();
__attribute__((always_inline)) void f2() {
  int i;
  f1();
}
void f3() {
  f2();
}
blaikie@blaikie-linux2:~/dev/scratch$ clang++-tot -O3 test.cpp -c -g && llvm-dwarfdump-tot test.o | grep "DW_TAG\|abstract_origin\|DW_AT_name"
0x0000000b: DW_TAG_compile_unit
              DW_AT_name        ("test.cpp")
0x0000002a:   DW_TAG_subprogram
                DW_AT_abstract_origin   (0x0000005b "_Z2f2v")
0x0000003d:     DW_TAG_variable
                  DW_AT_abstract_origin (0x00000067 "i")
0x00000042:     DW_TAG_GNU_call_site
                  DW_AT_abstract_origin (0x00000050 "_Z2f1v")
0x00000050:   DW_TAG_subprogram
                DW_AT_name      ("f1")
0x0000005b:   DW_TAG_subprogram
                DW_AT_name      ("f2")
0x00000067:     DW_TAG_variable
                  DW_AT_name    ("i")
0x00000073:   DW_TAG_base_type
                DW_AT_name      ("int")
0x0000007a:   DW_TAG_subprogram
                DW_AT_name      ("f3")
0x00000093:     DW_TAG_inlined_subroutine
                  DW_AT_abstract_origin (0x0000005b "_Z2f2v")
0x000000a7:     DW_TAG_GNU_call_site
                  DW_AT_abstract_origin (0x00000050 "_Z2f1v")

So currently LLVM still has the "emitting a DIE with only an abstract origin for variables that are optimized away" issue in the concrete out of line definition (0x2a/0x3d above) though it's been fixed in the inlined case (0x93 above). And this patch for statistics addresses the latter case (the inlined_subprogram) but doesn't address the former case (if LLVM were improved not to produce that basically dead concrete variable DIE).

InlinedFnsToBeProcessed -- doing recursive approach if we can firsly meet a DW_TAG_inlined_subroutine referencing a DW_TAG_subprogram copy with DW_AT_inline (since DWARF standard does not propose the order), this variable keeps the DW_TAG_inlined_subroutine DIEs that should be processed after we are finished with the dwarf tree

Let us know consider a big project (let it be GDB) compiled with -gsplit-dwarf. Main executable will have debug_info section:

$ llvm-dwarfdump gdb
0x0000000b: DW_TAG_compile_unit
           DW_AT_stmt_list   (0x00000000)
           DW_AT_comp_dir    ("/gdb")
           DW_AT_GNU_pubnames        (true)
           DW_AT_GNU_dwo_name        ("gdb.dwo")
           DW_AT_GNU_dwo_id  (0xb8ed71b1bb823706)
           DW_AT_low_pc      (0x0000000000482370)
           DW_AT_high_pc     (0x000000000048239a)
           DW_AT_GNU_addr_base       (0x00000000)
0x00000030: Compile Unit: length = 0x00000030, format = DWARF32, version = 0x0004, abbr_offset = 0x001a, addr_size = 0x08 (next unit at 0x00000064)

0x0000003b: DW_TAG_compile_unit
           DW_AT_stmt_list   (0x00000069)
           DW_AT_comp_dir    ("/gdb")
           DW_AT_GNU_pubnames        (true)
           DW_AT_GNU_dwo_name        ("amd64-tdep.dwo")
           DW_AT_GNU_dwo_id  (0x97c5a9e4ce1a43bf)
           DW_AT_GNU_ranges_base     (0x00000000)
           DW_AT_low_pc      (0x00000000004823a0)
           DW_AT_high_pc     (0x000000000048e982)
           DW_AT_GNU_addr_base       (0x00000018)
0x00000064: Compile Unit: length = 0x0000002c, format = DWARF32, version = 0x0004, abbr_offset = 0x0037, addr_size = 0x08 (next unit at 0x00000094)

0x0000006f: DW_TAG_compile_unit
....

We will keep filling both of these variables (GlobalInlinedFnInfo and InlinedFnsToBeProcessed) for all the CUs, but there could be a situation where we have inlined_subroutine referencing a subprogram, but we accidently use variables info from subprogram from previous CU with the same DIE offset -- that is why we have different numbers for this case.

Yep, that sounds reasonably buggy & in the absence of LTO/if this code isn't intended to handle the LTO case (which it seems it isn't) then moving the variables to the inner scope for now - pending work to address LTO - sounds right to me.

Here's a test case for the scope change fix/bug with split-dwarf:
a.cpp:

__attribute__((optnone)) static void x() {
}
__attribute__((always_inline)) static void y() {
  int var;
  x();
}
void f1() {
  y();
}

b.cpp:

__attribute__((optnone)) static void x() {
}
__attribute__((always_inline)) static void y() {
  int var;
  x();
}
void f2() {
  y();
}

c.cpp:

void f1();
void f2();
int main() {
  f1();
  f2();
}

$ clang++ a.cpp b.cpp c.cpp -g -gsplit-dwarf -O1
$ llvm-dwarfdump --statistics a.out

(you could compile c.cpp without debug info, or put main in a or b.cpp at the end, I think it'd still work).

With the current coed, this reports:

"#variables processed by location statistics": 3,                                                             
"#variables with 0% of parent scope covered by DW_AT_location": 3,    
...
"#local vars processed by location statistics": 3,                                                                        
"#local vars with 0% of parent scope covered by DW_AT_location": 3,

Moving the variables into the nested scope causes these to all change to 2, which is correct.

[djtodoro]

In D96045#2703754, @dblaikie wrote:

In D96045#2688579, @djtodoro wrote:

In D96045#2688025, @dblaikie wrote:

In D96045#2685000, @djtodoro wrote:

In D96045#2684570, @dblaikie wrote:

In D96045#2678117, @rdhindsa wrote:

After moving the variable InlinedFnsToBeProcessed to inside the for loop, I noticed a difference in two dwarfdump statistics for a quite large binary with fission=yes:

#variables processed by location statistics
#variables with 0% of parent scope covered by DW_AT_location

Since I am not very familiar with this code, I would request if you could investigate and apply the patch.

Hmm, looking at this trying to reproduce the behavior makes me think this wouldn't be the right fix & the change in numbers is probably reflective of a bug in the fix - specifically in LTO situations (where the abstract origin of the function may be in a different CU than the one it's referenced from/inlined into).

But there might be a reasonable alternative fix/improvement - pruning the contents of the list once it has been handled. Possibly sorting the list so it's easy to just visit the elements that need to.

I have taken a look into this and come up with the similar conclusion.
This could be reproduced with the Split DWARF -- when building e.g. GDB project with -gsplit-dwarf option enabled, it triggers the issue. I was struggling to make a simple test case (from a high level) that meets all the requirements, but it is very hard. That is why I've sent the email: https://lists.llvm.org/pipermail/llvm-dev/2021-April/149735.html.

Ah, hmm - why would this come up in Split DWARF, do you think? Split DWARF doesn't have any cross-CU references (the format has no way to encode them) so I'd expect no DIEs to be added to the InlinedFnsToBeProcessed list?

The example I had in my mind is a bit different (I haven't looked into cross-cu support yet, but we should have covered it).

Hmm, OK. If the intent is that this change isn't meant to work for/fix the LTO case - then changing the scope sounds like something worth doing immediately. (& the discussion below about Split DWARF sounds like it'd be a place where the current buggy code would exhibit different (incorrect) behavior). I can probably try to come up with a test case that demonstrates that buggy behavior.

Let me first explain two main variables here:

GlobalInlinedFnInfo -- keeps track of DW_TAG_subprogram copies with DW_AT_inline, so when we face a DW_TAG_inlined_subroutine we know for how many variables we have zero % location coverage

As an aside: This issue can come up for the concrete DW_TAG_subprogram that refers to the abstract origin, not only for the DW_TAG_inlined_subroutines that refer to the abstract origin. ie:

$ cat test.cpp
void f1();
__attribute__((always_inline)) void f2() {
  int i;
  f1();
}
void f3() {
  f2();
}
blaikie@blaikie-linux2:~/dev/scratch$ clang++-tot -O3 test.cpp -c -g && llvm-dwarfdump-tot test.o | grep "DW_TAG\|abstract_origin\|DW_AT_name"
0x0000000b: DW_TAG_compile_unit
              DW_AT_name        ("test.cpp")
0x0000002a:   DW_TAG_subprogram
                DW_AT_abstract_origin   (0x0000005b "_Z2f2v")
0x0000003d:     DW_TAG_variable
                  DW_AT_abstract_origin (0x00000067 "i")
0x00000042:     DW_TAG_GNU_call_site
                  DW_AT_abstract_origin (0x00000050 "_Z2f1v")
0x00000050:   DW_TAG_subprogram
                DW_AT_name      ("f1")
0x0000005b:   DW_TAG_subprogram
                DW_AT_name      ("f2")
0x00000067:     DW_TAG_variable
                  DW_AT_name    ("i")
0x00000073:   DW_TAG_base_type
                DW_AT_name      ("int")
0x0000007a:   DW_TAG_subprogram
                DW_AT_name      ("f3")
0x00000093:     DW_TAG_inlined_subroutine
                  DW_AT_abstract_origin (0x0000005b "_Z2f2v")
0x000000a7:     DW_TAG_GNU_call_site
                  DW_AT_abstract_origin (0x00000050 "_Z2f1v")

So currently LLVM still has the "emitting a DIE with only an abstract origin for variables that are optimized away" issue in the concrete out of line definition (0x2a/0x3d above) though it's been fixed in the inlined case (0x93 above). And this patch for statistics addresses the latter case (the inlined_subprogram) but doesn't address the former case (if LLVM were improved not to produce that basically dead concrete variable DIE).

Oh, yes, I think the D54217 have introduced a bug here (since we should consider dw_subprogram with abstract_origin as inlined function) and I guess we should make something like this:

diff --git a/llvm/tools/llvm-dwarfdump/Statistics.cpp b/llvm/tools/llvm-dwarfdump/Statistics.cpp
index 3758a56da7a2..4d0cf6b5fc8d 100644
--- a/llvm/tools/llvm-dwarfdump/Statistics.cpp
+++ b/llvm/tools/llvm-dwarfdump/Statistics.cpp
@@ -462,9 +462,15 @@ static void collectStatsRecursive(DWARFDie Die, std::string FnPrefix,
     return;

   // Handle any kind of lexical scope.
-  const bool IsFunction = Tag == dwarf::DW_TAG_subprogram;
+  const bool HasAbstractOriginAttr =
+      Die.find(dwarf::DW_AT_abstract_origin) != None;
+  const bool IsFunction =
+      ((Tag == dwarf::DW_TAG_subprogram) && !HasAbstractOriginAttr);
   const bool IsBlock = Tag == dwarf::DW_TAG_lexical_block;
-  const bool IsInlinedFunction = Tag == dwarf::DW_TAG_inlined_subroutine;
+  const bool IsInlinedFunction =
+      (Tag == dwarf::DW_TAG_inlined_subroutine ||
+       (Tag == dwarf::DW_TAG_subprogram && HasAbstractOriginAttr));
+
   InlinedVarsTy InlinedVars;
   // Get the vars of the inlined fn, so the locstats
   // reports the missing vars (with coverage 0%).

InlinedFnsToBeProcessed -- doing recursive approach if we can firsly meet a DW_TAG_inlined_subroutine referencing a DW_TAG_subprogram copy with DW_AT_inline (since DWARF standard does not propose the order), this variable keeps the DW_TAG_inlined_subroutine DIEs that should be processed after we are finished with the dwarf tree

Let us know consider a big project (let it be GDB) compiled with -gsplit-dwarf. Main executable will have debug_info section:

$ llvm-dwarfdump gdb
0x0000000b: DW_TAG_compile_unit
           DW_AT_stmt_list   (0x00000000)
           DW_AT_comp_dir    ("/gdb")
           DW_AT_GNU_pubnames        (true)
           DW_AT_GNU_dwo_name        ("gdb.dwo")
           DW_AT_GNU_dwo_id  (0xb8ed71b1bb823706)
           DW_AT_low_pc      (0x0000000000482370)
           DW_AT_high_pc     (0x000000000048239a)
           DW_AT_GNU_addr_base       (0x00000000)
0x00000030: Compile Unit: length = 0x00000030, format = DWARF32, version = 0x0004, abbr_offset = 0x001a, addr_size = 0x08 (next unit at 0x00000064)

0x0000003b: DW_TAG_compile_unit
           DW_AT_stmt_list   (0x00000069)
           DW_AT_comp_dir    ("/gdb")
           DW_AT_GNU_pubnames        (true)
           DW_AT_GNU_dwo_name        ("amd64-tdep.dwo")
           DW_AT_GNU_dwo_id  (0x97c5a9e4ce1a43bf)
           DW_AT_GNU_ranges_base     (0x00000000)
           DW_AT_low_pc      (0x00000000004823a0)
           DW_AT_high_pc     (0x000000000048e982)
           DW_AT_GNU_addr_base       (0x00000018)
0x00000064: Compile Unit: length = 0x0000002c, format = DWARF32, version = 0x0004, abbr_offset = 0x0037, addr_size = 0x08 (next unit at 0x00000094)

0x0000006f: DW_TAG_compile_unit
....

We will keep filling both of these variables (GlobalInlinedFnInfo and InlinedFnsToBeProcessed) for all the CUs, but there could be a situation where we have inlined_subroutine referencing a subprogram, but we accidently use variables info from subprogram from previous CU with the same DIE offset -- that is why we have different numbers for this case.

Yep, that sounds reasonably buggy & in the absence of LTO/if this code isn't intended to handle the LTO case (which it seems it isn't) then moving the variables to the inner scope for now - pending work to address LTO - sounds right to me.

Here's a test case for the scope change fix/bug with split-dwarf:
a.cpp:

__attribute__((optnone)) static void x() {
}
__attribute__((always_inline)) static void y() {
  int var;
  x();
}
void f1() {
  y();
}

b.cpp:

__attribute__((optnone)) static void x() {
}
__attribute__((always_inline)) static void y() {
  int var;
  x();
}
void f2() {
  y();
}

c.cpp:

void f1();
void f2();
int main() {
  f1();
  f2();
}

$ clang++ a.cpp b.cpp c.cpp -g -gsplit-dwarf -O1
$ llvm-dwarfdump --statistics a.out

(you could compile c.cpp without debug info, or put main in a or b.cpp at the end, I think it'd still work).

With the current coed, this reports:

"#variables processed by location statistics": 3,                                                             
"#variables with 0% of parent scope covered by DW_AT_location": 3,    
...
"#local vars processed by location statistics": 3,                                                                        
"#local vars with 0% of parent scope covered by DW_AT_location": 3,

Moving the variables into the nested scope causes these to all change to 2, which is correct.

Great! Thanks a lot!
This is the patch: https://reviews.llvm.org/D100951.

[dblaikie]

In D96045#2704967, @djtodoro wrote:

In D96045#2703754, @dblaikie wrote:

In D96045#2688579, @djtodoro wrote:

In D96045#2688025, @dblaikie wrote:

In D96045#2685000, @djtodoro wrote:

In D96045#2684570, @dblaikie wrote:

In D96045#2678117, @rdhindsa wrote:

After moving the variable InlinedFnsToBeProcessed to inside the for loop, I noticed a difference in two dwarfdump statistics for a quite large binary with fission=yes:

#variables processed by location statistics
#variables with 0% of parent scope covered by DW_AT_location

Since I am not very familiar with this code, I would request if you could investigate and apply the patch.

Hmm, looking at this trying to reproduce the behavior makes me think this wouldn't be the right fix & the change in numbers is probably reflective of a bug in the fix - specifically in LTO situations (where the abstract origin of the function may be in a different CU than the one it's referenced from/inlined into).

But there might be a reasonable alternative fix/improvement - pruning the contents of the list once it has been handled. Possibly sorting the list so it's easy to just visit the elements that need to.

I have taken a look into this and come up with the similar conclusion.
This could be reproduced with the Split DWARF -- when building e.g. GDB project with -gsplit-dwarf option enabled, it triggers the issue. I was struggling to make a simple test case (from a high level) that meets all the requirements, but it is very hard. That is why I've sent the email: https://lists.llvm.org/pipermail/llvm-dev/2021-April/149735.html.

Ah, hmm - why would this come up in Split DWARF, do you think? Split DWARF doesn't have any cross-CU references (the format has no way to encode them) so I'd expect no DIEs to be added to the InlinedFnsToBeProcessed list?

The example I had in my mind is a bit different (I haven't looked into cross-cu support yet, but we should have covered it).

Hmm, OK. If the intent is that this change isn't meant to work for/fix the LTO case - then changing the scope sounds like something worth doing immediately. (& the discussion below about Split DWARF sounds like it'd be a place where the current buggy code would exhibit different (incorrect) behavior). I can probably try to come up with a test case that demonstrates that buggy behavior.

Let me first explain two main variables here:

GlobalInlinedFnInfo -- keeps track of DW_TAG_subprogram copies with DW_AT_inline, so when we face a DW_TAG_inlined_subroutine we know for how many variables we have zero % location coverage

As an aside: This issue can come up for the concrete DW_TAG_subprogram that refers to the abstract origin, not only for the DW_TAG_inlined_subroutines that refer to the abstract origin. ie:

$ cat test.cpp
void f1();
__attribute__((always_inline)) void f2() {
  int i;
  f1();
}
void f3() {
  f2();
}
blaikie@blaikie-linux2:~/dev/scratch$ clang++-tot -O3 test.cpp -c -g && llvm-dwarfdump-tot test.o | grep "DW_TAG\|abstract_origin\|DW_AT_name"
0x0000000b: DW_TAG_compile_unit
              DW_AT_name        ("test.cpp")
0x0000002a:   DW_TAG_subprogram
                DW_AT_abstract_origin   (0x0000005b "_Z2f2v")
0x0000003d:     DW_TAG_variable
                  DW_AT_abstract_origin (0x00000067 "i")
0x00000042:     DW_TAG_GNU_call_site
                  DW_AT_abstract_origin (0x00000050 "_Z2f1v")
0x00000050:   DW_TAG_subprogram
                DW_AT_name      ("f1")
0x0000005b:   DW_TAG_subprogram
                DW_AT_name      ("f2")
0x00000067:     DW_TAG_variable
                  DW_AT_name    ("i")
0x00000073:   DW_TAG_base_type
                DW_AT_name      ("int")
0x0000007a:   DW_TAG_subprogram
                DW_AT_name      ("f3")
0x00000093:     DW_TAG_inlined_subroutine
                  DW_AT_abstract_origin (0x0000005b "_Z2f2v")
0x000000a7:     DW_TAG_GNU_call_site
                  DW_AT_abstract_origin (0x00000050 "_Z2f1v")

So currently LLVM still has the "emitting a DIE with only an abstract origin for variables that are optimized away" issue in the concrete out of line definition (0x2a/0x3d above) though it's been fixed in the inlined case (0x93 above). And this patch for statistics addresses the latter case (the inlined_subprogram) but doesn't address the former case (if LLVM were improved not to produce that basically dead concrete variable DIE).

Oh, yes, I think the D54217 have introduced a bug here (since we should consider dw_subprogram with abstract_origin as inlined function) and I guess we should make something like this:

diff --git a/llvm/tools/llvm-dwarfdump/Statistics.cpp b/llvm/tools/llvm-dwarfdump/Statistics.cpp
index 3758a56da7a2..4d0cf6b5fc8d 100644
--- a/llvm/tools/llvm-dwarfdump/Statistics.cpp
+++ b/llvm/tools/llvm-dwarfdump/Statistics.cpp
@@ -462,9 +462,15 @@ static void collectStatsRecursive(DWARFDie Die, std::string FnPrefix,
     return;

   // Handle any kind of lexical scope.
-  const bool IsFunction = Tag == dwarf::DW_TAG_subprogram;
+  const bool HasAbstractOriginAttr =
+      Die.find(dwarf::DW_AT_abstract_origin) != None;
+  const bool IsFunction =
+      ((Tag == dwarf::DW_TAG_subprogram) && !HasAbstractOriginAttr);
   const bool IsBlock = Tag == dwarf::DW_TAG_lexical_block;
-  const bool IsInlinedFunction = Tag == dwarf::DW_TAG_inlined_subroutine;
+  const bool IsInlinedFunction =
+      (Tag == dwarf::DW_TAG_inlined_subroutine ||
+       (Tag == dwarf::DW_TAG_subprogram && HasAbstractOriginAttr));
+
   InlinedVarsTy InlinedVars;
   // Get the vars of the inlined fn, so the locstats
   // reports the missing vars (with coverage 0%).

More or less - I think we'll need to find another name other than "inlined" for these "concrete and inlined instances with abstract origins", otherwise it's liable to be confusing to future readers/maintainers.

InlinedFnsToBeProcessed -- doing recursive approach if we can firsly meet a DW_TAG_inlined_subroutine referencing a DW_TAG_subprogram copy with DW_AT_inline (since DWARF standard does not propose the order), this variable keeps the DW_TAG_inlined_subroutine DIEs that should be processed after we are finished with the dwarf tree

Let us know consider a big project (let it be GDB) compiled with -gsplit-dwarf. Main executable will have debug_info section:

$ llvm-dwarfdump gdb
0x0000000b: DW_TAG_compile_unit
           DW_AT_stmt_list   (0x00000000)
           DW_AT_comp_dir    ("/gdb")
           DW_AT_GNU_pubnames        (true)
           DW_AT_GNU_dwo_name        ("gdb.dwo")
           DW_AT_GNU_dwo_id  (0xb8ed71b1bb823706)
           DW_AT_low_pc      (0x0000000000482370)
           DW_AT_high_pc     (0x000000000048239a)
           DW_AT_GNU_addr_base       (0x00000000)
0x00000030: Compile Unit: length = 0x00000030, format = DWARF32, version = 0x0004, abbr_offset = 0x001a, addr_size = 0x08 (next unit at 0x00000064)

0x0000003b: DW_TAG_compile_unit
           DW_AT_stmt_list   (0x00000069)
           DW_AT_comp_dir    ("/gdb")
           DW_AT_GNU_pubnames        (true)
           DW_AT_GNU_dwo_name        ("amd64-tdep.dwo")
           DW_AT_GNU_dwo_id  (0x97c5a9e4ce1a43bf)
           DW_AT_GNU_ranges_base     (0x00000000)
           DW_AT_low_pc      (0x00000000004823a0)
           DW_AT_high_pc     (0x000000000048e982)
           DW_AT_GNU_addr_base       (0x00000018)
0x00000064: Compile Unit: length = 0x0000002c, format = DWARF32, version = 0x0004, abbr_offset = 0x0037, addr_size = 0x08 (next unit at 0x00000094)

0x0000006f: DW_TAG_compile_unit
....

We will keep filling both of these variables (GlobalInlinedFnInfo and InlinedFnsToBeProcessed) for all the CUs, but there could be a situation where we have inlined_subroutine referencing a subprogram, but we accidently use variables info from subprogram from previous CU with the same DIE offset -- that is why we have different numbers for this case.

Yep, that sounds reasonably buggy & in the absence of LTO/if this code isn't intended to handle the LTO case (which it seems it isn't) then moving the variables to the inner scope for now - pending work to address LTO - sounds right to me.

Here's a test case for the scope change fix/bug with split-dwarf:
a.cpp:

__attribute__((optnone)) static void x() {
}
__attribute__((always_inline)) static void y() {
  int var;
  x();
}
void f1() {
  y();
}

b.cpp:

__attribute__((optnone)) static void x() {
}
__attribute__((always_inline)) static void y() {
  int var;
  x();
}
void f2() {
  y();
}

c.cpp:

void f1();
void f2();
int main() {
  f1();
  f2();
}

$ clang++ a.cpp b.cpp c.cpp -g -gsplit-dwarf -O1
$ llvm-dwarfdump --statistics a.out

(you could compile c.cpp without debug info, or put main in a or b.cpp at the end, I think it'd still work).

With the current coed, this reports:

"#variables processed by location statistics": 3,                                                             
"#variables with 0% of parent scope covered by DW_AT_location": 3,    
...
"#local vars processed by location statistics": 3,                                                                        
"#local vars with 0% of parent scope covered by DW_AT_location": 3,

Moving the variables into the nested scope causes these to all change to 2, which is correct.

Great! Thanks a lot!
This is the patch: https://reviews.llvm.org/D100951.

Thanks!

[dblaikie]

Oh, and it'd also be really good/important to make this work correctly with LTO (though that'll be more challenging/expensive due to the memory usage - it'll mean basically saving the details of every abstract origin (which means identifying them as such, since they don't expliictly identify themselves as abstract origins) (the expensive part) and then searching through that whenever we see a concrete-or-inlined (& if the search fails, putting the concrete-or-inlined in the list of things to resolve against future abstract origins that are parsed)). Will you be able to do that?

[djtodoro]

In D96045#2705748, @dblaikie wrote:

In D96045#2704967, @djtodoro wrote:

In D96045#2703754, @dblaikie wrote:

In D96045#2688579, @djtodoro wrote:

In D96045#2688025, @dblaikie wrote:

In D96045#2685000, @djtodoro wrote:

In D96045#2684570, @dblaikie wrote:

In D96045#2678117, @rdhindsa wrote:

After moving the variable InlinedFnsToBeProcessed to inside the for loop, I noticed a difference in two dwarfdump statistics for a quite large binary with fission=yes:

#variables processed by location statistics
#variables with 0% of parent scope covered by DW_AT_location

Since I am not very familiar with this code, I would request if you could investigate and apply the patch.

Hmm, looking at this trying to reproduce the behavior makes me think this wouldn't be the right fix & the change in numbers is probably reflective of a bug in the fix - specifically in LTO situations (where the abstract origin of the function may be in a different CU than the one it's referenced from/inlined into).

But there might be a reasonable alternative fix/improvement - pruning the contents of the list once it has been handled. Possibly sorting the list so it's easy to just visit the elements that need to.

I have taken a look into this and come up with the similar conclusion.
This could be reproduced with the Split DWARF -- when building e.g. GDB project with -gsplit-dwarf option enabled, it triggers the issue. I was struggling to make a simple test case (from a high level) that meets all the requirements, but it is very hard. That is why I've sent the email: https://lists.llvm.org/pipermail/llvm-dev/2021-April/149735.html.

Ah, hmm - why would this come up in Split DWARF, do you think? Split DWARF doesn't have any cross-CU references (the format has no way to encode them) so I'd expect no DIEs to be added to the InlinedFnsToBeProcessed list?

The example I had in my mind is a bit different (I haven't looked into cross-cu support yet, but we should have covered it).

Hmm, OK. If the intent is that this change isn't meant to work for/fix the LTO case - then changing the scope sounds like something worth doing immediately. (& the discussion below about Split DWARF sounds like it'd be a place where the current buggy code would exhibit different (incorrect) behavior). I can probably try to come up with a test case that demonstrates that buggy behavior.

Let me first explain two main variables here:

GlobalInlinedFnInfo -- keeps track of DW_TAG_subprogram copies with DW_AT_inline, so when we face a DW_TAG_inlined_subroutine we know for how many variables we have zero % location coverage

As an aside: This issue can come up for the concrete DW_TAG_subprogram that refers to the abstract origin, not only for the DW_TAG_inlined_subroutines that refer to the abstract origin. ie:

$ cat test.cpp
void f1();
__attribute__((always_inline)) void f2() {
  int i;
  f1();
}
void f3() {
  f2();
}
blaikie@blaikie-linux2:~/dev/scratch$ clang++-tot -O3 test.cpp -c -g && llvm-dwarfdump-tot test.o | grep "DW_TAG\|abstract_origin\|DW_AT_name"
0x0000000b: DW_TAG_compile_unit
              DW_AT_name        ("test.cpp")
0x0000002a:   DW_TAG_subprogram
                DW_AT_abstract_origin   (0x0000005b "_Z2f2v")
0x0000003d:     DW_TAG_variable
                  DW_AT_abstract_origin (0x00000067 "i")
0x00000042:     DW_TAG_GNU_call_site
                  DW_AT_abstract_origin (0x00000050 "_Z2f1v")
0x00000050:   DW_TAG_subprogram
                DW_AT_name      ("f1")
0x0000005b:   DW_TAG_subprogram
                DW_AT_name      ("f2")
0x00000067:     DW_TAG_variable
                  DW_AT_name    ("i")
0x00000073:   DW_TAG_base_type
                DW_AT_name      ("int")
0x0000007a:   DW_TAG_subprogram
                DW_AT_name      ("f3")
0x00000093:     DW_TAG_inlined_subroutine
                  DW_AT_abstract_origin (0x0000005b "_Z2f2v")
0x000000a7:     DW_TAG_GNU_call_site
                  DW_AT_abstract_origin (0x00000050 "_Z2f1v")

So currently LLVM still has the "emitting a DIE with only an abstract origin for variables that are optimized away" issue in the concrete out of line definition (0x2a/0x3d above) though it's been fixed in the inlined case (0x93 above). And this patch for statistics addresses the latter case (the inlined_subprogram) but doesn't address the former case (if LLVM were improved not to produce that basically dead concrete variable DIE).

Oh, yes, I think the D54217 have introduced a bug here (since we should consider dw_subprogram with abstract_origin as inlined function) and I guess we should make something like this:

diff --git a/llvm/tools/llvm-dwarfdump/Statistics.cpp b/llvm/tools/llvm-dwarfdump/Statistics.cpp
index 3758a56da7a2..4d0cf6b5fc8d 100644
--- a/llvm/tools/llvm-dwarfdump/Statistics.cpp
+++ b/llvm/tools/llvm-dwarfdump/Statistics.cpp
@@ -462,9 +462,15 @@ static void collectStatsRecursive(DWARFDie Die, std::string FnPrefix,
     return;

   // Handle any kind of lexical scope.
-  const bool IsFunction = Tag == dwarf::DW_TAG_subprogram;
+  const bool HasAbstractOriginAttr =
+      Die.find(dwarf::DW_AT_abstract_origin) != None;
+  const bool IsFunction =
+      ((Tag == dwarf::DW_TAG_subprogram) && !HasAbstractOriginAttr);
   const bool IsBlock = Tag == dwarf::DW_TAG_lexical_block;
-  const bool IsInlinedFunction = Tag == dwarf::DW_TAG_inlined_subroutine;
+  const bool IsInlinedFunction =
+      (Tag == dwarf::DW_TAG_inlined_subroutine ||
+       (Tag == dwarf::DW_TAG_subprogram && HasAbstractOriginAttr));
+
   InlinedVarsTy InlinedVars;
   // Get the vars of the inlined fn, so the locstats
   // reports the missing vars (with coverage 0%).

More or less - I think we'll need to find another name other than "inlined" for these "concrete and inlined instances with abstract origins", otherwise it's liable to be confusing to future readers/maintainers.

Here is the patch -- D101025.

InlinedFnsToBeProcessed -- doing recursive approach if we can firsly meet a DW_TAG_inlined_subroutine referencing a DW_TAG_subprogram copy with DW_AT_inline (since DWARF standard does not propose the order), this variable keeps the DW_TAG_inlined_subroutine DIEs that should be processed after we are finished with the dwarf tree

Let us know consider a big project (let it be GDB) compiled with -gsplit-dwarf. Main executable will have debug_info section:

$ llvm-dwarfdump gdb
0x0000000b: DW_TAG_compile_unit
           DW_AT_stmt_list   (0x00000000)
           DW_AT_comp_dir    ("/gdb")
           DW_AT_GNU_pubnames        (true)
           DW_AT_GNU_dwo_name        ("gdb.dwo")
           DW_AT_GNU_dwo_id  (0xb8ed71b1bb823706)
           DW_AT_low_pc      (0x0000000000482370)
           DW_AT_high_pc     (0x000000000048239a)
           DW_AT_GNU_addr_base       (0x00000000)
0x00000030: Compile Unit: length = 0x00000030, format = DWARF32, version = 0x0004, abbr_offset = 0x001a, addr_size = 0x08 (next unit at 0x00000064)

0x0000003b: DW_TAG_compile_unit
           DW_AT_stmt_list   (0x00000069)
           DW_AT_comp_dir    ("/gdb")
           DW_AT_GNU_pubnames        (true)
           DW_AT_GNU_dwo_name        ("amd64-tdep.dwo")
           DW_AT_GNU_dwo_id  (0x97c5a9e4ce1a43bf)
           DW_AT_GNU_ranges_base     (0x00000000)
           DW_AT_low_pc      (0x00000000004823a0)
           DW_AT_high_pc     (0x000000000048e982)
           DW_AT_GNU_addr_base       (0x00000018)
0x00000064: Compile Unit: length = 0x0000002c, format = DWARF32, version = 0x0004, abbr_offset = 0x0037, addr_size = 0x08 (next unit at 0x00000094)

0x0000006f: DW_TAG_compile_unit
....

We will keep filling both of these variables (GlobalInlinedFnInfo and InlinedFnsToBeProcessed) for all the CUs, but there could be a situation where we have inlined_subroutine referencing a subprogram, but we accidently use variables info from subprogram from previous CU with the same DIE offset -- that is why we have different numbers for this case.

Yep, that sounds reasonably buggy & in the absence of LTO/if this code isn't intended to handle the LTO case (which it seems it isn't) then moving the variables to the inner scope for now - pending work to address LTO - sounds right to me.

Here's a test case for the scope change fix/bug with split-dwarf:
a.cpp:

__attribute__((optnone)) static void x() {
}
__attribute__((always_inline)) static void y() {
  int var;
  x();
}
void f1() {
  y();
}

b.cpp:

__attribute__((optnone)) static void x() {
}
__attribute__((always_inline)) static void y() {
  int var;
  x();
}
void f2() {
  y();
}

c.cpp:

void f1();
void f2();
int main() {
  f1();
  f2();
}

$ clang++ a.cpp b.cpp c.cpp -g -gsplit-dwarf -O1
$ llvm-dwarfdump --statistics a.out

(you could compile c.cpp without debug info, or put main in a or b.cpp at the end, I think it'd still work).

With the current coed, this reports:

"#variables processed by location statistics": 3,                                                             
"#variables with 0% of parent scope covered by DW_AT_location": 3,    
...
"#local vars processed by location statistics": 3,                                                                        
"#local vars with 0% of parent scope covered by DW_AT_location": 3,

Moving the variables into the nested scope causes these to all change to 2, which is correct.

Great! Thanks a lot!
This is the patch: https://reviews.llvm.org/D100951.

Thanks!

[djtodoro]

In D96045#2705750, @dblaikie wrote:

Oh, and it'd also be really good/important to make this work correctly with LTO (though that'll be more challenging/expensive due to the memory usage - it'll mean basically saving the details of every abstract origin (which means identifying them as such, since they don't expliictly identify themselves as abstract origins) (the expensive part) and then searching through that whenever we see a concrete-or-inlined (& if the search fails, putting the concrete-or-inlined in the list of things to resolve against future abstract origins that are parsed)). Will you be able to do that?

Sure, I'll put it on my TODO list. I have some opened issues I am working on -- as soon as I am finished with it, I'll start working on that. The first step is to make a simple test case.

[dblaikie]

In D96045#2707512, @djtodoro wrote:

In D96045#2705750, @dblaikie wrote:

Oh, and it'd also be really good/important to make this work correctly with LTO (though that'll be more challenging/expensive due to the memory usage - it'll mean basically saving the details of every abstract origin (which means identifying them as such, since they don't expliictly identify themselves as abstract origins) (the expensive part) and then searching through that whenever we see a concrete-or-inlined (& if the search fails, putting the concrete-or-inlined in the list of things to resolve against future abstract origins that are parsed)). Will you be able to do that?

Sure, I'll put it on my TODO list. I have some opened issues I am working on -- as soon as I am finished with it, I'll start working on that. The first step is to make a simple test case.

Thanks - happy to help with that any time. (roughly speaking - one translation unit with a function that's always_inline, and another translation unit that calls that first function - link them together with lto enabled and you'll get a cross-CU inlining)

llvm/test/tools/llvm-dwarfdump/X86/locstats-for-inlined-vars.yaml

+# RUN: yaml2obj %s | llvm-dwarfdump --statistics - | FileCheck %s
+
+## Check that zero coverage was reported for inlined variable with
+## DW_AT_abstract_origin with no location attribute as well as
+## for the variable that has not been generated within the inlined subroutine.
+##
+## The yaml represents DWARF as:
+##
+## DW_TAG_compile_unit
+##   DW_AT_low_pc      (0x0000000000000000)
+##   DW_TAG_subprogram <-- (0x00000014)
+##     DW_AT_decl_file (0x01)
+##     DW_AT_decl_line (1)
+##     DW_AT_inline    (DW_INL_inlined)
+##     DW_TAG_formal_parameter <-- (0x00000018)
+##       DW_AT_decl_file       (0x01)
+##       DW_AT_decl_line       (1)
+##     DW_TAG_formal_parameter <-- (0x0000001b)
+##       DW_AT_decl_file       (0x01)
+##       DW_AT_decl_line       (1)
+##     DW_TAG_variable <-- (0x0000001e)
+##       DW_AT_decl_file       (0x01)
+##       DW_AT_decl_line       (1)
+##   DW_TAG_subprogram
+##     DW_AT_decl_file (0x01)
+##     DW_AT_decl_line (1)
+##     DW_AT_low_pc    (0x0000000000000000)
+##     DW_AT_high_pc   (0x000000000000000b)
+##     DW_TAG_inlined_subroutine
+##       DW_AT_abstract_origin (0x00000014)
+##       DW_AT_low_pc  (0x0000000000000000)
+##       DW_AT_high_pc (0x0000000000000007)
+##       DW_AT_call_file       (0x01)
+##       DW_AT_call_line       (1)
+##       DW_AT_call_column     (0x01)
+##       DW_TAG_formal_parameter
+##         DW_AT_abstract_origin       (0x00000018)
+##       DW_TAG_variable
+##         DW_AT_abstract_origin       (0x0000001e)
+##         DW_AT_location      ()
+##     DW_TAG_inlined_subroutine
+##       DW_AT_abstract_origin       (0x00000014)
+##       DW_AT_low_pc        (0x0000000000000000)
+##       DW_AT_high_pc       (0x0000000000000007)
+##       DW_AT_call_file     (0x01)
+##       DW_AT_call_line     (1)
+##       DW_AT_call_column   (0x01)
+##       DW_TAG_formal_parameter
+##         DW_AT_abstract_origin     (0x00000018)
+##         DW_AT_location    ()
+##       DW_TAG_variable
+##         DW_AT_abstract_origin     (0x0000001e)
+##         DW_AT_location    ()
+
+# CHECK: "version": 6,
+# CHECK: "#variables processed by location statistics": 6,
+# CHECK: "#variables with 0% of parent scope covered by DW_AT_location": 3,
+# CHECK: "#variables with 100% of parent scope covered by DW_AT_location": 3,
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+DWARF:
+  debug_abbrev:
+    - Table:
+      - Code: 1
+        Tag: DW_TAG_compile_unit
+        Children: DW_CHILDREN_yes
+        Attributes:
+          - Attribute: DW_AT_low_pc
+            Form: DW_FORM_addr
+      - Code: 2
+        Tag: DW_TAG_subprogram
+        Children: DW_CHILDREN_yes
+        Attributes:
+          - Attribute: DW_AT_decl_file
+            Form: DW_FORM_data1
+          - Attribute: DW_AT_decl_line
+            Form: DW_FORM_data1
+          - Attribute: DW_AT_inline
+            Form: DW_FORM_data1
+      - Code: 3
+        Tag: DW_TAG_formal_parameter
+        Children: DW_CHILDREN_no
+        Attributes:
+          - Attribute: DW_AT_decl_file
+            Form: DW_FORM_data1
+          - Attribute: DW_AT_decl_line
+            Form: DW_FORM_data1
+      - Code: 4
+        Tag: DW_TAG_formal_parameter
+        Children: DW_CHILDREN_no
+        Attributes:
+          - Attribute: DW_AT_decl_file
+            Form: DW_FORM_data1
+          - Attribute: DW_AT_decl_line
+            Form: DW_FORM_data1
+      - Code: 5
+        Tag: DW_TAG_variable
+        Children: DW_CHILDREN_no
+        Attributes:
+          - Attribute: DW_AT_decl_file
+            Form: DW_FORM_data1
+          - Attribute: DW_AT_decl_line
+            Form: DW_FORM_data1
+      - Code: 6
+        Tag: DW_TAG_subprogram
+        Children: DW_CHILDREN_yes
+        Attributes:
+          - Attribute: DW_AT_decl_file
+            Form: DW_FORM_data1
+          - Attribute: DW_AT_decl_line
+            Form: DW_FORM_data1
+          - Attribute: DW_AT_low_pc
+            Form: DW_FORM_addr
+          - Attribute: DW_AT_high_pc
+            Form: DW_FORM_data4
+      - Code: 7
+        Tag: DW_TAG_inlined_subroutine
+        Children: DW_CHILDREN_yes
+        Attributes:
+          - Attribute: DW_AT_abstract_origin
+            Form:      DW_FORM_ref4
+          - Attribute: DW_AT_low_pc
+            Form: DW_FORM_addr
+          - Attribute: DW_AT_high_pc
+            Form: DW_FORM_data4
+          - Attribute: DW_AT_call_file
+            Form: DW_FORM_data1
+          - Attribute: DW_AT_call_line
+            Form: DW_FORM_data1
+          - Attribute: DW_AT_call_column
+            Form: DW_FORM_data1
+      - Code: 8
+        Tag: DW_TAG_formal_parameter
+        Children: DW_CHILDREN_no
+        Attributes:
+          - Attribute: DW_AT_abstract_origin
+            Form: DW_FORM_ref4
+      - Code: 9
+        Tag: DW_TAG_variable
+        Children: DW_CHILDREN_no
+        Attributes:
+          - Attribute: DW_AT_abstract_origin
+            Form: DW_FORM_ref4
+          - Attribute: DW_AT_location
+            Form: DW_FORM_exprloc
+      - Code: 10
+        Tag: DW_TAG_formal_parameter
+        Children: DW_CHILDREN_no
+        Attributes:
+          - Attribute: DW_AT_abstract_origin
+            Form: DW_FORM_ref4
+          - Attribute: DW_AT_location
+            Form: DW_FORM_exprloc
+  debug_info:
+    - Version:    4
+      AbbrOffset: 0x00
+      Entries:
+        - AbbrCode: 1 ## DW_TAG_compile_unit
+          Values:
+            - Value: 0x00 ## DW_AT_producer
+        - AbbrCode: 2 ## DW_TAG_subprogram
+          Values:
+            - Value: 1 ## DW_AT_decl_file
+            - Value: 1 ## DW_AT_decl_line
+            - Value: 1 ## DW_AT_inline
+        - AbbrCode: 3 ## DW_TAG_formal_parameter
+          Values:
+            - Value: 1 ## DW_AT_decl_file
+            - Value: 1 ## DW_AT_decl_line
+        - AbbrCode: 4 ## DW_TAG_formal_parameter
+          Values:
+            - Value: 1 ## DW_AT_decl_file
+            - Value: 1 ## DW_AT_decl_line
+        - AbbrCode: 5 ## DW_TAG_variable
+          Values:
+            - Value: 1 ## DW_AT_decl_file
+            - Value: 1 ## DW_AT_decl_line
+        - AbbrCode: 0 ## NULL
+        - AbbrCode: 6 ## DW_TAG_subprogram
+          Values:
+            - Value: 1 ## DW_AT_decl_file
+            - Value: 1 ## DW_AT_decl_line
+            - Value: 0x00 ## DW_AT_low_pc
+            - Value: 0x0b ## DW_AT_high_pc
+        - AbbrCode: 7 ## DW_TAG_inlined_subroutine
+          Values:
+            - Value: 0x14 ## DW_AT_abstract_origin
+            - Value: 0x00 ## DW_AT_low_pc
+            - Value: 0x07 ## DW_AT_high_pc
+            - Value: 1 ## DW_AT_call_file
+            - Value: 1 ## DW_AT_call_line
+            - Value: 1 ## DW_AT_call_column
+        - AbbrCode: 8 ## DW_TAG_formal_parameter
+          Values:
+            - Value: 0x18 ## DW_AT_abstract_origin
+        - AbbrCode: 9 ## DW_TAG_formal_parameter
+          Values:
+            - Value: 0x1e ## DW_AT_abstract_origin
+            - Value: 0x0 ## DW_AT_location
+        - AbbrCode: 0 ## NULL
+        - AbbrCode: 7 ## DW_TAG_inlined_subroutine
+          Values:
+            - Value: 0x14 ## DW_AT_abstract_origin
+            - Value: 0x02 ## DW_AT_low_pc
+            - Value: 0x08 ## DW_AT_high_pc
+            - Value: 2 ## DW_AT_call_file
+            - Value: 2 ## DW_AT_call_line
+            - Value: 2 ## DW_AT_call_column
+        - AbbrCode: 10 ## DW_TAG_formal_parameter
+          Values:
+            - Value: 0x18 ## DW_AT_abstract_origin
+            - Value: 0x0 ## DW_AT_location
+        - AbbrCode: 9 ## DW_TAG_formal_parameter
+          Values:
+            - Value: 0x1e ## DW_AT_abstract_origin
+            - Value: 0x0 ## DW_AT_location
+        - AbbrCode: 0 ## NULL
+        - AbbrCode: 0 ## NULL

llvm/tools/llvm-dwarfdump/Statistics.cpp

 //===-- Statistics.cpp - Debug Info quality metrics -----------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm-dwarfdump.h"
 #include "llvm/ADT/DenseMap.h"
-#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSet.h"
-#include "llvm/DebugInfo/DIContext.h"
 #include "llvm/DebugInfo/DWARF/DWARFContext.h"
 #include "llvm/DebugInfo/DWARF/DWARFDebugLoc.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/JSON.h"
 
 #define DEBUG_TYPE "dwarfdump"
 using namespace llvm;
 using namespace llvm::dwarfdump;
 using namespace llvm::object;
 
+namespace {
 /// This represents the number of categories of debug location coverage being
 /// calculated. The first category is the number of variables with 0% location
 /// coverage, but the last category is the number of variables with 100%
 /// location coverage.
 constexpr int NumOfCoverageCategories = 12;
 
-namespace {
+/// This represents variables DIE offsets.
+using InlinedVarsTy = llvm::SmallVector<uint64_t>;
+/// This maps function DIE offset to its variables.
+using InlinedVarsTyMap = llvm::DenseMap<uint64_t, InlinedVarsTy>;
+/// This maps inlined function site DIE offset to its variables.
+using InlinedSiteVarsTyMap = llvm::DenseMap<uint64_t, InlinedVarsTy>;
+
 /// Holds statistics for one function (or other entity that has a PC range and
 /// contains variables, such as a compile unit).
 struct PerFunctionStats {
   /// Number of inlined instances of this function.
   unsigned NumFnInlined = 0;
   /// Number of out-of-line instances of this function.
   unsigned NumFnOutOfLine = 0;
   /// Number of inlined instances that have abstract origins.
static void collectLocStats(uint64_t ScopeBytesCovered, uint64_t BytesInScope,
 
   unsigned CoverageBucket = getCoverageBucket();
   VarParamLocStats[CoverageBucket]++;
   if (IsParam)
     ParamLocStats[CoverageBucket]++;
   else if (IsLocalVar)
     LocalVarLocStats[CoverageBucket]++;
 }
+
 /// Construct an identifier for a given DIE from its Prefix, Name, DeclFileName
 /// and DeclLine. The identifier aims to be unique for any unique entities,
 /// but keeping the same among different instances of the same entity.
 static std::string constructDieID(DWARFDie Die,
                                   StringRef Prefix = StringRef()) {
   std::string IDStr;
   llvm::raw_string_ostream ID(IDStr);
   ID << Prefix

 }
 
 /// Collect debug info quality metrics for one DIE.
 static void collectStatsForDie(DWARFDie Die, std::string FnPrefix,
                                std::string VarPrefix, uint64_t BytesInScope,
                                uint32_t InlineDepth,
                                StringMap<PerFunctionStats> &FnStatMap,
                                GlobalStats &GlobalStats,
-                               LocationStats &LocStats) {
+                               LocationStats &LocStats,
+                               InlinedSiteVarsTyMap &InlinedVariables) {
   bool HasLoc = false;
   bool HasSrcLoc = false;
   bool HasType = false;
   uint64_t TotalBytesCovered = 0;
   uint64_t ScopeBytesCovered = 0;
   uint64_t BytesEntryValuesCovered = 0;
   auto &FnStats = FnStatMap[FnPrefix];
   bool IsParam = Die.getTag() == dwarf::DW_TAG_formal_parameter;
static void collectStatsForDie(DWARFDie Die, std::string FnPrefix,
 
   if (Die.findRecursively(dwarf::DW_AT_decl_file) &&
       Die.findRecursively(dwarf::DW_AT_decl_line))
     HasSrcLoc = true;
 
   if (Die.findRecursively(dwarf::DW_AT_type))
     HasType = true;
 
+  // Check if it is an inlined variable.
+  if (Die.find(dwarf::DW_AT_abstract_origin)) {
+    DWARFDie Parent = Die.getParent();
+    auto &InlineSiteVars = InlinedVariables[Parent.getOffset()];
+    if (InlineSiteVars.size()) {
+      if (Die.find(dwarf::DW_AT_location) ||
+          Die.find(dwarf::DW_AT_const_value)) {
+        auto Offset = Die.find(dwarf::DW_AT_abstract_origin);
+        // Do not track this inlined var any more, since it has location
+        // coverage.
+        InlineSiteVars.erase(std::remove(InlineSiteVars.begin(),
+                                         InlineSiteVars.end(),
+                                         (*Offset).getRawUValue()),
+                             InlineSiteVars.end());
+      } else {
+        // The rest of the stats will be handled at the end of
+        // the collectStatsRecursive().
+        return;
+      }
+    }
+  }
+
   auto IsEntryValue = [&](ArrayRef<uint8_t> D) -> bool {
     DWARFUnit *U = Die.getDwarfUnit();
     DataExtractor Data(toStringRef(D),
                        Die.getDwarfUnit()->getContext().isLittleEndian(), 0);
     DWARFExpression Expression(Data, U->getAddressByteSize(),
                                U->getFormParams().Format);
     // Consider the expression containing the DW_OP_entry_value as
     // an entry value.

 }
 
 /// Recursively collect debug info quality metrics.
 static void collectStatsRecursive(DWARFDie Die, std::string FnPrefix,
                                   std::string VarPrefix, uint64_t BytesInScope,
                                   uint32_t InlineDepth,
                                   StringMap<PerFunctionStats> &FnStatMap,
                                   GlobalStats &GlobalStats,
-                                  LocationStats &LocStats) {
+                                  LocationStats &LocStats,
+                                  InlinedVarsTyMap &GlobalInlinedFnInfo,
+                                  InlinedSiteVarsTyMap &InlinedVariables) {
   const dwarf::Tag Tag = Die.getTag();
   // Skip function types.
   if (Tag == dwarf::DW_TAG_subroutine_type)
     return;
 
   // Handle any kind of lexical scope.
   const bool IsFunction = Tag == dwarf::DW_TAG_subprogram;
   const bool IsBlock = Tag == dwarf::DW_TAG_lexical_block;
   const bool IsInlinedFunction = Tag == dwarf::DW_TAG_inlined_subroutine;
+  std::string FnID;
   if (IsFunction || IsInlinedFunction || IsBlock) {
 
     // Reset VarPrefix when entering a new function.
-    if (Die.getTag() == dwarf::DW_TAG_subprogram ||
-        Die.getTag() == dwarf::DW_TAG_inlined_subroutine)
+    if (IsFunction || IsInlinedFunction)
       VarPrefix = "v";
 
     // Ignore forward declarations.
     if (Die.find(dwarf::DW_AT_declaration))
       return;
 
     // Check for call sites.
     if (Die.find(dwarf::DW_AT_call_file) && Die.find(dwarf::DW_AT_call_line))
       GlobalStats.CallSiteEntries++;
 
     // PC Ranges.
     auto RangesOrError = Die.getAddressRanges();
     if (!RangesOrError) {
       llvm::consumeError(RangesOrError.takeError());
       return;
     }
 
     auto Ranges = RangesOrError.get();
     uint64_t BytesInThisScope = 0;
     for (auto Range : Ranges)
       BytesInThisScope += Range.HighPC - Range.LowPC;
 
     // Count the function.
     if (!IsBlock) {
-      // Skip over abstract origins.
-      if (Die.find(dwarf::DW_AT_inline))
+      // Skip over abstract origins, but collect variables
+      // from it so it can be used for location statistics
+      // for inlined instancies.
+      if (Die.find(dwarf::DW_AT_inline)) {
+        DWARFDie Child = Die.getFirstChild();
+        while (Child) {
+          const dwarf::Tag ChildTag = Child.getTag();
+          if (ChildTag == dwarf::DW_TAG_formal_parameter ||
+              ChildTag == dwarf::DW_TAG_variable)
+            GlobalInlinedFnInfo[Die.getOffset()].push_back(Child.getOffset());
+          Child = Child.getSibling();
+        }
         return;
-      std::string FnID = constructDieID(Die);
+      }
+
+      FnID = constructDieID(Die);
       // We've seen an instance of this function.
       auto &FnStats = FnStatMap[FnID];
       FnStats.IsFunction = true;
       if (IsInlinedFunction) {
         FnStats.NumFnInlined++;
         if (Die.findRecursively(dwarf::DW_AT_abstract_origin))
           FnStats.NumAbstractOrigins++;
       } else {
if (BytesInThisScope) {
       if (IsFunction)
         GlobalStats.FunctionSize += BytesInThisScope;
       else if (IsInlinedFunction && InlineDepth == 0)
         GlobalStats.InlineFunctionSize += BytesInThisScope;
     }
   } else {
     // Not a scope, visit the Die itself. It could be a variable.
     collectStatsForDie(Die, FnPrefix, VarPrefix, BytesInScope, InlineDepth,
-                       FnStatMap, GlobalStats, LocStats);
+                       FnStatMap, GlobalStats, LocStats, InlinedVariables);
   }
 
   // Set InlineDepth correctly for child recursion
   if (IsFunction)
     InlineDepth = 0;
   else if (IsInlinedFunction)
     ++InlineDepth;
 
+  // Get the vars of the inlined fn, so the locstats
+  // reports the missing vars (with coverage 0%).
+  if (IsInlinedFunction) {
+    auto OffsetFn = Die.find(dwarf::DW_AT_abstract_origin);
+    if (OffsetFn)
+      InlinedVariables[Die.getOffset()] =
+          GlobalInlinedFnInfo[(*OffsetFn).getRawUValue()];
+  }
+
   // Traverse children.
   unsigned LexicalBlockIndex = 0;
   unsigned FormalParameterIndex = 0;
   DWARFDie Child = Die.getFirstChild();
   while (Child) {
     std::string ChildVarPrefix = VarPrefix;
     if (Child.getTag() == dwarf::DW_TAG_lexical_block)
       ChildVarPrefix += toHex(LexicalBlockIndex++) + '.';
     if (Child.getTag() == dwarf::DW_TAG_formal_parameter)
       ChildVarPrefix += 'p' + toHex(FormalParameterIndex++) + '.';
 
     collectStatsRecursive(Child, FnPrefix, ChildVarPrefix, BytesInScope,
-                          InlineDepth, FnStatMap, GlobalStats, LocStats);
+                          InlineDepth, FnStatMap, GlobalStats, LocStats,
+                          GlobalInlinedFnInfo, InlinedVariables);
     Child = Child.getSibling();
   }
+
+  if (!IsInlinedFunction)
+    return;
+
+  // After we have processed all vars of the inlined function,
+  // we want to know how many variables have no location.
+  constexpr unsigned ZeroCoverageBucket = 0;
+  auto &InlinedFnStats = FnStatMap[FnID];
+  auto &InlineSiteVars = InlinedVariables[Die.getOffset()];
+  for (auto Offset : InlineSiteVars) {
+    // We are done with this variable of this inlined site.
+    InlineSiteVars.erase(
+        std::remove(InlineSiteVars.begin(), InlineSiteVars.end(), Offset),
+        InlineSiteVars.end());
+
+    auto InlineVarDie = Die.getDwarfUnit()->getDIEForOffset(Offset);
+    LocStats.NumVarParam++;
+    LocStats.VarParamLocStats[ZeroCoverageBucket]++;
+    bool HasSrcLoc = false;
+    bool HasType = false;
+
+    if (InlineVarDie.find(dwarf::DW_AT_decl_file) &&
+        InlineVarDie.find(dwarf::DW_AT_decl_line))
+      HasSrcLoc = true;
+
+    if (InlineVarDie.find(dwarf::DW_AT_type))
+      HasType = true;
+
+    if (BytesInScope)
+      GlobalStats.ScopeBytes += BytesInScope;
+    auto Tag = InlineVarDie.getTag();
+    if (Tag == dwarf::DW_TAG_formal_parameter) {
+      LocStats.NumParam++;
+      LocStats.ParamLocStats[ZeroCoverageBucket]++;
+      InlinedFnStats.NumParams++;
+      GlobalStats.ParamScopeBytes += BytesInScope;
+      if (HasType)
+        InlinedFnStats.NumParamTypes++;
+      if (HasSrcLoc)
+        InlinedFnStats.NumParamSourceLocations++;
+    } else if (Tag == dwarf::DW_TAG_variable) {
+      LocStats.NumVar++;
+      LocStats.LocalVarLocStats[ZeroCoverageBucket]++;
+      InlinedFnStats.NumLocalVars++;
+      GlobalStats.LocalVarScopeBytes += BytesInScope;
+      if (HasType)
+        InlinedFnStats.NumLocalVarTypes++;
+      if (HasSrcLoc)
+        InlinedFnStats.NumLocalVarSourceLocations++;
+    }
+  }
+  assert(!InlineSiteVars.size() &&
+         "the inlined var map for the site should be clean");
 }
 
 /// Print human-readable output.
 /// \{
 static void printDatum(json::OStream &J, const char *Key, json::Value Value) {
   J.attribute(Key, Value);
   LLVM_DEBUG(llvm::dbgs() << Key << ": " << Value << '\n');
 }

 /// useful, only the delta between compiling the same program with different
 /// compilers is.
 bool dwarfdump::collectStatsForObjectFile(ObjectFile &Obj, DWARFContext &DICtx,
                                           const Twine &Filename,
                                           raw_ostream &OS) {
   StringRef FormatName = Obj.getFileFormatName();
   GlobalStats GlobalStats;
   LocationStats LocStats;
+  InlinedVarsTyMap GlobalInlinedFnInfo;
+  InlinedSiteVarsTyMap InlinedVariables;
   StringMap<PerFunctionStats> Statistics;
   for (const auto &CU : static_cast<DWARFContext *>(&DICtx)->compile_units())
     if (DWARFDie CUDie = CU->getNonSkeletonUnitDIE(false))
       collectStatsRecursive(CUDie, "/", "g", 0, 0, Statistics, GlobalStats,
-                            LocStats);
+                            LocStats, GlobalInlinedFnInfo, InlinedVariables);
 
   /// Collect the sizes of debug sections.
   SectionSizes Sizes;
   calculateSectionSizes(Obj, Sizes, Filename);
 
   /// The version number should be increased every time the algorithm is changed
   /// (including bug fixes). New metrics may be added without increasing the
   /// version.
   unsigned Version = 6;
   unsigned VarParamTotal = 0;
   unsigned VarParamUnique = 0;
   unsigned VarParamWithLoc = 0;
   unsigned NumFunctions = 0;
   unsigned NumInlinedFunctions = 0;
   unsigned NumFuncsWithSrcLoc = 0;
   unsigned NumAbstractOrigins = 0;
   unsigned ParamTotal = 0;