Skip to content

[Dwarf] Support __ptrauth qualifier in metadata nodes #83862

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

Merged
merged 11 commits into from
Mar 19, 2024

Conversation

kovdan01
Copy link
Contributor

@kovdan01 kovdan01 commented Mar 4, 2024

Reland #82363 after fixing build failure https://lab.llvm.org/buildbot/#/builders/5/builds/41428.

Memory sanitizer detects usage of RawData union member which is not
filled directly. Instead, the code relies on filling Data union
member, which is a struct consisting of signing schema parameters.

According to https://en.cppreference.com/w/cpp/language/union, this is UB:
"It is undefined behavior to read from the member of the union that
wasn't most recently written".

Instead of relying on compiler allowing us to do dirty things, do not
use union and only store RawData. Particular ptrauth parameters are
obtained on demand via bit operations.

Original PR description below.

Emit __ptrauth-qualified types as DIDerivedType metadata nodes in IR with tag DW_TAG_LLVM_ptrauth_type, baseType referring to the type which has the qualifier applied, and the following parameters representing the signing schema:

  • ptrAuthKey (integer)
  • ptrAuthIsAddressDiscriminated (boolean)
  • ptrAuthExtraDiscriminator (integer)
  • ptrAuthIsaPointer (boolean)
  • ptrAuthAuthenticatesNullValues (boolean)

Co-authored-by: Ahmed Bougacha [email protected]

Emit `__ptrauth`-qualified types as `DIDerivedType` metadata nodes in IR with
tag `DW_TAG_LLVM_ptrauth_type`, baseType referring to the type which has the
qualifier applied, and the following parameters representing the signing
schema:

- `ptrAuthKey` (integer)
- `ptrAuthIsAddressDiscriminated` (boolean)
- `ptrAuthExtraDiscriminator` (integer)
- `ptrAuthIsaPointer` (boolean)
- `ptrAuthAuthenticatesNullValues` (boolean)
See build failure
https://lab.llvm.org/buildbot/#/builders/5/builds/41428.

Memory sanitizer detects usage of `RawData` union member which is not
filled directly. Instead, the code relies on filling `Data` union
member, which is a struct consisting of signing schema parameters.

According to https://en.cppreference.com/w/cpp/language/union, this is UB:
"It is undefined behavior to read from the member of the union that
wasn't most recently written".

Instead of relying on compiler allowing us to do dirty things, do not
use union and only store `RawData`. Particular ptrauth parameters are
obtained on demand via bit operations.
@llvmbot
Copy link
Member

llvmbot commented Mar 4, 2024

@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-mlir

Author: Daniil Kovalev (kovdan01)

Changes

Reland #82363 after fixing build failure https://lab.llvm.org/buildbot/#/builders/5/builds/41428.

Memory sanitizer detects usage of RawData union member which is not
filled directly. Instead, the code relies on filling Data union
member, which is a struct consisting of signing schema parameters.

According to https://en.cppreference.com/w/cpp/language/union, this is UB:
"It is undefined behavior to read from the member of the union that
wasn't most recently written".

Instead of relying on compiler allowing us to do dirty things, do not
use union and only store RawData. Particular ptrauth parameters are
obtained on demand via bit operations.

Original PR description below.

Emit __ptrauth-qualified types as DIDerivedType metadata nodes in IR with tag DW_TAG_LLVM_ptrauth_type, baseType referring to the type which has the qualifier applied, and the following parameters representing the signing schema:

  • ptrAuthKey (integer)
  • ptrAuthIsAddressDiscriminated (boolean)
  • ptrAuthExtraDiscriminator (integer)
  • ptrAuthIsaPointer (boolean)
  • ptrAuthAuthenticatesNullValues (boolean)

Co-authored-by: Ahmed Bougacha <[email protected]>


Patch is 50.44 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/83862.diff

16 Files Affected:

  • (modified) llvm/include/llvm/IR/DIBuilder.h (+7)
  • (modified) llvm/include/llvm/IR/DebugInfoMetadata.h (+96-23)
  • (modified) llvm/lib/AsmParser/LLParser.cpp (+19-4)
  • (modified) llvm/lib/Bitcode/Reader/MetadataLoader.cpp (+14-4)
  • (modified) llvm/lib/Bitcode/Writer/BitcodeWriter.cpp (+5)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (+14)
  • (modified) llvm/lib/IR/AsmWriter.cpp (+11)
  • (modified) llvm/lib/IR/DIBuilder.cpp (+37-21)
  • (modified) llvm/lib/IR/DebugInfo.cpp (+3-3)
  • (modified) llvm/lib/IR/DebugInfoMetadata.cpp (+21-11)
  • (modified) llvm/lib/IR/LLVMContextImpl.h (+10-5)
  • (modified) llvm/lib/IR/Verifier.cpp (+1)
  • (modified) llvm/test/Assembler/debug-info.ll (+14-2)
  • (added) llvm/test/DebugInfo/AArch64/ptrauth.ll (+70)
  • (modified) llvm/unittests/IR/MetadataTest.cpp (+59-26)
  • (modified) mlir/lib/Target/LLVMIR/DebugTranslation.cpp (+2-1)
diff --git a/llvm/include/llvm/IR/DIBuilder.h b/llvm/include/llvm/IR/DIBuilder.h
index edec161b397155..010dcbfdadcac2 100644
--- a/llvm/include/llvm/IR/DIBuilder.h
+++ b/llvm/include/llvm/IR/DIBuilder.h
@@ -262,6 +262,13 @@ namespace llvm {
                       std::optional<unsigned> DWARFAddressSpace = std::nullopt,
                       StringRef Name = "", DINodeArray Annotations = nullptr);
 
+    /// Create a __ptrauth qualifier.
+    DIDerivedType *createPtrAuthQualifiedType(DIType *FromTy, unsigned Key,
+                                              bool IsAddressDiscriminated,
+                                              unsigned ExtraDiscriminator,
+                                              bool IsaPointer,
+                                              bool authenticatesNullValues);
+
     /// Create debugging information entry for a pointer to member.
     /// \param PointeeTy Type pointed to by this pointer.
     /// \param SizeInBits  Size.
diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index 156f6eb49253de..00c39693ed9fbf 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -745,7 +745,7 @@ class DIType : public DIScope {
 
   unsigned getLine() const { return Line; }
   uint64_t getSizeInBits() const { return SizeInBits; }
-  uint32_t getAlignInBits() const { return SubclassData32; }
+  uint32_t getAlignInBits() const;
   uint32_t getAlignInBytes() const { return getAlignInBits() / CHAR_BIT; }
   uint64_t getOffsetInBits() const { return OffsetInBits; }
   DIFlags getFlags() const { return Flags; }
@@ -972,6 +972,35 @@ class DIStringType : public DIType {
 ///
 /// TODO: Split out members (inheritance, fields, methods, etc.).
 class DIDerivedType : public DIType {
+public:
+  /// Pointer authentication (__ptrauth) metadata.
+  struct PtrAuthData {
+    unsigned RawData;
+
+    PtrAuthData(unsigned FromRawData) : RawData(FromRawData) {}
+    PtrAuthData(unsigned Key, bool IsDiscr, unsigned Discriminator,
+                bool IsaPointer, bool AuthenticatesNullValues) {
+      assert(Key < 16);
+      assert(Discriminator <= 0xffff);
+      RawData = (Key << 0) | (IsDiscr ? (1 << 4) : 0) | (Discriminator << 5) |
+                (IsaPointer ? (1 << 21) : 0) |
+                (AuthenticatesNullValues ? (1 << 22) : 0);
+    }
+    bool operator==(struct PtrAuthData Other) const {
+      return RawData == Other.RawData;
+    }
+    bool operator!=(struct PtrAuthData Other) const {
+      return !(*this == Other);
+    }
+
+    unsigned Key() { return (RawData >> 0) & 0b1111; }
+    bool IsAddressDiscriminated() { return (RawData >> 4) & 1; }
+    unsigned ExtraDiscriminator() { return (RawData >> 5) & 0xffff; }
+    bool IsaPointer() { return (RawData >> 21) & 1; }
+    bool AuthenticatesNullValues() { return (RawData >> 22) & 1; }
+  };
+
+private:
   friend class LLVMContextImpl;
   friend class MDNode;
 
@@ -982,59 +1011,70 @@ class DIDerivedType : public DIType {
   DIDerivedType(LLVMContext &C, StorageType Storage, unsigned Tag,
                 unsigned Line, uint64_t SizeInBits, uint32_t AlignInBits,
                 uint64_t OffsetInBits,
-                std::optional<unsigned> DWARFAddressSpace, DIFlags Flags,
+                std::optional<unsigned> DWARFAddressSpace,
+                std::optional<PtrAuthData> PtrAuthData, DIFlags Flags,
                 ArrayRef<Metadata *> Ops)
       : DIType(C, DIDerivedTypeKind, Storage, Tag, Line, SizeInBits,
                AlignInBits, OffsetInBits, Flags, Ops),
-        DWARFAddressSpace(DWARFAddressSpace) {}
+        DWARFAddressSpace(DWARFAddressSpace) {
+    if (PtrAuthData)
+      SubclassData32 = PtrAuthData->RawData;
+  }
   ~DIDerivedType() = default;
   static DIDerivedType *
   getImpl(LLVMContext &Context, unsigned Tag, StringRef Name, DIFile *File,
           unsigned Line, DIScope *Scope, DIType *BaseType, uint64_t SizeInBits,
           uint32_t AlignInBits, uint64_t OffsetInBits,
-          std::optional<unsigned> DWARFAddressSpace, DIFlags Flags,
+          std::optional<unsigned> DWARFAddressSpace,
+          std::optional<PtrAuthData> PtrAuthData, DIFlags Flags,
           Metadata *ExtraData, DINodeArray Annotations, StorageType Storage,
           bool ShouldCreate = true) {
     return getImpl(Context, Tag, getCanonicalMDString(Context, Name), File,
                    Line, Scope, BaseType, SizeInBits, AlignInBits, OffsetInBits,
-                   DWARFAddressSpace, Flags, ExtraData, Annotations.get(),
-                   Storage, ShouldCreate);
+                   DWARFAddressSpace, PtrAuthData, Flags, ExtraData,
+                   Annotations.get(), Storage, ShouldCreate);
   }
   static DIDerivedType *
   getImpl(LLVMContext &Context, unsigned Tag, MDString *Name, Metadata *File,
           unsigned Line, Metadata *Scope, Metadata *BaseType,
           uint64_t SizeInBits, uint32_t AlignInBits, uint64_t OffsetInBits,
-          std::optional<unsigned> DWARFAddressSpace, DIFlags Flags,
+          std::optional<unsigned> DWARFAddressSpace,
+          std::optional<PtrAuthData> PtrAuthData, DIFlags Flags,
           Metadata *ExtraData, Metadata *Annotations, StorageType Storage,
           bool ShouldCreate = true);
 
   TempDIDerivedType cloneImpl() const {
-    return getTemporary(
-        getContext(), getTag(), getName(), getFile(), getLine(), getScope(),
-        getBaseType(), getSizeInBits(), getAlignInBits(), getOffsetInBits(),
-        getDWARFAddressSpace(), getFlags(), getExtraData(), getAnnotations());
+    return getTemporary(getContext(), getTag(), getName(), getFile(), getLine(),
+                        getScope(), getBaseType(), getSizeInBits(),
+                        getAlignInBits(), getOffsetInBits(),
+                        getDWARFAddressSpace(), getPtrAuthData(), getFlags(),
+                        getExtraData(), getAnnotations());
   }
 
 public:
-  DEFINE_MDNODE_GET(
-      DIDerivedType,
-      (unsigned Tag, MDString *Name, Metadata *File, unsigned Line,
-       Metadata *Scope, Metadata *BaseType, uint64_t SizeInBits,
-       uint32_t AlignInBits, uint64_t OffsetInBits,
-       std::optional<unsigned> DWARFAddressSpace, DIFlags Flags,
-       Metadata *ExtraData = nullptr, Metadata *Annotations = nullptr),
-      (Tag, Name, File, Line, Scope, BaseType, SizeInBits, AlignInBits,
-       OffsetInBits, DWARFAddressSpace, Flags, ExtraData, Annotations))
+  DEFINE_MDNODE_GET(DIDerivedType,
+                    (unsigned Tag, MDString *Name, Metadata *File,
+                     unsigned Line, Metadata *Scope, Metadata *BaseType,
+                     uint64_t SizeInBits, uint32_t AlignInBits,
+                     uint64_t OffsetInBits,
+                     std::optional<unsigned> DWARFAddressSpace,
+                     std::optional<PtrAuthData> PtrAuthData, DIFlags Flags,
+                     Metadata *ExtraData = nullptr,
+                     Metadata *Annotations = nullptr),
+                    (Tag, Name, File, Line, Scope, BaseType, SizeInBits,
+                     AlignInBits, OffsetInBits, DWARFAddressSpace, PtrAuthData,
+                     Flags, ExtraData, Annotations))
   DEFINE_MDNODE_GET(DIDerivedType,
                     (unsigned Tag, StringRef Name, DIFile *File, unsigned Line,
                      DIScope *Scope, DIType *BaseType, uint64_t SizeInBits,
                      uint32_t AlignInBits, uint64_t OffsetInBits,
-                     std::optional<unsigned> DWARFAddressSpace, DIFlags Flags,
+                     std::optional<unsigned> DWARFAddressSpace,
+                     std::optional<PtrAuthData> PtrAuthData, DIFlags Flags,
                      Metadata *ExtraData = nullptr,
                      DINodeArray Annotations = nullptr),
                     (Tag, Name, File, Line, Scope, BaseType, SizeInBits,
-                     AlignInBits, OffsetInBits, DWARFAddressSpace, Flags,
-                     ExtraData, Annotations))
+                     AlignInBits, OffsetInBits, DWARFAddressSpace, PtrAuthData,
+                     Flags, ExtraData, Annotations))
 
   TempDIDerivedType clone() const { return cloneImpl(); }
 
@@ -1048,6 +1088,39 @@ class DIDerivedType : public DIType {
     return DWARFAddressSpace;
   }
 
+  std::optional<PtrAuthData> getPtrAuthData() const;
+
+  /// \returns The PointerAuth key.
+  std::optional<unsigned> getPtrAuthKey() const {
+    if (auto PtrAuthData = getPtrAuthData())
+      return PtrAuthData->Key();
+    return std::nullopt;
+  }
+  /// \returns The PointerAuth address discrimination bit.
+  std::optional<bool> isPtrAuthAddressDiscriminated() const {
+    if (auto PtrAuthData = getPtrAuthData())
+      return PtrAuthData->IsAddressDiscriminated();
+    return std::nullopt;
+  }
+  /// \returns The PointerAuth extra discriminator.
+  std::optional<unsigned> getPtrAuthExtraDiscriminator() const {
+    if (auto PtrAuthData = getPtrAuthData())
+      return PtrAuthData->ExtraDiscriminator();
+    return std::nullopt;
+  }
+  /// \returns The PointerAuth IsaPointer bit.
+  std::optional<bool> isPtrAuthIsaPointer() const {
+    if (auto PtrAuthData = getPtrAuthData())
+      return PtrAuthData->IsaPointer();
+    return std::nullopt;
+  }
+  /// \returns The PointerAuth authenticates null values bit.
+  std::optional<bool> getPtrAuthAuthenticatesNullValues() const {
+    if (auto PtrAuthData = getPtrAuthData())
+      return PtrAuthData->AuthenticatesNullValues();
+    return std::nullopt;
+  }
+
   /// Get extra data associated with this derived type.
   ///
   /// Class type for pointer-to-members, objective-c property node for ivars,
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index a91e2f690999e0..e91abaf0780a3d 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -5130,7 +5130,11 @@ bool LLParser::parseDIStringType(MDNode *&Result, bool IsDistinct) {
 ///   ::= !DIDerivedType(tag: DW_TAG_pointer_type, name: "int", file: !0,
 ///                      line: 7, scope: !1, baseType: !2, size: 32,
 ///                      align: 32, offset: 0, flags: 0, extraData: !3,
-///                      dwarfAddressSpace: 3)
+///                      dwarfAddressSpace: 3, ptrAuthKey: 1,
+///                      ptrAuthIsAddressDiscriminated: true,
+///                      ptrAuthExtraDiscriminator: 0x1234,
+///                      ptrAuthIsaPointer: 1, ptrAuthAuthenticatesNullValues:1
+///                      )
 bool LLParser::parseDIDerivedType(MDNode *&Result, bool IsDistinct) {
 #define VISIT_MD_FIELDS(OPTIONAL, REQUIRED)                                    \
   REQUIRED(tag, DwarfTagField, );                                              \
@@ -5145,19 +5149,30 @@ bool LLParser::parseDIDerivedType(MDNode *&Result, bool IsDistinct) {
   OPTIONAL(flags, DIFlagField, );                                              \
   OPTIONAL(extraData, MDField, );                                              \
   OPTIONAL(dwarfAddressSpace, MDUnsignedField, (UINT32_MAX, UINT32_MAX));      \
-  OPTIONAL(annotations, MDField, );
+  OPTIONAL(annotations, MDField, );                                            \
+  OPTIONAL(ptrAuthKey, MDUnsignedField, (0, 7));                               \
+  OPTIONAL(ptrAuthIsAddressDiscriminated, MDBoolField, );                      \
+  OPTIONAL(ptrAuthExtraDiscriminator, MDUnsignedField, (0, 0xffff));           \
+  OPTIONAL(ptrAuthIsaPointer, MDBoolField, );                                  \
+  OPTIONAL(ptrAuthAuthenticatesNullValues, MDBoolField, );
   PARSE_MD_FIELDS();
 #undef VISIT_MD_FIELDS
 
   std::optional<unsigned> DWARFAddressSpace;
   if (dwarfAddressSpace.Val != UINT32_MAX)
     DWARFAddressSpace = dwarfAddressSpace.Val;
+  std::optional<DIDerivedType::PtrAuthData> PtrAuthData;
+  if (ptrAuthKey.Val)
+    PtrAuthData = DIDerivedType::PtrAuthData(
+        (unsigned)ptrAuthKey.Val, ptrAuthIsAddressDiscriminated.Val,
+        (unsigned)ptrAuthExtraDiscriminator.Val, ptrAuthIsaPointer.Val,
+        ptrAuthAuthenticatesNullValues.Val);
 
   Result = GET_OR_DISTINCT(DIDerivedType,
                            (Context, tag.Val, name.Val, file.Val, line.Val,
                             scope.Val, baseType.Val, size.Val, align.Val,
-                            offset.Val, DWARFAddressSpace, flags.Val,
-                            extraData.Val, annotations.Val));
+                            offset.Val, DWARFAddressSpace, PtrAuthData,
+                            flags.Val, extraData.Val, annotations.Val));
   return false;
 }
 
diff --git a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
index 770eb83af17f9b..bdc2db82dfbe0e 100644
--- a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
+++ b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
@@ -1556,7 +1556,7 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
     break;
   }
   case bitc::METADATA_DERIVED_TYPE: {
-    if (Record.size() < 12 || Record.size() > 14)
+    if (Record.size() < 12 || Record.size() > 15)
       return error("Invalid record");
 
     // DWARF address space is encoded as N->getDWARFAddressSpace() + 1. 0 means
@@ -1566,8 +1566,18 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
       DWARFAddressSpace = Record[12] - 1;
 
     Metadata *Annotations = nullptr;
-    if (Record.size() > 13 && Record[13])
-      Annotations = getMDOrNull(Record[13]);
+    std::optional<DIDerivedType::PtrAuthData> PtrAuthData;
+
+    // Only look for annotations/ptrauth if both are allocated.
+    // If not, we can't tell which was intended to be embedded, as both ptrauth
+    // and annotations have been expected at Record[13] at various times.
+    if (Record.size() > 14) {
+      if (Record[13])
+        Annotations = getMDOrNull(Record[13]);
+
+      if (Record[14])
+        PtrAuthData = DIDerivedType::PtrAuthData(Record[14]);
+    }
 
     IsDistinct = Record[0];
     DINode::DIFlags Flags = static_cast<DINode::DIFlags>(Record[10]);
@@ -1577,7 +1587,7 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
                          getMDOrNull(Record[3]), Record[4],
                          getDITypeRefOrNull(Record[5]),
                          getDITypeRefOrNull(Record[6]), Record[7], Record[8],
-                         Record[9], DWARFAddressSpace, Flags,
+                         Record[9], DWARFAddressSpace, PtrAuthData, Flags,
                          getDITypeRefOrNull(Record[11]), Annotations)),
         NextMetadataNo);
     NextMetadataNo++;
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index 13be0b0c3307fb..b21424b854b3e9 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -1804,6 +1804,11 @@ void ModuleBitcodeWriter::writeDIDerivedType(const DIDerivedType *N,
 
   Record.push_back(VE.getMetadataOrNullID(N->getAnnotations().get()));
 
+  if (auto PtrAuthData = N->getPtrAuthData())
+    Record.push_back(PtrAuthData->RawData);
+  else
+    Record.push_back(0);
+
   Stream.EmitRecord(bitc::METADATA_DERIVED_TYPE, Record, Abbrev);
   Record.clear();
 }
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
index d462859e489465..ae0226934804f7 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -803,6 +803,20 @@ void DwarfUnit::constructTypeDIE(DIE &Buffer, const DIDerivedType *DTy) {
   if (DTy->getDWARFAddressSpace())
     addUInt(Buffer, dwarf::DW_AT_address_class, dwarf::DW_FORM_data4,
             *DTy->getDWARFAddressSpace());
+  if (auto Key = DTy->getPtrAuthKey())
+    addUInt(Buffer, dwarf::DW_AT_LLVM_ptrauth_key, dwarf::DW_FORM_data1, *Key);
+  if (auto AddrDisc = DTy->isPtrAuthAddressDiscriminated())
+    if (AddrDisc.value())
+      addFlag(Buffer, dwarf::DW_AT_LLVM_ptrauth_address_discriminated);
+  if (auto Disc = DTy->getPtrAuthExtraDiscriminator())
+    addUInt(Buffer, dwarf::DW_AT_LLVM_ptrauth_extra_discriminator,
+            dwarf::DW_FORM_data2, *Disc);
+  if (auto IsaPointer = DTy->isPtrAuthIsaPointer())
+    if (*IsaPointer)
+      addFlag(Buffer, dwarf::DW_AT_LLVM_ptrauth_isa_pointer);
+  if (auto AuthenticatesNullValues = DTy->getPtrAuthAuthenticatesNullValues())
+    if (*AuthenticatesNullValues)
+      addFlag(Buffer, dwarf::DW_AT_LLVM_ptrauth_authenticates_null_values);
 }
 
 void DwarfUnit::constructSubprogramArguments(DIE &Buffer, DITypeRefArray Args) {
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index 251485a403fee6..3949aadaf564b7 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -2107,6 +2107,17 @@ static void writeDIDerivedType(raw_ostream &Out, const DIDerivedType *N,
     Printer.printInt("dwarfAddressSpace", *DWARFAddressSpace,
                      /* ShouldSkipZero */ false);
   Printer.printMetadata("annotations", N->getRawAnnotations());
+  if (auto Key = N->getPtrAuthKey())
+    Printer.printInt("ptrAuthKey", *Key);
+  if (auto AddrDisc = N->isPtrAuthAddressDiscriminated())
+    Printer.printBool("ptrAuthIsAddressDiscriminated", *AddrDisc);
+  if (auto Disc = N->getPtrAuthExtraDiscriminator())
+    Printer.printInt("ptrAuthExtraDiscriminator", *Disc);
+  if (auto IsaPointer = N->isPtrAuthIsaPointer())
+    Printer.printBool("ptrAuthIsaPointer", *IsaPointer);
+  if (auto AuthenticatesNullValues = N->getPtrAuthAuthenticatesNullValues())
+    Printer.printBool("ptrAuthAuthenticatesNullValues",
+                      *AuthenticatesNullValues);
   Out << ")";
 }
 
diff --git a/llvm/lib/IR/DIBuilder.cpp b/llvm/lib/IR/DIBuilder.cpp
index 62efaba025344b..2842cb15e78fb3 100644
--- a/llvm/lib/IR/DIBuilder.cpp
+++ b/llvm/lib/IR/DIBuilder.cpp
@@ -296,7 +296,20 @@ DIStringType *DIBuilder::createStringType(StringRef Name,
 
 DIDerivedType *DIBuilder::createQualifiedType(unsigned Tag, DIType *FromTy) {
   return DIDerivedType::get(VMContext, Tag, "", nullptr, 0, nullptr, FromTy, 0,
-                            0, 0, std::nullopt, DINode::FlagZero);
+                            0, 0, std::nullopt, std::nullopt, DINode::FlagZero);
+}
+
+DIDerivedType *DIBuilder::createPtrAuthQualifiedType(
+    DIType *FromTy, unsigned Key, bool IsAddressDiscriminated,
+    unsigned ExtraDiscriminator, bool IsaPointer,
+    bool AuthenticatesNullValues) {
+  return DIDerivedType::get(
+      VMContext, dwarf::DW_TAG_LLVM_ptrauth_type, "", nullptr, 0, nullptr,
+      FromTy, 0, 0, 0, std::nullopt,
+      std::optional<DIDerivedType::PtrAuthData>({Key, IsAddressDiscriminated,
+                                                 ExtraDiscriminator, IsaPointer,
+                                                 AuthenticatesNullValues}),
+      DINode::FlagZero);
 }
 
 DIDerivedType *
@@ -307,8 +320,8 @@ DIBuilder::createPointerType(DIType *PointeeTy, uint64_t SizeInBits,
   // FIXME: Why is there a name here?
   return DIDerivedType::get(VMContext, dwarf::DW_TAG_pointer_type, Name,
                             nullptr, 0, nullptr, PointeeTy, SizeInBits,
-                            AlignInBits, 0, DWARFAddressSpace, DINode::FlagZero,
-                            nullptr, Annotations);
+                            AlignInBits, 0, DWARFAddressSpace, std::nullopt,
+                            DINode::FlagZero, nullptr, Annotations);
 }
 
 DIDerivedType *DIBuilder::createMemberPointerType(DIType *PointeeTy,
@@ -318,7 +331,8 @@ DIDerivedType *DIBuilder::createMemberPointerType(DIType *PointeeTy,
                                                   DINode::DIFlags Flags) {
   return DIDerivedType::get(VMContext, dwarf::DW_TAG_ptr_to_member_type, "",
                             nullptr, 0, nullptr, PointeeTy, SizeInBits,
-                            AlignInBits, 0, std::nullopt, Flags, Base);
+                            AlignInBits, 0, std::nullopt, std::nullopt, Flags,
+                            Base);
 }
 
 DIDerivedType *
