Re: bpf_core_type_id_kernel is not consistent with bpf_core_type_id_local

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 11/1/23 7:16 PM, Andrii Nakryiko wrote:
On Wed, Nov 1, 2023 at 5:34 PM Yonghong Song <yonghong.song@xxxxxxxxx> wrote:

On 11/1/23 3:42 PM, Andrii Nakryiko wrote:
On Wed, Nov 1, 2023 at 7:18 AM Lorenz Bauer <lorenz.bauer@xxxxxxxxxxxxx> wrote:
On Tue, Oct 31, 2023 at 6:24 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
Did you get round to fixing this, or did you decide to leave it as is?
Trying to recall, was there anything to do on the libbpf side, or was
it purely a compiler-side change?
I'm not 100% sure TBH. I'd like clang to behave consistently for
local_id and target_id. I don't know whether that would break libbpf.

*checks code* libbpf just passes through whatever ID compiler
generated, so there doesn't seem to be any change to libbpf. Seems
like compiler-only change. cc'ing Eduard  as well, if he's curious
enough to check
Okay, let us try to have a consistent behavior in local/remote type_id
by changing local_id semantics to be the same as target_id.

The corresponding llvm change is similar to

[yhs@devbig309.ftw3 ~/work/llvm-project (ed)]$ git diff
diff --git a/llvm/lib/Target/BPF/BPFPreserveDIType.cpp b/llvm/lib/Target/BPF/BPFPreserveDIType.cpp
index 78e1bf90f1bd..1fbe1207dc6e 100644
--- a/llvm/lib/Target/BPF/BPFPreserveDIType.cpp
+++ b/llvm/lib/Target/BPF/BPFPreserveDIType.cpp
@@ -86,15 +86,17 @@ static bool BPFPreserveDITypeImpl(Function &F) {
         Reloc = BTF::BTF_TYPE_ID_LOCAL;
       } else {
         Reloc = BTF::BTF_TYPE_ID_REMOTE;
-      DIType *Ty = cast<DIType>(MD);
-      while (auto *DTy = dyn_cast<DIDerivedType>(Ty)) {
-        unsigned Tag = DTy->getTag();
-        if (Tag != dwarf::DW_TAG_const_type &&
-            Tag != dwarf::DW_TAG_volatile_type)
-          break;
-        Ty = DTy->getBaseType();
-      }
+    }
+    DIType *Ty = cast<DIType>(MD);
+    while (auto *DTy = dyn_cast<DIDerivedType>(Ty)) {
+      unsigned Tag = DTy->getTag();
+      if (Tag != dwarf::DW_TAG_const_type &&
+          Tag != dwarf::DW_TAG_volatile_type)
+        break;
+      Ty = DTy->getBaseType();
+    }

+    if (Reloc == BTF::BTF_TYPE_ID_REMOTE) {
         if (Ty->getName().empty()) {
           if (isa<DISubroutineType>(Ty))
             report_fatal_error(
@@ -102,8 +104,8 @@ static bool BPFPreserveDITypeImpl(Function &F) {
           else
             report_fatal_error("Empty type name for BTF_TYPE_ID_REMOTE reloc");
         }
-      MD = Ty;
       }
+    MD = Ty;

       BasicBlock *BB = Call->getParent();
       IntegerType *VarType = Type::getInt64Ty(BB->getContext());

Either Eduard or Myself will submit a llvm patch to fix this in llvm18.

The change is merged into upstream llvm-project trunk ('main' branch):

https://github.com/llvm/llvm-project/commit/32e35b21b5971cc939b1de1194145d9b934fcb54


Sounds good, and thank you!


Lorenz




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux