Non constant size and offset in DWARF #141106
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-llvm-ir Author: Tom Tromey (tromey) ChangesIn Ada, a record type can have a non-constant size, and a field can appear at a non-constant bit offset in a record. To support this, this patch changes DIType to record the size and offset using metadata, rather than plain integers. In addition to a constant offset, both DIVariable and DIExpression are now supported here. One thing of note in this patch is the choice of how exactly to represent a non-constant bit offset, with the difficulty being that DWARF 5 does not support this. DWARF 3 did have a way to support a non-constant byte offset, combined with a constant bit offset within the byte, but this was deprecated in DWARF 4 and removed from DWARF 5. This patch takes a simple approach: a DWARF extension allowing the use of an expression with DW_AT_data_bit_offset. There is a corresponding DWARF issue, see https://dwarfstd.org/issues/250501.1.html. The main reason for this approach is that it keeps API simplicity: just a single value is needed, rather than having separate data describing the byte offset and the bit within the byte. Patch is 124.35 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/141106.diff 13 Files Affected:
diff --git a/llvm/include/llvm/IR/DIBuilder.h b/llvm/include/llvm/IR/DIBuilder.h
index 4ce71bd3dad58..1e1a4cb0af9ba 100644
--- a/llvm/include/llvm/IR/DIBuilder.h
+++ b/llvm/include/llvm/IR/DIBuilder.h
@@ -365,6 +365,24 @@ namespace llvm {
uint64_t BaseOffset, uint32_t VBPtrOffset,
DINode::DIFlags Flags);
+ /// Create debugging information entry for a member.
+ /// \param Scope Member scope.
+ /// \param Name Member name.
+ /// \param File File where this member is defined.
+ /// \param LineNo Line number.
+ /// \param SizeInBits Member size.
+ /// \param AlignInBits Member alignment.
+ /// \param OffsetInBits Member offset.
+ /// \param Flags Flags to encode member attribute, e.g. private
+ /// \param Ty Parent type.
+ /// \param Annotations Member annotations.
+ DIDerivedType *createMemberType(DIScope *Scope, StringRef Name,
+ DIFile *File, unsigned LineNo,
+ Metadata *SizeInBits, uint32_t AlignInBits,
+ Metadata *OffsetInBits,
+ DINode::DIFlags Flags, DIType *Ty,
+ DINodeArray Annotations = nullptr);
+
/// Create debugging information entry for a member.
/// \param Scope Member scope.
/// \param Name Member name.
@@ -419,6 +437,25 @@ namespace llvm {
DIDerivedType *createVariantMemberType(DIScope *Scope, DINodeArray Elements,
Constant *Discriminant, DIType *Ty);
+ /// Create debugging information entry for a bit field member.
+ /// \param Scope Member scope.
+ /// \param Name Member name.
+ /// \param File File where this member is defined.
+ /// \param LineNo Line number.
+ /// \param SizeInBits Member size.
+ /// \param OffsetInBits Member offset.
+ /// \param StorageOffsetInBits Member storage offset.
+ /// \param Flags Flags to encode member attribute.
+ /// \param Ty Parent type.
+ /// \param Annotations Member annotations.
+ DIDerivedType *createBitFieldMemberType(DIScope *Scope, StringRef Name,
+ DIFile *File, unsigned LineNo,
+ Metadata *SizeInBits,
+ Metadata *OffsetInBits,
+ uint64_t StorageOffsetInBits,
+ DINode::DIFlags Flags, DIType *Ty,
+ DINodeArray Annotations = nullptr);
+
/// Create debugging information entry for a bit field member.
/// \param Scope Member scope.
/// \param Name Member name.
@@ -510,6 +547,29 @@ namespace llvm {
unsigned RunTimeLang = 0, DIType *VTableHolder = nullptr,
MDNode *TemplateParms = nullptr, StringRef UniqueIdentifier = "");
+ /// Create debugging information entry for a struct.
+ /// \param Scope Scope in which this struct is defined.
+ /// \param Name Struct name.
+ /// \param File File where this member is defined.
+ /// \param LineNumber Line number.
+ /// \param SizeInBits Member size.
+ /// \param AlignInBits Member alignment.
+ /// \param Flags Flags to encode member attribute, e.g. private
+ /// \param Elements Struct elements.
+ /// \param RunTimeLang Optional parameter, Objective-C runtime version.
+ /// \param UniqueIdentifier A unique identifier for the struct.
+ /// \param Specification The type that this type completes. This is used by
+ /// Swift to represent generic types.
+ /// \param NumExtraInhabitants The number of extra inhabitants of the type.
+ /// An extra inhabitant is a bit pattern that does not represent a valid
+ /// value for instances of a given type. This is used by the Swift language.
+ DICompositeType *createStructType(
+ DIScope *Scope, StringRef Name, DIFile *File, unsigned LineNumber,
+ Metadata *SizeInBits, uint32_t AlignInBits, DINode::DIFlags Flags,
+ DIType *DerivedFrom, DINodeArray Elements, unsigned RunTimeLang = 0,
+ DIType *VTableHolder = nullptr, StringRef UniqueIdentifier = "",
+ DIType *Specification = nullptr, uint32_t NumExtraInhabitants = 0);
+
/// Create debugging information entry for a struct.
/// \param Scope Scope in which this struct is defined.
/// \param Name Struct name.
diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index d82c69aebb026..2ab2da60b1b6e 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -711,40 +711,33 @@ std::optional<StringRef> DIScope::getSource() const {
class DIType : public DIScope {
unsigned Line;
DIFlags Flags;
- uint64_t SizeInBits;
- uint64_t OffsetInBits;
uint32_t NumExtraInhabitants;
protected:
+ static constexpr unsigned N_OPERANDS = 5;
+
DIType(LLVMContext &C, unsigned ID, StorageType Storage, unsigned Tag,
- unsigned Line, uint64_t SizeInBits, uint32_t AlignInBits,
- uint64_t OffsetInBits, uint32_t NumExtraInhabitants, DIFlags Flags,
- ArrayRef<Metadata *> Ops)
+ unsigned Line, uint32_t AlignInBits, uint32_t NumExtraInhabitants,
+ DIFlags Flags, ArrayRef<Metadata *> Ops)
: DIScope(C, ID, Storage, Tag, Ops) {
- init(Line, SizeInBits, AlignInBits, OffsetInBits, NumExtraInhabitants,
- Flags);
+ init(Line, AlignInBits, NumExtraInhabitants, Flags);
}
~DIType() = default;
- void init(unsigned Line, uint64_t SizeInBits, uint32_t AlignInBits,
- uint64_t OffsetInBits, uint32_t NumExtraInhabitants,
+ void init(unsigned Line, uint32_t AlignInBits, uint32_t NumExtraInhabitants,
DIFlags Flags) {
this->Line = Line;
this->Flags = Flags;
- this->SizeInBits = SizeInBits;
this->SubclassData32 = AlignInBits;
- this->OffsetInBits = OffsetInBits;
this->NumExtraInhabitants = NumExtraInhabitants;
}
/// Change fields in place.
- void mutate(unsigned Tag, unsigned Line, uint64_t SizeInBits,
- uint32_t AlignInBits, uint64_t OffsetInBits,
+ void mutate(unsigned Tag, unsigned Line, uint32_t AlignInBits,
uint32_t NumExtraInhabitants, DIFlags Flags) {
assert(isDistinct() && "Only distinct nodes can mutate");
setTag(Tag);
- init(Line, SizeInBits, AlignInBits, OffsetInBits, NumExtraInhabitants,
- Flags);
+ init(Line, AlignInBits, NumExtraInhabitants, Flags);
}
public:
@@ -753,10 +746,8 @@ class DIType : public DIScope {
}
unsigned getLine() const { return Line; }
- uint64_t getSizeInBits() const { return SizeInBits; }
uint32_t getAlignInBits() const;
uint32_t getAlignInBytes() const { return getAlignInBits() / CHAR_BIT; }
- uint64_t getOffsetInBits() const { return OffsetInBits; }
uint32_t getNumExtraInhabitants() const { return NumExtraInhabitants; }
DIFlags getFlags() const { return Flags; }
@@ -766,6 +757,26 @@ class DIType : public DIScope {
Metadata *getRawScope() const { return getOperand(1); }
MDString *getRawName() const { return getOperandAs<MDString>(2); }
+ Metadata *getRawSizeInBits() const { return getOperand(3); }
+ uint64_t getSizeInBits() const {
+ if (auto *MD = dyn_cast_or_null<ConstantAsMetadata>(getRawSizeInBits())) {
+ if (ConstantInt *CI = dyn_cast_or_null<ConstantInt>(MD->getValue())) {
+ return CI->getZExtValue();
+ }
+ }
+ return 0;
+ }
+
+ Metadata *getRawOffsetInBits() const { return getOperand(4); }
+ uint64_t getOffsetInBits() const {
+ if (auto *MD = dyn_cast_or_null<ConstantAsMetadata>(getRawOffsetInBits())) {
+ if (ConstantInt *CI = dyn_cast_or_null<ConstantInt>(MD->getValue())) {
+ return CI->getZExtValue();
+ }
+ }
+ return 0;
+ }
+
/// Returns a new temporary DIType with updated Flags
TempDIType cloneWithFlags(DIFlags NewFlags) const {
auto NewTy = clone();
@@ -831,18 +842,18 @@ class DIBasicType : public DIType {
protected:
DIBasicType(LLVMContext &C, StorageType Storage, unsigned Tag,
- uint64_t SizeInBits, uint32_t AlignInBits, unsigned Encoding,
+ uint32_t AlignInBits, unsigned Encoding,
uint32_t NumExtraInhabitants, DIFlags Flags,
ArrayRef<Metadata *> Ops)
- : DIType(C, DIBasicTypeKind, Storage, Tag, 0, SizeInBits, AlignInBits, 0,
+ : DIType(C, DIBasicTypeKind, Storage, Tag, 0, AlignInBits,
NumExtraInhabitants, Flags, Ops),
Encoding(Encoding) {}
DIBasicType(LLVMContext &C, unsigned ID, StorageType Storage, unsigned Tag,
- uint64_t SizeInBits, uint32_t AlignInBits, unsigned Encoding,
+ uint32_t AlignInBits, unsigned Encoding,
uint32_t NumExtraInhabitants, DIFlags Flags,
ArrayRef<Metadata *> Ops)
- : DIType(C, ID, Storage, Tag, 0, SizeInBits, AlignInBits, 0,
- NumExtraInhabitants, Flags, Ops),
+ : DIType(C, ID, Storage, Tag, 0, AlignInBits, NumExtraInhabitants, Flags,
+ Ops),
Encoding(Encoding) {}
~DIBasicType() = default;
@@ -859,11 +870,21 @@ class DIBasicType : public DIType {
MDString *Name, uint64_t SizeInBits,
uint32_t AlignInBits, unsigned Encoding,
uint32_t NumExtraInhabitants, DIFlags Flags,
+ StorageType Storage, bool ShouldCreate = true) {
+ auto *SizeInBitsNode = ConstantAsMetadata::get(
+ ConstantInt::get(Type::getInt64Ty(Context), SizeInBits));
+ return getImpl(Context, Tag, Name, SizeInBitsNode, AlignInBits, Encoding,
+ NumExtraInhabitants, Flags, Storage, ShouldCreate);
+ }
+ static DIBasicType *getImpl(LLVMContext &Context, unsigned Tag,
+ MDString *Name, Metadata *SizeInBits,
+ uint32_t AlignInBits, unsigned Encoding,
+ uint32_t NumExtraInhabitants, DIFlags Flags,
StorageType Storage, bool ShouldCreate = true);
TempDIBasicType cloneImpl() const {
- return getTemporary(getContext(), getTag(), getName(), getSizeInBits(),
- getAlignInBits(), getEncoding(),
+ return getTemporary(getContext(), getTag(), getRawName(),
+ getRawSizeInBits(), getAlignInBits(), getEncoding(),
getNumExtraInhabitants(), getFlags());
}
@@ -896,6 +917,12 @@ class DIBasicType : public DIType {
uint32_t NumExtraInhabitants, DIFlags Flags),
(Tag, Name, SizeInBits, AlignInBits, Encoding,
NumExtraInhabitants, Flags))
+ DEFINE_MDNODE_GET(DIBasicType,
+ (unsigned Tag, MDString *Name, Metadata *SizeInBits,
+ uint32_t AlignInBits, unsigned Encoding,
+ uint32_t NumExtraInhabitants, DIFlags Flags),
+ (Tag, Name, SizeInBits, AlignInBits, Encoding,
+ NumExtraInhabitants, Flags))
TempDIBasicType clone() const { return cloneImpl(); }
@@ -927,29 +954,28 @@ class DIFixedPointType : public DIBasicType {
APInt Denominator;
DIFixedPointType(LLVMContext &C, StorageType Storage, unsigned Tag,
- uint64_t SizeInBits, uint32_t AlignInBits, unsigned Encoding,
- DIFlags Flags, unsigned Kind, int Factor,
- ArrayRef<Metadata *> Ops)
- : DIBasicType(C, DIFixedPointTypeKind, Storage, Tag, SizeInBits,
- AlignInBits, Encoding, 0, Flags, Ops),
+ uint32_t AlignInBits, unsigned Encoding, DIFlags Flags,
+ unsigned Kind, int Factor, ArrayRef<Metadata *> Ops)
+ : DIBasicType(C, DIFixedPointTypeKind, Storage, Tag, AlignInBits,
+ Encoding, 0, Flags, Ops),
Kind(Kind), Factor(Factor) {
assert(Kind == FixedPointBinary || Kind == FixedPointDecimal);
}
DIFixedPointType(LLVMContext &C, StorageType Storage, unsigned Tag,
- uint64_t SizeInBits, uint32_t AlignInBits, unsigned Encoding,
- DIFlags Flags, unsigned Kind, APInt Numerator,
- APInt Denominator, ArrayRef<Metadata *> Ops)
- : DIBasicType(C, DIFixedPointTypeKind, Storage, Tag, SizeInBits,
- AlignInBits, Encoding, 0, Flags, Ops),
+ uint32_t AlignInBits, unsigned Encoding, DIFlags Flags,
+ unsigned Kind, APInt Numerator, APInt Denominator,
+ ArrayRef<Metadata *> Ops)
+ : DIBasicType(C, DIFixedPointTypeKind, Storage, Tag, AlignInBits,
+ Encoding, 0, Flags, Ops),
Kind(Kind), Factor(0), Numerator(Numerator), Denominator(Denominator) {
assert(Kind == FixedPointRational);
}
DIFixedPointType(LLVMContext &C, StorageType Storage, unsigned Tag,
- uint64_t SizeInBits, uint32_t AlignInBits, unsigned Encoding,
- DIFlags Flags, unsigned Kind, int Factor, APInt Numerator,
+ uint32_t AlignInBits, unsigned Encoding, DIFlags Flags,
+ unsigned Kind, int Factor, APInt Numerator,
APInt Denominator, ArrayRef<Metadata *> Ops)
- : DIBasicType(C, DIFixedPointTypeKind, Storage, Tag, SizeInBits,
- AlignInBits, Encoding, 0, Flags, Ops),
+ : DIBasicType(C, DIFixedPointTypeKind, Storage, Tag, AlignInBits,
+ Encoding, 0, Flags, Ops),
Kind(Kind), Factor(Factor), Numerator(Numerator),
Denominator(Denominator) {}
~DIFixedPointType() = default;
@@ -959,20 +985,42 @@ class DIFixedPointType : public DIBasicType {
uint64_t SizeInBits, uint32_t AlignInBits, unsigned Encoding,
DIFlags Flags, unsigned Kind, int Factor, APInt Numerator,
APInt Denominator, StorageType Storage, bool ShouldCreate = true) {
+ auto *SizeInBitsNode = ConstantAsMetadata::get(
+ ConstantInt::get(Type::getInt64Ty(Context), SizeInBits));
+ return getImpl(Context, Tag, getCanonicalMDString(Context, Name),
+ SizeInBitsNode, AlignInBits, Encoding, Flags, Kind, Factor,
+ Numerator, Denominator, Storage, ShouldCreate);
+ }
+ static DIFixedPointType *
+ getImpl(LLVMContext &Context, unsigned Tag, StringRef Name,
+ Metadata *SizeInBits, uint32_t AlignInBits, unsigned Encoding,
+ DIFlags Flags, unsigned Kind, int Factor, APInt Numerator,
+ APInt Denominator, StorageType Storage, bool ShouldCreate = true) {
return getImpl(Context, Tag, getCanonicalMDString(Context, Name),
SizeInBits, AlignInBits, Encoding, Flags, Kind, Factor,
Numerator, Denominator, Storage, ShouldCreate);
}
static DIFixedPointType *
getImpl(LLVMContext &Context, unsigned Tag, MDString *Name,
- uint64_t SizeInBits, uint32_t AlignInBits, unsigned Encoding,
+ uint32_t SizeInBits, uint32_t AlignInBits, unsigned Encoding,
+ DIFlags Flags, unsigned Kind, int Factor, APInt Numerator,
+ APInt Denominator, StorageType Storage, bool ShouldCreate = true) {
+ auto *SizeInBitsNode = ConstantAsMetadata::get(
+ ConstantInt::get(Type::getInt64Ty(Context), SizeInBits));
+ return getImpl(Context, Tag, Name, SizeInBitsNode, AlignInBits, Encoding,
+ Flags, Kind, Factor, Numerator, Denominator, Storage,
+ ShouldCreate);
+ }
+ static DIFixedPointType *
+ getImpl(LLVMContext &Context, unsigned Tag, MDString *Name,
+ Metadata *SizeInBits, uint32_t AlignInBits, unsigned Encoding,
DIFlags Flags, unsigned Kind, int Factor, APInt Numerator,
APInt Denominator, StorageType Storage, bool ShouldCreate = true);
TempDIFixedPointType cloneImpl() const {
- return getTemporary(getContext(), getTag(), getName(), getSizeInBits(),
- getAlignInBits(), getEncoding(), getFlags(), Kind,
- Factor, Numerator, Denominator);
+ return getTemporary(getContext(), getTag(), getRawName(),
+ getRawSizeInBits(), getAlignInBits(), getEncoding(),
+ getFlags(), Kind, Factor, Numerator, Denominator);
}
public:
@@ -1003,6 +1051,13 @@ class DIFixedPointType : public DIBasicType {
APInt Denominator),
(Tag, Name, SizeInBits, AlignInBits, Encoding, Flags, Kind,
Factor, Numerator, Denominator))
+ DEFINE_MDNODE_GET(DIFixedPointType,
+ (unsigned Tag, MDString *Name, Metadata *SizeInBits,
+ uint32_t AlignInBits, unsigned Encoding, DIFlags Flags,
+ unsigned Kind, int Factor, APInt Numerator,
+ APInt Denominator),
+ (Tag, Name, SizeInBits, AlignInBits, Encoding, Flags, Kind,
+ Factor, Numerator, Denominator))
TempDIFixedPointType clone() const { return cloneImpl(); }
@@ -1042,13 +1097,15 @@ class DIStringType : public DIType {
friend class LLVMContextImpl;
friend class MDNode;
+ static constexpr unsigned MY_FIRST_OPERAND = DIType::N_OPERANDS;
+
unsigned Encoding;
DIStringType(LLVMContext &C, StorageType Storage, unsigned Tag,
- uint64_t SizeInBits, uint32_t AlignInBits, unsigned Encoding,
+ uint32_t AlignInBits, unsigned Encoding,
ArrayRef<Metadata *> Ops)
- : DIType(C, DIStringTypeKind, Storage, Tag, 0, SizeInBits, AlignInBits, 0,
- 0, FlagZero, Ops),
+ : DIType(C, DIStringTypeKind, Storage, Tag, 0, AlignInBits, 0, FlagZero,
+ Ops),
Encoding(Encoding) {}
~DIStringType() = default;
@@ -1058,8 +1115,10 @@ class DIStringType : public DIType {
uint64_t SizeInBits, uint32_t AlignInBits,
unsigned Encoding, StorageType Storage,
bool ShouldCreate = true) {
+ auto *SizeInBitsNode = ConstantAsMetadata::get(
+ ConstantInt::get(Type::getInt64Ty(Context), SizeInBits));
return getImpl(Context, Tag, getCanonicalMDString(Context, Name),
- StringLength, StrLenExp, StrLocationExp, SizeInBits,
+ StringLength, StrLenExp, StrLocationExp, SizeInBitsNode,
AlignInBits, Encoding, Storage, ShouldCreate);
}
static DIStringType *getImpl(LLVMContext &Context, unsigned Tag,
@@ -1067,12 +1126,24 @@ class DIStringType : public DIType {
Metadata *StrLenExp, Metadata *StrLocationExp,
uint64_t SizeInBits, uint32_t AlignInBits,
unsigned Encoding, StorageType Storage,
+ bool ShouldCreate = true) {
+ auto *SizeInBitsNode = ConstantAsMetadata::get(
+ ConstantInt::get(Type::getInt64Ty(Context), SizeInBits));
+ return getImpl(Context, Tag, Name, StringLength, StrLenExp, StrLocationExp,
+ SizeInBitsNode, AlignInBits, Encoding, Storage,
+ ShouldCreate);
+ }
+ static DIStringType *getImpl(LLVMContext &Context, unsigned Tag,
+ MDString *Name, Metadata *StringLength,
+ Metadata *StrLenExp, Metadata *StrLocationExp,
+ Metadata *SizeInBits, uint32_t AlignInBits,
+ unsigned Encoding, StorageType Storage,
bool ShouldCreate = true);
TempDIStringType cloneImpl() const {
return getTemporary(getContext(), getTag(), getRawName(),
getRawStringLength(), getRawStringLengthExp(),
- getRawStringLocationExp(), getSizeInBits(),
+ getRawStringLocationExp(), getRawSizeInBits(),
...
[truncated]
|
|
Note that I've written this patch as a series of smaller patches. I know it'll all be squashed if it lands, but it may be simpler to review the pieces separately. |
|
I'm fixing up the failures. Sorry about that. Also as a follow-up I may implement the suggestion here: https://discourse.llvm.org/t/rfc-dynamic-sizes-and-field-offsets-in-dwarf/85992/3?u=tromey |
tromey
force-pushed
the
non-constant-size-and-offset
branch
from
2698846 to
90a46a7
Compare
llvmbot
added
clang
|
I rebased this and fixed up the mlir problem. I also changed how the overload resolution was done, so that a later patch that treats |
|
From what I can tell, those failures don't have anything to do with this patch. |
tromey
added a commit
to tromey/gdb
that referenced
this pull request
May 30, 2025
In Ada, a field can have a dynamic bit offset in its enclosing record.
In DWARF 3, this was handled using a dynamic
DW_AT_data_member_location, combined with a DW_AT_bit_offset -- this
combination worked out ok because in practice GNAT only needs a
dynamic byte offset with a fixed offset within the byte.
However, this approach was deprecated in DWARF 4 and then removed in
DWARF 5. No replacement approach was given, meaning that in strict
mode there is no way to express this.
This is a DWARF bug, see
https://dwarfstd.org/issues/250501.1.html
In a discussion on the DWARF mailing list, a couple people mentioned
that compilers could use the obvious extension of a dynamic
DW_AT_data_bit_offset. I've implemented this for LLVM:
llvm/llvm-project#141106
In preparation for that landing, this patch implements support for
this construct in gdb.
tromey
added a commit
to tromey/gdb
that referenced
this pull request
Jun 2, 2025
In Ada, a field can have a dynamic bit offset in its enclosing record.
In DWARF 3, this was handled using a dynamic
DW_AT_data_member_location, combined with a DW_AT_bit_offset -- this
combination worked out ok because in practice GNAT only needs a
dynamic byte offset with a fixed offset within the byte.
However, this approach was deprecated in DWARF 4 and then removed in
DWARF 5. No replacement approach was given, meaning that in strict
mode there is no way to express this.
This is a DWARF bug, see
https://dwarfstd.org/issues/250501.1.html
In a discussion on the DWARF mailing list, a couple people mentioned
that compilers could use the obvious extension of a dynamic
DW_AT_data_bit_offset. I've implemented this for LLVM:
llvm/llvm-project#141106
In preparation for that landing, this patch implements support for
this construct in gdb.
New in v2: renamed some constants and added a helper method, per
Simon's review.
Approved-By: Simon Marchi <[email protected]>
tromey
added a commit
to tromey/gdb
that referenced
this pull request
Jun 2, 2025
In Ada, a field can have a dynamic bit offset in its enclosing record.
In DWARF 3, this was handled using a dynamic
DW_AT_data_member_location, combined with a DW_AT_bit_offset -- this
combination worked out ok because in practice GNAT only needs a
dynamic byte offset with a fixed offset within the byte.
However, this approach was deprecated in DWARF 4 and then removed in
DWARF 5. No replacement approach was given, meaning that in strict
mode there is no way to express this.
This is a DWARF bug, see
https://dwarfstd.org/issues/250501.1.html
In a discussion on the DWARF mailing list, a couple people mentioned
that compilers could use the obvious extension of a dynamic
DW_AT_data_bit_offset. I've implemented this for LLVM:
llvm/llvm-project#141106
In preparation for that landing, this patch implements support for
this construct in gdb.
New in v2: renamed some constants and added a helper method, per
Simon's review.
New in v3: more renamings.
Approved-By: Simon Marchi <[email protected]>
saagarjha
pushed a commit
to ahjragaas/binutils-gdb
that referenced
this pull request
Jun 3, 2025
In Ada, a field can have a dynamic bit offset in its enclosing record.
In DWARF 3, this was handled using a dynamic
DW_AT_data_member_location, combined with a DW_AT_bit_offset -- this
combination worked out ok because in practice GNAT only needs a
dynamic byte offset with a fixed offset within the byte.
However, this approach was deprecated in DWARF 4 and then removed in
DWARF 5. No replacement approach was given, meaning that in strict
mode there is no way to express this.
This is a DWARF bug, see
https://dwarfstd.org/issues/250501.1.html
In a discussion on the DWARF mailing list, a couple people mentioned
that compilers could use the obvious extension of a dynamic
DW_AT_data_bit_offset. I've implemented this for LLVM:
llvm/llvm-project#141106
In preparation for that landing, this patch implements support for
this construct in gdb.
New in v2: renamed some constants and added a helper method, per
Simon's review.
New in v3: more renamings.
Approved-By: Simon Marchi <[email protected]>
tromey
force-pushed
the
non-constant-size-and-offset
branch
from
90a46a7 to
c80072c
Compare
|
Rebased and adapted to the |
tromey
commented
Jun 4, 2025
tromey
force-pushed
the
non-constant-size-and-offset
branch
from
c80072c to
90e7a94
Compare
|
Updated to fix a tiny error I'd introduced. |
|
Ping. |
dwblaikie
reviewed
Jun 16, 2025
dwblaikie
approved these changes
Jun 17, 2025
This patch changes code in DebugInfoMetadata.h to use symbolic constants to refer to metadata slots. This makes it a little simpler (but still not entirely seamless) to add metadata to DIType.
This changes DIType to use metadata for the size and offset information. This patch doesn't have any functional changes yet; the test is in the next patch.
This updates the DWARF writer to emit dynamic offsets and sizes for members.
tromey
force-pushed
the
non-constant-size-and-offset
branch
from
90e7a94 to
e265877
Compare
|
I've rebased this and addressed the review comments. I am leaving the one sub-thread open since I wasn't sure if I should send a separate PR (I don't have push rights so I can't land something separately on my own) -- just let me know if you'd like that, it's no trouble. Thanks. |
dwblaikie
approved these changes
Jun 24, 2025
There was a problem hiding this comment.
Yeah, let's just go with this as a single patch.
Looking closer, in retrospect/for next time, perhaps this could've been a few smaller patches (I guess the strtuct, member, and bit field member are all separate except that they depend on the new MDUnsignedOrMDField - either that could've been included in one of the patches that needed it, and the other two patches dependent on that one arbitrarily chosen - or if that could've been added/tested independently, then all 3 functional patches independently depend on it - (plus the already discussed clang change that could've been done separately - but yeah, annoying to have a dependent patch for such a small change, so probably fine to keep it in in situations like this then just send a tiny PR real quick to go in before the rest once approved)))
This adds some new DIBuilder methods to expose the new dynamic size and offset functionality.
tromey
force-pushed
the
non-constant-size-and-offset
branch
from
e265877 to
f43c0a1
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/155/builds/10442 Here is the relevant piece of the build log for the reference |
|
Hi @tromey,
there is the same problem with the |
If anyone wants to stamp that & I'll commit it once the presubmit checks run - hopefully that's adequate to address the nvptx issue - just at a very rough guess. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/160/builds/19862 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/180/builds/20005 Here is the relevant piece of the build log for the reference |
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Jun 26, 2025
Need dbg team to land this patch This reverts commit 3b90597.
anthonyhatran
pushed a commit
to anthonyhatran/llvm-project
that referenced
this pull request
Jun 26, 2025
In Ada, a record type can have a non-constant size, and a field can appear at a non-constant bit offset in a record. To support this, this patch changes DIType to record the size and offset using metadata, rather than plain integers. In addition to a constant offset, both DIVariable and DIExpression are now supported here. One thing of note in this patch is the choice of how exactly to represent a non-constant bit offset, with the difficulty being that DWARF 5 does not support this. DWARF 3 did have a way to support a non-constant byte offset, combined with a constant bit offset within the byte, but this was deprecated in DWARF 4 and removed from DWARF 5. This patch takes a simple approach: a DWARF extension allowing the use of an expression with DW_AT_data_bit_offset. There is a corresponding DWARF issue, see https://dwarfstd.org/issues/250501.1.html. The main reason for this approach is that it keeps API simplicity: just a single value is needed, rather than having separate data describing the byte offset and the bit within the byte.
|
It looks like this PR is causing failures in emscripten's LTO builds. I haven't looked into it yet, but if you have any thoughts on what may be the issue it would be helpful. |
|
I am seeing an assertion failure when building the Linux kernel for aarch64 with LTO enabled. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In Ada, a record type can have a non-constant size, and a field can appear at a non-constant bit offset in a record.
To support this, this patch changes DIType to record the size and offset using metadata, rather than plain integers. In addition to a constant offset, both DIVariable and DIExpression are now supported here.
One thing of note in this patch is the choice of how exactly to represent a non-constant bit offset, with the difficulty being that DWARF 5 does not support this. DWARF 3 did have a way to support a non-constant byte offset, combined with a constant bit offset within the byte, but this was deprecated in DWARF 4 and removed from DWARF 5.
This patch takes a simple approach: a DWARF extension allowing the use of an expression with DW_AT_data_bit_offset. There is a corresponding DWARF issue, see https://dwarfstd.org/issues/250501.1.html. The main reason for this approach is that it keeps API simplicity: just a single value is needed, rather than having separate data describing the byte offset and the bit within the byte.