[BPF] fix CO-RE incorrect index access string

Currently, we expect the CO-RE offset relocation records
a string encoding the original getelementptr access index,
so kernel bpf loader can decode it correctly.

For example,
  struct s { int a; int b; };
  struct t { int c; int d; };
  #define _(x) (__builtin_preserve_access_index(x))
  int get_value(const void *addr1, const void *addr2);
  int test(struct s *arg1, struct t *arg2) {
    return get_value(_(&arg1->b), _(&arg2->d));
  }

We expect two offset relocations:
  reloc 1: type s, access index 0, 1
  reloc 2: type t, access index 0, 1

Two globals are created to retain access indexes for the
above two relocations with global variable names.
The first global has a name "0:1:". Unfortunately,
the second global has the name "0:1:.1" as the llvm
internals automatically add suffix ".1" to a global
with the same name. Later on, the BPF peels the last
character and record "0:1" and "0:1:." in the
relocation table.

This is not desirable. BPF backend could use the global
variable suffix knowledge to generate correct access str.
This patch rather took an approach not relying on
that knowledge. It generates "s:0:1:" and "t:0:1:" to
avoid global variable suffixes and later on generate
correct index access string "0:1" for both records.

Signed-off-by: Yonghong Song <yhs@fb.com>
Differential Revision: https://reviews.llvm.org/D65258

llvm-svn: 367030

diff --git a/llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp b/llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp
index 51d4cbc..509484b 100644 (file)
--- a/llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp
+++ b/llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp
@@ -116,9 +116,8 @@ private:
   void replaceWithGEP(std::vector<CallInst *> &CallList,
                       uint32_t NumOfZerosIndex, uint32_t DIIndex);
 
-  Value *computeBaseAndAccessStr(CallInst *Call, std::string &AccessStr,
-                                 std::string &AccessKey, uint32_t Kind,
-                                 MDNode *&TypeMeta);
+  Value *computeBaseAndAccessKey(CallInst *Call, std::string &AccessKey,
+                                 uint32_t Kind, MDNode *&TypeMeta);
   bool getAccessIndex(const Value *IndexValue, uint64_t &AccessIndex);
   bool transformGEPChain(Module &M, CallInst *Call, uint32_t Kind);
 };