@@ -327,7 +341,7 @@ DIBuilder::createReferenceType(unsigned Tag, DIType *RTy, uint64_t SizeInBits,
                                std::optional<unsigned> DWARFAddressSpace) {
   assert(RTy && "Unable to create reference type");
   return DIDerivedType::get(VMContext, Tag, "", nullptr, 0, nullptr, RTy,
-                            SizeInBits, AlignInBits, 0, DWARFAddressSpace,
+                            SizeInBi...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Mar 4, 2024

@llvm/pr-subscribers-llvm-ir

Author: Daniil Kovalev (kovdan01)

Changes

Reland #82363 after fixing build failure https://lab.llvm.org/buildbot/#/builders/5/builds/41428.

Memory sanitizer detects usage of RawData union member which is not
filled directly. Instead, the code relies on filling Data union
member, which is a struct consisting of signing schema parameters.

According to https://en.cppreference.com/w/cpp/language/union, this is UB:
"It is undefined behavior to read from the member of the union that
wasn't most recently written".

Instead of relying on compiler allowing us to do dirty things, do not
use union and only store RawData. Particular ptrauth parameters are
obtained on demand via bit operations.

Original PR description below.

Emit __ptrauth-qualified types as DIDerivedType metadata nodes in IR with tag DW_TAG_LLVM_ptrauth_type, baseType referring to the type which has the qualifier applied, and the following parameters representing the signing schema:

  • ptrAuthKey (integer)
  • ptrAuthIsAddressDiscriminated (boolean)
  • ptrAuthExtraDiscriminator (integer)
  • ptrAuthIsaPointer (boolean)
  • ptrAuthAuthenticatesNullValues (boolean)

Co-authored-by: Ahmed Bougacha <[email protected]>


Patch is 50.44 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/83862.diff

16 Files Affected:

  • (modified) llvm/include/llvm/IR/DIBuilder.h (+7)
  • (modified) llvm/include/llvm/IR/DebugInfoMetadata.h (+96-23)
  • (modified) llvm/lib/AsmParser/LLParser.cpp (+19-4)
  • (modified) llvm/lib/Bitcode/Reader/MetadataLoader.cpp (+14-4)
  • (modified) llvm/lib/Bitcode/Writer/BitcodeWriter.cpp (+5)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (+14)
  • (modified) llvm/lib/IR/AsmWriter.cpp (+11)
  • (modified) llvm/lib/IR/DIBuilder.cpp (+37-21)
  • (modified) llvm/lib/IR/DebugInfo.cpp (+3-3)
  • (modified) llvm/lib/IR/DebugInfoMetadata.cpp (+21-11)
  • (modified) llvm/lib/IR/LLVMContextImpl.h (+10-5)
  • (modified) llvm/lib/IR/Verifier.cpp (+1)
  • (modified) llvm/test/Assembler/debug-info.ll (+14-2)
  • (added) llvm/test/DebugInfo/AArch64/ptrauth.ll (+70)
  • (modified) llvm/unittests/IR/MetadataTest.cpp (+59-26)
  • (modified) mlir/lib/Target/LLVMIR/DebugTranslation.cpp (+2-1)
diff --git a/llvm/include/llvm/IR/DIBuilder.h b/llvm/include/llvm/IR/DIBuilder.h
index edec161b397155..010dcbfdadcac2 100644
--- a/llvm/include/llvm/IR/DIBuilder.h
+++ b/llvm/include/llvm/IR/DIBuilder.h
@@ -262,6 +262,13 @@ namespace llvm {
                       std::optional<unsigned> DWARFAddressSpace = std::nullopt,
                       StringRef Name = "", DINodeArray Annotations = nullptr);
 
+    /// Create a __ptrauth qualifier.
+    DIDerivedType *createPtrAuthQualifiedType(DIType *FromTy, unsigned Key,
+                                              bool IsAddressDiscriminated,
+                                              unsigned ExtraDiscriminator,
+                                              bool IsaPointer,
+                                              bool authenticatesNullValues);
+
     /// Create debugging information entry for a pointer to member.
     /// \param PointeeTy Type pointed to by this pointer.
     /// \param SizeInBits  Size.
diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index 156f6eb49253de..00c39693ed9fbf 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -745,7 +745,7 @@ class DIType : public DIScope {
 
   unsigned getLine() const { return Line; }
   uint64_t getSizeInBits() const { return SizeInBits; }
-  uint32_t getAlignInBits() const { return SubclassData32; }
+  uint32_t getAlignInBits() const;
   uint32_t getAlignInBytes() const { return getAlignInBits() / CHAR_BIT; }
   uint64_t getOffsetInBits() const { return OffsetInBits; }
   DIFlags getFlags() const { return Flags; }
@@ -972,6 +972,35 @@ class DIStringType : public DIType {
 ///
 /// TODO: Split out members (inheritance, fields, methods, etc.).
 class DIDerivedType : public DIType {
+public:
+  /// Pointer authentication (__ptrauth) metadata.
+  struct PtrAuthData {
+    unsigned RawData;
+
+    PtrAuthData(unsigned FromRawData) : RawData(FromRawData) {}
+    PtrAuthData(unsigned Key, bool IsDiscr, unsigned Discriminator,
+                bool IsaPointer, bool AuthenticatesNullValues) {
+      assert(Key < 16);
+      assert(Discriminator <= 0xffff);
+      RawData = (Key << 0) | (IsDiscr ? (1 << 4) : 0) | (Discriminator << 5) |
+                (IsaPointer ? (1 << 21) : 0) |
+                (AuthenticatesNullValues ? (1 << 22) : 0);
+    }
+    bool operator==(struct PtrAuthData Other) const {
+      return RawData == Other.RawData;
+    }
+    bool operator!=(struct PtrAuthData Other) const {
+      return !(*this == Other);
+    }
+
+    unsigned Key() { return (RawData >> 0) & 0b1111; }
+    bool IsAddressDiscriminated() { return (RawData >> 4) & 1; }
+    unsigned ExtraDiscriminator() { return (RawData >> 5) & 0xffff; }
+    bool IsaPointer() { return (RawData >> 21) & 1; }
+    bool AuthenticatesNullValues() { return (RawData >> 22) & 1; }
+  };
+
+private:
   friend class LLVMContextImpl;
   friend class MDNode;
 
@@ -982,59 +1011,70 @@ class DIDerivedType : public DIType {
   DIDerivedType(LLVMContext &C, StorageType Storage, unsigned Tag,
                 unsigned Line, uint64_t SizeInBits, uint32_t AlignInBits,
                 uint64_t OffsetInBits,
-                std::optional<unsigned> DWARFAddressSpace, DIFlags Flags,
+                std::optional<unsigned> DWARFAddressSpace,
+                std::optional<PtrAuthData> PtrAuthData, DIFlags Flags,
                 ArrayRef<Metadata *> Ops)
       : DIType(C, DIDerivedTypeKind, Storage, Tag, Line, SizeInBits,
                AlignInBits, OffsetInBits, Flags, Ops),
-        DWARFAddressSpace(DWARFAddressSpace) {}
+        DWARFAddressSpace(DWARFAddressSpace) {
+    if (PtrAuthData)
+      SubclassData32 = PtrAuthData->RawData;
+  }
   ~DIDerivedType() = default;
   static DIDerivedType *
   getImpl(LLVMContext &Context, unsigned Tag, StringRef Name, DIFile *File,
           unsigned Line, DIScope *Scope, DIType *BaseType, uint64_t SizeInBits,
           uint32_t AlignInBits, uint64_t OffsetInBits,
-          std::optional<unsigned> DWARFAddressSpace, DIFlags Flags,
+          std::optional<unsigned> DWARFAddressSpace,
+          std::optional<PtrAuthData> PtrAuthData, DIFlags Flags,
           Metadata *ExtraData, DINodeArray Annotations, StorageType Storage,
           bool ShouldCreate = true) {
     return getImpl(Context, Tag, getCanonicalMDString(Context, Name), File,
                    Line, Scope, BaseType, SizeInBits, AlignInBits, OffsetInBits,
-                   DWARFAddressSpace, Flags, ExtraData, Annotations.get(),
-                   Storage, ShouldCreate);
+                   DWARFAddressSpace, PtrAuthData, Flags, ExtraData,
+                   Annotations.get(), Storage, ShouldCreate);
   }
   static DIDerivedType *
   getImpl(LLVMContext &Context, unsigned Tag, MDString *Name, Metadata *File,
           unsigned Line, Metadata *Scope, Metadata *BaseType,
           uint64_t SizeInBits, uint32_t AlignInBits, uint64_t OffsetInBits,
-          std::optional<unsigned> DWARFAddressSpace, DIFlags Flags,
+          std::optional<unsigned> DWARFAddressSpace,
+          std::optional<PtrAuthData> PtrAuthData, DIFlags Flags,
           Metadata *ExtraData, Metadata *Annotations, StorageType Storage,
           bool ShouldCreate = true);
 
   TempDIDerivedType cloneImpl() const {
-    return getTemporary(
-        getContext(), getTag(), getName(), getFile(), getLine(), getScope(),
-        getBaseType(), getSizeInBits(), getAlignInBits(), getOffsetInBits(),
-        getDWARFAddressSpace(), getFlags(), getExtraData(), getAnnotations());
+    return getTemporary(getContext(), getTag(), getName(), getFile(), getLine(),
+                        getScope(), getBaseType(), getSizeInBits(),
+                        getAlignInBits(), getOffsetInBits(),
+                        getDWARFAddressSpace(), getPtrAuthData(), getFlags(),
+                        getExtraData(), getAnnotations());
   }
 
 public:
-  DEFINE_MDNODE_GET(
-      DIDerivedType,
-      (unsigned Tag, MDString *Name, Metadata *File, unsigned Line,
-       Metadata *Scope, Metadata *BaseType, uint64_t SizeInBits,
-       uint32_t AlignInBits, uint64_t OffsetInBits,
-       std::optional<unsigned> DWARFAddressSpace, DIFlags Flags,
-       Metadata *ExtraData = nullptr, Metadata *Annotations = nullptr),
-      (Tag, Name, File, Line, Scope, BaseType, SizeInBits, AlignInBits,
-       OffsetInBits, DWARFAddressSpace, Flags, ExtraData, Annotations))
+  DEFINE_MDNODE_GET(DIDerivedType,
+                    (unsigned Tag, MDString *Name, Metadata *File,
+                     unsigned Line, Metadata *Scope, Metadata *BaseType,
+                     uint64_t SizeInBits, uint32_t AlignInBits,
+                     uint64_t OffsetInBits,
+                     std::optional<unsigned> DWARFAddressSpace,
+                     std::optional<PtrAuthData> PtrAuthData, DIFlags Flags,
+                     Metadata *ExtraData = nullptr,
+                     Metadata *Annotations = nullptr),
+                    (Tag, Name, File, Line, Scope, BaseType, SizeInBits,
+                     AlignInBits, OffsetInBits, DWARFAddressSpace, PtrAuthData,
+                     Flags, ExtraData, Annotations))
   DEFINE_MDNODE_GET(DIDerivedType,
                     (unsigned Tag, StringRef Name, DIFile *File, unsigned Line,
                      DIScope *Scope, DIType *BaseType, uint64_t SizeInBits,
                      uint32_t AlignInBits, uint64_t OffsetInBits,
-                     std::optional<unsigned> DWARFAddressSpace, DIFlags Flags,
+                     std::optional<unsigned> DWARFAddressSpace,
+                     std::optional<PtrAuthData> PtrAuthData, DIFlags Flags,
                      Metadata *ExtraData = nullptr,
                      DINodeArray Annotations = nullptr),
                     (Tag, Name, File, Line, Scope, BaseType, SizeInBits,
-                     AlignInBits, OffsetInBits, DWARFAddressSpace, Flags,
-                     ExtraData, Annotations))
+                     AlignInBits, OffsetInBits, DWARFAddressSpace, PtrAuthData,
+                     Flags, ExtraData, Annotations))
 
   TempDIDerivedType clone() const { return cloneImpl(); }
 
@@ -1048,6 +1088,39 @@ class DIDerivedType : public DIType {
     return DWARFAddressSpace;
   }
 
+  std::optional<PtrAuthData> getPtrAuthData() const;
+
+  /// \returns The PointerAuth key.
+  std::optional<unsigned> getPtrAuthKey() const {
+    if (auto PtrAuthData = getPtrAuthData())
+      return PtrAuthData->Key();
+    return std::nullopt;
+  }
+  /// \returns The PointerAuth address discrimination bit.
+  std::optional<bool> isPtrAuthAddressDiscriminated() const {
+    if (auto PtrAuthData = getPtrAuthData())
+      return PtrAuthData->IsAddressDiscriminated();
+    return std::nullopt;
+  }
+  /// \returns The PointerAuth extra discriminator.
+  std::optional<unsigned> getPtrAuthExtraDiscriminator() const {
+    if (auto PtrAuthData = getPtrAuthData())
+      return PtrAuthData->ExtraDiscriminator();
+    return std::nullopt;
+  }
+  /// \returns The PointerAuth IsaPointer bit.
+  std::optional<bool> isPtrAuthIsaPointer() const {
+    if (auto PtrAuthData = getPtrAuthData())
+      return PtrAuthData->IsaPointer();
+    return std::nullopt;
+  }
+  /// \returns The PointerAuth authenticates null values bit.
+  std::optional<bool> getPtrAuthAuthenticatesNullValues() const {
+    if (auto PtrAuthData = getPtrAuthData())
+      return PtrAuthData->AuthenticatesNullValues();
+    return std::nullopt;
+  }
+
   /// Get extra data associated with this derived type.
   ///
   /// Class type for pointer-to-members, objective-c property node for ivars,
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index a91e2f690999e0..e91abaf0780a3d 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -5130,7 +5130,11 @@ bool LLParser::parseDIStringType(MDNode *&Result, bool IsDistinct) {
 ///   ::= !DIDerivedType(tag: DW_TAG_pointer_type, name: "int", file: !0,
 ///                      line: 7, scope: !1, baseType: !2, size: 32,
 ///                      align: 32, offset: 0, flags: 0, extraData: !3,
-///                      dwarfAddressSpace: 3)
+///                      dwarfAddressSpace: 3, ptrAuthKey: 1,
+///                      ptrAuthIsAddressDiscriminated: true,
+///                      ptrAuthExtraDiscriminator: 0x1234,
+///                      ptrAuthIsaPointer: 1, ptrAuthAuthenticatesNullValues:1
+///                      )
 bool LLParser::parseDIDerivedType(MDNode *&Result, bool IsDistinct) {
 #define VISIT_MD_FIELDS(OPTIONAL, REQUIRED)                                    \
   REQUIRED(tag, DwarfTagField, );                                              \
@@ -5145,19 +5149,30 @@ bool LLParser::parseDIDerivedType(MDNode *&Result, bool IsDistinct) {
   OPTIONAL(flags, DIFlagField, );                                              \
   OPTIONAL(extraData, MDField, );                                              \
   OPTIONAL(dwarfAddressSpace, MDUnsignedField, (UINT32_MAX, UINT32_MAX));      \
-  OPTIONAL(annotations, MDField, );
+  OPTIONAL(annotations, MDField, );                                            \
+  OPTIONAL(ptrAuthKey, MDUnsignedField, (0, 7));                               \
+  OPTIONAL(ptrAuthIsAddressDiscriminated, MDBoolField, );                      \
+  OPTIONAL(ptrAuthExtraDiscriminator, MDUnsignedField, (0, 0xffff));           \
+  OPTIONAL(ptrAuthIsaPointer, MDBoolField, );                                  \
+  OPTIONAL(ptrAuthAuthenticatesNullValues, MDBoolField, );
   PARSE_MD_FIELDS();
 #undef VISIT_MD_FIELDS
 
   std::optional<unsigned> DWARFAddressSpace;
   if (dwarfAddressSpace.Val != UINT32_MAX)
     DWARFAddressSpace = dwarfAddressSpace.Val;
+  std::optional<DIDerivedType::PtrAuthData> PtrAuthData;
+  if (ptrAuthKey.Val)
+    PtrAuthData = DIDerivedType::PtrAuthData(
+        (unsigned)ptrAuthKey.Val, ptrAuthIsAddressDiscriminated.Val,
+        (unsigned)ptrAuthExtraDiscriminator.Val, ptrAuthIsaPointer.Val,
+        ptrAuthAuthenticatesNullValues.Val);
 
   Result = GET_OR_DISTINCT(DIDerivedType,
                            (Context, tag.Val, name.Val, file.Val, line.Val,
                             scope.Val, baseType.Val, size.Val, align.Val,
-                            offset.Val, DWARFAddressSpace, flags.Val,
-                            extraData.Val, annotations.Val));
+                            offset.Val, DWARFAddressSpace, PtrAuthData,
+                            flags.Val, extraData.Val, annotations.Val));
   return false;
 }
 
diff --git a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
index 770eb83af17f9b..bdc2db82dfbe0e 100644
--- a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
+++ b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
@@ -1556,7 +1556,7 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
     break;
   }
   case bitc::METADATA_DERIVED_TYPE: {
-    if (Record.size() < 12 || Record.size() > 14)
+    if (Record.size() < 12 || Record.size() > 15)
       return error("Invalid record");
 
     // DWARF address space is encoded as N->getDWARFAddressSpace() + 1. 0 means
@@ -1566,8 +1566,18 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
       DWARFAddressSpace = Record[12] - 1;
 
     Metadata *Annotations = nullptr;
-    if (Record.size() > 13 && Record[13])
-      Annotations = getMDOrNull(Record[13]);
+    std::optional<DIDerivedType::PtrAuthData> PtrAuthData;
+
+    // Only look for annotations/ptrauth if both are allocated.
+    // If not, we can't tell which was intended to be embedded, as both ptrauth
+    // and annotations have been expected at Record[13] at various times.
+    if (Record.size() > 14) {
+      if (Record[13])
+        Annotations = getMDOrNull(Record[13]);
+
+      if (Record[14])
+        PtrAuthData = DIDerivedType::PtrAuthData(Record[14]);
+    }
 
     IsDistinct = Record[0];
     DINode::DIFlags Flags = static_cast<DINode::DIFlags>(Record[10]);
@@ -1577,7 +1587,7 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
                          getMDOrNull(Record[3]), Record[4],
                          getDITypeRefOrNull(Record[5]),
                          getDITypeRefOrNull(Record[6]), Record[7], Record[8],
-                         Record[9], DWARFAddressSpace, Flags,
+                         Record[9], DWARFAddressSpace, PtrAuthData, Flags,
                          getDITypeRefOrNull(Record[11]), Annotations)),
         NextMetadataNo);
     NextMetadataNo++;
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index 13be0b0c3307fb..b21424b854b3e9 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -1804,6 +1804,11 @@ void ModuleBitcodeWriter::writeDIDerivedType(const DIDerivedType *N,
 
   Record.push_back(VE.getMetadataOrNullID(N->getAnnotations().get()));
 
+  if (auto PtrAuthData = N->getPtrAuthData())
+    Record.push_back(PtrAuthData->RawData);
+  else
+    Record.push_back(0);
+
   Stream.EmitRecord(bitc::METADATA_DERIVED_TYPE, Record, Abbrev);
   Record.clear();
 }
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
index d462859e489465..ae0226934804f7 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -803,6 +803,20 @@ void DwarfUnit::constructTypeDIE(DIE &Buffer, const DIDerivedType *DTy) {
   if (DTy->getDWARFAddressSpace())
     addUInt(Buffer, dwarf::DW_AT_address_class, dwarf::DW_FORM_data4,
             *DTy->getDWARFAddressSpace());
+  if (auto Key = DTy->getPtrAuthKey())
+    addUInt(Buffer, dwarf::DW_AT_LLVM_ptrauth_key, dwarf::DW_FORM_data1, *Key);
+  if (auto AddrDisc = DTy->isPtrAuthAddressDiscriminated())
+    if (AddrDisc.value())
+      addFlag(Buffer, dwarf::DW_AT_LLVM_ptrauth_address_discriminated);
+  if (auto Disc = DTy->getPtrAuthExtraDiscriminator())
+    addUInt(Buffer, dwarf::DW_AT_LLVM_ptrauth_extra_discriminator,
+            dwarf::DW_FORM_data2, *Disc);
+  if (auto IsaPointer = DTy->isPtrAuthIsaPointer())
+    if (*IsaPointer)
+      addFlag(Buffer, dwarf::DW_AT_LLVM_ptrauth_isa_pointer);
+  if (auto AuthenticatesNullValues = DTy->getPtrAuthAuthenticatesNullValues())
+    if (*AuthenticatesNullValues)
+      addFlag(Buffer, dwarf::DW_AT_LLVM_ptrauth_authenticates_null_values);
 }
 
 void DwarfUnit::constructSubprogramArguments(DIE &Buffer, DITypeRefArray Args) {
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index 251485a403fee6..3949aadaf564b7 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -2107,6 +2107,17 @@ static void writeDIDerivedType(raw_ostream &Out, const DIDerivedType *N,
     Printer.printInt("dwarfAddressSpace", *DWARFAddressSpace,
                      /* ShouldSkipZero */ false);
   Printer.printMetadata("annotations", N->getRawAnnotations());
+  if (auto Key = N->getPtrAuthKey())
+    Printer.printInt("ptrAuthKey", *Key);
+  if (auto AddrDisc = N->isPtrAuthAddressDiscriminated())
+    Printer.printBool("ptrAuthIsAddressDiscriminated", *AddrDisc);
+  if (auto Disc = N->getPtrAuthExtraDiscriminator())
+    Printer.printInt("ptrAuthExtraDiscriminator", *Disc);
+  if (auto IsaPointer = N->isPtrAuthIsaPointer())
+    Printer.printBool("ptrAuthIsaPointer", *IsaPointer);
+  if (auto AuthenticatesNullValues = N->getPtrAuthAuthenticatesNullValues())
+    Printer.printBool("ptrAuthAuthenticatesNullValues",
+                      *AuthenticatesNullValues);
   Out << ")";
 }
 
diff --git a/llvm/lib/IR/DIBuilder.cpp b/llvm/lib/IR/DIBuilder.cpp
index 62efaba025344b..2842cb15e78fb3 100644
--- a/llvm/lib/IR/DIBuilder.cpp
+++ b/llvm/lib/IR/DIBuilder.cpp
@@ -296,7 +296,20 @@ DIStringType *DIBuilder::createStringType(StringRef Name,
 
 DIDerivedType *DIBuilder::createQualifiedType(unsigned Tag, DIType *FromTy) {
   return DIDerivedType::get(VMContext, Tag, "", nullptr, 0, nullptr, FromTy, 0,
-                            0, 0, std::nullopt, DINode::FlagZero);
+                            0, 0, std::nullopt, std::nullopt, DINode::FlagZero);
+}
+
+DIDerivedType *DIBuilder::createPtrAuthQualifiedType(
+    DIType *FromTy, unsigned Key, bool IsAddressDiscriminated,
+    unsigned ExtraDiscriminator, bool IsaPointer,
+    bool AuthenticatesNullValues) {
+  return DIDerivedType::get(
+      VMContext, dwarf::DW_TAG_LLVM_ptrauth_type, "", nullptr, 0, nullptr,
+      FromTy, 0, 0, 0, std::nullopt,
+      std::optional<DIDerivedType::PtrAuthData>({Key, IsAddressDiscriminated,
+                                                 ExtraDiscriminator, IsaPointer,
+                                                 AuthenticatesNullValues}),
+      DINode::FlagZero);
 }
 
 DIDerivedType *
@@ -307,8 +320,8 @@ DIBuilder::createPointerType(DIType *PointeeTy, uint64_t SizeInBits,
   // FIXME: Why is there a name here?
   return DIDerivedType::get(VMContext, dwarf::DW_TAG_pointer_type, Name,
                             nullptr, 0, nullptr, PointeeTy, SizeInBits,
-                            AlignInBits, 0, DWARFAddressSpace, DINode::FlagZero,
-                            nullptr, Annotations);
+                            AlignInBits, 0, DWARFAddressSpace, std::nullopt,
+                            DINode::FlagZero, nullptr, Annotations);
 }
 
 DIDerivedType *DIBuilder::createMemberPointerType(DIType *PointeeTy,
@@ -318,7 +331,8 @@ DIDerivedType *DIBuilder::createMemberPointerType(DIType *PointeeTy,
                                                   DINode::DIFlags Flags) {
   return DIDerivedType::get(VMContext, dwarf::DW_TAG_ptr_to_member_type, "",
                             nullptr, 0, nullptr, PointeeTy, SizeInBits,
-                            AlignInBits, 0, std::nullopt, Flags, Base);
+                            AlignInBits, 0, std::nullopt, std::nullopt, Flags,
+                            Base);
 }
 
 DIDerivedType *
