Page MenuHomePhabricator

yonghong-song (Yonghong Song)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 4 2018, 11:50 PM (141 w, 1 d)

Recent Activity

Mon, Jun 7

yonghong-song committed rG8ce45f972834: BPF: fix relocation types in lib/Object/RelocationResolver.cpp (authored by yonghong-song).
BPF: fix relocation types in lib/Object/RelocationResolver.cpp
Mon, Jun 7, 9:00 PM
yonghong-song closed D103864: BPF: fix relocation types in lib/Object/RelocationResolver.cpp.
Mon, Jun 7, 8:59 PM · Restricted Project
yonghong-song requested review of D103864: BPF: fix relocation types in lib/Object/RelocationResolver.cpp.
Mon, Jun 7, 5:27 PM · Restricted Project
yonghong-song added a comment to D61524: [BPF] Support for compile once and run everywhere.

@aeubanks Thanks for the background information. Please feel free to make changes so that BPFAbstractMemberAccess.cpp does not use getPointerElementType. For example, if needed you could extend IR intrinsics to pass additional information from clang frontend codegen to llvm. I am happy to do a code review then.

Mon, Jun 7, 9:26 AM · Restricted Project

Fri, Jun 4

yonghong-song added a comment to D61524: [BPF] Support for compile once and run everywhere.

Sorry to come to this years later, but the new intrinsics look at the base pointer's pointee type to determine size. The whole point of the opaque pointers transition is that pointee types are meaningless (this was sorta mentioned in the review comments). GEP instructions have an explicit type as part of the instruction to know what type the base pointer is pointing to. I think it's a bit harder to associate intrinsics with a type. We could have a dummy argument with an overloaded type, but that's a bit weird. Any ideas?

Fri, Jun 4, 12:17 AM · Restricted Project

Thu, Jun 3

yonghong-song added a comment to D103667: [WIP] BPF: add support for DWARF_AT_LLVM_annotations attribute.

Is the C source syntax already fixed? Could it use a more specific name - then maybe wouldn't need the flag to opt-in if the only places the attribute appears is where it should be propagated?

Thu, Jun 3, 11:57 PM · Restricted Project, Restricted Project, debug-info
yonghong-song added a comment to D101336: [LLD][BPF] Add bpf support.

@MaskRay Did you get some time to look at the new revision?

Thu, Jun 3, 9:31 PM · lld, Restricted Project
yonghong-song requested review of D103667: [WIP] BPF: add support for DWARF_AT_LLVM_annotations attribute.
Thu, Jun 3, 9:29 PM · Restricted Project, Restricted Project, debug-info

Wed, Jun 2

yonghong-song added a comment to D103549: [POC] Put annotation strings into debuginfo..

@dblaikie Thanks for the pointer! I will work on the new patch, starting with DW_AT_BTF_annotation (to limit the scope where annotations will be processed) and post a new patch soon.

Wed, Jun 2, 5:18 PM · Restricted Project, Restricted Project, debug-info
yonghong-song added a comment to D103549: [POC] Put annotation strings into debuginfo..