@@ -340,8 +339,7 @@ bool BPFAbstractMemberAccess::getAccessIndex(const Value *IndexValue,
 /// Compute the base of the whole preserve_*_access_index chains, i.e., the base
 /// pointer of the first preserve_*_access_index call, and construct the access
 /// string, which will be the name of a global variable.
-Value *BPFAbstractMemberAccess::computeBaseAndAccessStr(CallInst *Call,
-                                                        std::string &AccessStr,
+Value *BPFAbstractMemberAccess::computeBaseAndAccessKey(CallInst *Call,
                                                         std::string &AccessKey,
                                                         uint32_t Kind,
                                                         MDNode *&TypeMeta) {
@@ -392,16 +390,16 @@ Value *BPFAbstractMemberAccess::computeBaseAndAccessStr(CallInst *Call,
   if (!LastTypeName.size() || AccessIndices.size() > TypeNameIndex + 2)
     return nullptr;
 
-  // Construct the type string AccessStr.
+  // Construct the type string AccessKey.
   for (unsigned I = 0; I < AccessIndices.size(); ++I)
-    AccessStr = std::to_string(AccessIndices[I]) + ":" + AccessStr;
+    AccessKey = std::to_string(AccessIndices[I]) + ":" + AccessKey;
 
   if (TypeNameIndex == AccessIndices.size() - 1)
-    AccessStr = "0:" + AccessStr;
+    AccessKey = "0:" + AccessKey;
 
   // Access key is the type name + access string, uniquely identifying
   // one kernel memory access.
-  AccessKey = LastTypeName + ":" + AccessStr;
+  AccessKey = LastTypeName + ":" + AccessKey;
 
   return Base;
 }
@@ -410,10 +408,10 @@ Value *BPFAbstractMemberAccess::computeBaseAndAccessStr(CallInst *Call,
 /// transformation to a chain of relocable GEPs.
 bool BPFAbstractMemberAccess::transformGEPChain(Module &M, CallInst *Call,
                                                 uint32_t Kind) {
-  std::string AccessStr, AccessKey;
+  std::string AccessKey;
   MDNode *TypeMeta = nullptr;
   Value *Base =
-      computeBaseAndAccessStr(Call, AccessStr, AccessKey, Kind, TypeMeta);
+      computeBaseAndAccessKey(Call, AccessKey, Kind, TypeMeta);
   if (!Base)
     return false;
 
@@ -432,7 +430,7 @@ bool BPFAbstractMemberAccess::transformGEPChain(Module &M, CallInst *Call,
 
   if (GEPGlobals.find(AccessKey) == GEPGlobals.end()) {
     GV = new GlobalVariable(M, Type::getInt64Ty(BB->getContext()), false,
-                            GlobalVariable::ExternalLinkage, NULL, AccessStr);
+                            GlobalVariable::ExternalLinkage, NULL, AccessKey);
     GV->addAttribute(BPFCoreSharedInfo::AmaAttr);
     // Set the metadata (debuginfo types) for the global.
     if (TypeMeta)

diff --git a/llvm/lib/Target/BPF/BTFDebug.cpp b/llvm/lib/Target/BPF/BTFDebug.cpp
index fa35c66..3105eae 100644 (file)
--- a/llvm/lib/Target/BPF/BTFDebug.cpp
+++ b/llvm/lib/Target/BPF/BTFDebug.cpp
@@ -1006,19 +1006,20 @@ void BTFDebug::generateOffsetReloc(const MachineInstr *MI,
   unsigned RootId = populateStructType(RootTy);
   setTypeFromId(RootId, &PrevStructType, &PrevArrayType);
   unsigned RootTySize = PrevStructType->getStructSize();
+  StringRef IndexPattern = AccessPattern.substr(AccessPattern.find_first_of(':') + 1);
 
   BTFOffsetReloc OffsetReloc;
   OffsetReloc.Label = ORSym;
-  OffsetReloc.OffsetNameOff = addString(AccessPattern.drop_back());
+  OffsetReloc.OffsetNameOff = addString(IndexPattern.drop_back());
   OffsetReloc.TypeID = RootId;
 
   uint32_t Start = 0, End = 0, Offset = 0;
   bool FirstAccess = true;
-  for (auto C : AccessPattern) {
+  for (auto C : IndexPattern) {
     if (C != ':') {
       End++;
     } else {
-      std::string SubStr = AccessPattern.substr(Start, End - Start);
+      std::string SubStr = IndexPattern.substr(Start, End - Start);
       int Loc = std::stoi(SubStr);
 
       if (FirstAccess) {
@@ -1043,7 +1044,7 @@ void BTFDebug::generateOffsetReloc(const MachineInstr *MI,
       End = Start;
     }
   }
-  AccessOffsets[RootTy->getName().str() + ":" + AccessPattern.str()] = Offset;
+  AccessOffsets[AccessPattern.str()] = Offset;
   OffsetRelocTable[SecNameOff].push_back(OffsetReloc);
 }
 
@@ -1227,7 +1228,7 @@ bool BTFDebug::InstLower(const MachineInstr *MI, MCInst &OutMI) {
         MDNode *MDN = GVar->getMetadata(LLVMContext::MD_preserve_access_index);
         DIType *Ty = dyn_cast<DIType>(MDN);
         std::string TypeName = Ty->getName();
-        int64_t Imm = AccessOffsets[TypeName + ":" + GVar->getName().str()];
+        int64_t Imm = AccessOffsets[GVar->getName().str()];
 
         // Emit "mov ri, <imm>" for abstract member accesses.
         OutMI.setOpcode(BPF::MOV_ri);

diff --git a/llvm/test/CodeGen/BPF/CORE/offset-reloc-access-str.ll b/llvm/test/CodeGen/BPF/CORE/offset-reloc-access-str.ll
new file mode 100644 (file)
index 0000000..5625bf9
--- /dev/null
+++ b/llvm/test/CodeGen/BPF/CORE/offset-reloc-access-str.ll
@@ -0,0 +1,95 @@
+; RUN: llc -march=bpfel -filetype=asm -o - %s | FileCheck %s
+; RUN: llc -march=bpfeb -filetype=asm -o - %s | FileCheck %s
+;
+; Source code:
+;   struct s { int a; int b; };
+;   struct t { int c; int d; };
+;   #define _(x) (__builtin_preserve_access_index(x))
+;   int get_value(const void *addr1, const void *addr2);
+;   int test(struct s *arg1, struct t *arg2) {
+;     return get_value(_(&arg1->b), _(&arg2->d));
+;   }
+; clang -target bpf -S -O2 -g -emit-llvm test.c
+
+%struct.s = type { i32, i32 }
+%struct.t = type { i32, i32 }
+
+; Function Attrs: nounwind
+define dso_local i32 @test(%struct.s* %arg1, %struct.t* %arg2) local_unnamed_addr #0 !dbg !7 {
+entry:
+  call void @llvm.dbg.value(metadata %struct.s* %arg1, metadata !22, metadata !DIExpression()), !dbg !24
+  call void @llvm.dbg.value(metadata %struct.t* %arg2, metadata !23, metadata !DIExpression()), !dbg !24
+  %0 = tail call i32* @llvm.preserve.struct.access.index.p0i32.p0s_struct.ss(%struct.s* %arg1, i32 1, i32 1), !dbg !25, !llvm.preserve.access.index !12
+  %1 = bitcast i32* %0 to i8*, !dbg !25
+  %2 = tail call i32* @llvm.preserve.struct.access.index.p0i32.p0s_struct.ts(%struct.t* %arg2, i32 1, i32 1), !dbg !26, !llvm.preserve.access.index !17
+  %3 = bitcast i32* %2 to i8*, !dbg !26
+  %call = tail call i32 @get_value(i8* %1, i8* %3) #4, !dbg !27
+  ret i32 %call, !dbg !28
+}
+
+; CHECK:             .section        .BTF,"",@progbits
+; CHECK:             .ascii  ".text"                 # string offset=[[SEC_INDEX:[0-9]+]]
+; CHECK-NEXT:        .byte   0
+; CHECK:             .ascii  "0:1"                   # string offset=[[ACCESS_STR:[0-9]+]]
+; CHECK-NEXT:        .byte   0
+; CHECK:             .section        .BTF.ext,"",@progbits
+; CHECK:             .long   12                      # OffsetReloc
+; CHECK-NEXT:        .long   [[SEC_INDEX]]           # Offset reloc section string offset=[[SEC_INDEX]]
+; CHECK-NEXT:        .long   2
+; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   {{[0-9]+}}
+; CHECK-NEXT:        .long   [[ACCESS_STR]]
+; CHECK-NEXT:        .long   .Ltmp{{[0-9]+}}
+; CHECK-NEXT:        .long   {{[0-9]+}}
+; CHECK-NEXT:        .long   [[ACCESS_STR]]
+
+declare dso_local i32 @get_value(i8*, i8*) local_unnamed_addr #1
+
+; Function Attrs: nounwind readnone
+declare i32* @llvm.preserve.struct.access.index.p0i32.p0s_struct.ss(%struct.s*, i32 immarg, i32 immarg) #2
+
+; Function Attrs: nounwind readnone
+declare i32* @llvm.preserve.struct.access.index.p0i32.p0s_struct.ts(%struct.t*, i32 immarg, i32 immarg) #2
+
+; Function Attrs: nounwind readnone speculatable
+declare void @llvm.dbg.value(metadata, metadata, metadata) #3
+
+attributes #0 = { nounwind "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 = { "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="all" "less-precise-fpmad"="false" "no-infs-fp-math"="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 #2 = { nounwind readnone }
+attributes #3 = { nounwind readnone speculatable }
+attributes #4 = { nounwind }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 10.0.0 (trunk 366831) (llvm/trunk 366867)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "test.c", directory: "/tmp/home/yhs/work/tests/llvm/core-bugs")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 10.0.0 (trunk 366831) (llvm/trunk 366867)"}
+!7 = distinct !DISubprogram(name: "test", scope: !1, file: !1, line: 5, type: !8, scopeLine: 5, flags: DIFlagPrototyped, isDefinition: true, isOptimized: true, unit: !0, retainedNodes: !21)
+!8 = !DISubroutineType(types: !9)
+!9 = !{!10, !11, !16}
+!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!11 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !12, size: 64)
+!12 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "s", file: !1, line: 1, size: 64, elements: !13)
+!13 = !{!14, !15}
+!14 = !DIDerivedType(tag: DW_TAG_member, name: "a", scope: !12, file: !1, line: 1, baseType: !10, size: 32)
+!15 = !DIDerivedType(tag: DW_TAG_member, name: "b", scope: !12, file: !1, line: 1, baseType: !10, size: 32, offset: 32)
+!16 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !17, size: 64)
+!17 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "t", file: !1, line: 2, size: 64, elements: !18)
+!18 = !{!19, !20}
+!19 = !DIDerivedType(tag: DW_TAG_member, name: "c", scope: !17, file: !1, line: 2, baseType: !10, size: 32)
+!20 = !DIDerivedType(tag: DW_TAG_member, name: "d", scope: !17, file: !1, line: 2, baseType: !10, size: 32, offset: 32)
+!21 = !{!22, !23}
+!22 = !DILocalVariable(name: "arg1", arg: 1, scope: !7, file: !1, line: 5, type: !11)
+!23 = !DILocalVariable(name: "arg2", arg: 2, scope: !7, file: !1, line: 5, type: !16)
+!24 = !DILocation(line: 0, scope: !7)
+!25 = !DILocation(line: 6, column: 20, scope: !7)
+!26 = !DILocation(line: 6, column: 33, scope: !7)
+!27 = !DILocation(line: 6, column: 10, scope: !7)
+!28 = !DILocation(line: 6, column: 3, scope: !7)