Skip to content

Emit debuginfo for foreign fns/statics - #164715

Open
yonghong-song wants to merge 2 commits into
rust-lang:mainfrom
yonghong-song:fix-llvm-extern
Open

yonghong-song wants to merge 2 commits into
rust-lang:mainfrom
yonghong-song:fix-llvm-extern

Conversation

@yonghong-song

@yonghong-song yonghong-song commented Oct 22, 2026 •

Copy link
Copy Markdown

View all comments

Currently rustc does not support debuginfo for foreign fns and statics
(i.e. fns and statics defined elsewhere), even with -Cdebuginfo=2. On BPF,
that means the BPF backend emits no BTF for them, but libbpf and the kernel
rely on those BTF entries in order to resolve kfuncs and ksyms.

So let us support foreign functions and statics with the following:
. Allow #[link_section] on foreign fns and statics.
. Implement debuginfo and codegen to support foreign fns and statics.
. All architectures are supported.
. Add tests with codegen-llvm, run-make object, and some others
due to this feature.

LLM Disclosure

I guided claude to support foreign fns/statics. I wrote some tests by
myself. All code changes and new tests are reviewed and tested manually.

@rustbot

rustbot commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_codegen_llvm/src/debuginfo

cc @Walnut356

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 22, 2026
@rustbot

rustbot commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the pull request, and welcome! The Rust Project has assigned @adwinwhite (or someone else) to review your changes, you should hear from them (or someone else) within the next two weeks.

Please see the contribution instructions and our LLM policy for more information.

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler, debuginfo
  • compiler, debuginfo expanded to 77 candidates
  • Random selection from 20 candidates

@rustbot

This comment has been minimized.

@yonghong-song

Copy link
Copy Markdown
Author

r? Michal Rostecki

@rustbot

rustbot commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator

Error: Failed to set assignee to Michal: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #triagebot on Zulip.

@rust-log-analyzer

This comment has been minimized.

@rustbot

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@vadorovsky

Copy link
Copy Markdown
Contributor

r? @vadorovsky @nagisa

if tcx.sess.target.arch == Arch::Bpf {
let attrs = tcx.codegen_fn_attrs(instance_def_id);
base::set_link_section(llfn, attrs);
// Declare kfuncs weak, as vmlinux.h does with `__weak __ksym`, so a kernel

@vadorovsky vadorovsky Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems to mark all the symbols in the .ksyms section as week, but I would say we still want to differentiate between weak and strong ksyms like we do in clang.

There is an #[linkage = "extern_weak"] attribute that we should honor, and in the absence of it we should mark the ksym as strong.

I will propose an exact code change later today.

View changes since the review

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I actually debate myself whether I should have default 'extern_weak' or not. The reason is the current vmlinux.h which has 'extern_weak' like below:

/* BPF kfuncs */
#ifndef BPF_NO_KFUNC_PROTOTYPES
extern void __bpf_trap(void) __weak __ksym;
extern bool __scx_bpf_dsq_insert_vtime(struct task_struct *p, struct scx_bpf_dsq_insert_vtime_args *args) __weak __ksym;
extern s32 __scx_bpf_select_cpu_and(struct task_struct *p, const struct cpumask *cpus_allowed, struct scx_bpf_select_cpu_and_args *args) __weak __ksym;
extern void bbr_cwnd_event_tx_start(struct sock *sk) __weak __ksym;
extern void bbr_init(struct sock *sk) __weak __ksym;
extern void bbr_main(struct sock *sk, u32 ack, int flag, const struct rate_sample *rs) __weak __ksym;
extern u32 bbr_min_tso_segs(struct sock *sk) __weak __ksym;
extern void bbr_set_state(struct sock *sk, u8 new_state) __weak __ksym;
extern u32 bbr_sndbuf_expand(struct sock *sk) __weak __ksym;
extern u32 bbr_ssthresh(struct sock *sk) __weak __ksym;
extern u32 bbr_undo_cwnd(struct sock *sk) __weak __ksym;
...

But agree with you that should not have '__weak' as the default in rust frontend. It is easier for users to mark it if needed. Also it may have other non kfunc functions which may not need __weak.


/// The type a static with `#[linkage]` points to: a raw pointer, or an `Option` of a reference
/// (see `check_static_linkage`). `None` if it is unsized or not one of those.
fn extern_pointee_ty<'tcx>(cx: &CodegenCx<'_, 'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {

@vadorovsky vadorovsky Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems to be a helper with only a single user. Perhaps just move the code there, to check_and_apply_linkage?

View changes since the review

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Right, it is a single user, I can inline into the call site.

);

fn convert(cx: &mut AcceptContext<'_, '_>, args: &ArgParser) -> Option<AttributeKind> {
// On BPF, an extern placed in ".ksyms" is a kfunc or ksym, as with clang's __ksym.

@vadorovsky vadorovsky Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we really need to differentiate between allowed targets for BPF and for other CPU targets? Having such a differentiation in compiler frontend is unfortunate.

Later you say "Elsewhere a section on an extern declaration has no effect" - but what does it exactly mean? If we allow compiling programs with foreign functions and statics with custom link sections for, let's say, x86_64-unknown-linux-gnu target, do we risk emitting some invalid code that can cause UBI, or is it just an useless, but harmless thing that the ELF loaders will ignore? I'm guessing the latter, but let me know if I'm wrong.

Also, the comment is specifically mentioning the .ksyms link section, but the code allows any foreign functions and statics to be placed in any link section. One could write the following BPF code:

unsafe extern "C" {
    #[unsafe(link_section = ".foo")]
    fn bar(a: u32, p: *mut c_void) -> u32;
}

That this code also has no effect, but I think that with your proposed change, the compiler would allow it. But that's not a big issue, because BPF program loader like libbpf will just ignore it.

Of course you could extend this if statement to check also for the section name, and allow only .ksyms, but I'm personally leaning towards not policing this and just allowing any foreign fns/statics in any section for any target.arch, so we don't end up bloating compiler frontend with implementation details of different CPU architectures.

View changes since the review

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hm, I'm not sure how the new attribute infrastructure treats attributes that are not relevant or applicable. Like, it is useful to inform users that they did something that doesn't actually have any effect. This was only partly the case with the old attribute infrastructure where nobody reading the attribute would produce a warning that it wasn't used. In that scheme allowing this attribute through would still be fine.

But I wouldn't be surprised if now this warning infrastructure is no longer present as the new attribute parsing scheme is much more strict about what's allowed where. And if that's the case I would be mildly reluctant to allow this attribute wholesale.

That said, I'm not too familiar with post-attr-rework guidelines for how things should be done. Maybe @jdonszelmann has better insight.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Do we really need to differentiate between allowed targets for BPF and for other CPU targets? Having such a differentiation in compiler frontend is unfortunate.

Good question. I am working on BPF so my implementation focus on BPF. I have no rejection to expand to all targets as long as people agrees.

@vadorovsky vadorovsky Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually the point about extern functions in custom sections "not taking effect outside BPF" is not even true. I tried the following experiment:

$ cat foo.c
#include <stdint.h>

__attribute__((section(".multiply")))
uint32_t multiply(uint32_t x, uint32_t y) {
  return x * y;
}

$ clang -shared -fPIC foo.c -o libfoo.so
$ cat extern.rs
unsafe extern "C" {
    #[unsafe(link_section = ".multiply")]
    fn multiply(x: u32, y: u32) -> u32;
}

pub fn main() {
    let x = unsafe { multiply(2, 3) };
    println!("x: {x}");
}

$ rustc -C link-arg=-L. -C link-arg=-lfoo extern.rs
warning: the `link_section` attribute cannot be used on foreign functions
 --> extern.rs:2:14
  |
2 |     #[unsafe(link_section = ".multiply")]
  |              ^^^^^^^^^^^^
  |
  = help: the `link_section` attribute can be applied to functions with a body and statics
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: `#[warn(unused_attributes)]` (part of `#[warn(unused)]`) on by default
                                                                                                                                                              warning: 1 warning emitted
$ LD_LIBRARY_PATH="/tmp/playground" ./extern
x: 6

The program works! Placing custom functions in custom sections is absolutely fine - both GNU ld and glibc were able to link it.

The answer to @nagisa's question is - rustc emits just a warning. But given that the program works, I think it shouldn't even warn. We should just allow link_section on extern functions indiscriminately. I suppose the same program would work on most operating systems.

@vadorovsky vadorovsky Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Even better example, with .so file emitted by Rust and with extern statics:

$ cat foo.rs
#[unsafe(no_mangle)]
#[unsafe(link_section = ".foo")]
pub static FOO: u32 = 2;

#[unsafe(no_mangle)]
#[unsafe(link_section = ".bar")]
pub static BAR: u32 = 3;

#[unsafe(no_mangle)]
#[unsafe(link_section = ".multiply")]
pub extern "C" fn multiply(x: u32, y: u32) -> u32 {
    x * y
}

$ rustc --crate-type=cdylib foo.rs -o libfoo.so
$ cat extern.rs
unsafe extern "C" {
    #[unsafe(link_section = ".foo")]
    static FOO: u32;

    #[unsafe(link_section = ".bar")]
    static BAR: u32;

    #[unsafe(link_section = ".multiply")]
    fn multiply(x: u32, y: u32) -> u32;
}

pub fn main() {
    let x = unsafe { multiply(FOO, BAR) };
    println!("x: {x}");
}

$ rustc -C link-arg=-L. -C link-arg=-lfoo extern.rs
warning: the `link_section` attribute cannot be used on foreign statics
 --> extern.rs:2:14
  |
2 |     #[unsafe(link_section = ".foo")]
  |              ^^^^^^^^^^^^
  |
  = help: the `link_section` attribute can be applied to functions and statics
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: `#[warn(unused_attributes)]` (part of `#[warn(unused)]`) on by default

warning: the `link_section` attribute cannot be used on foreign statics
 --> extern.rs:5:14
  |
5 |     #[unsafe(link_section = ".bar")]
  |              ^^^^^^^^^^^^
  |
  = help: the `link_section` attribute can be applied to functions and statics
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!

warning: the `link_section` attribute cannot be used on foreign functions
 --> extern.rs:8:14
  |
8 |     #[unsafe(link_section = ".multiply")]
  |              ^^^^^^^^^^^^
  |
  = help: the `link_section` attribute can be applied to functions with a body and statics
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!

warning: 3 warnings emitted
$ LD_LIBRARY_PATH="/tmp/playground" ./extern
x: 6

All the warnings seem useless, since everything works just fine.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

$ readelf -S libfoo.so | grep -e foo -e bar -e multiply
  [11] .bar              PROGBITS         000000000000760c  0000760c
  [12] .foo              PROGBITS         0000000000007610  00007610
  [19] .multiply         PROGBITS         000000000004a1e0  000491e0

fn wants_foreign_item_debuginfo(&self) -> bool {
self.dbg_cx.is_some()
&& self.sess().opts.debuginfo == DebugInfo::Full
&& self.sess().target.arch == Arch::Bpf

@vadorovsky vadorovsky Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Similar question here - do we really have to differentiate here? DWARF information containing a foreign fn/static might be of no effect, but it shouldn't be incorrect in a sense of breaking the standard or causing some bad behavior. I would also lean towards not policing it and emitting DI for such items indiscriminately.

View changes since the review

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The reason I have guard is because in clang frontend. See

diff --git a/clang/lib/Basic/Targets/BPF.h b/clang/lib/Basic/Targets/BPF.h
index 117f81430bf4..b2f1831e960e 100644
--- a/clang/lib/Basic/Targets/BPF.h
+++ b/clang/lib/Basic/Targets/BPF.h
@@ -76,6 +76,8 @@ public:
     return None;
   }
 
+  bool allowDebugInfoForExternalVar() const override { return true; }
+
   CallingConvCheckResult checkCallingConvention(CallingConv CC) const override {
     switch (CC) {
     default:

The default is false, and BPF is true.

I have no rejection if people want support all architectures.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think #164715 (comment) and #164715 (comment) are sufficient to conclude - let's support all architectures.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think #164715 (comment) and #164715 (comment) are sufficient to conclude - let's support all architectures.

Thanks for clarification! Let me move forward with this.


// Arguments types
if cx.sess().target.is_like_msvc {
// FIXME(#42800):

@vadorovsky vadorovsky Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems like we should try to actually fix that bug (#42800), instead of merging a workaround. I can try to do it.

View changes since the review

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is preexisting though.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Indeed, I see that. it is a good idea to fix it.

@vadorovsky vadorovsky Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's remove this workaround for now, I think it's fine to continue the work on allowing foreign fns/statics without it.

Sure, ksyms with array types as arguments will be broken, but BPF is an experimental target, and the bug itself (mismatched DI) already exists independently from BPF/BTF/ksyms. Let's fix the bug separately, and let's trim this PR to one topic.

@vadorovsky

Copy link
Copy Markdown
Contributor

There were already two PRs aiming to do the same thing:

These two being very similar to each other, and yours being a reimplementation from scratch that looks very differently. On surface, the other PRs look simplier. From the quick read, the practical differences I could notice are:

  • Your PR goes deeper with the BPF-only gating of allowing custom link sections for items.
  • You added a workaround for debuginfo: Subroutine types are not ABI-adjusted #42800, while the other PRs don't seem to address the issue at all.
    • I think the issue should be solved properly on its own. The question is if it should be a blocker for the ksyms work done in this PR. Probably not, since we can still emit BTF and it's going work as long as we don't deal with array arguments. Seems like we can fix these things separately.

Anything else I missed? Would be great to mention the difference (perhaps in the PR description).

// is called directly, only when it exists, as clang does. libbpf resolves a missing weak
// kfunc to 0, and the verifier then prunes the call.

// CHECK-LABEL: call_weak:

@nagisa nagisa Oct 26, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Any chance you could move assertions closer to the relevant code?

View changes since the review

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ack

Comment on lines +1076 to +1078
// LLVM-C's `LLVMDIBuilderCreateGlobalVariableExpression` always marks the
// variable as defined, which is wrong for an extern global that is only
// declared in this module.

@nagisa nagisa Oct 26, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This comment looks a lot like Claudeism. I don't think this comment is particularly useful, given that a single look to the C definition will reveal the difference right away.

View changes since the review

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Will remove

Comment on lines +436 to +437
// Only BPF accepts `#[link_section]` on foreign statics (".ksyms" for ksyms);
// other targets warn that it is unused, so ignore it.

@nagisa nagisa Oct 26, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Inverse reasoning. The comment is superfluous regardless as the code implies exactly what's written in the comment. Just remove it.

View changes since the review

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Will remove

/// (see `check_static_linkage`). `None` if it is unsized or not one of those.
fn extern_pointee_ty<'tcx>(cx: &CodegenCx<'_, 'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
let ptr_ty = match ty.kind() {
ty::Adt(def, args) if cx.tcx.is_lang_item(def.did(), LangItem::Option) => args.type_at(0),

@nagisa nagisa Oct 26, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

AFAIR is only valid for types that participate in layout optimization. e.g. Option<&T> is indeed just a *const T with null representing None, while Option<*const T> is a proper tagged enum.

I'm sure your LLM should be able to find a ready-made function or method or a pattern used elsewhere that's applicable here if you re-query it.

View changes since the review

&& self.sess().target.arch == Arch::Bpf
}

/// Attaches a declaration `DISubprogram` to `llfn`, the declaration of a foreign function.

@nagisa nagisa Oct 26, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This sentence is hard to parse.

How about

Suggested change
/// Attaches a declaration `DISubprogram` to `llfn`, the declaration of a foreign function.
/// Attaches `DISubprogram` to a foreign function declaration `llfn`

Please also review the other comments you've added for obvious claudeisms.

View changes since the review

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Will do. Thanks!

Comment on lines +144 to +146
if !self.wants_foreign_item_debuginfo() {
return;
}

@nagisa nagisa Oct 26, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How harmful would it be if we did this for all targets not just bpf?

View changes since the review

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

If we allow all other targets, the other targets will have debug info for those extern functions. And then other targets can decide whether they want to do anything with thoese extern function debuginfo.

In BPF, we will use them to generate BTF.

For other architectures, it depends on their specific needs. They can certainly ignore them.


// Arguments types
if cx.sess().target.is_like_msvc {
// FIXME(#42800):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is preexisting though.

@rustbot rustbot added the A-run-make Area: port run-make Makefiles to rmake.rs label Oct 30, 2026
@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

@yonghong-song yonghong-song changed the title Emit debuginfo for extern fns/statics on BPF Emit debuginfo for foreign fns/statics Oct 30, 2026
@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added A-tidy Area: The tidy tool T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Oct 30, 2026
@rust-log-analyzer

This comment has been minimized.

@yonghong-song

Copy link
Copy Markdown
Author

Okay, I pushed the second revision. The following is the update:

  • extern_weak attribute will not implicitly attach to link_section = ".ksyms". This, in user rust code, extern_weak may or may not attach to fns or statics.
  • Allow all architectures for foreign fns/statics with debuginfo.
  • One bpf exception: a bpf type cannot directly be used from the corresponding rust type, so one line change (guarded with bpf) is needed.
  • Inline a single func in consts.rs
  • To have better BTF output, a tests/run-make/btf-debuginfo/print_btf.py is added with run-make. The print_btf.py file is copies from llvm bpf selftest.
  • A couple of x86_64 tests for foreign fns/statics tests.
  • A few tests need update due to all architecture mode

One more thing, the above mentioned print_btf.py has some issues in CI, so workaround like below:

diff --git a/src/tools/tidy/config/ruff.toml b/src/tools/tidy/config/ruff.toml
index b38006f8e8c..1c97fd241c3 100644
--- a/src/tools/tidy/config/ruff.toml
+++ b/src/tools/tidy/config/ruff.toml
@@ -54,3 +54,16 @@ ignore = [
     "F403", # undefined-local-with-import-star
     "F405", # undefined-local-with-import-star-usage
 ]
+
+[lint.per-file-ignores]
+"tests/run-make/btf-debuginfo/print_btf.py" = [
+    "B023", # function-uses-loop-variable
+    "EXE001", # shebang-not-executable
+    "F821", # undefined-name
+]
+# Hack: CI runs from a subdirectory under the main checkout
+"../tests/run-make/btf-debuginfo/print_btf.py" = [
+    "B023", # function-uses-loop-variable
+    "EXE001", # shebang-not-executable
+    "F821", # undefined-name
+]

Let me know if you have better solution.

@rust-bors

This comment has been minimized.

Problem
=======

Currently rustc does not support debuginfo for foreign functions and foreign
statics (i.e. variables defined elsewhere), even with -Cdebuginfo=2. On BPF,
that means the BPF backend emits no BTF for them, but libbpf and the kernel
rely on those BTF entries in order to resolve kfuncs and ksyms.

For example, without this patch set, compiling

  unsafe extern "C" {
      #[unsafe(link_section = ".ksyms")]
      #[linkage = "extern_weak"]
      static bpf_kfunc: Option<unsafe extern "C" fn(i32, u64) -> i32>;
      #[unsafe(link_section = ".ksyms")]
      static bpf_ksym_var: i32;
      fn do_work(x: i32) -> i32;
  }

  #[no_mangle]
  pub fn prog() -> i32 { unsafe { match bpf_kfunc { Some(f) => f(1, 2), None => 0 } } }
  #[no_mangle]
  pub fn read_var() -> i32 { unsafe { bpf_ksym_var } }
  #[no_mangle]
  pub fn prog2() -> unsafe extern "C" fn(i32) -> i32 { do_work }

with --target bpfel-unknown-none -Copt-level=2 -Cdebuginfo=2, rustc will
issue a warning:

  ...
  warning: the `link_section` attribute cannot be used on foreign statics
  ...

and the compiler gives declarations with no !dbg and no section:

  @bpf_ksym_var = external local_unnamed_addr global i32
  declare extern_weak noundef i32 @bpf_kfunc(i32 noundef, i64 noundef) ...
  declare noundef i32 @do_work(i32 noundef) ...

and BTF only for the functions defined in the crate:

  [3] FUNC 'prog' type_id=1 linkage=global
  [7] FUNC 'prog2' type_id=4 linkage=global
  [9] FUNC 'read_var' type_id=8 linkage=global

with no entries for bpf_kfunc, do_work or bpf_ksym_var and no .ksyms
DATASEC, so libbpf cannot resolve the kfunc or the ksym.

How clang solved this
=====================

Clang solved this in its frontend for BPF. LLVM commit e3d8ee3
("[DebugInfo] Support to emit debugInfo for extern variables") emits a
non-defining DIGlobalVariable for extern variables, and commit
a2a3ca8 ("BPF: emit debuginfo for Function of DeclRefExpr if
requested") emits a declaration DISubprogram for extern functions
referenced by address. Both are guarded by
TargetInfo::allowDebugInfoForExternalRef, which only BPF enables.
Commit ae0d2244a235 later moved extern functions onto the same
mechanism as extern variables, so clang now covers every extern
function a BPF program references.

With the C equivalent of the example above,

  extern int bpf_kfunc(int a, unsigned long b) __weak __ksym;
  extern const int bpf_ksym_var __ksym;
  extern int do_work(int);

clang -O2 -g gives

  @bpf_ksym_var = external ... constant i32, section ".ksyms", ... !dbg !0
  declare !dbg !25 extern_weak ... i32 @bpf_kfunc(...) ... section ".ksyms"
  declare !dbg !44 ... i32 @do_work(i32 noundef) rust-lang#1

and the BTF has the entries libbpf needs:

  [6] FUNC 'bpf_kfunc' type_id=4 linkage=extern
  [11] FUNC 'do_work' type_id=10 linkage=extern
  [16] VAR 'bpf_ksym_var' type_id=15, linkage=extern
  [17] DATASEC '.ksyms' size=0 vlen=2
          type_id=6 offset=0 size=0 (FUNC 'bpf_kfunc')
          type_id=16 offset=0 size=4 (VAR 'bpf_ksym_var')

How rustc solves this
=====================

A declaration DISubprogram is attached to each declared foreign function
and a non-defining DIGlobalVariable to each declared foreign static when
compiling with full debuginfo (-Cdebuginfo=2). The BPF backend then lowers
debuginfo to BTF.

A kfunc or ksym is marked with #[link_section = ".ksyms"], as clang's
__ksym does. The kfunc or ksym can also have #[linkage = "extern_weak"].

The above warning in the 'Problem' section disappears and the 'link_section'
attribute is applied.

Where Rust differs from C, rustc does the following:

- Names: the debuginfo is named after the symbol rather than the Rust
  item name, since BTF matches extern functions and variables by name.
  This also honors #[link_name].

- void *: Rust's c_void is an enum, so pointers to c_void get void *
  debuginfo instead of a pointer to that enum; otherwise kfunc
  prototypes taking void * would not match the kernel's BTF.

Support Other Architectures
===========================

The `link_section` attribute is now accepted on foreign items on all targets,
not just BPF. The major benefits include simpler code and no or minimal
per-target checks, and all these architectures can get debuginfo
(with -Cdebuginfo=2) for foreign fns and statics. Users can decide how to deal
with debuginfo for those foreign fns and statics. As an example, some x86 tests
are added to show debuginfo from foreign fns and statics.

On BPF, `*mut c_void` needs special handling in order to have BTF compatibility.
The specific effect is to treat it like `void *`, not a pointer to Rust's
`c_void` enum.

Test results
============

With this commit, the example from "Problem" compiles without
warnings, and the declarations get debuginfo and the .ksyms section,
with bpf_kfunc declared weak:

  @bpf_ksym_var = external local_unnamed_addr global i32,
                  section ".ksyms", !dbg !0
  declare !dbg !33 extern_weak noundef i32 @bpf_kfunc(...) ...
                  section ".ksyms"
  declare !dbg !37 noundef i32 @do_work(i32 noundef) ...

The BTF now has the same extern entries and .ksyms DATASEC as clang's:

  [3] FUNC 'prog' type_id=1 linkage=global
  [6] FUNC 'bpf_kfunc' type_id=4 linkage=extern
  [10] FUNC 'prog2' type_id=7 linkage=global
  [12] FUNC 'do_work' type_id=11 linkage=extern
  [14] FUNC 'read_var' type_id=13 linkage=global
  [15] VAR 'bpf_ksym_var' type_id=2, linkage=extern
  [16] DATASEC '.ksyms' size=0 vlen=2
          type_id=6 offset=0 size=0 (FUNC 'bpf_kfunc')
          type_id=15 offset=0 size=4 (VAR 'bpf_ksym_var')

and bpf_kfunc is a weak undefined symbol in the object file:

  8: 0000000000000000  0 NOTYPE  WEAK   DEFAULT  UND bpf_kfunc
 10: 0000000000000000  0 NOTYPE  GLOBAL DEFAULT  UND do_work
 12: 0000000000000000  0 NOTYPE  GLOBAL DEFAULT  UND bpf_ksym_var

LLM Disclosure
==============

I guided claude to support foreign fns/statics. I wrote some tests by
myself. All code changes and new tests are reviewed and tested manually.
@yonghong-song

Copy link
Copy Markdown
Author

☔ The latest upstream changes (presumably #165375) made this pull request unmergeable. Please resolve the merge conflicts by rebasing.

Okay, I will rebase on latest main branch.

The following is a list of tests:

  - Codegen debuginfo tests. Tests bpf-foreign-debuginfo.rs and
    x86-foreign-debuginfo.rs are tested with debuginfo.
  - The test link-section-foreign-items.rs validates foreign
    declarations, with no debuginfo.
  - The test link-section.rs previously expected the "being phased out"
    warning. Now the test passes without a warning.
  - ui/attributes/positions/link-section-bpf.rs accepts foreign fns and
    statics with no lint.
  - run-make btf tests. A helper print_btf.py is used to dump
    easy-to-understand BTF. extern.rs covers strong foreign items, and
    extern_weak.rs covers weak ones. extern_weak.rs also checks the
    generated code and symbols.
  - Some other tests include attr-on-mac-call,
    codegen_attr_on_required_trait_method,
    cfg_attr-attr-syntax-validation and
    issue-43106-gating-of-builtin-attrs. Only the `help:` line listing
    allowed targets changed.
@rustbot

rustbot commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@rustbot

rustbot commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator

⚠️ Warning ⚠️

  • There are issue links (such as #123) in the commit messages of the following commits.
    Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtrees.

@workingjubilee workingjubilee added the llm-assisted An LLM-assisted PR as defined by the LLM policy. Requires ahead-of-time consent by assignee. label Nov 4, 2026
@workingjubilee

Copy link
Copy Markdown
Member

@yonghong-song Please make the PR description significantly more concise.

@workingjubilee

workingjubilee commented Nov 4, 2026 •

Copy link
Copy Markdown
Member

The LLM policy's experiment clause for LLM-driven code changes addresses code changes. It does not, by my reading, offer a different policy for things like PR descriptions. My understanding is those should still be human-written. Also, it asks that review is arranged prior to opening them.

This PR seems to not fulfill that, as the PR description includes blather and I do not see such a request on Zulip, at the very least. Unrelated, this PR is also a de facto duplicate with two other PRs. It also, by targeting a wider usage than just BPF, veers dangerously close to not being about "non-critical" sections of the compiler. I believe this PR should be closed.

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 5, 2026
@yonghong-song yonghong-song reopened this Nov 5, 2026
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 5, 2026
@yonghong-song

Copy link
Copy Markdown
Author

@yonghong-song Please make the PR description significantly more concise.

Sorry. This is my first rust pull request as I am not aware of such requirement. I just shorted the PR description.

@yonghong-song

Copy link
Copy Markdown
Author

The LLM policy's experiment clause for LLM-driven code changes addresses code changes. It does not, by my reading, offer a different policy for things like PR descriptions. My understanding is those should still be human-written. Also, it asks that review is arranged prior to opening them.

Actually the commit message is written by myself. Maybe I have too many examples in the PR. I guess I should ask a reviewer for a preview to ensure to have proper format, motivation, etc. before posting.

This PR seems to not fulfill that, as the PR description includes blather and I do not see such a request on Zulip, at the very least. Unrelated, this PR is also a de facto duplicate with two other PRs. It also, by targeting a wider usage than just BPF, veers dangerously close to not being about "non-critical" sections of the compiler. I believe this PR should be closed.

Sorry, this is my first patch. For the next one, I will post in Zulip to have a discussion first with a RFC. Yes, I find two other very similar PRs, both from @altugbozkurt07.

#161390 (new version)
#152899 (old version)

I will take a look at #161390 as well. I am okay to close/abandon this PR and happy to work with @altugbozkurt07 as the goal is to make this work and get merged in rust. My PR has some bpf specific corner cases and quite some tests (bpf anx x86) and this can be contributed to #161390.

@vadorovsky vadorovsky left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Left some more feedback.


The LLM policy's experiment clause for LLM-driven code changes addresses code changes. It does not, by my reading, offer a different policy for things like PR descriptions. My understanding is those should still be human-written. Also, it asks that review is arranged prior to opening them.

To be fair, @yonghong-song did contact me privately before filing a PR, I told him I would be fine with reviewing the AI-assisted PR.

He didn't make a thread on Zulip, although I don't see this channel being mentioned as a requirement in the policy. But I agree that given the impact outside BPF, it would be better if it was pre-arranged there. Sorry for that!

Unrelated, this PR is also a de facto duplicate with two other PRs.

I have asked already for listing the differences between this implementation and other two PRs, I don't see that being addressed. That said, I think it's fine to superseed the other PRs, given that there was no follow up to the comments since summer in both of them.

It also, by targeting a wider usage than just BPF, veers dangerously close to not being about "non-critical" sections of the compiler. I believe this PR should be closed.

To be honest, from the policy is not clear to me whether this change is already ciritical:

Changes that have a strong soundness impact, like the trait system, MIR building, or the query system are probably not ok.

After addressing my points, this PR should be limited to:

  • Not emitting the (useless, in my opinion) warning about extern functions/statics in custom link sections. Note that it's just the warning, and compilation of programs with these items works (and probably always worked). At this point, there are no real changes to codegen. I would argue this point is trivial.
  • Emitting debug info for these extern types. Definitely not trivial, but also with no soundness impact?

Do these qualify as critical? For me neither of those points seem to match the examples from the quote.

That said, if it ends up being closed, I volunteer to work on an implementation from scratch, without clankers.

View changes since this review

@@ -0,0 +1,295 @@
#!/usr/bin/env python3

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm wondering if we could use https://github.com/anakryiko/btfdump instead.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I tested. This works. Just not sure how we could utilize it (https://github.com/anakryiko/btfdump) in this repo.

// On BPF, treat `*mut c_void` as C's `void *`. libbpf and the kernel match kfunc
// prototypes by BTF kind, so emitting the pointee as `c_void` would not match
// a `void *` parameter. Return `None` here to avoid emitting a concrete pointee type.
_ if cx.sess().target.arch == Arch::Bpf && is_c_void(cx, pointee_type) => None,

@vadorovsky vadorovsky Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As I understand, the problem here is that the BTF produced by C kernel functions with mutable void pointers as arguments has void* as a type, and the BTF produced by Rust's extern "C" block has to match it. It does not match if the BTF type emitted by Rust says *mut void, so then the kernel will fail to find the matching function. It's a matter of acknowledging that a representation of a mutable pointer T in BTF is T* and not *mut T, regardless of which language produced it.

But I don't think this is a good solution.

First of all, this workaround is just limited to *mut void. Other pointer types would be still broken.

Then, it affects DWARF. I think type names in DWARF should remain same as in the code, and it's fine if they differ because of a language that a program was written in.

Last but not least, this is a problem specific to BTF. Given that Rust frontend doesn't really know what BTF is, and only BPF backend in LLVM does, the latter seems like a better place to do such adjustments. We could make a change in LLVM that replaces *mut T with T*. How does it sound to you? Happy to make a PR, since I have the idea in my head already.

Also, probably worth checking what pahole does for Rust mutable pointer types, and sync the appropriate change there as well.

@workingjubilee workingjubilee Nov 5, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

From the Rust perspective, the type of *mut T is supposed to be the type of T* (...well, arguably only if the pointer is a thin pointer). Usually the fact that we tend to use different strings is unimportant. In cases where the compiler's emitted string is important, because the "debuginfo" is actually real type information for a platform's built-in typechecking, then we should emit whatever string will cause the relevant programs to see them as the same.

@workingjubilee workingjubilee Nov 5, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note the same conceptually applies if there is some other part of the type tag, instead of the string, that we are mismatched on.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My understanding of this specific part of the code change is that it is only concerned with the pointee type and what it does there cannot affect whether the type is written down as *mut void or void*.

compute_debuginfo_type_name is what we would want to affect if just the string is the primary concern.

@vadorovsky vadorovsky Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

After thinking about it more, I'm turning against doing any name fixups in any compiler part (neither in Rust, nor in LLVM).

I've come to realization that such sanitization wouldn't be as simple as turning *mut T to T*. Even in the hard-coded case here, it's actually *mut c_void turned into void* (not c_void*!), so it sanitizes the type alias name from core::ffi to one matching exactly the expected type. This would become even more hairy with *mut core::ffi::c_void or *mut ffi::c_void, depending on how developers import the type.

Also, I'm not even sure if the name mismatch is even a problem for the kernel. I just assumed so by the fact that @yonghong-song added such a quirk here. But is it true? I'm going to properly check that today.

In case it's true, I think we should rather try to make kernel smarter, so it doesn't require exact arg type name match, but rather just the arg name + its type size.

@vadorovsky vadorovsky Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Qwen after reading kernel sources says it's not an issue, and that Rust's extern, despite different arg type name, would resolve just fine. Still going to check myself and give my final human judgement later today, but usually my experience with Claude and Qwen disagreeing in the topic of BPF was that Qwen was more often right. 🙃

ksyms

@yonghong-song yonghong-song Nov 6, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I did some investigation on this. I think we can remove this code " _ if cx.sess().target.arch == Arch::Bpf && is_c_void(cx, pointee_type) => None," in rustc. Instead, we can do something in bpf-linker.

In compiler/rustc_target/src/spec/base/bpf.rs, we have

linker_flavor: LinkerFlavor::Bpf

and in compiler/rustc_codegen_ssa/src/back/link.rs, we have

LinkerFlavor::Bpf => "bpf-linker"

which is bpf specific.

So we can make change in bpf-linker (https://github.com/aya-rs/bpf-linker) to
change 'pointer to enum' to 'pointer to void' if the following is satisfied
from high level:

  • di_composite_type.tag() == DW_TAG_enumeration_type
  • di_composite_type.name() == Some(b"c_void")
  • di_composite_type.namespace_path() == [b"core".as_slice(), b"ffi"]
    Related file is src/llvm/di.rs in bpf-linker repo.

We cannot keep 'ptr -> enum' in the object file. In libbpf, it will compare parameter types between bpf-generated btf vs. kernel-generated btf. In this case, bpf-generted btf from rustc, and kernel-generated btf is from gcc or clang. Here the bpf only has extern function, and kernel has implementation.

Without rustc or bpf-linker fix, the libbpf will fail since two parameter mismatch.

Do you think bpf-linker approach is a viable solution? If this is the case, I can send a patch there. I already has a prototype.

BTW, I looked at a similar diff: #161390 which only supports bpf (in most places).

@workingjubilee workingjubilee Nov 6, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I spent more time looking at the kernel documentation for BTF co-re locations.

@vadorovsky The type matching is not based on strings, as far as I can tell. It is based on a data structure that uses a specific binary encoding.

c_void is a lang_item. That means it is a magic type implemented by the compiler. If we are leaking the fact that it is a pointer to an enum into debuginfo, that is incorrect, as the type encoding should appear identical in debuginfo to void*. That is the specific definition of c_void: "it's the void part of void*".

This would remain true if c_void was defined, in the Rust code, as a struct that occupied 9000 bytes, or a newtype around i32, or anything else. The Rust source code it adorns is actually irrelevant, and treating it as relevant is the actual bug here. The fact that it is a 2-variant enum is just to make certain aspects of the implementation easier.

@workingjubilee workingjubilee Nov 6, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

To be more specific:

Fixing this in the linker is leaving the underlying bug in Rust unaddressed.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Then may be the following

" _ if is_c_void(cx, pointee_type) => None,"

will be good enough? It hides details and also applies to all architectures?


// Arguments types
if cx.sess().target.is_like_msvc {
// FIXME(#42800):

@vadorovsky vadorovsky Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's remove this workaround for now, I think it's fine to continue the work on allowing foreign fns/statics without it.

Sure, ksyms with array types as arguments will be broken, but BPF is an experimental target, and the bug itself (mismatched DI) already exists independently from BPF/BTF/ksyms. Let's fix the bug separately, and let's trim this PR to one topic.

@workingjubilee

Copy link
Copy Markdown
Member

@vadorovsky Alright. If it is prearranged, then it is probably permissible modulo the rest of the policy. The concern I raised about the PR description where it should be human-written does stand, as that seems to me to be an important check on "does anyone understand this?"

re: criticality, that was... mostly... being alarmed about apparent scope creep. I had not fully digested the PR's contents beyond "enough to realize this has expanded far beyond 1~2 platforms in impact".

I will note that we have had patch releases that, to my memory, were strongly motivated by severe-enough regressions in debuginfo generation and debuggability of programs for tier 1 platforms. Test coverage in this area is unfortunately known-inadequate because many of the tests we did have were fragile and unmaintainable.

The way we produce debuginfo can also already cause LLVM to stack overflow on some programs with many locals, though that is because they compile to something that causes the debuginfo generation code in LLVM to recurse, despite source code that might seem linear. Crashing the compiler from resource exhaustion is permissible, at least, so that part won't matter if it's not an extreme difference.

The "debuginfo" code in question for BPF being type information that has to be correct for programs to run if they use CO-RE makes all of this somewhat awkward to talk about. My understanding is this puts a nonzero correctness requirement on how our debuginfo generation works.

@workingjubilee

workingjubilee commented Nov 5, 2026 •

Copy link
Copy Markdown
Member

Actually the commit message is written by myself. Maybe I have too many examples in the PR. I guess I should ask a reviewer for a preview to ensure to have proper format, motivation, etc. before posting.

Sorry, just noticed this... Huh. Okay. My apologies?

Then yes, please and thank you. In general it is not necessary for the PR description to reprise the contents of the PR like describing specific test outputs, and such should be avoided. This is both because of the literal duplication when a test is part of a PR, and because our CI is run on every merge. So yes, the description is mostly aimed at the motivation and a verrry high-level summary, not specific steps of driving clang. I assumed it was mechanical in origin because being so comprehensive is approximately free for machines while programmers are typically averse to duplication.

@yonghong-song

Copy link
Copy Markdown
Author

Actually the commit message is written by myself. Maybe I have too many examples in the PR. I guess I should ask a reviewer for a preview to ensure to have proper format, motivation, etc. before posting.

Sorry, just noticed this... Huh. Okay. My apologies?

Then yes, please and thank you. In general it is not necessary for the PR description to reprise the contents of the PR like describing specific test outputs, and such should be avoided. This is both because of the literal duplication when a test is part of a PR, and because our CI is run on every merge. So yes, the description is mostly aimed at the motivation and a verrry high-level summary, not specific steps of driving clang. I assumed it was mechanical in origin because being so comprehensive is approximately free for machines while programmers are typically averse to duplication.

No problem. Thanks! I am working in llvm and kernel/bpf, sometimes we do have long commit message to have detailed explain. Here, in rust, I will keep commit message short as this is the convention here.

@yonghong-song

Copy link
Copy Markdown
Author

For this

In case it's true, I think we should rather try to make kernel smarter, so it doesn't require exact arg type name match, but rather just the arg name + its type size.

A kernel only solution will be tricky because clang frontend may generate similar code pattern as rustc and it will be tricky to distinguish in libbpf. In general, libbpf may not want to relax the checking condition ...

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-run-make Area: port run-make Makefiles to rmake.rs A-tidy Area: The tidy tool llm-assisted An LLM-assisted PR as defined by the LLM policy. Requires ahead-of-time consent by assignee. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants