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

19

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

19

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);
     }
     return;
   }

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 constant" -b %t | FileCheck %s
+
+# CHECK: (U) constant = (m = 0)
+
+# 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.
+#
+# This is the code used to generate the assembly:
+#
+# namespace {
+# struct U {
+#   static const int s = 0;
+#  int m;
+# };
+# }
+#
+# U constant;
+#
+# int main() {
+#   return constant.m;
+# }
+#
+# Compiled with:
+#   gcc -S -g test.cpp
+#
+
+  .file "test.cpp"
+  .text
+.Ltext0:
+  .local  constant
+  .comm constant,4,4
+  .globl  main
+  .type main, @function
+main:
+.LFB0:
+  .file 1 "test.cpp"
+  .loc 1 10 12
+  .cfi_startproc
+  endbr64
+  pushq %rbp
+  .cfi_def_cfa_offset 16
+  .cfi_offset 6, -16
+  movq  %rsp, %rbp
+  .cfi_def_cfa_register 6
+  .loc 1 11 19
+  movl  constant(%rip), %eax
+  .loc 1 12 1
+  popq  %rbp
+  .cfi_def_cfa 7, 8
+  ret
+  .cfi_endproc
+.LFE0:
+  .size main, .-main
+.Letext0:
+  .section  .debug_info,"",@progbits
+.Ldebug_info0:
+  .long 0x96
+  .value  0x4
+  .long .Ldebug_abbrev0
+  .byte 0x8
+  .uleb128 0x1
+  .long .LASF0
+  .byte 0x4
+  .long .LASF1
+  .long .LASF2
+  .quad .Ltext0
+  .quad .Letext0-.Ltext0
+  .long .Ldebug_line0
+  .uleb128 0x2
+  .long 0x51
+  .uleb128 0x3
+  .string "U"
+  .byte 0x4
+  .byte 0x1
+  .byte 0x2
+  .byte 0x8
+  .uleb128 0x4
+  .string "s"
+  .byte 0x1
+  .byte 0x3
+  .byte 0x14
+  .long 0x60
+  .byte 0
+  .uleb128 0x5
+  .string "m"
+  .byte 0x1
+  .byte 0x4
+  .byte 0x6
+  .long 0x59
+  .byte 0
+  .byte 0
+  .byte 0
+  .uleb128 0x6
+  .byte 0x1
+  .byte 0x1
+  .byte 0x1
+  .long 0x2d
+  .uleb128 0x7
+  .byte 0x4
+  .byte 0x5
+  .string "int"
+  .uleb128 0x8
+  .long 0x59
+  .uleb128 0x9
+  .long .LASF3
+  .byte 0x1
+  .byte 0x8
+  .byte 0x3
+  .long 0x32
+  .uleb128 0x9
+  .byte 0x3
+  .quad constant
+  .uleb128 0xa
+  .long .LASF4
+  .byte 0x1
+  .byte 0xa
+  .byte 0x5
+  .long 0x59
+  .quad .LFB0
+  .quad .LFE0-.LFB0
+  .uleb128 0x1
+  .byte 0x9c
+  .byte 0
+  .section  .debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+  .uleb128 0x1
+  .uleb128 0x11
+  .byte 0x1
+  .uleb128 0x25
+  .uleb128 0xe
+  .uleb128 0x13
+  .uleb128 0xb
+  .uleb128 0x3
+  .uleb128 0xe
+  .uleb128 0x1b
+  .uleb128 0xe
+  .uleb128 0x11
+  .uleb128 0x1
+  .uleb128 0x12
+  .uleb128 0x7
+  .uleb128 0x10
+  .uleb128 0x17
+  .byte 0
+  .byte 0
+  .uleb128 0x2
+  .uleb128 0x39
+  .byte 0x1
+  .uleb128 0x1
+  .uleb128 0x13
+  .byte 0
+  .byte 0
+  .uleb128 0x3
+  .uleb128 0x13
+  .byte 0x1
+  .uleb128 0x3
+  .uleb128 0x8
+  .uleb128 0xb
+  .uleb128 0xb
+  .uleb128 0x3a
+  .uleb128 0xb
+  .uleb128 0x3b
+  .uleb128 0xb
+  .uleb128 0x39
+  .uleb128 0xb
+  .byte 0
+  .byte 0
+  .uleb128 0x4
+  .uleb128 0xd
+  .byte 0
+  .uleb128 0x3
+  .uleb128 0x8
+  .uleb128 0x3a
+  .uleb128 0xb
+  .uleb128 0x3b
+  .uleb128 0xb
+  .uleb128 0x39
+  .uleb128 0xb
+  .uleb128 0x49
+  .uleb128 0x13
+  .uleb128 0x3c
+  .uleb128 0x19
+  .uleb128 0x1c
+  .uleb128 0xb
+  .byte 0
+  .byte 0
+  .uleb128 0x5
+  .uleb128 0xd
+  .byte 0
+  .uleb128 0x3
+  .uleb128 0x8
+  .uleb128 0x3a
+  .uleb128 0xb
+  .uleb128 0x3b
+  .uleb128 0xb
+  .uleb128 0x39
+  .uleb128 0xb
+  .uleb128 0x49
+  .uleb128 0x13
+  .uleb128 0x38
+  .uleb128 0xb
+  .byte 0
+  .byte 0
+  .uleb128 0x6
+  .uleb128 0x3a
+  .byte 0
+  .uleb128 0x3a
+  .uleb128 0xb
+  .uleb128 0x3b
+  .uleb128 0xb
+  .uleb128 0x39
+  .uleb128 0xb
+  .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 0xe
+  .uleb128 0x3a
+  .uleb128 0xb
+  .uleb128 0x3b
+  .uleb128 0xb
+  .uleb128 0x39
+  .uleb128 0xb
+  .uleb128 0x49
+  .uleb128 0x13
+  .uleb128 0x2
+  .uleb128 0x18
+  .byte 0
+  .byte 0
+  .uleb128 0xa
+  .uleb128 0x2e
+  .byte 0
+  .uleb128 0x3f
+  .uleb128 0x19
+  .uleb128 0x3
+  .uleb128 0xe
+  .uleb128 0x3a
+  .uleb128 0xb
+  .uleb128 0x3b
+  .uleb128 0xb
+  .uleb128 0x39
+  .uleb128 0xb
+  .uleb128 0x49
+  .uleb128 0x13
+  .uleb128 0x11
+  .uleb128 0x1
+  .uleb128 0x12
+  .uleb128 0x7
+  .uleb128 0x40
+  .uleb128 0x18
+  .uleb128 0x2117
+  .uleb128 0x19
+  .byte 0
+  .byte 0
+  .byte 0
+  .section  .debug_aranges,"",@progbits
+  .long 0x2c
+  .value  0x2
+  .long .Ldebug_info0
+  .byte 0x8
+  .byte 0
+  .value  0
+  .value  0
+  .quad .Ltext0
+  .quad .Letext0-.Ltext0
+  .quad 0
+  .quad 0
+  .section  .debug_line,"",@progbits
+.Ldebug_line0:
+  .section  .debug_str,"MS",@progbits,1
+.LASF2:
+  .string "/workspaces/llvm-codespace/llvm-project/lldb/test/Shell/SymbolFile/DWARF/x86"
+.LASF3:
+  .string "constant"
+.LASF1:
+  .string "test.cpp"
+.LASF0:
+  .string "GNU C++14 10.3.0 -mtune=generic -march=x86-64 -g -O0 -fasynchronous-unwind-tables -fstack-protector-strong -fstack-clash-protection -fcf-protection"
+.LASF4:
+  .string "main"
+  .ident  "GCC: (Ubuntu 10.3.0-1ubuntu1) 10.3.0"
+  .section  .note.GNU-stack,"",@progbits
+  .section  .note.gnu.property,"a"
+  .align 8
+  .long  1f - 0f
+  .long  4f - 1f
+  .long  5
+0:
+  .string  "GNU"
+1:
+  .align 8
+  .long  0xc0000002
+  .long  3f - 2f
+2:
+  .long  0x3
+3:
+  .align 8
+4: