Skip to content

Conversation

@yonghong-song
Copy link
Contributor

Nikita Popov reported an issue ([1]) where a dangling weak symbol __bpf_trap is in the final binary and this caused libbpf failing like below:

  $ veristat -v ./t.o
  Processing 't.o'...
  libbpf: elf: skipping unrecognized data section(4) .eh_frame
  libbpf: elf: skipping relo section(5) .rel.eh_frame for section(4) .eh_frame
  libbpf: failed to find BTF for extern '__bpf_trap': -3
  Failed to open './t.o': -3

In llvm, the dag selection phase generates __bpf_trap in code. Later the UnreachableBlockElim pass removed __bpf_trap from the code, but __bpf_trap symbol survives in the symbol table.

Having a dangling __bpf_trap weak symbol is not good for old kernels as seen in the above veristat failure. Although users could use compiler flag -mllvm -bpf-disable-trap-unreachable to workaround the issue, this patch fixed the issue by removing the dangling __bpf_trap.

[1] #165696

@github-actions
Copy link

github-actions bot commented Nov 1, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Nikita Popov reported an issue ([1]) where a dangling weak symbol
__bpf_trap is in the final binary and this caused libbpf failing
like below:

  $ veristat -v ./t.o
  Processing 't.o'...
  libbpf: elf: skipping unrecognized data section(4) .eh_frame
  libbpf: elf: skipping relo section(5) .rel.eh_frame for section(4) .eh_frame
  libbpf: failed to find BTF for extern '__bpf_trap': -3
  Failed to open './t.o': -3

In llvm, the dag selection phase generates __bpf_trap in code. Later
the UnreachableBlockElim pass removed __bpf_trap from the code, but
__bpf_trap symbol survives in the symbol table.

Having a dangling __bpf_trap weak symbol is not good for old kernels
as seen in the above veristat failure. Although users could use compiler
flag '-mllvm -bpf-disable-trap-unreachable' to workaround the issue,
this patch fixed the issue by removing the dangling __bpf_trap.

  [1] llvm#165696
@nikic
Copy link
Contributor

nikic commented Nov 1, 2025

cc @tuliom Would be good to confirm whether this fixes the issues you saw on RHEL 8.

@yonghong-song yonghong-song merged commit 8fd1bf2 into llvm:main Nov 3, 2025
10 checks passed
if (GV->getName() == BPF_TRAP)
SawTrapCall = true;
} else if (Op.isSymbol()) {
if (const MCSymbol *Sym = Op.getMCSymbol())
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will always fire an assertion:

  MCSymbol *getMCSymbol() const {
    assert(isMCSymbol() && "Wrong MachineOperand accessor");
    return Contents.Sym;
  }

But isMCSymbol() is always false since isSymbol() is true instead.

What's the right fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used Op.isSymbol() as a possible target but running bpf selftest, the branch with Op.isSymbol() never to be true, at least when running linux kernel bpf selftests.

Let me double check whether this branch isSymbol() is really necessary or not.

@yonghong-song
Copy link
Contributor Author

The BPF_TRAP is created as

  Function::Create(FT, GlobalValue::ExternalWeakLinkage, BPF_TRAP, M);

So BPF_TRAP will be used as a global in instructions. So the branch

  else if (Op.isSymbol()) { ... }

should be considered as dead code and can be removed.

WDYT?

yonghong-song pushed a commit to yonghong-song/llvm-project that referenced this pull request Nov 4, 2025
In [1], the symbol __bpf_trap (macro BPF_TRAP) is removed if it
is not used in the code. In the discussion in [1], it is found
that the branch "if (Op.isSymbol())" is actually always false.
Remove it to avoid confusion.

  [1] llvm#166003
yonghong-song added a commit that referenced this pull request Nov 4, 2025
In [1], the symbol __bpf_trap (macro BPF_TRAP) is removed if it is not
used in the code. In the discussion in [1], it is found that the branch
"if (Op.isSymbol())" is actually always false. Remove it to avoid
confusion.

  [1] #166003
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Nov 4, 2025
…166440)

In [1], the symbol __bpf_trap (macro BPF_TRAP) is removed if it is not
used in the code. In the discussion in [1], it is found that the branch
"if (Op.isSymbol())" is actually always false. Remove it to avoid
confusion.

  [1] llvm/llvm-project#166003
@tuliom
Copy link
Contributor

tuliom commented Nov 5, 2025

cc @tuliom Would be good to confirm whether this fixes the issues you saw on RHEL 8.

For the record: I've just confirmed this commit did solve #165696.

Thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants