[BPF] handle typedef of struct/union for CO-RE relocations

[yonghong-song]

Summary

Linux commit

https://github.com/torvalds/linux/commit/1cf5b23988ea0086a252a5c8b005b075f1e9b030#diff-289313b9fec99c6f0acfea19d9cfd949

uses "#pragma clang attribute push (attribute((preserve_access_index)),

apply_to = record)"

to apply CO-RE relocations to all records including the following pattern:

#pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record)
typedef struct {
  int a;
} __t;
#pragma clang attribute pop 
int test(__t *arg) { return arg->a; }

The current approach to use struct/union type in the relocation record will
result in an anonymous struct, which make later type matching difficult
in bpf loader. In fact, current BPF backend will fail the above program
with assertion:

clang: ../lib/Target/BPF/BPFAbstractMemberAccess.cpp:796: ... 
   Assertion `TypeName.size()' failed.

clang will change to use the type of the base of the member access
which will preserve the typedef modifier for the
preserve_{struct,union}_access_index intrinsics in the above example.
Here we adjust BPF backend to accept that the debuginfo
type metadata may be 'typedef' and handle them properly.

[anakryiko]

llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp

715

this comment won't be accurate anymore? should it be updated to include typedef?

732–734

I wonder if capturing typedef always is a better approach, even if underlying struct/union is named? If that causes problems due to typedef name not being found, we can always teach libbpf to treat typedef as an alias and try to find candidate types first using typedef name, if that fails - using underlying struct name? WDYT?

[yonghong-song]

llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp

715

Yes, this needs to updated.

732–734

Yes, we can do this. This probably will give libbpf more information about what is going on, i.e., user actually use type with typedef to access variable instead of the structure (even with explicit names). Not sure how useful this information will be used. But it should not hurt.

[yonghong-song]

address Andrii's comments. Always use the typedef type name if it is available.

llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp

bool BPFAbstractMemberAccess::runOnModule(Module &M) {
   // Bail out if no debug info.
   if (M.debug_compile_units().empty())
     return false;
 
   DL = &M.getDataLayout();
   return doTransformation(M);
 }
 
-static bool SkipDIDerivedTag(unsigned Tag) {
+static bool SkipDIDerivedTag(unsigned Tag, bool skipTypedef) {
   if (Tag != dwarf::DW_TAG_typedef && Tag != dwarf::DW_TAG_const_type &&
       Tag != dwarf::DW_TAG_volatile_type &&
       Tag != dwarf::DW_TAG_restrict_type &&
       Tag != dwarf::DW_TAG_member)
-     return false;
+  if (Tag == dwarf::DW_TAG_typedef && !skipTypedef)
+    return false;
   return true;
 }
 
-static DIType * stripQualifiers(DIType *Ty) {
+static DIType * stripQualifiers(DIType *Ty, bool skipTypedef = true) {
   while (auto *DTy = dyn_cast<DIDerivedType>(Ty)) {
-    if (!SkipDIDerivedTag(DTy->getTag()))
+    if (!SkipDIDerivedTag(DTy->getTag(), skipTypedef))
       break;
     Ty = DTy->getBaseType();
   }
   return Ty;
 }
 
 static const DIType * stripQualifiers(const DIType *Ty) {
   while (auto *DTy = dyn_cast<DIDerivedType>(Ty)) {
-    if (!SkipDIDerivedTag(DTy->getTag()))
+    if (!SkipDIDerivedTag(DTy->getTag(), true))
       break;
     Ty = DTy->getBaseType();
   }
   return Ty;
 }
 
 static uint32_t calcArraySize(const DICompositeType *CTy, uint32_t StartDim) {
   DINodeArray Elements = CTy->getElements();
while (Call) {
     CInfo = AIChain[Call].second;
     Call = AIChain[Call].first;
   }
 
   // The access offset from the base of the head of chain is also
   // calculated here as all debuginfo types are available.
 
   // Get type name and calculate the first index.
-  // We only want to get type name from structure or union.
+  // We only want to get type name from typedef, structure or union.
   // If user wants a relocation like
   //    int *p; ... __builtin_preserve_access_index(&p[4]) ...
   // or
   //    int a[10][20]; ... __builtin_preserve_access_index(&a[2][3]) ...
   // we will skip them.
   uint32_t FirstIndex = 0;
   uint32_t PatchImm = 0; // AccessOffset or the requested field info
   uint32_t InfoKind = BPFCoreSharedInfo::FIELD_BYTE_OFFSET;
   while (CallStack.size()) {
     auto StackElem = CallStack.top();
     Call = StackElem.first;
     CInfo = StackElem.second;
 
     if (!Base)
       Base = CInfo.Base;
 
-    DIType *Ty = stripQualifiers(cast<DIType>(CInfo.Metadata));
+    DIType *PossibleTypeDef = stripQualifiers(cast<DIType>(CInfo.Metadata),
+                                              false);
+    DIType *Ty = stripQualifiers(PossibleTypeDef);
     if (CInfo.Kind == BPFPreserveUnionAI ||
         CInfo.Kind == BPFPreserveStructAI) {
-      // struct or union type
-      TypeName = std::string(Ty->getName());
-      TypeMeta = Ty;
+      // struct or union type. If the typedef is in the metadata, always
+      // use the typedef.
+      TypeName = std::string(PossibleTypeDef->getName());
+      TypeMeta = PossibleTypeDef;
       PatchImm += FirstIndex * (Ty->getSizeInBits() >> 3);
       break;
     }
 
     assert(CInfo.Kind == BPFPreserveArrayAI);
 
     // Array entries will always be consumed for accumulative initial index.
     CallStack.pop();

llvm/test/CodeGen/BPF/CORE/offset-reloc-typedef-struct-2.ll

+; RUN: llc -march=bpfel -filetype=asm -o - %s | FileCheck %s
+; RUN: llc -march=bpfel -mattr=+alu32 -filetype=asm -o - %s | FileCheck %s
+;
+; Source code:
+;   #pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record)
+;   typedef struct {
+;     int a;
+;   } __t;
+;   #pragma clang attribute pop
+;
+;   int test(__t *arg) { return arg->a; }
+; Compiler flag to generate IR:
+;   clang -target bpf -S -O2 -g -emit-llvm test.c
+
+%struct.__t = type { i32 }
+
+; Function Attrs: nounwind readonly
+define dso_local i32 @test(%struct.__t* readonly %arg) local_unnamed_addr #0 !dbg !13 {
+entry:
+  call void @llvm.dbg.value(metadata %struct.__t* %arg, metadata !18, metadata !DIExpression()), !dbg !19
+  %0 = tail call i32* @llvm.preserve.struct.access.index.p0i32.p0s_struct.__ts(%struct.__t* %arg, i32 0, i32 0), !dbg !20, !llvm.preserve.access.index !4
+  %1 = load i32, i32* %0, align 4, !dbg !20, !tbaa !21
+  ret i32 %1, !dbg !26
+}
+
+; CHECK:             .long   1                       # BTF_KIND_TYPEDEF(id = 2)
+; CHECK-NEXT:        .long   134217728               # 0x8000000
+; CHECK-NEXT:        .long   3
+; CHECK-NEXT:        .long   0                       # BTF_KIND_STRUCT(id = 3)
+; CHECK-NEXT:        .long   67108865                # 0x4000001
+; CHECK-NEXT:        .long   4
+; CHECK-NEXT:        .long   5
+; CHECK-NEXT:        .long   4
+; CHECK-NEXT:        .long   0                       # 0x0
+;
+; CHECK:             .ascii  "__t"                   # string offset=1
+; CHECK:             .byte   97                      # string offset=5
+; CHECK:             .ascii  ".text"                 # string offset=20
+; CHECK:             .ascii  "0:0"                   # string offset=26
+;
+; CHECK:             .long   16                      # FieldReloc
+; CHECK-NEXT:        .long   20                      # Field reloc section string offset=20
+; CHECK-NEXT:        .long   1
+; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   2
+; CHECK-NEXT:        .long   26
+; CHECK-NEXT:        .long   0
+
+; Function Attrs: nounwind readnone
+declare i32* @llvm.preserve.struct.access.index.p0i32.p0s_struct.__ts(%struct.__t*, i32, i32) #1
+
+; Function Attrs: nounwind readnone speculatable
+declare void @llvm.dbg.value(metadata, metadata, metadata) #2
+
+attributes #0 = { nounwind readonly "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="all" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind readnone }
+attributes #2 = { nounwind readnone speculatable}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!9, !10, !11}
+!llvm.ident = !{!12}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 11.0.0 (https://github.com/llvm/llvm-project.git 5125d1c934efa69ffc1902ce3b8f2f288653a92f)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "test.c", directory: "/tmp/home/yhs/work/tests/core_bug")
+!2 = !{}
+!3 = !{!4}
+!4 = !DIDerivedType(tag: DW_TAG_typedef, name: "__t", file: !1, line: 4, baseType: !5)
+!5 = distinct !DICompositeType(tag: DW_TAG_structure_type, file: !1, line: 2, size: 32, elements: !6)
+!6 = !{!7}
+!7 = !DIDerivedType(tag: DW_TAG_member, name: "a", scope: !5, file: !1, line: 3, baseType: !8, size: 32)
+!8 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!9 = !{i32 7, !"Dwarf Version", i32 4}
+!10 = !{i32 2, !"Debug Info Version", i32 3}
+!11 = !{i32 1, !"wchar_size", i32 4}
+!12 = !{!"clang version 11.0.0 (https://github.com/llvm/llvm-project.git 5125d1c934efa69ffc1902ce3b8f2f288653a92f)"}
+!13 = distinct !DISubprogram(name: "test", scope: !1, file: !1, line: 7, type: !14, scopeLine: 7, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, isDefinition: true, isOptimized: true, unit: !0, retainedNodes: !17)
+!14 = !DISubroutineType(types: !15)
+!15 = !{!8, !16}
+!16 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !4, size: 64)
+!17 = !{!18}
+!18 = !DILocalVariable(name: "arg", arg: 1, scope: !13, file: !1, line: 7, type: !16)
+!19 = !DILocation(line: 0, scope: !13)
+!20 = !DILocation(line: 7, column: 34, scope: !13)
+!21 = !{!22, !23, i64 0}
+!22 = !{!"", !23, i64 0}
+!23 = !{!"int", !24, i64 0}
+!24 = !{!"omnipotent char", !25, i64 0}
+!25 = !{!"Simple C/C++ TBAA"}
+!26 = !DILocation(line: 7, column: 22, scope: !13)

llvm/test/CodeGen/BPF/CORE/offset-reloc-typedef-union-2.ll

+; RUN: llc -march=bpfel -filetype=asm -o - %s | FileCheck %s
+; RUN: llc -march=bpfel -mattr=+alu32 -filetype=asm -o - %s | FileCheck %s
+;
+; Source code:
+;   #pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record)
+;   typedef union {
+;     int a;
+;   } __t;
+;   #pragma clang attribute pop
+;
+;   int test(__t *arg) { return arg->a; }
+; Compiler flag to generate IR:
+;   clang -target bpf -S -O2 -g -emit-llvm test.c
+
+%union.__t = type { i32 }
+
+; Function Attrs: nounwind readonly
+define dso_local i32 @test(%union.__t* readonly %arg) local_unnamed_addr #0 !dbg !13 {
+entry:
+  call void @llvm.dbg.value(metadata %union.__t* %arg, metadata !18, metadata !DIExpression()), !dbg !19
+  %0 = tail call %union.__t* @llvm.preserve.union.access.index.p0s_union.__ts.p0s_union.__ts(%union.__t* %arg, i32 0), !dbg !20, !llvm.preserve.access.index !4
+  %a = getelementptr %union.__t, %union.__t* %0, i64 0, i32 0, !dbg !20
+  %1 = load i32, i32* %a, align 4, !dbg !20, !tbaa !21
+  ret i32 %1, !dbg !24
+}
+
+; CHECK:             .long   1                       # BTF_KIND_TYPEDEF(id = 2)
+; CHECK-NEXT:        .long   134217728               # 0x8000000
+; CHECK-NEXT:        .long   3
+; CHECK-NEXT:        .long   0                       # BTF_KIND_UNION(id = 3)
+; CHECK-NEXT:        .long   83886081                # 0x5000001
+; CHECK-NEXT:        .long   4
+; CHECK-NEXT:        .long   5
+; CHECK-NEXT:        .long   4
+; CHECK-NEXT:        .long   0                       # 0x0
+;
+; CHECK:             .ascii  "__t"                   # string offset=1
+; CHECK:             .byte   97                      # string offset=5
+; CHECK:             .ascii  ".text"                 # string offset=20
+; CHECK:             .ascii  "0:0"                   # string offset=26
+;
+; CHECK:             .long   16                      # FieldReloc
+; CHECK-NEXT:        .long   20                      # Field reloc section string offset=20
+; CHECK-NEXT:        .long   1
+; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   2
+; CHECK-NEXT:        .long   26
+; CHECK-NEXT:        .long   0
+
+; Function Attrs: nounwind readnone
+declare %union.__t* @llvm.preserve.union.access.index.p0s_union.__ts.p0s_union.__ts(%union.__t*, i32) #1
+
+; Function Attrs: nounwind readnone speculatable
+declare void @llvm.dbg.value(metadata, metadata, metadata) #2
+
+attributes #0 = { nounwind readonly "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="all" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind readnone }
+attributes #2 = { nounwind readnone speculatable}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!9, !10, !11}
+!llvm.ident = !{!12}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 11.0.0 (https://github.com/llvm/llvm-project.git 5125d1c934efa69ffc1902ce3b8f2f288653a92f)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "test.c", directory: "/tmp/home/yhs/work/tests/core_bug")
+!2 = !{}
+!3 = !{!4}
+!4 = !DIDerivedType(tag: DW_TAG_typedef, name: "__t", file: !1, line: 4, baseType: !5)
+!5 = distinct !DICompositeType(tag: DW_TAG_union_type, file: !1, line: 2, size: 32, elements: !6)
+!6 = !{!7}
+!7 = !DIDerivedType(tag: DW_TAG_member, name: "a", scope: !5, file: !1, line: 3, baseType: !8, size: 32)
+!8 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!9 = !{i32 7, !"Dwarf Version", i32 4}
+!10 = !{i32 2, !"Debug Info Version", i32 3}
+!11 = !{i32 1, !"wchar_size", i32 4}
+!12 = !{!"clang version 11.0.0 (https://github.com/llvm/llvm-project.git 5125d1c934efa69ffc1902ce3b8f2f288653a92f)"}
+!13 = distinct !DISubprogram(name: "test", scope: !1, file: !1, line: 7, type: !14, scopeLine: 7, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, isDefinition: true, isOptimized: true, unit: !0, retainedNodes: !17)
+!14 = !DISubroutineType(types: !15)
+!15 = !{!8, !16}
+!16 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !4, size: 64)
+!17 = !{!18}
+!18 = !DILocalVariable(name: "arg", arg: 1, scope: !13, file: !1, line: 7, type: !16)
+!19 = !DILocation(line: 0, scope: !13)
+!20 = !DILocation(line: 7, column: 34, scope: !13)
+!21 = !{!22, !22, i64 0}
+!22 = !{!"omnipotent char", !23, i64 0}
+!23 = !{!"Simple C/C++ TBAA"}
+!24 = !DILocation(line: 7, column: 22, scope: !13)