@@ -327,7 +341,7 @@ DIBuilder::createReferenceType(unsigned Tag, DIType *RTy, uint64_t SizeInBits,
                                std::optional<unsigned> DWARFAddressSpace) {
   assert(RTy && "Unable to create reference type");
   return DIDerivedType::get(VMContext, Tag, "", nullptr, 0, nullptr, RTy,
-                            SizeInBits, AlignInBits, 0, DWARFAddressSpace,
+                            SizeInBi...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Mar 4, 2024

@llvm/pr-subscribers-mlir-llvm

Author: Daniil Kovalev (kovdan01)

Changes

Reland #82363 after fixing build failure https://lab.llvm.org/buildbot/#/builders/5/builds/41428.

Memory sanitizer detects usage of RawData union member which is not
filled directly. Instead, the code relies on filling Data union
member, which is a struct consisting of signing schema parameters.

According to https://en.cppreference.com/w/cpp/language/union, this is UB:
"It is undefined behavior to read from the member of the union that
wasn't most recently written".

Instead of relying on compiler allowing us to do dirty things, do not
use union and only store RawData. Particular ptrauth parameters are
obtained on demand via bit operations.

Original PR description below.

Emit __ptrauth-qualified types as DIDerivedType metadata nodes in IR with tag DW_TAG_LLVM_ptrauth_type, baseType referring to the type which has the qualifier applied, and the following parameters representing the signing schema:

  • ptrAuthKey (integer)
  • ptrAuthIsAddressDiscriminated (boolean)
  • ptrAuthExtraDiscriminator (integer)
  • ptrAuthIsaPointer (boolean)
  • ptrAuthAuthenticatesNullValues (boolean)

Co-authored-by: Ahmed Bougacha <[email protected]>


Patch is 50.44 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/83862.diff

16 Files Affected:

  • (modified) llvm/include/llvm/IR/DIBuilder.h (+7)
  • (modified) llvm/include/llvm/IR/DebugInfoMetadata.h (+96-23)
  • (modified) llvm/lib/AsmParser/LLParser.cpp (+19-4)
  • (modified) llvm/lib/Bitcode/Reader/MetadataLoader.cpp (+14-4)
  • (modified) llvm/lib/Bitcode/Writer/BitcodeWriter.cpp (+5)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (+14)
  • (modified) llvm/lib/IR/AsmWriter.cpp (+11)
  • (modified) llvm/lib/IR/DIBuilder.cpp (+37-21)
  • (modified) llvm/lib/IR/DebugInfo.cpp (+3-3)
  • (modified) llvm/lib/IR/DebugInfoMetadata.cpp (+21-11)
  • (modified) llvm/lib/IR/LLVMContextImpl.h (+10-5)
  • (modified) llvm/lib/IR/Verifier.cpp (+1)
  • (modified) llvm/test/Assembler/debug-info.ll (+14-2)
  • (added) llvm/test/DebugInfo/AArch64/ptrauth.ll (+70)
  • (modified) llvm/unittests/IR/MetadataTest.cpp (+59-26)
  • (modified) mlir/lib/Target/LLVMIR/DebugTranslation.cpp (+2-1)
diff --git a/llvm/include/llvm/IR/DIBuilder.h b/llvm/include/llvm/IR/DIBuilder.h
index edec161b397155..010dcbfdadcac2 100644
--- a/llvm/include/llvm/IR/DIBuilder.h
+++ b/llvm/include/llvm/IR/DIBuilder.h
@@ -262,6 +262,13 @@ namespace llvm {
                       std::optional<unsigned> DWARFAddressSpace = std::nullopt,
                       StringRef Name = "", DINodeArray Annotations = nullptr);
 
+    /// Create a __ptrauth qualifier.
+    DIDerivedType *createPtrAuthQualifiedType(DIType *FromTy, unsigned Key,
+                                              bool IsAddressDiscriminated,
+                                              unsigned ExtraDiscriminator,
+                                              bool IsaPointer,
+                                              bool authenticatesNullValues);
+
     /// Create debugging information entry for a pointer to member.
     /// \param PointeeTy Type pointed to by this pointer.
     /// \param SizeInBits  Size.
diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index 156f6eb49253de..00c39693ed9fbf 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -745,7 +745,7 @@ class DIType : public DIScope {
 
   unsigned getLine() const { return Line; }
   uint64_t getSizeInBits() const { return SizeInBits; }
-  uint32_t getAlignInBits() const { return SubclassData32; }
+  uint32_t getAlignInBits() const;
   uint32_t getAlignInBytes() const { return getAlignInBits() / CHAR_BIT; }
   uint64_t getOffsetInBits() const { return OffsetInBits; }
   DIFlags getFlags() const { return Flags; }
@@ -972,6 +972,35 @@ class DIStringType : public DIType {
 ///
 /// TODO: Split out members (inheritance, fields, methods, etc.).
 class DIDerivedType : public DIType {
+public:
+  /// Pointer authentication (__ptrauth) metadata.
+  struct PtrAuthData {
+    unsigned RawData;
+
+    PtrAuthData(unsigned FromRawData) : RawData(FromRawData) {}
+    PtrAuthData(unsigned Key, bool IsDiscr, unsigned Discriminator,
+                bool IsaPointer, bool AuthenticatesNullValues) {
+      assert(Key < 16);
+      assert(Discriminator <= 0xffff);
+      RawData = (Key << 0) | (IsDiscr ? (1 << 4) : 0) | (Discriminator << 5) |
+                (IsaPointer ? (1 << 21) : 0) |
+                (AuthenticatesNullValues ? (1 << 22) : 0);
+    }
+    bool operator==(struct PtrAuthData Other) const {
+      return RawData == Other.RawData;
+    }
+    bool operator!=(struct PtrAuthData Other) const {
+      return !(*this == Other);
+    }
+
+    unsigned Key() { return (RawData >> 0) & 0b1111; }
+    bool IsAddressDiscriminated() { return (RawData >> 4) & 1; }
+    unsigned ExtraDiscriminator() { return (RawData >> 5) & 0xffff; }
+    bool IsaPointer() { return (RawData >> 21) & 1; }
+    bool AuthenticatesNullValues() { return (RawData >> 22) & 1; }
+  };
+
+private:
   friend class LLVMContextImpl;
   friend class MDNode;
 
@@ -982,59 +1011,70 @@ class DIDerivedType : public DIType {
   DIDerivedType(LLVMContext &C, StorageType Storage, unsigned Tag,
                 unsigned Line, uint64_t SizeInBits, uint32_t AlignInBits,
                 uint64_t OffsetInBits,
-                std::optional<unsigned> DWARFAddressSpace, DIFlags Flags,
+                std::optional<unsigned> DWARFAddressSpace,
+                std::optional<PtrAuthData> PtrAuthData, DIFlags Flags,
                 ArrayRef<Metadata *> Ops)
       : DIType(C, DIDerivedTypeKind, Storage, Tag, Line, SizeInBits,
                AlignInBits, OffsetInBits, Flags, Ops),
-        DWARFAddressSpace(DWARFAddressSpace) {}
+        DWARFAddressSpace(DWARFAddressSpace) {
+    if (PtrAuthData)
+      SubclassData32 = PtrAuthData->RawData;
+  }
   ~DIDerivedType() = default;
   static DIDerivedType *
   getImpl(LLVMContext &Context, unsigned Tag, StringRef Name, DIFile *File,
           unsigned Line, DIScope *Scope, DIType *BaseType, uint64_t SizeInBits,
           uint32_t AlignInBits, uint64_t OffsetInBits,
-          std::optional<unsigned> DWARFAddressSpace, DIFlags Flags,
+          std::optional<unsigned> DWARFAddressSpace,
+          std::optional<PtrAuthData> PtrAuthData, DIFlags Flags,
           Metadata *ExtraData, DINodeArray Annotations, StorageType Storage,
           bool ShouldCreate = true) {
     return getImpl(Context, Tag, getCanonicalMDString(Context, Name), File,
                    Line, Scope, BaseType, SizeInBits, AlignInBits, OffsetInBits,
-                   DWARFAddressSpace, Flags, ExtraData, Annotations.get(),
-                   Storage, ShouldCreate);
+                   DWARFAddressSpace, PtrAuthData, Flags, ExtraData,
+                   Annotations.get(), Storage, ShouldCreate);
   }
   static DIDerivedType *
   getImpl(LLVMContext &Context, unsigned Tag, MDString *Name, Metadata *File,
           unsigned Line, Metadata *Scope, Metadata *BaseType,
           uint64_t SizeInBits, uint32_t AlignInBits, uint64_t OffsetInBits,
-          std::optional<unsigned> DWARFAddressSpace, DIFlags Flags,
+          std::optional<unsigned> DWARFAddressSpace,
+          std::optional<PtrAuthData> PtrAuthData, DIFlags Flags,
           Metadata *ExtraData, Metadata *Annotations, StorageType Storage,
           bool ShouldCreate = true);
 
   TempDIDerivedType cloneImpl() const {
-    return getTemporary(
-        getContext(), getTag(), getName(), getFile(), getLine(), getScope(),
-        getBaseType(), getSizeInBits(), getAlignInBits(), getOffsetInBits(),
-        getDWARFAddressSpace(), getFlags(), getExtraData(), getAnnotations());
+    return getTemporary(getContext(), getTag(), getName(), getFile(), getLine(),
+                        getScope(), getBaseType(), getSizeInBits(),
+                        getAlignInBits(), getOffsetInBits(),
+                        getDWARFAddressSpace(), getPtrAuthData(), getFlags(),
+                        getExtraData(), getAnnotations());
   }
 
 public:
-  DEFINE_MDNODE_GET(
-      DIDerivedType,
-      (unsigned Tag, MDString *Name, Metadata *File, unsigned Line,
-       Metadata *Scope, Metadata *BaseType, uint64_t SizeInBits,
-       uint32_t AlignInBits, uint64_t OffsetInBits,
-       std::optional<unsigned> DWARFAddressSpace, DIFlags Flags,
-       Metadata *ExtraData = nullptr, Metadata *Annotations = nullptr),
-      (Tag, Name, File, Line, Scope, BaseType, SizeInBits, AlignInBits,
-       OffsetInBits, DWARFAddressSpace, Flags, ExtraData, Annotations))
+  DEFINE_MDNODE_GET(DIDerivedType,
+                    (unsigned Tag, MDString *Name, Metadata *File,
+                     unsigned Line, Metadata *Scope, Metadata *BaseType,
+                     uint64_t SizeInBits, uint32_t AlignInBits,
+                     uint64_t OffsetInBits,
+                     std::optional<unsigned> DWARFAddressSpace,
+                     std::optional<PtrAuthData> PtrAuthData, DIFlags Flags,
+                     Metadata *ExtraData = nullptr,
+                     Metadata *Annotations = nullptr),
+                    (Tag, Name, File, Line, Scope, BaseType, SizeInBits,
+                     AlignInBits, OffsetInBits, DWARFAddressSpace, PtrAuthData,
+                     Flags, ExtraData, Annotations))
   DEFINE_MDNODE_GET(DIDerivedType,
                     (unsigned Tag, StringRef Name, DIFile *File, unsigned Line,
                      DIScope *Scope, DIType *BaseType, uint64_t SizeInBits,
                      uint32_t AlignInBits, uint64_t OffsetInBits,
-                     std::optional<unsigned> DWARFAddressSpace, DIFlags Flags,
+                     std::optional<unsigned> DWARFAddressSpace,
+                     std::optional<PtrAuthData> PtrAuthData, DIFlags Flags,
                      Metadata *ExtraData = nullptr,
                      DINodeArray Annotations = nullptr),
                     (Tag, Name, File, Line, Scope, BaseType, SizeInBits,
-                     AlignInBits, OffsetInBits, DWARFAddressSpace, Flags,
-                     ExtraData, Annotations))
+                     AlignInBits, OffsetInBits, DWARFAddressSpace, PtrAuthData,
+                     Flags, ExtraData, Annotations))
 
   TempDIDerivedType clone() const { return cloneImpl(); }
 
@@ -1048,6 +1088,39 @@ class DIDerivedType : public DIType {
     return DWARFAddressSpace;
   }
 
+  std::optional<PtrAuthData> getPtrAuthData() const;
+
+  /// \returns The PointerAuth key.
+  std::optional<unsigned> getPtrAuthKey() const {
+    if (auto PtrAuthData = getPtrAuthData())
+      return PtrAuthData->Key();
+    return std::nullopt;
+  }
+  /// \returns The PointerAuth address discrimination bit.
+  std::optional<bool> isPtrAuthAddressDiscriminated() const {
+    if (auto PtrAuthData = getPtrAuthData())
+      return PtrAuthData->IsAddressDiscriminated();
+    return std::nullopt;
+  }
+  /// \returns The PointerAuth extra discriminator.
+  std::optional<unsigned> getPtrAuthExtraDiscriminator() const {
+    if (auto PtrAuthData = getPtrAuthData())
+      return PtrAuthData->ExtraDiscriminator();
+    return std::nullopt;
+  }
+  /// \returns The PointerAuth IsaPointer bit.
+  std::optional<bool> isPtrAuthIsaPointer() const {
+    if (auto PtrAuthData = getPtrAuthData())
+      return PtrAuthData->IsaPointer();
+    return std::nullopt;
+  }
+  /// \returns The PointerAuth authenticates null values bit.
+  std::optional<bool> getPtrAuthAuthenticatesNullValues() const {
+    if (auto PtrAuthData = getPtrAuthData())
+      return PtrAuthData->AuthenticatesNullValues();
+    return std::nullopt;
+  }
+
   /// Get extra data associated with this derived type.
   ///
   /// Class type for pointer-to-members, objective-c property node for ivars,
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index a91e2f690999e0..e91abaf0780a3d 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -5130,7 +5130,11 @@ bool LLParser::parseDIStringType(MDNode *&Result, bool IsDistinct) {
 ///   ::= !DIDerivedType(tag: DW_TAG_pointer_type, name: "int", file: !0,
 ///                      line: 7, scope: !1, baseType: !2, size: 32,
 ///                      align: 32, offset: 0, flags: 0, extraData: !3,
-///                      dwarfAddressSpace: 3)
+///                      dwarfAddressSpace: 3, ptrAuthKey: 1,
+///                      ptrAuthIsAddressDiscriminated: true,
+///                      ptrAuthExtraDiscriminator: 0x1234,
+///                      ptrAuthIsaPointer: 1, ptrAuthAuthenticatesNullValues:1
+///                      )
 bool LLParser::parseDIDerivedType(MDNode *&Result, bool IsDistinct) {
 #define VISIT_MD_FIELDS(OPTIONAL, REQUIRED)                                    \
   REQUIRED(tag, DwarfTagField, );                                              \
@@ -5145,19 +5149,30 @@ bool LLParser::parseDIDerivedType(MDNode *&Result, bool IsDistinct) {
   OPTIONAL(flags, DIFlagField, );                                              \
   OPTIONAL(extraData, MDField, );                                              \
   OPTIONAL(dwarfAddressSpace, MDUnsignedField, (UINT32_MAX, UINT32_MAX));      \
-  OPTIONAL(annotations, MDField, );
+  OPTIONAL(annotations, MDField, );                                            \
+  OPTIONAL(ptrAuthKey, MDUnsignedField, (0, 7));                               \
+  OPTIONAL(ptrAuthIsAddressDiscriminated, MDBoolField, );                      \
+  OPTIONAL(ptrAuthExtraDiscriminator, MDUnsignedField, (0, 0xffff));           \
+  OPTIONAL(ptrAuthIsaPointer, MDBoolField, );                                  \
+  OPTIONAL(ptrAuthAuthenticatesNullValues, MDBoolField, );
   PARSE_MD_FIELDS();
 #undef VISIT_MD_FIELDS
 
   std::optional<unsigned> DWARFAddressSpace;
   if (dwarfAddressSpace.Val != UINT32_MAX)
     DWARFAddressSpace = dwarfAddressSpace.Val;
+  std::optional<DIDerivedType::PtrAuthData> PtrAuthData;
+  if (ptrAuthKey.Val)
+    PtrAuthData = DIDerivedType::PtrAuthData(
+        (unsigned)ptrAuthKey.Val, ptrAuthIsAddressDiscriminated.Val,
+        (unsigned)ptrAuthExtraDiscriminator.Val, ptrAuthIsaPointer.Val,
+        ptrAuthAuthenticatesNullValues.Val);
 
   Result = GET_OR_DISTINCT(DIDerivedType,
                            (Context, tag.Val, name.Val, file.Val, line.Val,
                             scope.Val, baseType.Val, size.Val, align.Val,
-                            offset.Val, DWARFAddressSpace, flags.Val,
-                            extraData.Val, annotations.Val));
+                            offset.Val, DWARFAddressSpace, PtrAuthData,
+                            flags.Val, extraData.Val, annotations.Val));
   return false;
 }
 
diff --git a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
index 770eb83af17f9b..bdc2db82dfbe0e 100644
--- a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
+++ b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
@@ -1556,7 +1556,7 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
     break;
   }
   case bitc::METADATA_DERIVED_TYPE: {
-    if (Record.size() < 12 || Record.size() > 14)
+    if (Record.size() < 12 || Record.size() > 15)
       return error("Invalid record");
 
     // DWARF address space is encoded as N->getDWARFAddressSpace() + 1. 0 means
@@ -1566,8 +1566,18 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
       DWARFAddressSpace = Record[12] - 1;
 
     Metadata *Annotations = nullptr;
-    if (Record.size() > 13 && Record[13])
-      Annotations = getMDOrNull(Record[13]);
+    std::optional<DIDerivedType::PtrAuthData> PtrAuthData;
+
+    // Only look for annotations/ptrauth if both are allocated.
+    // If not, we can't tell which was intended to be embedded, as both ptrauth
+    // and annotations have been expected at Record[13] at various times.
+    if (Record.size() > 14) {
+      if (Record[13])
+        Annotations = getMDOrNull(Record[13]);
+
+      if (Record[14])
+        PtrAuthData = DIDerivedType::PtrAuthData(Record[14]);
+    }
 
     IsDistinct = Record[0];
     DINode::DIFlags Flags = static_cast<DINode::DIFlags>(Record[10]);
@@ -1577,7 +1587,7 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
                          getMDOrNull(Record[3]), Record[4],
                          getDITypeRefOrNull(Record[5]),
                          getDITypeRefOrNull(Record[6]), Record[7], Record[8],
-                         Record[9], DWARFAddressSpace, Flags,
+                         Record[9], DWARFAddressSpace, PtrAuthData, Flags,
                          getDITypeRefOrNull(Record[11]), Annotations)),
         NextMetadataNo);
     NextMetadataNo++;
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index 13be0b0c3307fb..b21424b854b3e9 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -1804,6 +1804,11 @@ void ModuleBitcodeWriter::writeDIDerivedType(const DIDerivedType *N,
 
   Record.push_back(VE.getMetadataOrNullID(N->getAnnotations().get()));
 
+  if (auto PtrAuthData = N->getPtrAuthData())
+    Record.push_back(PtrAuthData->RawData);
+  else
+    Record.push_back(0);
+
   Stream.EmitRecord(bitc::METADATA_DERIVED_TYPE, Record, Abbrev);
   Record.clear();
 }
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
index d462859e489465..ae0226934804f7 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -803,6 +803,20 @@ void DwarfUnit::constructTypeDIE(DIE &Buffer, const DIDerivedType *DTy) {
   if (DTy->getDWARFAddressSpace())
     addUInt(Buffer, dwarf::DW_AT_address_class, dwarf::DW_FORM_data4,
             *DTy->getDWARFAddressSpace());
+  if (auto Key = DTy->getPtrAuthKey())
+    addUInt(Buffer, dwarf::DW_AT_LLVM_ptrauth_key, dwarf::DW_FORM_data1, *Key);
+  if (auto AddrDisc = DTy->isPtrAuthAddressDiscriminated())
+    if (AddrDisc.value())
+      addFlag(Buffer, dwarf::DW_AT_LLVM_ptrauth_address_discriminated);
+  if (auto Disc = DTy->getPtrAuthExtraDiscriminator())
+    addUInt(Buffer, dwarf::DW_AT_LLVM_ptrauth_extra_discriminator,
+            dwarf::DW_FORM_data2, *Disc);
+  if (auto IsaPointer = DTy->isPtrAuthIsaPointer())
+    if (*IsaPointer)
+      addFlag(Buffer, dwarf::DW_AT_LLVM_ptrauth_isa_pointer);
+  if (auto AuthenticatesNullValues = DTy->getPtrAuthAuthenticatesNullValues())
+    if (*AuthenticatesNullValues)
+      addFlag(Buffer, dwarf::DW_AT_LLVM_ptrauth_authenticates_null_values);
 }
 
 void DwarfUnit::constructSubprogramArguments(DIE &Buffer, DITypeRefArray Args) {
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index 251485a403fee6..3949aadaf564b7 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -2107,6 +2107,17 @@ static void writeDIDerivedType(raw_ostream &Out, const DIDerivedType *N,
     Printer.printInt("dwarfAddressSpace", *DWARFAddressSpace,
                      /* ShouldSkipZero */ false);
   Printer.printMetadata("annotations", N->getRawAnnotations());
+  if (auto Key = N->getPtrAuthKey())
+    Printer.printInt("ptrAuthKey", *Key);
+  if (auto AddrDisc = N->isPtrAuthAddressDiscriminated())
+    Printer.printBool("ptrAuthIsAddressDiscriminated", *AddrDisc);
+  if (auto Disc = N->getPtrAuthExtraDiscriminator())
+    Printer.printInt("ptrAuthExtraDiscriminator", *Disc);
+  if (auto IsaPointer = N->isPtrAuthIsaPointer())
+    Printer.printBool("ptrAuthIsaPointer", *IsaPointer);
+  if (auto AuthenticatesNullValues = N->getPtrAuthAuthenticatesNullValues())
+    Printer.printBool("ptrAuthAuthenticatesNullValues",
+                      *AuthenticatesNullValues);
   Out << ")";
 }
 
diff --git a/llvm/lib/IR/DIBuilder.cpp b/llvm/lib/IR/DIBuilder.cpp
index 62efaba025344b..2842cb15e78fb3 100644
--- a/llvm/lib/IR/DIBuilder.cpp
+++ b/llvm/lib/IR/DIBuilder.cpp
@@ -296,7 +296,20 @@ DIStringType *DIBuilder::createStringType(StringRef Name,
 
 DIDerivedType *DIBuilder::createQualifiedType(unsigned Tag, DIType *FromTy) {
   return DIDerivedType::get(VMContext, Tag, "", nullptr, 0, nullptr, FromTy, 0,
-                            0, 0, std::nullopt, DINode::FlagZero);
+                            0, 0, std::nullopt, std::nullopt, DINode::FlagZero);
+}
+
+DIDerivedType *DIBuilder::createPtrAuthQualifiedType(
+    DIType *FromTy, unsigned Key, bool IsAddressDiscriminated,
+    unsigned ExtraDiscriminator, bool IsaPointer,
+    bool AuthenticatesNullValues) {
+  return DIDerivedType::get(
+      VMContext, dwarf::DW_TAG_LLVM_ptrauth_type, "", nullptr, 0, nullptr,
+      FromTy, 0, 0, 0, std::nullopt,
+      std::optional<DIDerivedType::PtrAuthData>({Key, IsAddressDiscriminated,
+                                                 ExtraDiscriminator, IsaPointer,
+                                                 AuthenticatesNullValues}),
+      DINode::FlagZero);
 }
 
 DIDerivedType *
@@ -307,8 +320,8 @@ DIBuilder::createPointerType(DIType *PointeeTy, uint64_t SizeInBits,
   // FIXME: Why is there a name here?
   return DIDerivedType::get(VMContext, dwarf::DW_TAG_pointer_type, Name,
                             nullptr, 0, nullptr, PointeeTy, SizeInBits,
-                            AlignInBits, 0, DWARFAddressSpace, DINode::FlagZero,
-                            nullptr, Annotations);
+                            AlignInBits, 0, DWARFAddressSpace, std::nullopt,
+                            DINode::FlagZero, nullptr, Annotations);
 }
 
 DIDerivedType *DIBuilder::createMemberPointerType(DIType *PointeeTy,
@@ -318,7 +331,8 @@ DIDerivedType *DIBuilder::createMemberPointerType(DIType *PointeeTy,
                                                   DINode::DIFlags Flags) {
   return DIDerivedType::get(VMContext, dwarf::DW_TAG_ptr_to_member_type, "",
                             nullptr, 0, nullptr, PointeeTy, SizeInBits,
-                            AlignInBits, 0, std::nullopt, Flags, Base);
+                            AlignInBits, 0, std::nullopt, std::nullopt, Flags,
+                            Base);
 }
 
 DIDerivedType *
@@ -327,7 +341,7 @@ DIBuilder::createReferenceType(unsigned Tag, DIType *RTy, uint64_t SizeInBits,
                                std::optional<unsigned> DWARFAddressSpace) {
   assert(RTy && "Unable to create reference type");
   return DIDerivedType::get(VMContext, Tag, "", nullptr, 0, nullptr, RTy,
-                            SizeInBits, AlignInBits, 0, DWARFAddressSpace,
+                            SizeInBi...
[truncated]

Payload.Data.ExtraDiscriminator = Discriminator;
Payload.Data.IsaPointer = IsaPointer;
Payload.Data.AuthenticatesNullValues = AuthenticatesNullValues;
RawData = (Key << 0) | (IsDiscr ? (1 << 4) : 0) | (Discriminator << 5) |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add comment describing the layout of RawData.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks, see f765247

Comment on lines 989 to 995
bool operator==(struct PtrAuthData Other) const {
return RawData == Other.RawData;
}
bool operator!=(struct PtrAuthData Other) const {
return !(*this == Other);
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer non-member operator overloads (they can be friends and defined inline if that's useful/more convenient than defining them outside the class definition) to allow equal conversions on LHS and RHS.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks, see f765247

(IsaPointer ? (1 << 21) : 0) |
(AuthenticatesNullValues ? (1 << 22) : 0);
}
bool operator==(struct PtrAuthData Other) const {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop the struct qualifier as it's not needed (well, it's needed if the name is overloaded to be a struct and something else, but probably best not to do that) in C++.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks, see f765247

std::optional<PtrAuthData> getPtrAuthData() const;

/// \returns The PointerAuth key.
std::optional<unsigned> getPtrAuthKey() const {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these helpers particularly useful? It seems like they change the usage code from this:

if (auto PtrAuthData = getPtrAuthData())
  do_something_with(PtrAuthData->Key());

to

if (auto Key = getPtrAuthKey())
  do_something_with(*Key);

Which doesn't seem like a huge win?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are just small shortcuts which do not give any value except what you described above. I personally have no strong preference, so accepting your suggestion - deleted the helpers. See f765247

@kovdan01 kovdan01 requested review from dwblaikie and asl March 6, 2024 00:10
@kovdan01
Copy link
Contributor Author

kovdan01 commented Mar 7, 2024

Update to current main branch to trigger buildkite (previous fail seemed unrelated to the changes)

@kovdan01
Copy link
Contributor Author

kovdan01 commented Mar 7, 2024

Would be glad to see @JDevlieghere's and anyone else's interested feedback on the changes

}

unsigned Key() { return (RawData >> 0) & 0b1111; }
bool IsAddressDiscriminated() { return (RawData >> 4) & 1; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: functionName. The code base is unfortunately inconsistent, but newer functions mostly stick with the recommended convention.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks, see 1adcade

PARSE_MD_FIELDS();
#undef VISIT_MD_FIELDS

std::optional<unsigned> DWARFAddressSpace;
if (dwarfAddressSpace.Val != UINT32_MAX)
DWARFAddressSpace = dwarfAddressSpace.Val;
std::optional<DIDerivedType::PtrAuthData> PtrAuthData;
if (ptrAuthKey.Val)
PtrAuthData = DIDerivedType::PtrAuthData(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

@kovdan01 kovdan01 Mar 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks, see 1adcade. Also fixed in other places & used in-place construction where applicable (see (6) here https://en.cppreference.com/w/cpp/utility/optional/optional)


target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"

@p = common global i8* null, align 8, !dbg !0
Copy link
Member

@MaskRay MaskRay Mar 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i8* indicates a typed pointer, from which LLVM is migrating away. Use ptr.
Consider a non-common linkage. COMMON linkage is also obsoleted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks, see 1adcade. Regarding linkage - any type is applicable for the test, so just removed that, and by default that should be external.

@kovdan01 kovdan01 requested a review from MaskRay March 8, 2024 11:22
@kovdan01
Copy link
Contributor Author

Would be glad to see @JDevlieghere's and anyone else's interested feedback on the changes.

@MaskRay I've addressed your comments in 1adcade - would be glad if you let me know if the fixes are OK.

Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would've probably done the inverse and shifted the RawData in the constructor, stored them as individual fields in a struct and have the getters operate on that. And of course have a new vector to return the raw data. But it looks like we rely on RawData more than on the getters, so this might be slightly more efficient. LGTM!

// If not, we can't tell which was intended to be embedded, as both ptrauth
// and annotations have been expected at Record[13] at various times.
if (Record.size() > 14) {
if (Record[13])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if is redundant?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleting this does not break any tests. However, even w/o this patch, when we had the code below, deleting check against Record[13] did not break any tests.

if (Record.size() > 13 && Record[13])
  Annotations = getMDOrNull(Record[13]);

So, there are either no test cases which check that or the if is redundant and probably already was redundant before. Since there are no other objections and the PR already got several approvals, I'll merge the PR and investigate this particular question later.

@MaskRay
Copy link
Member

MaskRay commented Mar 15, 2024

Would be glad to see @JDevlieghere's and anyone else's interested feedback on the changes.

@MaskRay I've addressed your comments in 1adcade - would be glad if you let me know if the fixes are OK.

Thanks. I'm trying to study this patch as I am not so familiar with debug info IR.

If you don't mind waiting a bit, waiting for @dwblaikie (another debug information code owner) could be useful.

@MaskRay
Copy link
Member

MaskRay commented Mar 16, 2024

I was confused as I haven't attended any LLVM Pointer Authentication sync-ups and I am definitely lacking context. It seems that there is an upstreaming proposal back in 2019 https://discourse.llvm.org/t/rfc-pointer-authentication-for-arm64e/53433 and apple/llvm-project contains these DW_TAG_LLVM_ptrauth_type changes. This PR is part of the upstream efforts. Yeah, looks good to me.

Normally, when reviewers see createPtrAuthQualifiedType with no in-tree user, they'd ask that you prepare links for the immediate user. rg createPtrAuthQualifiedType in the Apple repo reveals that clang/lib/CodeGen/CGDebugInfo.cpp will be the user in a subsequent patch I think.

@JDevlieghere
Copy link
Member

@dwblaikie Did you want to have a look at this before we merge this? This PR is going to cause quite a bit of conflicts downstream so I would be nice to merge this somewhat early in the week.

@dwblaikie
Copy link
Collaborator

@dwblaikie Did you want to have a look at this before we merge this? This PR is going to cause quite a bit of conflicts downstream so I would be nice to merge this somewhat early in the week.

glances What sort of conflicts/breakages are you expecting? I guess API breaks on DIBuilder for custom frontends? Other things to look out for?

@JDevlieghere
Copy link
Member

What sort of conflicts/breakages are you expecting? I guess API breaks on DIBuilder for custom frontends? Other things to look out for?

Just regular merge conflicts when we merge this into the Swift fork. This PR is upstreaming ptrauth support and started off based on the downstream code, but after a few iterations of review it looks a bit different. That's totally fine, as the result is better for it, but git will need a hand with the merge. The only reason I'm bringing it up is because last time this PR landed on a Friday afternoon (before it got revert) which left us little to deal with all this.

@dwblaikie
Copy link
Collaborator

What sort of conflicts/breakages are you expecting? I guess API breaks on DIBuilder for custom frontends? Other things to look out for?

Just regular merge conflicts when we merge this into the Swift fork. This PR is upstreaming ptrauth support and started off based on the downstream code, but after a few iterations of review it looks a bit different. That's totally fine, as the result is better for it, but git will need a hand with the merge. The only reason I'm bringing it up is because last time this PR landed on a Friday afternoon (before it got revert) which left us little to deal with all this.

Ah, your own downstream, fair enough. It looks right enough to me, FWIW :)

@kovdan01 kovdan01 merged commit 924a1dc into llvm:main Mar 19, 2024
chencha3 pushed a commit to chencha3/llvm-project that referenced this pull request Mar 23, 2024
Reland llvm#82363 after fixing build failure
https://lab.llvm.org/buildbot/#/builders/5/builds/41428.

Memory sanitizer detects usage of `RawData` union member which is not
filled directly. Instead, the code relies on filling `Data` union
member, which is a struct consisting of signing schema parameters.

According to https://en.cppreference.com/w/cpp/language/union, this is
UB:
"It is undefined behavior to read from the member of the union that
wasn't most recently written".

Instead of relying on compiler allowing us to do dirty things, do not
use union and only store `RawData`. Particular ptrauth parameters are
obtained on demand via bit operations.

Original PR description below.

Emit `__ptrauth`-qualified types as `DIDerivedType` metadata nodes in IR
with tag `DW_TAG_LLVM_ptrauth_type`, baseType referring to the type
which has the qualifier applied, and the following parameters
representing the signing schema:

- `ptrAuthKey` (integer)
- `ptrAuthIsAddressDiscriminated` (boolean)
- `ptrAuthExtraDiscriminator` (integer)
- `ptrAuthIsaPointer` (boolean)
- `ptrAuthAuthenticatesNullValues` (boolean)

Co-authored-by: Ahmed Bougacha <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants