Skip to content
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

[DebugInfo][BPF] Add 'btf:type_tag' annotation in DWARF #91423

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

eddyz87
Copy link
Contributor

@eddyz87 eddyz87 commented May 8, 2024

This commit is a follow-up for BPF mailing list discussion at [1]. It changes the way __attribute__((btf_type_tag("...")))s are represented in DWARF.

Prior to this commit type tags could only be attached to pointers. Such attachments associated the tags with a pointee type. E.g. for the following C code:

int __attribute__((btf_type_tag("tag1"))) *g;

Generated DWARF looked as follows:

0x0000001e:   DW_TAG_variable
                DW_AT_name      ("g")
                DW_AT_type      (0x00000029 "int *")

0x00000029:   DW_TAG_pointer_type
                DW_AT_type      (0x00000032 "int")

0x0000002e:     DW_TAG_LLVM_annotation
                  DW_AT_name    ("btf_type_tag")
                  DW_AT_const_value     ("tag1")

0x00000032:   DW_TAG_base_type
                DW_AT_name      ("int")

The goal of this commit is to allow attachment of type tags to the tagged types instead. E.g. for the same example DWARF should look as follows:

0x0000001e:   DW_TAG_variable
                DW_AT_name      ("g")
                DW_AT_type      (0x00000029 "int *")

0x00000029:   DW_TAG_pointer_type
                DW_AT_type      (0x00000032 "int")

0x00000032:   DW_TAG_base_type
                DW_AT_name      ("int")

0x00000036:     DW_TAG_LLVM_annotation
                  DW_AT_name    ("btf:type_tag")
                  DW_AT_const_value     ("tag1")

A new tag name, btf:type_tag, is used so that DWARF consumers could distinguish between old and new attachment semantics.

This feature is mostly used by Linux Kernel in combination with tool named pahole [2]. Reasonably recent versions of pahole generate errors (1.23, 1.24) or warnings (1.25) when DW_TAG_LLVM_annotation is attached to DW_TAG_base_type or DW_TAG_unspecified_type. Hence the btf:type_tag generation is controlled by a hidden option -mllvm -btf-type-tag-v2. The goal is to provide a way for tooling to work on adding support btf:type_tag and eventually replace
btf_type_tag by btf:type_tag, removing the above option.

The commit includes the following changes:

  • Changes in debug info generation:

    • New method DIBuilder::createAnnotationsPlaceholder() is added, it creates a temporary DIDerivedType that plays as annotations placeholder while debug info metadata is being constructed;

    • New overload for CGDebugInfo::CreateType method is added:

      llvm::DIType *CGDebugInfo::CreateType(const BTFTagAttributedType *Ty,
      llvm::DIFile *Unit);

      This overload collects BTF type tags in Ty, creates annotations placeholder pointing to the base type of Ty, registers the placeholder in the CGDebugInfo::AnnotationsPlaceholder vector.

    • CGDebugInfo::finalize() is updated to do the following for each annotation placeholder:

      • clone underlying base type;
      • attach annotations the clone using replaceAnnotations() call;
      • replace all placeholder usages by a clone.
        Such scheme allows to deal with type cycles.
  • Changes in AST construction:

    • ASTContext::getBTFTagAttributedType() is updated to ensure that BTFTagAttributedType always wraps QualType w/o local constant/volatile/restricted qualifiers. This simplifies debug info generation.

[1] https://lore.kernel.org/bpf/87r0w9jjoq.fsf@oracle.com/
[2] https://git.kernel.org/pub/scm/devel/pahole/pahole.git/

This was previously tracked as differential revision:
https://reviews.llvm.org/D143967

Note: the new place for annotation is DWARF was already agreed as a part of D143967, however the switch enabling or disabling this is a new addition.

Copy link

github-actions bot commented May 8, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@eddyz87
Copy link
Contributor Author

eddyz87 commented May 8, 2024

Hi @dwblaikie, @yonghong-song,

Could you please take a look at this pull request?
Rationale is here: #91422 (comment)
The CI failure seems to be a fluke as it is reported for DataFlowSanitizer-x86_64 :: release_shadow_space.c which is completely unrelated (also CI for 91424 passed).

@dafaust, @jemarch, @4ast, fyi.

@eddyz87 eddyz87 marked this pull request as ready for review May 8, 2024 18:07
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen debuginfo llvm:ir labels May 8, 2024
@llvmbot
Copy link

llvmbot commented May 8, 2024

@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-debuginfo
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-llvm-ir

Author: None (eddyz87)

Changes

This commit is a follow-up for BPF mailing list discussion at [1]. It changes the way __attribute__((btf_type_tag("...")))s are represented in DWARF.

Prior to this commit type tags could only be attached to pointers. Such attachments associated the tags with a pointee type. E.g. for the following C code:

int __attribute__((btf_type_tag("tag1"))) *g;

Generated DWARF looked as follows:

0x0000001e:   DW_TAG_variable
                DW_AT_name      ("g")
                DW_AT_type      (0x00000029 "int *")

0x00000029:   DW_TAG_pointer_type
                DW_AT_type      (0x00000032 "int")

0x0000002e:     DW_TAG_LLVM_annotation
                  DW_AT_name    ("btf_type_tag")
                  DW_AT_const_value     ("tag1")

0x00000032:   DW_TAG_base_type
                DW_AT_name      ("int")

The goal of this commit is to allow attachment of type tags to the tagged types instead. E.g. for the same example DWARF should look as follows:

0x0000001e:   DW_TAG_variable
                DW_AT_name      ("g")
                DW_AT_type      (0x00000029 "int *")

0x00000029:   DW_TAG_pointer_type
                DW_AT_type      (0x00000032 "int")

0x00000032:   DW_TAG_base_type
                DW_AT_name      ("int")

0x00000036:     DW_TAG_LLVM_annotation
                  DW_AT_name    ("btf:type_tag")
                  DW_AT_const_value     ("tag1")

A new tag name, btf:type_tag, is used so that DWARF consumers could distinguish between old and new attachment semantics.

This feature is mostly used by Linux Kernel in combination with tool named pahole [2]. Reasonably recent versions of pahole generate errors (1.23, 1.24) or warnings (1.25) when DW_TAG_LLVM_annotation is attached to DW_TAG_base_type or DW_TAG_unspecified_type. Hence the btf:type_tag generation is controlled by a hidden option -mllvm -btf-type-tag-v2. The goal is to provide a way for tooling to work on adding support btf:type_tag and eventually replace
btf_type_tag by btf:type_tag, removing the above option.

The commit includes the following changes:

  • Changes in debug info generation:

    • New method DIBuilder::createAnnotationsPlaceholder() is added, it creates a temporary DIDerivedType that plays as annotations placeholder while debug info metadata is being constructed;

    • New overload for CGDebugInfo::CreateType method is added:

      llvm::DIType *CGDebugInfo::CreateType(const BTFTagAttributedType *Ty,
      llvm::DIFile *Unit);

      This overload collects BTF type tags in Ty, creates annotations placeholder pointing to the base type of Ty, registers the placeholder in the CGDebugInfo::AnnotationsPlaceholder vector.

    • CGDebugInfo::finalize() is updated to do the following for each annotation placeholder:

      • clone underlying base type;
      • attach annotations the clone using replaceAnnotations() call;
      • replace all placeholder usages by a clone.
        Such scheme allows to deal with type cycles.
  • Changes in AST construction:

    • ASTContext::getBTFTagAttributedType() is updated to ensure that BTFTagAttributedType always wraps QualType w/o local constant/volatile/restricted qualifiers. This simplifies debug info generation.

[1] https://lore.kernel.org/bpf/87r0w9jjoq.fsf@oracle.com/
[2] https://git.kernel.org/pub/scm/devel/pahole/pahole.git/

This was previously tracked as differential revision:
https://reviews.llvm.org/D143967

Note: the new place for annotation is DWARF was already agreed as a part of D143967, however the switch enabling or disabling this is a new addition.


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

25 Files Affected:

  • (modified) clang/lib/CodeGen/CGDebugInfo.cpp (+180-24)
  • (modified) clang/lib/CodeGen/CGDebugInfo.h (+3)
  • (added) clang/test/CodeGen/attr-btf_type_tag-circular.c (+18)
  • (added) clang/test/CodeGen/attr-btf_type_tag-const.c (+41)
  • (modified) clang/test/CodeGen/attr-btf_type_tag-func-ptr.c (+14-5)
  • (modified) clang/test/CodeGen/attr-btf_type_tag-func.c (+37-13)
  • (added) clang/test/CodeGen/attr-btf_type_tag-restrict.c (+21)
  • (modified) clang/test/CodeGen/attr-btf_type_tag-similar-type.c (+34-13)
  • (modified) clang/test/CodeGen/attr-btf_type_tag-typedef-field.c (+46-20)
  • (modified) clang/test/CodeGen/attr-btf_type_tag-var.c (+55-22)
  • (added) clang/test/CodeGen/attr-btf_type_tag-void.c (+12)
  • (added) clang/test/CodeGen/attr-btf_type_tag-volatile.c (+18)
  • (modified) llvm/include/llvm/IR/DIBuilder.h (+3)
  • (modified) llvm/include/llvm/IR/DebugInfoMetadata.h (+58-22)
  • (modified) llvm/lib/AsmParser/LLParser.cpp (+10-7)
  • (modified) llvm/lib/Bitcode/Reader/MetadataLoader.cpp (+12-4)
  • (modified) llvm/lib/Bitcode/Writer/BitcodeWriter.cpp (+2)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (+5-1)
  • (modified) llvm/lib/IR/AsmWriter.cpp (+4-2)
  • (modified) llvm/lib/IR/DIBuilder.cpp (+11)
  • (modified) llvm/lib/IR/DebugInfoMetadata.cpp (+8-7)
  • (modified) llvm/lib/IR/LLVMContextImpl.h (+20-10)
  • (added) llvm/test/Bitcode/attr-btf_tag-dibasic.ll (+36)
  • (added) llvm/test/Bitcode/attr-btf_tag-disubroutine.ll (+41)
  • (modified) llvm/test/DebugInfo/attr-btf_type_tag.ll (+99-37)
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index fac278f0e20a4..9518a75a3b1f1 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -56,6 +56,16 @@
 using namespace clang;
 using namespace clang::CodeGen;
 
+// Temporarily hide new format for btf_type_tags / DW_TAG_LLVM_annotation
+// behind an option to allow transitory period for tooling dependent on
+// this annotation. The goal is to remove this flag after transitory period.
+static llvm::cl::opt<bool> BTFTypeTagV2(
+    "btf-type-tag-v2", llvm::cl::Hidden,
+    llvm::cl::desc("For __attribute__((btf_type_tag(...))) generate "
+                   "DW_TAG_LLVM_annotation tags with DW_AT_name 'btf:type_tag' "
+                   "attached to annotated type itself"),
+    llvm::cl::init(false));
+
 static uint32_t getTypeAlignIfRequired(const Type *Ty, const ASTContext &Ctx) {
   auto TI = Ctx.getTypeInfo(Ty);
   return TI.isAlignRequired() ? TI.Align : 0;
@@ -1185,6 +1195,129 @@ CGDebugInfo::getOrCreateRecordFwdDecl(const RecordType *Ty,
   return RetTy;
 }
 
+static QualType collectBTFTypeTagAnnotations(
+    llvm::LLVMContext &Context, llvm::DIBuilder &DBuilder,
+    llvm::SmallVectorImpl<llvm::Metadata *> &Annots,
+    const BTFTagAttributedType *BTFAttrTy, const char *TagName) {
+  QualType WrappedTy;
+
+  do {
+    StringRef TagValue = BTFAttrTy->getAttr()->getBTFTypeTag();
+    if (!TagValue.empty()) {
+      llvm::Metadata *Ops[] = {
+          llvm::MDString::get(Context, TagName),
+          llvm::MDString::get(Context, TagValue),
+      };
+      Annots.insert(Annots.begin(), llvm::MDNode::get(Context, Ops));
+    }
+    WrappedTy = BTFAttrTy->getWrappedType();
+    BTFAttrTy = dyn_cast<BTFTagAttributedType>(WrappedTy);
+  } while (BTFAttrTy);
+
+  return WrappedTy;
+}
+
+static bool retreiveCVR(llvm::DIDerivedType *DTy, QualifierCollector &Qc) {
+  switch (DTy->getTag()) {
+  case llvm::dwarf::DW_TAG_const_type:
+    Qc.addConst();
+    return true;
+  case llvm::dwarf::DW_TAG_volatile_type:
+    Qc.addVolatile();
+    return true;
+  case llvm::dwarf::DW_TAG_restrict_type:
+    Qc.addRestrict();
+    return true;
+  default:
+    return false;
+  }
+}
+
+// Tags returned by QualifierCollector::getNextQualifier() should be
+// applied in the reverse order, thus use recursive function.
+static llvm::DIType *applyQualifiers(llvm::DIBuilder &DBuilder,
+                                     llvm::DIType *Ty, QualifierCollector &Qc) {
+  llvm::dwarf::Tag Tag = getNextQualifier(Qc);
+  if (!Tag)
+    return Ty;
+  Ty = applyQualifiers(DBuilder, Ty, Qc);
+  return DBuilder.createQualifiedType(Tag, Ty);
+}
+
+static bool isAnnotationsPlaceholder(llvm::DIDerivedType *DTy) {
+  return DTy->isTemporary() &&
+         DTy->getTag() == llvm::dwarf::DW_TAG_LLVM_annotation;
+}
+
+llvm::DIType *CGDebugInfo::CreateType(const BTFTagAttributedType *Ty,
+                                      llvm::DIFile *Unit) {
+  SmallVector<llvm::Metadata *, 4> Annotations;
+  auto WrappedTy = collectBTFTypeTagAnnotations(
+      CGM.getLLVMContext(), DBuilder, Annotations, Ty, "btf:type_tag");
+
+  if (!BTFTypeTagV2 || Annotations.empty())
+    return getOrCreateType(WrappedTy, Unit);
+
+  // After discussion with GCC BPF team in [1] it was decided to avoid
+  // attaching BTF type tags to const/volatile/restrict DWARF DIEs.
+  // So, strip qualifiers from WrappedTy and apply those to a final
+  // annotations placeholder instance at the end of this function.
+  //
+  // [1] https://reviews.llvm.org/D143967
+  QualifierCollector Qc;
+  Qc.addCVRQualifiers(WrappedTy.getLocalCVRQualifiers());
+  WrappedTy.removeLocalFastQualifiers(Qualifiers::CVRMask);
+
+  llvm::DIType *WrappedDI = getOrCreateType(WrappedTy, Unit);
+  if (!WrappedDI)
+    WrappedDI = DBuilder.createUnspecifiedType("void");
+
+  // Stripping local CVR qualifiers might not be enough in cases like this:
+  //
+  //   #define __tag __attribute__((btf_type_tag("tag")))
+  //   const int *foo;
+  //   const int *bar(void) {
+  //     return (typeof(*foo) __tag *)(0);
+  //   }
+  //
+  // Here the AST looks like:
+  //
+  //   BTFTagAttributedType
+  //   |  'typeof (*foo) __attribute__((btf_type_tag("tag")))' sugar
+  //   `-TypeOfExprType 'typeof (*foo)' sugar
+  //     |-ParenExpr 'const int' lvalue
+  //     | `- ...
+  //     `-QualType 'const int' const
+  //       `-BuiltinType 'int'
+  //
+  // The BTFTagAttributedType is applied to TypeOfExpr.
+  // For TypeOfExpr the getOrCreateType(), would return instance of
+  // DIDerivedType with tag DW_TAG_const_type.
+  //
+  // To avoid repeating UnwrapTypeForDebugInfo() logic here just
+  // rebuild CVR metadata nodes if necessary.
+  // The above local CVR qualifiers processing is redundant,
+  // but avoids rebuilding metadata nodes in the most common case.
+  while (auto *DTy = dyn_cast<llvm::DIDerivedType>(WrappedDI)) {
+    if (!retreiveCVR(DTy, Qc))
+      break;
+    WrappedDI = DTy->getBaseType();
+  }
+
+  if (auto *DTy = dyn_cast<llvm::DIDerivedType>(WrappedDI))
+    if (isAnnotationsPlaceholder(DTy)) {
+      WrappedDI = DTy->getBaseType();
+      for (llvm::Metadata *O : DTy->getAnnotations()->operands())
+        Annotations.push_back(O);
+    }
+
+  auto *Placeholder = DBuilder.createAnnotationsPlaceholder(
+      WrappedDI, DBuilder.getOrCreateArray(Annotations));
+  AnnotationPlaceholders.push_back(Placeholder);
+
+  return applyQualifiers(DBuilder, Placeholder, Qc);
+}
+
 llvm::DIType *CGDebugInfo::CreatePointerLikeType(llvm::dwarf::Tag Tag,
                                                  const Type *Ty,
                                                  QualType PointeeTy,
@@ -1197,32 +1330,23 @@ llvm::DIType *CGDebugInfo::CreatePointerLikeType(llvm::dwarf::Tag Tag,
       CGM.getTarget().getDWARFAddressSpace(
           CGM.getTypes().getTargetAddressSpace(PointeeTy));
 
-  SmallVector<llvm::Metadata *, 4> Annots;
-  auto *BTFAttrTy = dyn_cast<BTFTagAttributedType>(PointeeTy);
-  while (BTFAttrTy) {
-    StringRef Tag = BTFAttrTy->getAttr()->getBTFTypeTag();
-    if (!Tag.empty()) {
-      llvm::Metadata *Ops[2] = {
-          llvm::MDString::get(CGM.getLLVMContext(), StringRef("btf_type_tag")),
-          llvm::MDString::get(CGM.getLLVMContext(), Tag)};
-      Annots.insert(Annots.begin(),
-                    llvm::MDNode::get(CGM.getLLVMContext(), Ops));
-    }
-    BTFAttrTy = dyn_cast<BTFTagAttributedType>(BTFAttrTy->getWrappedType());
-  }
-
   llvm::DINodeArray Annotations = nullptr;
-  if (Annots.size() > 0)
-    Annotations = DBuilder.getOrCreateArray(Annots);
+  auto *BTFAttrTy = dyn_cast<BTFTagAttributedType>(PointeeTy.getTypePtr());
+  if (!BTFTypeTagV2 && BTFAttrTy) {
+    SmallVector<llvm::Metadata *, 4> AnnotationsVec;
+    collectBTFTypeTagAnnotations(CGM.getLLVMContext(), DBuilder, AnnotationsVec,
+                                 BTFAttrTy, "btf_type_tag");
+    Annotations = DBuilder.getOrCreateArray(AnnotationsVec);
+  }
 
   if (Tag == llvm::dwarf::DW_TAG_reference_type ||
       Tag == llvm::dwarf::DW_TAG_rvalue_reference_type)
     return DBuilder.createReferenceType(Tag, getOrCreateType(PointeeTy, Unit),
                                         Size, Align, DWARFAddressSpace);
-  else
-    return DBuilder.createPointerType(getOrCreateType(PointeeTy, Unit), Size,
-                                      Align, DWARFAddressSpace, StringRef(),
-                                      Annotations);
+
+  return DBuilder.createPointerType(getOrCreateType(PointeeTy, Unit), Size,
+                                    Align, DWARFAddressSpace, StringRef(),
+                                    Annotations);
 }
 
 llvm::DIType *CGDebugInfo::getOrCreateStructPtrType(StringRef Name,
@@ -3543,9 +3667,6 @@ static QualType UnwrapTypeForDebugInfo(QualType T, const ASTContext &C) {
     case Type::Attributed:
       T = cast<AttributedType>(T)->getEquivalentType();
       break;
-    case Type::BTFTagAttributed:
-      T = cast<BTFTagAttributedType>(T)->getWrappedType();
-      break;
     case Type::CountAttributed:
       T = cast<CountAttributedType>(T)->desugar();
       break;
@@ -3745,10 +3866,12 @@ llvm::DIType *CGDebugInfo::CreateTypeNode(QualType Ty, llvm::DIFile *Unit) {
   case Type::TemplateSpecialization:
     return CreateType(cast<TemplateSpecializationType>(Ty), Unit);
 
+  case Type::BTFTagAttributed:
+    return CreateType(cast<BTFTagAttributedType>(Ty), Unit);
+
   case Type::CountAttributed:
   case Type::Auto:
   case Type::Attributed:
-  case Type::BTFTagAttributed:
   case Type::Adjusted:
   case Type::Decayed:
   case Type::DeducedTemplateSpecialization:
@@ -5921,6 +6044,35 @@ void CGDebugInfo::setDwoId(uint64_t Signature) {
   TheCU->setDWOId(Signature);
 }
 
+static llvm::DIType *copyAnnotations(llvm::DIBuilder &DBuilder,
+                                     llvm::DIDerivedType *Placeholder) {
+  auto *WrappedDI = Placeholder->getBaseType();
+  SmallVector<llvm::Metadata *, 4> Annotations;
+
+  for (const llvm::Metadata *O : Placeholder->getAnnotations()->operands())
+    Annotations.push_back(const_cast<llvm::Metadata *>(O));
+
+  auto AddAnnotations = [&](auto *Type) {
+    if (llvm::DINodeArray OldAnnotations = Type->getAnnotations())
+      for (const llvm::Metadata *O : OldAnnotations->operands())
+        Annotations.push_back(const_cast<llvm::Metadata *>(O));
+    auto Clone = Type->clone();
+    Clone->replaceAnnotations(DBuilder.getOrCreateArray(Annotations));
+    return llvm::MDNode::replaceWithPermanent(std::move(Clone));
+  };
+
+  if (auto *Ty = dyn_cast<llvm::DIBasicType>(WrappedDI))
+    return AddAnnotations(Ty);
+  if (auto *Ty = dyn_cast<llvm::DICompositeType>(WrappedDI))
+    return AddAnnotations(Ty);
+  if (auto *Ty = dyn_cast<llvm::DIDerivedType>(WrappedDI))
+    return AddAnnotations(Ty);
+  if (auto *Ty = dyn_cast<llvm::DISubroutineType>(WrappedDI))
+    return AddAnnotations(Ty);
+
+  return WrappedDI;
+}
+
 void CGDebugInfo::finalize() {
   // Creating types might create further types - invalidating the current
   // element and the size(), so don't cache/reference them.
@@ -5994,6 +6146,10 @@ void CGDebugInfo::finalize() {
     if (auto MD = TypeCache[RT])
       DBuilder.retainType(cast<llvm::DIType>(MD));
 
+  for (auto &Placeholder : AnnotationPlaceholders)
+    DBuilder.replaceTemporary(llvm::TempDIType(Placeholder),
+                              copyAnnotations(DBuilder, Placeholder));
+
   DBuilder.finalize();
 }
 
diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index d6db4d711366a..43a093c725f6a 100644
--- a/clang/lib/CodeGen/CGDebugInfo.h
+++ b/clang/lib/CodeGen/CGDebugInfo.h
@@ -170,6 +170,8 @@ class CGDebugInfo {
   /// The key is coroutine real parameters, value is DIVariable in LLVM IR.
   Param2DILocTy ParamDbgMappings;
 
+  std::vector<llvm::DIDerivedType *> AnnotationPlaceholders;
+
   /// Helper functions for getOrCreateType.
   /// @{
   /// Currently the checksum of an interface includes the number of
@@ -217,6 +219,7 @@ class CGDebugInfo {
   llvm::DIType *CreateType(const MemberPointerType *Ty, llvm::DIFile *F);
   llvm::DIType *CreateType(const AtomicType *Ty, llvm::DIFile *F);
   llvm::DIType *CreateType(const PipeType *Ty, llvm::DIFile *F);
+  llvm::DIType *CreateType(const BTFTagAttributedType *Ty, llvm::DIFile *F);
   /// Get enumeration type.
   llvm::DIType *CreateEnumType(const EnumType *Ty);
   llvm::DIType *CreateTypeDefinition(const EnumType *Ty);
diff --git a/clang/test/CodeGen/attr-btf_type_tag-circular.c b/clang/test/CodeGen/attr-btf_type_tag-circular.c
new file mode 100644
index 0000000000000..4bcb20e63bead
--- /dev/null
+++ b/clang/test/CodeGen/attr-btf_type_tag-circular.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 \
+// RUN:   -triple %itanium_abi_triple -debug-info-kind=limited \
+// RUN:   -mllvm -btf-type-tag-v2 -S -emit-llvm -o - %s | FileCheck %s
+
+#define __tag1 __attribute__((btf_type_tag("tag1")))
+
+struct st {
+  struct st __tag1 *self;
+} g;
+
+// CHECK: distinct !DIGlobalVariable(name: "g", scope: ![[#]], file: ![[#]], line: [[#]], type: ![[L1:[0-9]+]], isLocal: false, isDefinition: true)
+// CHECK: ![[L1]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "st", file: ![[#]], line: [[#]], size: [[#]], elements: ![[L2:[0-9]+]])
+// CHECK: ![[L2]] = !{![[L3:[0-9]+]]}
+// CHECK: ![[L3]] = !DIDerivedType(tag: DW_TAG_member, name: "self", scope: ![[L1]], file: ![[#]], line: [[#]], baseType: ![[L4:[0-9]+]], size: [[#]])
+// CHECK: ![[L4]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[L5:[0-9]+]], size: [[#]])
+// CHECK: ![[L5]] = !DICompositeType(tag: DW_TAG_structure_type, name: "st", file: ![[#]], line: [[#]], size: [[#]], elements: ![[L2]], annotations: ![[L7:[0-9]+]])
+// CHECK: ![[L7]] = !{![[L8:[0-9]+]]}
+// CHECK: ![[L8]] = !{!"btf:type_tag", !"tag1"}
diff --git a/clang/test/CodeGen/attr-btf_type_tag-const.c b/clang/test/CodeGen/attr-btf_type_tag-const.c
new file mode 100644
index 0000000000000..94d9c05f5345a
--- /dev/null
+++ b/clang/test/CodeGen/attr-btf_type_tag-const.c
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 \
+// RUN:   -triple %itanium_abi_triple -debug-info-kind=limited \
+// RUN:   -mllvm -btf-type-tag-v2 -S -emit-llvm -o - %s | FileCheck %s
+
+// Check that BTF type tags are not attached to DW_TAG_const_type DIEs
+// in presence of "sugar" expressions that are transparent for
+// CGDebugInfo.cpp:UnwrapTypeForDebugInfo(), but are not transparent
+// for local qualifiers.
+//
+// For details see:
+//   CGDebugInfo::CreateType(const BTFTagAttributedType, llvm::DIFile)
+
+#define __tag1 __attribute__((btf_type_tag("tag1")))
+#define __tag2 __attribute__((btf_type_tag("tag2")))
+#define __tag3 __attribute__((btf_type_tag("tag3")))
+
+const int *foo;
+typeof(*foo) __tag1 bar;
+
+// CHECK: distinct !DIGlobalVariable(name: "bar", {{.*}}, type: ![[L01:[0-9]+]], {{.*}})
+// CHECK: ![[L01]] = !DIDerivedType(tag: DW_TAG_const_type, baseType: ![[L02:[0-9]+]])
+// CHECK: ![[L02]] = !DIBasicType(name: "int", {{.*}}, annotations: ![[L03:[0-9]+]])
+// CHECK: ![[L03]] = !{![[L04:[0-9]+]]}
+// CHECK: ![[L04]] = !{!"btf:type_tag", !"tag1"}
+
+const int __tag2 *buz;
+
+// CHECK: distinct !DIGlobalVariable(name: "buz", {{.*}}, type: ![[L05:[0-9]+]], {{.*}})
+// CHECK: ![[L05]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[L06:[0-9]+]], {{.*}})
+// CHECK: ![[L06]] = !DIDerivedType(tag: DW_TAG_const_type, baseType: ![[L08:[0-9]+]])
+// CHECK: ![[L08]] = !DIBasicType(name: "int", size: [[#]], {{.*}}, annotations: ![[L09:[0-9]+]])
+// CHECK: ![[L09]] = !{![[L10:[0-9]+]]}
+// CHECK: ![[L10]] = !{!"btf:type_tag", !"tag2"}
+
+typeof(*buz) __tag3 quux;
+
+// CHECK: distinct !DIGlobalVariable(name: "quux", {{.*}}, type: ![[L12:[0-9]+]], {{.*}})
+// CHECK: ![[L12]] = !DIDerivedType(tag: DW_TAG_const_type, baseType: ![[L13:[0-9]+]])
+// CHECK: ![[L13]] = !DIBasicType(name: "int", {{.*}}, annotations: ![[L14:[0-9]+]])
+// CHECK: ![[L14]] = !{![[L15:[0-9]+]], ![[L10]]}
+// CHECK: ![[L15]] = !{!"btf:type_tag", !"tag3"}
diff --git a/clang/test/CodeGen/attr-btf_type_tag-func-ptr.c b/clang/test/CodeGen/attr-btf_type_tag-func-ptr.c
index 26935c882a017..8567864692202 100644
--- a/clang/test/CodeGen/attr-btf_type_tag-func-ptr.c
+++ b/clang/test/CodeGen/attr-btf_type_tag-func-ptr.c
@@ -1,4 +1,8 @@
 // RUN: %clang_cc1 -triple %itanium_abi_triple -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 \
+// RUN:   -triple %itanium_abi_triple -debug-info-kind=limited \
+// RUN:   -mllvm -btf-type-tag-v2 -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefix CHECK-V2 %s
 
 struct t {
  int (__attribute__((btf_type_tag("rcu"))) *f)();
@@ -8,8 +12,13 @@ int foo(struct t *arg) {
   return arg->a;
 }
 
-// CHECK:      !DIDerivedType(tag: DW_TAG_member, name: "f"
-// CHECK-SAME: baseType: ![[L18:[0-9]+]]
-// CHECK:      ![[L18]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[#]], size: [[#]], annotations: ![[L21:[0-9]+]])
-// CHECK:      ![[L21]] = !{![[L22:[0-9]+]]}
-// CHECK:      ![[L22]] = !{!"btf_type_tag", !"rcu"}
+// CHECK: !DIDerivedType(tag: DW_TAG_member, name: "f", scope: ![[#]], file: ![[#]], line: [[#]], baseType: ![[L1:[0-9]+]], size: [[#]])
+// CHECK: ![[L1]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[#]], size: [[#]], annotations: ![[L2:[0-9]+]])
+// CHECK: ![[L2]] = !{![[L3:[0-9]+]]}
+// CHECK: ![[L3]] = !{!"btf_type_tag", !"rcu"}
+
+// CHECK-V2: !DIDerivedType(tag: DW_TAG_member, name: "f", scope: ![[#]], file: ![[#]], line: [[#]], baseType: ![[L1:[0-9]+]], size: [[#]])
+// CHECK-V2: ![[L1]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[L2:[0-9]+]], size: [[#]])
+// CHECK-V2: ![[L2]] = !DISubroutineType(types: ![[#]], annotations: ![[L4:[0-9]+]])
+// CHECK-V2: ![[L4]] = !{![[L5:[0-9]+]]}
+// CHECK-V2: ![[L5]] = !{!"btf:type_tag", !"rcu"}
diff --git a/clang/test/CodeGen/attr-btf_type_tag-func.c b/clang/test/CodeGen/attr-btf_type_tag-func.c
index dbb8864759148..890d3ab35428b 100644
--- a/clang/test/CodeGen/attr-btf_type_tag-func.c
+++ b/clang/test/CodeGen/attr-btf_type_tag-func.c
@@ -1,5 +1,17 @@
-// RUN: %clang_cc1 -triple %itanium_abi_triple -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
-// RUN: %clang_cc1 -triple %itanium_abi_triple -DDOUBLE_BRACKET_ATTRS=1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 \
+// RUN:   -triple %itanium_abi_triple -debug-info-kind=limited \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 \
+// RUN:   -triple %itanium_abi_triple -DDOUBLE_BRACKET_ATTRS=1 -debug-info-kind=limited \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 \
+// RUN:   -triple %itanium_abi_triple -debug-info-kind=limited \
+// RUN:   -mllvm -btf-type-tag-v2 -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK-V2 %s
+// RUN: %clang_cc1 \
+// RUN:   -triple %itanium_abi_triple -DDOUBLE_BRACKET_ATTRS=1 \
+// RUN:   -debug-info-kind=limited -mllvm -btf-type-tag-v2 -emit-llvm -o - %s \
+// RUN: | FileCheck --check-prefixes CHECK-V2 %s
 
 #if DOUBLE_BRACKET_ATTRS
 #define __tag1 [[clang::btf_type_tag("tag1")]]
@@ -15,14 +27,26 @@
 
 int __tag1 * __tag2 *foo(int __tag1 * __tag2 *arg) { return arg; }
 
-// CHECK: distinct !DISubprogram(name: "foo", scope: ![[#]], file: ![[#]], line: [[#]], type: ![[L9:[0-9]+]]
-// CHECK: ![[L9]] = !DISubroutineType(types: ![[L10:[0-9]+]]
-// CHECK: ![[L10]] = !{![[L11:[0-9]+]], ![[L11]]}
-// CHECK: ![[L11]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[L12:[0-9]+]], size: [[#]], annotations: ![[L16:[0-9]+]]
-// CHECK: ![[L12]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[L13:[0-9]+]], size: [[#]], annotations: ![[L14:[0-9]+]]
-// CHECK: ![[L13]] = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed
-// CHECK: ![[L14]] = !{![[L15:[0-9]+]]}
-// CHECK: ![[L15]] = !{!"btf_type_tag", !"tag1"}
-// CHECK: ![[L16]] = !{![[L17:[0-9]+]]}
-// CHECK: ![[L17]] = !{!"btf_type_tag", !"tag2"}
-// CHECK: !DILocalVariable(name: "arg", arg: 1, scope: ![[#]], file: ![[#]], line: [[#]], type: ![[L11]])
+// CHECK: distinct !DISubprogram(name: "foo", scope: ![[#]], file: ![[#]], line: [[#]], type: ![[L01:[0-9]+]], {{.*}})
+// CHECK: ![[L01]] = !DISubroutineType(types: ![[L02:[0-9]+]])
+// CHECK: ![[L02]] = !{![[L03:[0-9]+]], ![[L03]]}
+// CHECK: ![[L03]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[L04:[0-9]+]], size: [[#]], annotations: ![[L05:[0-9]+]])
+// CHECK: ![[L04]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[L06:[0-9]+]], size: [[#]], annotations: ![[L07:[0-9]+]])
+// CHECK: ![[L06]] = !DIBasicType(name: "int", size: [[#]], encoding: DW_ATE_signed)
+// CHECK: ![[L07]] = !{![[L11:[0-9]+]]}
+// CHECK: ![[L11]] = !{!"btf_type_tag", !"tag1"}
+// CHECK: ![[L05]] = !{![[L12:[0-9]+]]}
+// CHECK: ![[L12]] = !{!"btf_type_tag", !"tag2"}
+// CHECK: !DILocalVariable(name: "arg", arg: 1, scope: ![[#]], file: ![[#]], line: [[#]], type: ![[L03]])
+
+// CHECK-V2: distinct !DISubprogram(name: "foo", scope: ![[#]], file: ![[#]], line: [[#]], type: ![[L01:[0-9]+]], {{.*}})
+// CHECK-V2: ![[L01]] = !DISubroutineType(types: ![[L02:[0-9]+]])
+// CHECK-V2: ![[L02]] = !{![[L03:[0-9]+]], ![[L03]]}
+// CHECK-V2: ![[L03]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[L04:[0-9]+]], size: [[#]])
+// CHECK-V2: ![[L04]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[L06:[0-9]+]], size: [[#]], annotations: ![[L07:[0-9...
[truncated]

@yonghong-song
Copy link
Contributor

Looks like some changes are duplicate from #91422, e.g., llvm/lib/Bitcode/Reader/MetadataLoader.cpp. There are some other files are duplicated as well. Could you do a cleanup here? This will make it easy to compare to https://reviews.llvm.org/D143967.

@eddyz87
Copy link
Contributor Author

eddyz87 commented May 18, 2024

Looks like some changes are duplicate from #91422, e.g., llvm/lib/Bitcode/Reader/MetadataLoader.cpp. There are some other files are duplicated as well. Could you do a cleanup here? This will make it easy to compare to https://reviews.llvm.org/D143967.

As far as I understand, this is how one makes "revision stacks" after migration from Phabriactor to Github: I have three branches, one for each patch I want to land, each branch is forked from a previous one. Until first branch is landed, Github will show two pending commits (one from the previous branch, one from this branch). To check changes specific to this branch please refer to the top-most commit of the branch: ba6a989

Please let me know if I understand the new process in a wrong way.

@yonghong-song
Copy link
Contributor

Okay, I see. Thanks for explanation. I checked the top commit and it looks good to me. it is very similar to https://reviews.llvm.org/D143967 except a few additions like new llvm internal flag and a couple of places to generate type tags based on v1 format.

Test "buildkite/github-pull-requests " failed and I don't know what is the reason. I think it is worthwhile to rerun the test to ensure all tests passed.

@eddyz87
Copy link
Contributor Author

eddyz87 commented May 18, 2024 via email

@yonghong-song
Copy link
Contributor

Test "buildkite/github-pull-requests " failed and I don't know what is the reason. I think it is worthwhile to rerun the test to ensure all tests passed.
CI failure seems to be a fluke as it is reported for DataFlowSanitizer-x86_64 :: release_shadow_space.c which is completely unrelated, also CI for 91424 passed (91424 adds one commit to the bpf backend on top of this revision).

Right, I am aware of this. I just want to have test status 'green' :-)

Copy link
Collaborator

@dwblaikie dwblaikie left a comment

Choose a reason for hiding this comment

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

approving mostly on the basis of my own previous approval on phab

@eddyz87 eddyz87 force-pushed the btf-type-tag-v2-dwarf-changes branch from ba6a989 to 57822bc Compare June 5, 2024 07:01
@eddyz87 eddyz87 force-pushed the btf-type-tag-v2-dwarf-changes branch from 57822bc to 06555f0 Compare June 12, 2024 17:50
This commit is a follow-up for BPF mailing list discussion at [1].
It changes the way `__attribute__((btf_type_tag("...")))`s are
represented in DWARF.

Prior to this commit type tags could only be attached to pointers.
Such attachments associated the tags with a pointee type.
E.g. for the following C code:

    int __attribute__((btf_type_tag("tag1"))) *g;

Generated DWARF looked as follows:

    0x0000001e:   DW_TAG_variable
                    DW_AT_name      ("g")
                    DW_AT_type      (0x00000029 "int *")

    0x00000029:   DW_TAG_pointer_type
                    DW_AT_type      (0x00000032 "int")

    0x0000002e:     DW_TAG_LLVM_annotation
                      DW_AT_name    ("btf_type_tag")
                      DW_AT_const_value     ("tag1")

    0x00000032:   DW_TAG_base_type
                    DW_AT_name      ("int")

The goal of this commit is to allow attachment of type tags to the
tagged types instead. E.g. for the same example DWARF should look as
follows:

    0x0000001e:   DW_TAG_variable
                    DW_AT_name      ("g")
                    DW_AT_type      (0x00000029 "int *")

    0x00000029:   DW_TAG_pointer_type
                    DW_AT_type      (0x00000032 "int")

    0x00000032:   DW_TAG_base_type
                    DW_AT_name      ("int")

    0x00000036:     DW_TAG_LLVM_annotation
                      DW_AT_name    ("btf:type_tag")
                      DW_AT_const_value     ("tag1")

A new tag name, `btf:type_tag`, is used so that DWARF consumers
could distinguish between old and new attachment semantics.

This feature is mostly used by Linux Kernel in combination with tool
named pahole [2]. Reasonably recent versions of pahole generate
errors (1.23, 1.24) or warnings (1.25) when `DW_TAG_LLVM_annotation`
is attached to `DW_TAG_base_type` or `DW_TAG_unspecified_type`.
Hence the `btf:type_tag` generation is controlled by a hidden option
`-mllvm -btf-type-tag-v2`. The goal is to provide a way for tooling to
work on adding support `btf:type_tag` and eventually replace
`btf_type_tag` by `btf:type_tag`, removing the above option.

The commit includes the following changes:
- Changes in debug info generation:
  - New method `DIBuilder::createAnnotationsPlaceholder()` is added,
    it creates a temporary `DIDerivedType` that plays as annotations
    placeholder while debug info metadata is being constructed;
  - New overload for `CGDebugInfo::CreateType` method is added:

      llvm::DIType *CGDebugInfo::CreateType(const BTFTagAttributedType *Ty,
                                            llvm::DIFile *Unit);

    This overload collects BTF type tags in `Ty`, creates annotations
    placeholder pointing to the base type of `Ty`, registers the
    placeholder in the `CGDebugInfo::AnnotationsPlaceholder` vector.
  - `CGDebugInfo::finalize()` is updated to do the following for each
    annotation placeholder:
    - clone underlying base type;
    - attach annotations the clone using `replaceAnnotations()` call;
    - replace all placeholder usages by a clone.
  Such scheme allows to deal with type cycles.

- Changes in AST construction:
  - `ASTContext::getBTFTagAttributedType()` is updated to ensure that
    `BTFTagAttributedType` always wraps `QualType` w/o local
    constant/volatile/restricted qualifiers. This simplifies debug info
    generation.

[1] https://lore.kernel.org/bpf/87r0w9jjoq.fsf@oracle.com/
[2] https://git.kernel.org/pub/scm/devel/pahole/pahole.git/

This was previously tracked as differential revision:
https://reviews.llvm.org/D143967
Newly added btf_type_tag related tests use 'clang -S -emit-llvm',
which is now not accepted by 'clang'.
Fix these tests by using only '-emit-llvm'.

To be squashed with the main commit when when branch would be merged.
@eddyz87 eddyz87 force-pushed the btf-type-tag-v2-dwarf-changes branch from 06555f0 to a66cec3 Compare June 18, 2024 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen clang Clang issues not falling into any other category debuginfo llvm:ir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants