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

6

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

23–26

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

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

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

struct MemberAttributes {
   uint64_t data_bit_offset = UINT64_MAX;
   AccessType accessibility = eAccessNone;
   llvm::Optional<uint64_t> byte_size;
   DWARFFormValue encoding_form;
   /// Indicates the byte offset of the word from the base address of the
   /// structure.
   uint32_t member_byte_offset;
   bool is_artificial = false;
-  /// On DW_TAG_members, this means the member is static.
-  bool is_external = false;
 };
 
 /// Parsed form of all attributes that are relevant for parsing Objective-C
 /// properties.
 struct PropertyAttributes {
   explicit PropertyAttributes(const DWARFDIE &die);
   const char *prop_name = nullptr;
   const char *prop_getter_name = nullptr;
if (attributes.ExtractFormValueAtIndex(i, form_value)) {
         break;
 
       case DW_AT_accessibility:
         accessibility = DW_ACCESS_to_AccessType(form_value.Unsigned());
         break;
       case DW_AT_artificial:
         is_artificial = form_value.Boolean();
         break;
-      case DW_AT_external:
-        is_external = form_value.Boolean();
-        break;
       default:
         break;
       }
     }
   }
 
   // Clang has a DWARF generation bug where sometimes it represents
   // fields that are references with bad byte size and bit size/offset
void DWARFASTParserClang::ParseSingleMember(
 
   const bool class_is_objc_object_or_interface =
       TypeSystemClang::IsObjCObjectOrInterfaceType(class_clang_type);
 
   // FIXME: Make Clang ignore Objective-C accessibility for expressions
   if (class_is_objc_object_or_interface)
     attrs.accessibility = eAccessNone;
 
-  // Handle static members
-  if (attrs.is_external && attrs.member_byte_offset == UINT32_MAX) {
+  // Handle static members, which is any member that doesn't have a bit or a
+  // byte member offset.
+  if (attrs.member_byte_offset == UINT32_MAX &&
+      attrs.data_bit_offset == UINT64_MAX) {
     Type *var_type = die.ResolveTypeUID(attrs.encoding_form.Reference());
 
     if (var_type) {
       if (attrs.accessibility == eAccessNone)
         attrs.accessibility = eAccessPublic;
       TypeSystemClang::AddVariableToRecordType(
           class_clang_type, attrs.name, var_type->GetForwardCompilerType(),
           attrs.accessibility);
void DWARFASTParserClang::ParseSingleMember(
 
   if (attrs.accessibility == eAccessNone)
     attrs.accessibility = default_accessibility;
 
   uint64_t field_bit_offset = (attrs.member_byte_offset == UINT32_MAX
                                    ? 0
                                    : (attrs.member_byte_offset * 8));
 
   if (attrs.bit_size > 0) {
     FieldInfo this_field_info;
     this_field_info.bit_offset = field_bit_offset;
     this_field_info.bit_size = attrs.bit_size;
 
     if (attrs.data_bit_offset != UINT64_MAX) {
       this_field_info.bit_offset = attrs.data_bit_offset;
     } else {
       if (!attrs.byte_size)

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

+# RUN: llvm-mc --triple=x86_64-pc-linux --filetype=obj %s -o %t
+# RUN: %lldb -o "target variable ug U::s" -b %t | FileCheck %s
+
+# CHECK: (lldb) target variable ug
+# CHECK: (U) ug = (m = 14159265)
+# CHECK: (int) U::s = 65295141
+
+# This tests that a static member in a class declared in the anonymous namespace
+# does not appear as a field of the class. There is a difference between the
+# debug info generated by gcc and clang, where clang flags the static member
+# with DW_AT_external, but gcc does not.
+#
+# Roughly corresponds to this source code:
+#
+# namespace {
+# struct U {
+#   static int s;
+#   int m = 14159265;
+# };
+# int U::s = 65295141;
+# }
+#
+# U ug;
+
+        .file   "test.cpp"
+        .data
+        .quad 0
+ug:
+        .long 14159265
+.Lug_s:
+        .long 65295141
+
+        .section        .debug_info,"",@progbits
+.Ldebug_info0:
+        .long   .Lcu_end-.Lcu_begin
+.Lcu_begin:
+        .value  0x4
+        .long   .Ldebug_abbrev0
+        .byte   0x8
+        .uleb128 0x1
+        .asciz  "GCC DWARF reduced by hand"
+        .byte   0x4
+        .asciz  "test.cpp"
+        .uleb128 0x2
+.LU:
+        .uleb128 0x3
+        .string "U"
+        .byte   0x4
+.LU_s:
+        .uleb128 0x4
+        .string "s"
+        .long   .Lint-.Ldebug_info0
+        .uleb128 0x5
+        .string "m"
+        .long   .Lint-.Ldebug_info0
+        .byte   0
+        .byte   0
+        .byte   0
+        .uleb128 0x6
+        .long   0x2d
+.Lint:
+        .uleb128 0x7
+        .byte   0x4
+        .byte   0x5
+        .string "int"
+        .uleb128 0x9
+        .string "ug"
+        .long   .LU-.Ldebug_info0
+        .uleb128 0x9
+        .byte   0x3
+        .quad   ug
+        .uleb128 0xa
+        .long   .LU_s-.Ldebug_info0
+        .uleb128 0x9
+        .byte   0x3
+        .quad .Lug_s
+        .byte   0
+.Lcu_end:
+        .section        .debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+        .uleb128 0x1
+        .uleb128 0x11
+        .byte   0x1
+        .uleb128 0x25
+        .uleb128 0x8
+        .uleb128 0x13
+        .uleb128 0xb
+        .uleb128 0x3
+        .uleb128 0x8
+        .byte   0
+        .byte   0
+        .uleb128 0x2
+        .uleb128 0x39
+        .byte   0x1
+        .byte   0
+        .byte   0
+        .uleb128 0x3
+        .uleb128 0x13
+        .byte   0x1
+        .uleb128 0x3
+        .uleb128 0x8
+        .uleb128 0xb
+        .uleb128 0xb
+        .byte   0
+        .byte   0
+        .uleb128 0x4
+        .uleb128 0xd
+        .byte   0
+        .uleb128 0x3
+        .uleb128 0x8
+        .uleb128 0x49
+        .uleb128 0x13
+        .uleb128 0x3c
+        .uleb128 0x19
+        .byte   0
+        .byte   0
+        .uleb128 0x5
+        .uleb128 0xd
+        .byte   0
+        .uleb128 0x3
+        .uleb128 0x8
+        .uleb128 0x49
+        .uleb128 0x13
+        .uleb128 0x38
+        .uleb128 0xb
+        .byte   0
+        .byte   0
+        .uleb128 0x6
+        .uleb128 0x3a
+        .byte   0
+        .uleb128 0x18
+        .uleb128 0x13
+        .byte   0
+        .byte   0
+        .uleb128 0x7
+        .uleb128 0x24
+        .byte   0
+        .uleb128 0xb
+        .uleb128 0xb
+        .uleb128 0x3e
+        .uleb128 0xb
+        .uleb128 0x3
+        .uleb128 0x8
+        .byte   0
+        .byte   0
+        .uleb128 0x8
+        .uleb128 0x26
+        .byte   0
+        .uleb128 0x49
+        .uleb128 0x13
+        .byte   0
+        .byte   0
+        .uleb128 0x9
+        .uleb128 0x34
+        .byte   0
+        .uleb128 0x3
+        .uleb128 0x8
+        .uleb128 0x49
+        .uleb128 0x13
+        .uleb128 0x2
+        .uleb128 0x18
+        .byte   0
+        .byte   0
+  .uleb128 0xa
+  .uleb128 0x34
+  .byte 0
+  .uleb128 0x47
+  .uleb128 0x13
+  .uleb128 0x2
+  .uleb128 0x18
+  .byte 0
+  .byte 0
+        .byte   0