Skip to content

Commit 30ef50b

Browse files
committed
[lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members
**Background** Prior to DWARFv4, there was no clear normative text on how to handle static data members. Non-normative text suggested we compilers should use `DW_AT_external` to mark static data members of structrues/unions. Clang does this consistently. However, GCC doesn't, e.g., when the structure/union is in an anonymous namespace (which is C++ standard conformant). Additionally, GCC never emits `DW_AT_data_member_location`s for union members (regardless of storage linkage and storage duration). Since DWARFv5 (issue 161118.1), static data members get emitted as `DW_TAG_variable`. LLDB used to differentiate between static and non-static members by checking the `DW_AT_external` flag and the absence of `DW_AT_data_member_location`. With D18008 LLDB started to pretend that union members always have a `0` `DW_AT_data_member_location` by default (because GCC never emits these locations). In D124409 LLDB stopped checking the `DW_AT_external` flag to account for the case where GCC doesn't emit the flag for types in anonymous namespaces; instead we only check for presence of `DW_AT_data_member_location`s. The combination of these changes then meant that LLDB would never correctly detect that a union has static data members. **Solution** Instead of unconditionally initializing the `member_byte_offset` to `0` specifically for union members, this patch proposes to check for both the absence of `DW_AT_data_member_location` and `DW_AT_declaration`, which consistently gets emitted for static data members on GCC and Clang. We initialize the `member_byte_offset` to `0` anyway if we determine it wasn't a static. So removing the special case for unions makes this code simpler to reason about. Long-term, we should just use DWARFv5's new representation for static data members. Fixes llvm#68135
1 parent 1964118 commit 30ef50b

File tree

4 files changed

+87
-6
lines changed

4 files changed

+87
-6
lines changed

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

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2482,8 +2482,9 @@ struct MemberAttributes {
24822482
DWARFFormValue encoding_form;
24832483
/// Indicates the byte offset of the word from the base address of the
24842484
/// structure.
2485-
uint32_t member_byte_offset;
2485+
uint32_t member_byte_offset = UINT32_MAX;
24862486
bool is_artificial = false;
2487+
bool is_declaration = false;
24872488
};
24882489

24892490
/// Parsed form of all attributes that are relevant for parsing Objective-C
@@ -2656,8 +2657,6 @@ DiscriminantValue &VariantPart::discriminant() { return this->_discriminant; }
26562657
MemberAttributes::MemberAttributes(const DWARFDIE &die,
26572658
const DWARFDIE &parent_die,
26582659
ModuleSP module_sp) {
2659-
member_byte_offset = (parent_die.Tag() == DW_TAG_union_type) ? 0 : UINT32_MAX;
2660-
26612660
DWARFAttributes attributes = die.GetAttributes();
26622661
for (size_t i = 0; i < attributes.Size(); ++i) {
26632662
const dw_attr_t attr = attributes.AttributeAtIndex(i);
@@ -2717,6 +2716,9 @@ MemberAttributes::MemberAttributes(const DWARFDIE &die,
27172716
case DW_AT_artificial:
27182717
is_artificial = form_value.Boolean();
27192718
break;
2719+
case DW_AT_declaration:
2720+
is_declaration = form_value.Boolean();
2721+
break;
27202722
default:
27212723
break;
27222724
}
@@ -2923,10 +2925,18 @@ void DWARFASTParserClang::ParseSingleMember(
29232925
if (class_is_objc_object_or_interface)
29242926
attrs.accessibility = eAccessNone;
29252927

2926-
// Handle static members, which is any member that doesn't have a bit or a
2927-
// byte member offset.
2928+
// Handle static members, which are typically members without
2929+
// locations. However, GCC *never* emits DW_AT_data_member_location
2930+
// for static data members of unions.
2931+
// Non-normative text pre-DWARFv5 recommends marking static
2932+
// data members with an DW_AT_external flag. Clang emits this consistently
2933+
// whereas GCC emits it only for static data members if not part of an
2934+
// anonymous namespace. The flag that is consistently emitted for static
2935+
// data members is DW_AT_declaration, so we check it instead.
2936+
// FIXME: Since DWARFv5, static data members are marked DW_AT_variable so we can
2937+
// consistently detect them on both GCC and Clang without below heuristic.
29282938
if (attrs.member_byte_offset == UINT32_MAX &&
2929-
attrs.data_bit_offset == UINT64_MAX) {
2939+
attrs.data_bit_offset == UINT64_MAX && attrs.is_declaration) {
29302940
Type *var_type = die.ResolveTypeUID(attrs.encoding_form.Reference());
29312941

29322942
if (var_type) {
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
CXX_SOURCES := main.cpp
2+
3+
include Makefile.rules
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
"""
2+
Tests that frame variable and expr work for
3+
C++ unions and their static data members.
4+
"""
5+
import lldb
6+
from lldbsuite.test.lldbtest import *
7+
from lldbsuite.test.decorators import *
8+
import lldbsuite.test.lldbutil as lldbutil
9+
10+
class CppUnionStaticMembersTestCase(TestBase):
11+
def test(self):
12+
"""Tests that frame variable and expr work
13+
for union static data members"""
14+
self.build()
15+
16+
(target, process, main_thread, _) = lldbutil.run_to_source_breakpoint(
17+
self, "return 0", lldb.SBFileSpec("main.cpp")
18+
)
19+
20+
self.expect("frame variable foo", substrs=["val = 42"])
21+
self.expect("frame variable bar", substrs=["val = 137"])
22+
23+
self.expect_expr("foo", result_type="Foo", result_children=[ValueCheck(
24+
name="val", value="42"
25+
)])
26+
self.expect_expr("bar", result_type="Bar", result_children=[ValueCheck(
27+
name="val", value="137"
28+
)])
29+
30+
self.expect_expr("Foo::sVal1", result_type="const int", result_value="-42")
31+
self.expect_expr("Foo::sVal2", result_type="Foo", result_children=[ValueCheck(
32+
name="val", value="42"
33+
)])
34+
35+
@expectedFailureAll
36+
def test_union_in_anon_namespace(self):
37+
"""Tests that frame variable and expr work
38+
for union static data members in anonymous
39+
namespaces"""
40+
self.expect_expr("Bar::sVal1", result_type="const int", result_value="-137")
41+
self.expect_expr("Bar::sVal2", result_type="Bar", result_children=[ValueCheck(
42+
name="val", value="137"
43+
)])
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
union Foo {
2+
int val = 42;
3+
static const int sVal1 = -42;
4+
static Foo sVal2;
5+
};
6+
7+
Foo Foo::sVal2{};
8+
9+
namespace {
10+
union Bar {
11+
int val = 137;
12+
static const int sVal1 = -137;
13+
static Bar sVal2;
14+
};
15+
16+
Bar Bar::sVal2{};
17+
} // namespace
18+
19+
int main() {
20+
Foo foo;
21+
Bar bar;
22+
auto sum = Bar::sVal1 + Foo::sVal1 + Foo::sVal2.val + Bar::sVal2.val;
23+
24+
return 0;
25+
}

0 commit comments

Comments
 (0)