Filter non-external static members from SBType::GetFieldAtIndex.

[siggi-alpheus]

Summary

See issue 55040 where static members of classes declared in the anonymous namespace are incorrectly returned as member fields from lldb::SBType::GetFieldAtIndex(). It appears that attrs.member_byte_offset contains a sentinel value for members that don't have a DW_AT_data_member_location.

[labath]

For future reference, it's better to generate a full diff when uploading a patch manually. Here are some ways to do that.

Apart from the inline comment this patch seems fine. We should also add a test case with the non-external static member. I think the test case could be as simple as declaring a global variable of the given type, printing it, and making sure the output is correct. You can use one of the assembly tests in test/Shell/SymbolFile/DWARF/x86/ for inspiration. Or you can wait until I get a bit of time to create one...

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

2672

Have you run the test suite? I'd guess that you'd need to check for DW_AT_data_bit_offset here as well.

[siggi-alpheus]

Thanks for taking a look. Let me see about fixing the test failures and writing that test.

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

2672

Yeah, I ran the tests overnight and I see a couple of tests failing. Will see about fixing and writing a test for the case.

[siggi-alpheus]

Now passes all tests, still needs a test.

[siggi-alpheus]

Now passes all tests - but I still need to write a test for the problem.

[siggi-alpheus]

Add a test that fails against the ToT lldb.

[siggi-alpheus]

Please take a look. This now passes all tests locally, and adds a test that fails against the current ToT lldb.

[siggi-alpheus]

A full diff from origin/main - I hope.

[siggi-alpheus]

Fix awkwardly named variable in the test.

[siggi-alpheus]

This is passing lldb-checks locally. Is there anything else I need to add or tweak, or is this ready for review?

[labath]

Looks good. Thanks for the patch. Do you need me to commit this for you?

Left two comments inline, but I can also fix that when I'm pushing it.

lldb/test/Shell/SymbolFile/DWARF/x86/debug_static-member-anonymous-namespace.s

5 ↗

(On Diff #426452)

Better use some other number here, as it's fairly easy for a value to accidentally come out as zero.

22–25 ↗

(On Diff #426452)

We're not going to be running this, so you don't really need the main function.

[siggi-alpheus]

Thanks!

lldb/test/Shell/SymbolFile/DWARF/x86/debug_static-member-anonymous-namespace.s

18 ↗

(On Diff #428023)

I wonder why I couldn't get the query to work against an initialized global without the main function. Is the assembly directly out of gcc, or did you need to tweak it?
PS: Now I fancy some pie.

[labath]

lldb/test/Shell/SymbolFile/DWARF/x86/debug_static-member-anonymous-namespace.s

18 ↗

(On Diff #428023)

I produced this by modifying/reducing the input you provided (with looking at some gcc output as reference).

Without more details, I can only guess what was the problem for you, but one of issue that comes up in tests like this is that lldb does not like to display variables whose "file address" is zero -- it mistakes them for a null pointer (one day I'll have to fix that). I work around that here, by adding some padding so that the variable does not end up at the address zero.

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

     attrs.accessibility = eAccessNone;
 
   // Handle static members
-  if (attrs.is_external && attrs.member_byte_offset == UINT32_MAX) {
+  if (attrs.member_byte_offset == UINT32_MAX) {
     Type *var_type = die.ResolveTypeUID(attrs.encoding_form.Reference());
 
     if (var_type) {