Skip to content

Add flags check to createVariantMemberType #139261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tromey
Copy link
Contributor

@tromey tromey commented May 9, 2025

I noticed that DIDerivedType overloads the "ExtraData" member depending on the precise type being implemented. A variant part uses this to store the discriminant (a reference to another member), but a bit field uses it to store the storage offset.

This patch changes createVariantMemberType to ensure that the FlagBitField is not used when creating a variant part. If this flag is used, the ExtraData field would be erroneously used in two different ways.

The patch also updates a comment in DIDerivedType to list a couple more cases.

I noticed that DIDerivedType overloads the "ExtraData" member
depending on the precise type being implemented.  A variant part uses
this to store the discriminant (a reference to another member), but a
bit field uses it to store the storage offset.

This patch changes createVariantMemberType to ensure that the
FlagBitField is not used when creating a variant part.  If this flag
is used, the ExtraData field would be erroneously used in two
different ways.

The patch also updates a comment in DIDerivedType to list a couple
more cases.
@llvmbot
Copy link
Member

llvmbot commented May 9, 2025

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-debuginfo

Author: Tom Tromey (tromey)

Changes

I noticed that DIDerivedType overloads the "ExtraData" member depending on the precise type being implemented. A variant part uses this to store the discriminant (a reference to another member), but a bit field uses it to store the storage offset.

This patch changes createVariantMemberType to ensure that the FlagBitField is not used when creating a variant part. If this flag is used, the ExtraData field would be erroneously used in two different ways.

The patch also updates a comment in DIDerivedType to list a couple more cases.


Full diff: https://github.com/llvm/llvm-project/pull/139261.diff

2 Files Affected:

  • (modified) llvm/include/llvm/IR/DebugInfoMetadata.h (+2-1)
  • (modified) llvm/lib/IR/DIBuilder.cpp (+3)
diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index d82c69aebb026..8531424cc136f 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -1253,7 +1253,8 @@ class DIDerivedType : public DIType {
   ///
   /// Class type for pointer-to-members, objective-c property node for ivars,
   /// global constant wrapper for static members, virtual base pointer offset
-  /// for inheritance, or a tuple of template parameters for template aliases.
+  /// for inheritance, a tuple of template parameters for template aliases,
+  /// discriminant for a variant, or storage offset for a bit field.
   ///
   /// TODO: Separate out types that need this extra operand: pointer-to-member
   /// types and member fields (static members and ivars).
diff --git a/llvm/lib/IR/DIBuilder.cpp b/llvm/lib/IR/DIBuilder.cpp
index d9cc49fdad89c..bf34dd5014323 100644
--- a/llvm/lib/IR/DIBuilder.cpp
+++ b/llvm/lib/IR/DIBuilder.cpp
@@ -438,6 +438,9 @@ DIDerivedType *DIBuilder::createVariantMemberType(
     DIScope *Scope, StringRef Name, DIFile *File, unsigned LineNumber,
     uint64_t SizeInBits, uint32_t AlignInBits, uint64_t OffsetInBits,
     Constant *Discriminant, DINode::DIFlags Flags, DIType *Ty) {
+  // "ExtraData" is overloaded for bit fields and for variants, so
+  // make sure to disallow this.
+  assert((Flags & DINode::FlagBitField) == 0);
   return DIDerivedType::get(
       VMContext, dwarf::DW_TAG_member, Name, File, LineNumber,
       getNonCompileUnitScope(Scope), Ty, SizeInBits, AlignInBits, OffsetInBits,

@tromey
Copy link
Contributor Author

tromey commented May 9, 2025

AFAICT those failures aren't related to this patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants