[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
Michael137 wrote: @antmox Addressed in https://github.com/llvm/llvm-project/pull/68408 https://github.com/llvm/llvm-project/pull/68300 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
Michael137 wrote: > Hello! It looks like this broke lldb-aarch64-windows bot: > https://lab.llvm.org/buildbot/#/builders/219/builds/6130 Could you please > look at this ? Yup! Was in the process of fixing. I think we just skip this test on windows https://github.com/llvm/llvm-project/pull/68300 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
antmox wrote: Hello! It looks like this broke lldb-aarch64-windows bot: https://lab.llvm.org/buildbot/#/builders/219/builds/6130 Could you please look at this ? https://github.com/llvm/llvm-project/pull/68300 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/68300 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
https://github.com/adrian-prantl approved this pull request. https://github.com/llvm/llvm-project/pull/68300 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/68300
>From 30ef50b808a8458a60bbd3cdc52b866ee296b6ba Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Thu, 5 Oct 2023 12:13:12 +0100
Subject: [PATCH 1/2] [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 #68135
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 22 +++---
.../cpp/union-static-data-members/Makefile| 3 ++
.../TestCppUnionStaticMembers.py | 43 +++
.../cpp/union-static-data-members/main.cpp| 25 +++
4 files changed, 87 insertions(+), 6 deletions(-)
create mode 100644 lldb/test/API/lang/cpp/union-static-data-members/Makefile
create mode 100644
lldb/test/API/lang/cpp/union-static-data-members/TestCppUnionStaticMembers.py
create mode 100644 lldb/test/API/lang/cpp/union-static-data-members/main.cpp
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 37fb16d4e0351c9..ee35a7de80c1e18 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2482,8 +2482,9 @@ struct MemberAttributes {
DWARFFormValue encoding_form;
/// Indicates the byte offset of the word from the base address of the
/// structure.
- uint32_t member_byte_offset;
+ uint32_t member_byte_offset = UINT32_MAX;
bool is_artificial = false;
+ bool is_declaration = false;
};
/// Parsed form of all attributes that are relevant for parsing Objective-C
@@ -2656,8 +2657,6 @@ DiscriminantValue &VariantPart::discriminant() { return
this->_discriminant; }
MemberAttributes::MemberAttributes(const DWARFDIE &die,
const DWARFDIE &parent_die,
ModuleSP module_sp) {
- member_byte_offset = (parent_die.Tag() == DW_TAG_union_type) ? 0 :
UINT32_MAX;
-
DWARFAttributes attributes = die.GetAttributes();
for (size_t i = 0; i < attributes.Size(); ++i) {
const dw_attr_t attr = attributes.AttributeAtIndex(i);
@@ -2717,6 +2716,9 @@ MemberAttributes::MemberAttributes(const DWARFDIE &die,
case DW_AT_artificial:
is_artificial = form_value.Boolean();
break;
+ case DW_AT_declaration:
+is_declaration = form_value.Boolean();
+break;
default:
break;
}
@@ -2923,10 +2925,18 @@ void DWARFASTParserClang::ParseSingleMember(
if (class_is_objc_object_or_interface)
attrs.accessibility = eAccessNone;
- // Handle static members, which is any member that doesn't have a bit or a
- // byte member offset.
+ // Handle static members, which are typically members without
+ // locations. However, GCC *never* emits DW_AT_data_member_location
+ // for static data members of unions.
+ // Non-normative text pre-DWARFv5 recommends marking static
+ // data members with an DW_AT_external flag. Clang emits this consistently
+ // whereas GCC emits it only for static data members if not part of an
+ // anonymous namespace. The flag that is consistently emitted for static
+ // data member
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 777a6e6f10b2b90496d248b7fa904fce834484be
30ef50b808a8458a60bbd3cdc52b866ee296b6ba --
lldb/test/API/lang/cpp/union-static-data-members/main.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
``
View the diff from clang-format here.
``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index ee35a7de80c1..436632473816 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2933,8 +2933,8 @@ void DWARFASTParserClang::ParseSingleMember(
// whereas GCC emits it only for static data members if not part of an
// anonymous namespace. The flag that is consistently emitted for static
// data members is DW_AT_declaration, so we check it instead.
- // FIXME: Since DWARFv5, static data members are marked DW_AT_variable so we
can
- // consistently detect them on both GCC and Clang without below heuristic.
+ // FIXME: Since DWARFv5, static data members are marked DW_AT_variable so we
+ // can consistently detect them on both GCC and Clang without below
heuristic.
if (attrs.member_byte_offset == UINT32_MAX &&
attrs.data_bit_offset == UINT64_MAX && attrs.is_declaration) {
Type *var_type = die.ResolveTypeUID(attrs.encoding_form.Reference());
``
https://github.com/llvm/llvm-project/pull/68300
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/68300 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/68300 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/68300 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Changes
**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 #68135
---
Full diff: https://github.com/llvm/llvm-project/pull/68300.diff
4 Files Affected:
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
(+16-6)
- (added) lldb/test/API/lang/cpp/union-static-data-members/Makefile (+3)
- (added)
lldb/test/API/lang/cpp/union-static-data-members/TestCppUnionStaticMembers.py
(+43)
- (added) lldb/test/API/lang/cpp/union-static-data-members/main.cpp (+25)
``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 37fb16d4e0351c9..ee35a7de80c1e18 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2482,8 +2482,9 @@ struct MemberAttributes {
DWARFFormValue encoding_form;
/// Indicates the byte offset of the word from the base address of the
/// structure.
- uint32_t member_byte_offset;
+ uint32_t member_byte_offset = UINT32_MAX;
bool is_artificial = false;
+ bool is_declaration = false;
};
/// Parsed form of all attributes that are relevant for parsing Objective-C
@@ -2656,8 +2657,6 @@ DiscriminantValue &VariantPart::discriminant() { return
this->_discriminant; }
MemberAttributes::MemberAttributes(const DWARFDIE &die,
const DWARFDIE &parent_die,
ModuleSP module_sp) {
- member_byte_offset = (parent_die.Tag() == DW_TAG_union_type) ? 0 :
UINT32_MAX;
-
DWARFAttributes attributes = die.GetAttributes();
for (size_t i = 0; i < attributes.Size(); ++i) {
const dw_attr_t attr = attributes.AttributeAtIndex(i);
@@ -2717,6 +2716,9 @@ MemberAttributes::MemberAttributes(const DWARFDIE &die,
case DW_AT_artificial:
is_artificial = form_value.Boolean();
break;
+ case DW_AT_declaration:
+is_declaration = form_value.Boolean();
+break;
default:
break;
}
@@ -2923,10 +2925,18 @@ void DWARFASTParserClang::ParseSingleMember(
if (class_is_objc_object_or_interface)
attrs.accessibility = eAccessNone;
- // Handle static members, which is any member that doesn't have a bit or a
- // byte member offset.
+ // Handle static members, which are typically members without
+ // locations. However, GCC *never* emits DW_AT_data_member_location
+ // for static data members of unions.
+ // Non-normative text pre-DWARFv5 recommends marking static
+ // data members with an DW_AT_external flag. Clang emits this consistently
+ // whereas GCC emits it only for static data members if not part of an
+ // anonymous namespace. The flag that is consistently emitted for static
+ // data members is DW_AT_declaration, so we check it instead.
+ // FIXME: Since DWARFv5, static data members are marked DW_AT_variable so we
can
+ // consistently detect them on both GCC and Clang without below heuristic.
if (attrs.member_byte_offset == UINT32_MAX &&
- attrs.data_bit_offset == UINT64_MAX) {
+ attrs.data_bit_offset == UINT64_MAX && attrs.is_declaration) {
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
Michael137 wrote: Alternatively, we could start checking `DW_AT_external` again, at the cost of not supporting some GCC cases pre-DWARFv5 https://github.com/llvm/llvm-project/pull/68300 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/68300
**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 #68135
>From 30ef50b808a8458a60bbd3cdc52b866ee296b6ba Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Thu, 5 Oct 2023 12:13:12 +0100
Subject: [PATCH] [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 #68135
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 22 +++---
.../cpp/union-static-data-members/Makefile| 3 ++
.../TestCppUnionStaticMembers.py | 43 +++
.../cpp/union-static-data-members/main.cpp| 25 +++
4 files changed, 87 insertions(+), 6 deletions(-)
create mode 100644 lldb/test/API/lang/cpp/union-static-data-members/Makefile
create mode 100644
lldb/test/API/lang/cpp/union-static-data-members/TestCppUnionStaticMembers.py
create mode 100644 lldb/test/API/lang/cpp/union-static-data-members/main.cpp
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 37fb16d4e0351c9..ee35a7de80c1e18 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2482,8 +2482,9 @@ struct MemberAttributes {
DWARFFormValue encoding_form;
/// Indicates the byte offset of the word from the base addre