As for supporting it in DWARF, probably with a custom attribute (DW_AT_BTF_annotation? (or "LLVM" instead of "BTF" perhaps, I'm not sure)) with a standard form (DW_FORM_strp/strxN/etc - the usual way we emit strings).

Wed, Jun 2, 12:41 PM · Restricted Project, Restricted Project, debug-info
yonghong-song requested review of D103549: [POC] Put annotation strings into debuginfo..
Wed, Jun 2, 12:06 PM · Restricted Project, Restricted Project, debug-info

Tue, May 25

yonghong-song updated the diff for D101336: [LLD][BPF] Add bpf support.
Tue, May 25, 11:25 AM · lld, Restricted Project
yonghong-song committed rG6a2ea84600ba: BPF: Add more relocation kinds (authored by yonghong-song).
BPF: Add more relocation kinds
Tue, May 25, 8:20 AM
yonghong-song closed D102712: BPF: Add more relocation kinds.
Tue, May 25, 8:20 AM · Restricted Project

Mon, May 24

yonghong-song updated the diff for D102712: BPF: Add more relocation kinds.
  • add one test case to generate R_BPF_64_ABS64 relocation for global data.
Mon, May 24, 11:59 AM · Restricted Project

Sun, May 23

yonghong-song updated the diff for D102712: BPF: Add more relocation kinds.
  • enumerate all relocations in RuntimeDyld.c such that R_BPF_64_{ABS64,ABS32} relocations will be resolved and others will be ignored.
  • added one more test case that R_BPF_64_ABS64 is properly resolved.
Sun, May 23, 11:57 AM · Restricted Project

Fri, May 21

yonghong-song updated the diff for D102712: BPF: Add more relocation kinds.
  • use FK_SecRel_8 Fixup kind for LD_imm64 operands instead of FK_SecRel_4.
Fri, May 21, 4:48 PM · Restricted Project
yonghong-song added inline comments to D102712: BPF: Add more relocation kinds.
Fri, May 21, 4:32 PM · Restricted Project
yonghong-song updated the diff for D102712: BPF: Add more relocation kinds.
  • add a test for ExecutionEngine/RuntimeDyld for added BPF relocation case. Without the patch in RuntimeDyldELF.cpp, the test will crash
  • rename R_BPF_64_ALLOC32 to R_BPF_64_NODYLD32.
  • added @MaskRay as the reviewer
  • remaining known issues to be resolved:
    • can BPF backend use FK_SecRel_4 for R_BPF_64_64?
    • Is the relocation name R_BPF_64_NODYLD32 okay or we could have a better one?
Fri, May 21, 8:14 AM · Restricted Project

Thu, May 20

yonghong-song added inline comments to D102712: BPF: Add more relocation kinds.
Thu, May 20, 8:20 PM · Restricted Project
yonghong-song added inline comments to D102712: BPF: Add more relocation kinds.
Thu, May 20, 8:17 PM · Restricted Project

May 19 2021

yonghong-song updated the diff for D102712: BPF: Add more relocation kinds.
  • rename R_BPF_64_D{32,64} to R_BPF_64_ABS{32,64} to better reflect the intention of relocation. *ABS{32,64} names have been used by arm, aarch64 and amdgpu.
May 19 2021, 9:27 PM · Restricted Project
yonghong-song updated the diff for D102712: BPF: Add more relocation kinds.
May 19 2021, 7:51 PM · Restricted Project

May 18 2021

yonghong-song updated the diff for D102712: BPF: Add more relocation kinds.
  • removed FK_SecRel_8 and used FK_SecRel_4 for LD_imm64 fixup kind which reflects better that fact only the first 4-byte insn->off is allowed for storing the section offset.
May 18 2021, 11:11 PM · Restricted Project
yonghong-song updated the diff for D102712: BPF: Add more relocation kinds.
  • add more comments and descriptions for relocations.
May 18 2021, 7:08 PM · Restricted Project
yonghong-song added inline comments to D102712: BPF: Add more relocation kinds.
May 18 2021, 5:36 PM · Restricted Project
yonghong-song added a comment to D102712: BPF: Add more relocation kinds.

These ELF flags should be documented somewhere. Even a draft will help.

For example, riscv is using https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md#relocations , though not as sophisticated as other psABIs (x86-64,aarch64,ppc64 ELFv2)

May 18 2021, 5:34 PM · Restricted Project
yonghong-song added a comment to D101336: [LLD][BPF] Add bpf support.

Discussed with Alexei, we are going to add more relocations in BPF backend. Here is the patch https://reviews.llvm.org/D102712. You can take a look if you are interested.
Once it or its variant is merged, I will remove getImplicitAddendAlloc() and relocateAlloc(). Then we will have no non-target changes except disabling -ffunction-sections for BPF LTO relocatable mode. Thanks!

May 18 2021, 1:11 PM · lld, Restricted Project
yonghong-song updated the summary of D102712: BPF: Add more relocation kinds.
May 18 2021, 12:58 PM · Restricted Project
yonghong-song added a comment to D102712: BPF: Add more relocation kinds.

This patch will require bpf-next libbpf patch like below:

diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index ee426226928f..ccc5d4591801 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -28,6 +28,12 @@
 #ifndef R_BPF_64_64
 #define R_BPF_64_64 1
 #endif
+#ifndef R_BPF_64_LD64
+#define R_BPF_64_LD64 2
+#endif
+#ifndef R_BPF_64_PC32
+#define R_BPF_64_PC32 3
+#endif
 #ifndef R_BPF_64_32
 #define R_BPF_64_32 10
 #endif
diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
index b594a88620ce..3e3cd64025dd 100644
--- a/tools/lib/bpf/linker.c
+++ b/tools/lib/bpf/linker.c
@@ -892,7 +892,8 @@ static int linker_sanity_check_elf_relos(struct src_obj *obj, struct src_sec *se
                size_t sym_idx = ELF64_R_SYM(relo->r_info);
                size_t sym_type = ELF64_R_TYPE(relo->r_info);
May 18 2021, 12:46 PM · Restricted Project
yonghong-song requested review of D102712: BPF: Add more relocation kinds.
May 18 2021, 12:36 PM · Restricted Project

May 17 2021

yonghong-song updated subscribers of D101336: [LLD][BPF] Add bpf support.

@alessandrod FYI. This is to support BPF in lld. Maybe this may help your rust bpf-linker work?

May 17 2021, 11:50 AM · lld, Restricted Project
yonghong-song updated the diff for D101336: [LLD][BPF] Add bpf support.
  • removed hack in InputSection.cpp to prevent emitting relocations for relocatable BPF.
  • added two new target hooks to differentiate between ALLOC and non-ALLOC as BPF targets need them to take different actions.
  • implement target relocate*() and getImplicitAddend*() properly.
May 17 2021, 11:47 AM · lld, Restricted Project
yonghong-song added a comment to D101336: [LLD][BPF] Add bpf support.

For

I am also very concerned with potentially other differently interpreted concepts, e.g. the re-defined STV_INTERNAL (https://lists.llvm.org/pipermail/cfe-dev/2021-May/068134.html).

This would potentially diverge from other implementations and add some symbol resolution complexity to the generic code (say, we would have to check STV_INTERNAL in Symbols.cpp just for BPF)

May 17 2021, 11:43 AM · lld, Restricted Project
yonghong-song added a comment to D101336: [LLD][BPF] Add bpf support.

For this Why is this <unknown>? if you remove if (config->emachine != EM_BPF && type != target->noneRel), this is because I didn't implement BPF target relocate properly. In current llvm BPF backend, BPFAsmBackend.cpp file, we have

void BPFAsmBackend::applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
                               const MCValue &Target,
                               MutableArrayRef<char> Data, uint64_t Value,
                               bool IsResolved,
                               const MCSubtargetInfo *STI) const {
  if (Fixup.getKind() == FK_SecRel_4 || Fixup.getKind() == FK_SecRel_8) {
    // The Value is 0 for global variables, and the in-section offset
    // for static variables. Write to the immediate field of the inst.
    assert(Value <= UINT32_MAX);
    support::endian::write<uint32_t>(&Data[Fixup.getOffset() + 4],
                                     static_cast<uint32_t>(Value),
                                     Endian);
  } else if (Fixup.getKind() == FK_Data_4) {
    support::endian::write<uint32_t>(&Data[Fixup.getOffset()], Value, Endian);
  } else if (Fixup.getKind() == FK_Data_8) {
    support::endian::write<uint64_t>(&Data[Fixup.getOffset()], Value, Endian);
  } else if (Fixup.getKind() == FK_PCRel_4) {
    Value = (uint32_t)((Value - 8) / 8);
    if (Endian == support::little) {
      Data[Fixup.getOffset() + 1] = 0x10;
      support::endian::write32le(&Data[Fixup.getOffset() + 4], Value);
    } else {
      Data[Fixup.getOffset() + 1] = 0x1;
      support::endian::write32be(&Data[Fixup.getOffset() + 4], Value);
    } 
  } else {
    assert(Fixup.getKind() == FK_PCRel_2);
    Value = (uint16_t)((Value - 8) / 8);
    support::endian::write<uint16_t>(&Data[Fixup.getOffset() + 2], Value,
                                     Endian);
  }                                  
}
May 17 2021, 8:43 AM · lld, Restricted Project

May 16 2021

yonghong-song committed rG833e9b2ea7a7: [BPF] add support for 32 bit registers in inline asm (authored by alessandrod).
[BPF] add support for 32 bit registers in inline asm
May 16 2021, 11:03 AM
yonghong-song closed D102118: [BPF] add support for 32 bit registers in inline asm.
May 16 2021, 11:03 AM · Restricted Project, Restricted Project
yonghong-song added a comment to D102118: [BPF] add support for 32 bit registers in inline asm.

Sure. I will merge the patch for you.

May 16 2021, 9:18 AM · Restricted Project, Restricted Project

May 15 2021

yonghong-song accepted D102118: [BPF] add support for 32 bit registers in inline asm.

LGTM except one minor issue. Please do address the nit before merging. Thanks!

May 15 2021, 9:25 AM · Restricted Project, Restricted Project

May 12 2021

yonghong-song updated the diff for D101336: [LLD][BPF] Add bpf support.
  • Addressed @MaskRay's comments. Thanks for quick reviewing!
May 12 2021, 8:46 PM · lld, Restricted Project

May 11 2021

yonghong-song updated the diff for D101336: [LLD][BPF] Add bpf support.

handling static functions (int a different section) properly. static variables should be similar.

May 11 2021, 7:55 PM · lld, Restricted Project
yonghong-song updated subscribers of D101336: [LLD][BPF] Add bpf support.
May 11 2021, 11:02 AM · lld, Restricted Project
yonghong-song updated the diff for D101336: [LLD][BPF] Add bpf support.

add tests/docs and target hook emitFuncDataSections(). Removed WIP tag.
@MaskRay Could you take a look? Thanks!

May 11 2021, 11:01 AM · lld, Restricted Project

May 9 2021

yonghong-song updated subscribers of D102118: [BPF] add support for 32 bit registers in inline asm.
May 9 2021, 11:24 AM · Restricted Project, Restricted Project
yonghong-song added a comment to D102118: [BPF] add support for 32 bit registers in inline asm.

Generally looks good. One suggestion and a few nits. Thanks!

May 9 2021, 11:24 AM · Restricted Project, Restricted Project

May 6 2021

yonghong-song added a comment to D102036: BPF: fix FIELD_EXISTS relocation with array subscripts.

That is true that bar[1] is not an field. We also support field relocation like bar[1] since if sizeof(bar[1]) changed we can adjust offset properly. So there is
a precedence to support array like bar[1] for *field* relocations. So I think supporting "bar[1]" for existence relocation is not a bad thing.

May 6 2021, 11:04 PM · Restricted Project
yonghong-song committed rG605c811d2b0f: BPF: fix FIELD_EXISTS relocation with array subscripts (authored by yonghong-song).
BPF: fix FIELD_EXISTS relocation with array subscripts
May 6 2021, 10:42 PM
yonghong-song closed D102036: BPF: fix FIELD_EXISTS relocation with array subscripts.
May 6 2021, 10:42 PM · Restricted Project
yonghong-song requested review of D102036: BPF: fix FIELD_EXISTS relocation with array subscripts.
May 6 2021, 5:05 PM · Restricted Project

May 5 2021

yonghong-song accepted D101866: [BPF][Test] Disable codegen test on AIX.

Ok, sounds good then.

May 5 2021, 7:36 PM · Restricted Project
yonghong-song added a comment to D101866: [BPF][Test] Disable codegen test on AIX.

This looks better. Do you have a plan to eventually remove this fix as it really looks incompatible with other platforms?

May 5 2021, 7:32 PM · Restricted Project
yonghong-song added a comment to D101866: [BPF][Test] Disable codegen test on AIX.

Toolchain can choose whether they want to override the default handling of triple. AIX toolchain choose not to override and use the default one.

Maybe it is better to do what other platform is doing? What is the advantage of not overriding the default triple for not supported platform?

May 5 2021, 5:24 PM · Restricted Project
yonghong-song added a comment to D101866: [BPF][Test] Disable codegen test on AIX.

If I understand correctly, on AIX system, BPF is not supported, so if -march=bpf is specified, it will internally expand to -mtriple=bpfel-ibm-aix and the test will fail since it is not supported, if llc uses -mtriple=bpfel and then llvm understands it is not really related to AIX and it is fine, is that right?

May 5 2021, 4:40 PM · Restricted Project

May 4 2021

yonghong-song added a comment to D101866: [BPF][Test] Disable codegen test on AIX.

We have a lot of tests using -march=bpf[eleb], why only these two tests? Is this due to the fact llvm-objdump won't work any more? Also, we have quite some objdump-* tests,

$ ls objdump*.ll
objdump_atomics.ll    objdump_cond_op.ll  objdump_imm_hex.ll     objdump_nop.ll         objdump_trivial.ll
objdump_cond_op_2.ll  objdump_dis_all.ll  objdump_intrinsics.ll  objdump_static_var.ll  objdump_two_funcs.ll

They all have similar llc | llvm-objdump | FileCheck test commands? Do other objdump_* tests also need to change to triple?

May 4 2021, 9:44 PM · Restricted Project

Apr 26 2021

yonghong-song requested review of D101336: [LLD][BPF] Add bpf support.
Apr 26 2021, 5:38 PM · lld, Restricted Project
yonghong-song committed rGbba7338b8f5d: BPF: generate BTF info for LD_imm64 loaded function pointer (authored by yonghong-song).
BPF: generate BTF info for LD_imm64 loaded function pointer
Apr 26 2021, 5:24 PM
yonghong-song closed D100568: BPF: generate BTF info for LD_imm64 loaded function pointer.
Apr 26 2021, 5:23 PM · Restricted Project
yonghong-song committed rGa2a3ca8d9796: BPF: emit debuginfo for Function of DeclRefExpr if requested (authored by yonghong-song).
BPF: emit debuginfo for Function of DeclRefExpr if requested
Apr 26 2021, 4:57 PM
yonghong-song closed D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.
Apr 26 2021, 4:57 PM · Restricted Project, debug-info
yonghong-song added inline comments to D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.
Apr 26 2021, 3:36 PM · Restricted Project, debug-info
yonghong-song updated the diff for D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.

formatting to mode "DI = CGM.getModuleDebugInfo()" and use cast<...> instead of dyn_cast<...> since there should be no possibility of null ptr.

Apr 26 2021, 3:35 PM · Restricted Project, debug-info
yonghong-song updated the diff for D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.
  • use stripPointerCasts() to remove the cast of DeclRefExpr pointer
Apr 26 2021, 2:22 PM · Restricted Project, debug-info
yonghong-song added a comment to D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.

Looks like stripPointerCasts() is enough to make it work (no null Function pointer any more). This is consistent with

llvm::Constant *CodeGenModule::GetOrCreateLLVMFunction(
    StringRef MangledName, llvm::Type *Ty, GlobalDecl GD, bool ForVTable,
    bool DontDefer, bool IsThunk, llvm::AttributeList ExtraAttrs,
    ForDefinition_t IsForDefinition) {
  const Decl *D = GD.getDecl();
...
  // Make sure the result is of the requested type.
  if (!IsIncompleteFunction) {
    assert(F->getFunctionType() == Ty);
    return F;
  }
Apr 26 2021, 2:11 PM · Restricted Project, debug-info
yonghong-song added inline comments to D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.
Apr 26 2021, 12:06 PM · Restricted Project, debug-info
yonghong-song updated the diff for D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.
  • somehow previous test case didn't trigger nullptr for Fn. Replaced it with a working test case.

With the following change,

diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index a784aade88da..fc86c1a72056 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -2841,6 +2841,7 @@ LValue CodeGenFunction::EmitDeclRefLValue(const DeclRefExpr *E) {
     if (getContext().getTargetInfo().allowDebugInfoForExternalRef()) {
       CGDebugInfo *DI = CGM.getModuleDebugInfo();
       auto *Fn = dyn_cast<llvm::Function>(LV.getPointer(*this));
+      fprintf(stderr, "Fn = %p\n", (void *)Fn);
       if (DI && Fn && !Fn->getSubprogram())
         DI->EmitFunctionDecl(FD, FD->getLocation(), T, Fn);
     }

I have the following for the added test case,

$ clang -cc1 -x c -debug-info-kind=limited -triple bpf-linux-gnu -emit-llvm debug-info-extern-callback.c
Fn = 0x7afcf28
Fn = 0x7afce98
Fn = (nil)
Apr 26 2021, 12:05 PM · Restricted Project, debug-info
yonghong-song added a comment to D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.

@dblaikie ping. Did you get a chance of looking at it?

Apr 26 2021, 10:52 AM · Restricted Project, debug-info

Apr 20 2021

yonghong-song added a comment to D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.

ping @dblaikie could you take a look of my new investigation?

Apr 20 2021, 11:02 AM · Restricted Project, debug-info

Apr 19 2021

yonghong-song added a comment to D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.

@dblaikie I did some investigation, found the root cause and provided an alternative solution. Could you check whether you are okay with either approach?

Apr 19 2021, 6:12 PM · Restricted Project, debug-info

Apr 18 2021

yonghong-song added a comment to D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.

Did some further investigation, the following is the callchain:

EmitFunctionDeclLValue
   EmitFunctionDeclPointer
       GetAddrOfFunction
           GetOrCreateLLVMFunction
Apr 18 2021, 11:50 PM · Restricted Project, debug-info
yonghong-song added a comment to D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.

To answer one of your questions above, if there is a function definition before, dyn_cast<llvm::Function>(...) will return nullptr. I tried starting from "Value" class and found dyn_cast to llvm::ConstantExpr will result in a non-null object, but I did not trace down further with subclasses of llvm::ConstantExpr.

Apr 18 2021, 11:27 AM · Restricted Project, debug-info
yonghong-song added a comment to D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.

Sorry, I know what is the segfault now after some debugging. It is because auto *Fn = dyn_cast<llvm::Function>(LV.getPointer(*this)); is a NULL pointer after there is a definition before this.

Hmm, not sure I follow - is LV.getPointer(*this) returning null, or is the dyn_cast returning null because the thing isn't an llvm::Function? If it's not a function, what is it?

Apr 18 2021, 11:23 AM · Restricted Project, debug-info

Apr 17 2021

yonghong-song added a comment to D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.

I did some debugging on why DeclRefExpr evaluated not to be a Function pointer. Here is what I got.
I made the following clang change to dump out the pointer to the emited value ('&foo'):

Apr 17 2021, 10:16 AM · Restricted Project, debug-info
yonghong-song updated the diff for D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.
  • add checking CGDebugInfo pointer. In my original patch but got lost in the revision.
Apr 17 2021, 10:06 AM · Restricted Project, debug-info

Apr 16 2021

yonghong-song updated the diff for D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.
  • isDefined() is not necessary, segfault is caused by a nullptr by casting a LV.getPointer(). Check nullptr instead.
Apr 16 2021, 3:28 PM · Restricted Project, debug-info
yonghong-song added a comment to D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.

Sorry, I know what is the segfault now after some debugging. It is because auto *Fn = dyn_cast<llvm::Function>(LV.getPointer(*this)); is a NULL pointer after there is a definition before this. Will pose another patch but with test case remains to cover mix of declaration, pointer reference and definition.

Apr 16 2021, 3:14 PM · Restricted Project, debug-info
yonghong-song added a comment to D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.

Generally looks OK, but could you explain more about the problems with/motivation for the declaration/definition issue? (probably worth noting in the patch description maybe comment in code and test)

Apr 16 2021, 2:49 PM · Restricted Project, debug-info
yonghong-song added a comment to D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.

@dblaikie I renamed the variable, added a few guards to ensure safety and avoid unnecessary attempt to generate duplicated debuginfo, and enhanced the test case to cover mix of declaration, accessing extern func address and definition of the function. Could you take a look?

Apr 16 2021, 1:02 PM · Restricted Project, debug-info
yonghong-song updated the diff for D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.

fix a clang-format warning

Apr 16 2021, 12:34 PM · Restricted Project, debug-info

Apr 15 2021

yonghong-song updated the diff for D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.
  • checked both !FD->isDefined() and !Fn->getSubprogram() before emitting debuginfo for the declaration
  • added additional tests to mix DeclRefExpr and actual function definition.
Apr 15 2021, 11:53 PM · Restricted Project, debug-info
yonghong-song updated the diff for D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.
  • check Fn->getSubprogram() before emit debuginfo. This is consistent with EmitFuncDeclForCallSite() and is better than checking FD->isDefined() as we may hit multiple DeclRefExpr and all of them will go through emitting debuginfo.
Apr 15 2021, 11:09 PM · Restricted Project, debug-info
yonghong-song updated the diff for D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.

check FD->isDefined() as well before emit debuginfo for the declaration. It is okay to emit a declaration subprogram and later refined to be with definition. But it is not okay to refine a definition to a declaration. So add check here.

Apr 15 2021, 10:41 PM · Restricted Project, debug-info
yonghong-song updated the diff for D100568: BPF: generate BTF info for LD_imm64 loaded function pointer.
  • change the test case to have a section name for the extern func which should be the most common use case. We already have tests for extern func (direct call instead of callback) which called the same internal llvm function, so the case without section name should be covered as well.
Apr 15 2021, 5:26 PM · Restricted Project
yonghong-song updated the diff for D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.

Rename TargetInfo.allowDebugInfoForExternalVar to TargetInfo.allowDebugInfoForExternalRef.

Apr 15 2021, 5:00 PM · Restricted Project, debug-info
yonghong-song added inline comments to D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.
Apr 15 2021, 3:49 PM · Restricted Project, debug-info
yonghong-song added a comment to D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.

For the first example, actually clang is smart enough to remove all dead code, so nothing generated.

Apr 15 2021, 3:44 PM · Restricted Project, debug-info
yonghong-song added a comment to D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.

Yes.

Apr 15 2021, 1:53 PM · Restricted Project, debug-info
yonghong-song added a comment to D100568: BPF: generate BTF info for LD_imm64 loaded function pointer.

The corresponding clang patch is here: https://reviews.llvm.org/D100567

Apr 15 2021, 8:29 AM · Restricted Project
yonghong-song added a comment to D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.

The corresponding llvm patch is here: https://reviews.llvm.org/D100568

Apr 15 2021, 8:28 AM · Restricted Project, debug-info
yonghong-song requested review of D100568: BPF: generate BTF info for LD_imm64 loaded function pointer.
Apr 15 2021, 8:28 AM · Restricted Project
yonghong-song requested review of D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested.
Apr 15 2021, 8:20 AM · Restricted Project, debug-info

Apr 13 2021

yonghong-song committed rGa285bdb56fb4: BPF: remove default .extern data section (authored by yonghong-song).
BPF: remove default .extern data section
Apr 13 2021, 11:38 AM
yonghong-song closed D100392: BPF: remove default .extern data section.
Apr 13 2021, 11:38 AM · Restricted Project
yonghong-song requested review of D100392: BPF: remove default .extern data section.
Apr 13 2021, 9:30 AM · Restricted Project
yonghong-song committed rG968292cb9319: BPF: generate proper BTF for globals with WeakODRLinkage (authored by yonghong-song).
BPF: generate proper BTF for globals with WeakODRLinkage
Apr 13 2021, 8:54 AM
yonghong-song closed D100362: BPF: generate proper BTF for globals with WeakODRLinkage.
Apr 13 2021, 8:54 AM · Restricted Project

Apr 12 2021

yonghong-song requested review of D100362: BPF: generate proper BTF for globals with WeakODRLinkage.
Apr 12 2021, 10:59 PM · Restricted Project

Mar 25 2021

yonghong-song committed rG886f9ff53155: BPF: add extern func to data sections if specified (authored by yonghong-song).
BPF: add extern func to data sections if specified
Mar 25 2021, 4:04 PM
yonghong-song closed D93563: BPF: add extern func to data sections if specified.
Mar 25 2021, 4:04 PM · Restricted Project

Mar 11 2021

yonghong-song committed rG379d90884807: BPF: provide better error message for unsupported atomic operations (authored by yonghong-song).
BPF: provide better error message for unsupported atomic operations
Mar 11 2021, 7:27 PM
yonghong-song closed D98471: BPF: provide better error message for unsupported atomic operations.
Mar 11 2021, 7:27 PM · Restricted Project