* [PATCH bpf] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF
@ 2020-03-17 1:16 Fangrui Song
2020-03-17 3:10 ` Andrii Nakryiko
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Fangrui Song @ 2020-03-17 1:16 UTC (permalink / raw)
To: bpf, netdev
Cc: daniel, Nick Desaulniers, Nathan Chancellor, clang-built-linux,
Stanislav Fomichev, davem, ast
Simplify gen_btf logic to make it work with llvm-objcopy and
llvm-objdump. We just need to retain one section .BTF. To do so, we can
use a simple objcopy --only-section=.BTF instead of jumping all the
hoops via an architecture-less binary file.
We use a dd comment to change the e_type field in the ELF header from
ET_EXEC to ET_REL so that .btf.vmlinux.bin.o will be accepted by lld.
Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux")
Cc: Stanislav Fomichev <sdf@google.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>
Reported-by: Nathan Chancellor <natechancellor@gmail.com>
Link: https://github.com/ClangBuiltLinux/linux/issues/871
Signed-off-by: Fangrui Song <maskray@google.com>
---
scripts/link-vmlinux.sh | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index dd484e92752e..84be8d7c361d 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -120,18 +120,9 @@ gen_btf()
info "BTF" ${2}
vmlinux_link ${1}
- LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1}
- # dump .BTF section into raw binary file to link with final vmlinux
- bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \
- cut -d, -f1 | cut -d' ' -f2)
- bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \
- awk '{print $4}')
- ${OBJCOPY} --change-section-address .BTF=0 \
- --set-section-flags .BTF=alloc -O binary \
- --only-section=.BTF ${1} .btf.vmlinux.bin
- ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \
- --rename-section .data=.BTF .btf.vmlinux.bin ${2}
+ # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux
+ ${OBJCOPY} --only-section=.BTF ${1} ${2} && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16
}
# Create ${2} .o file with all symbols from the ${1} object file
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH bpf] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF 2020-03-17 1:16 [PATCH bpf] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF Fangrui Song @ 2020-03-17 3:10 ` Andrii Nakryiko 2020-03-17 3:37 ` [PATCH bpf v2] " Fangrui Song 2020-03-17 17:02 ` [PATCH bpf] " kbuild test robot 2020-03-17 18:34 ` kbuild test robot 2 siblings, 1 reply; 17+ messages in thread From: Andrii Nakryiko @ 2020-03-17 3:10 UTC (permalink / raw) To: Fangrui Song Cc: bpf, Networking, Daniel Borkmann, Nick Desaulniers, Nathan Chancellor, clang-built-linux, Stanislav Fomichev, David S. Miller, Alexei Starovoitov On Mon, Mar 16, 2020 at 6:17 PM Fangrui Song <maskray@google.com> wrote: > > Simplify gen_btf logic to make it work with llvm-objcopy and > llvm-objdump. We just need to retain one section .BTF. To do so, we can > use a simple objcopy --only-section=.BTF instead of jumping all the > hoops via an architecture-less binary file. > > We use a dd comment to change the e_type field in the ELF header from > ET_EXEC to ET_REL so that .btf.vmlinux.bin.o will be accepted by lld. > > Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") > Cc: Stanislav Fomichev <sdf@google.com> > Cc: Nick Desaulniers <ndesaulniers@google.com> > Tested-by: Nick Desaulniers <ndesaulniers@google.com> > Reported-by: Nathan Chancellor <natechancellor@gmail.com> > Link: https://github.com/ClangBuiltLinux/linux/issues/871 > Signed-off-by: Fangrui Song <maskray@google.com> > --- > scripts/link-vmlinux.sh | 13 ++----------- > 1 file changed, 2 insertions(+), 11 deletions(-) > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > index dd484e92752e..84be8d7c361d 100755 > --- a/scripts/link-vmlinux.sh > +++ b/scripts/link-vmlinux.sh > @@ -120,18 +120,9 @@ gen_btf() > > info "BTF" ${2} > vmlinux_link ${1} > - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} Is it really tested? Seems like you just dropped .BTF generation step completely... > > - # dump .BTF section into raw binary file to link with final vmlinux > - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ > - cut -d, -f1 | cut -d' ' -f2) > - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ > - awk '{print $4}') > - ${OBJCOPY} --change-section-address .BTF=0 \ > - --set-section-flags .BTF=alloc -O binary \ > - --only-section=.BTF ${1} .btf.vmlinux.bin > - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ > - --rename-section .data=.BTF .btf.vmlinux.bin ${2} > + # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux > + ${OBJCOPY} --only-section=.BTF ${1} ${2} && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 > } > > # Create ${2} .o file with all symbols from the ${1} object file > -- > 2.25.1.481.gfbce0eb801-goog ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH bpf v2] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF 2020-03-17 3:10 ` Andrii Nakryiko @ 2020-03-17 3:37 ` Fangrui Song 2020-03-17 5:09 ` Andrii Nakryiko 0 siblings, 1 reply; 17+ messages in thread From: Fangrui Song @ 2020-03-17 3:37 UTC (permalink / raw) To: Andrii Nakryiko Cc: bpf, Networking, Daniel Borkmann, Nick Desaulniers, Nathan Chancellor, clang-built-linux, Stanislav Fomichev, David S. Miller, Alexei Starovoitov On 2020-03-16, Andrii Nakryiko wrote: >On Mon, Mar 16, 2020 at 6:17 PM Fangrui Song <maskray@google.com> wrote: >> >> Simplify gen_btf logic to make it work with llvm-objcopy and >> llvm-objdump. We just need to retain one section .BTF. To do so, we can >> use a simple objcopy --only-section=.BTF instead of jumping all the >> hoops via an architecture-less binary file. >> >> We use a dd comment to change the e_type field in the ELF header from >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o will be accepted by lld. >> >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") >> Cc: Stanislav Fomichev <sdf@google.com> >> Cc: Nick Desaulniers <ndesaulniers@google.com> >> Tested-by: Nick Desaulniers <ndesaulniers@google.com> >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 >> Signed-off-by: Fangrui Song <maskray@google.com> >> --- >> scripts/link-vmlinux.sh | 13 ++----------- >> 1 file changed, 2 insertions(+), 11 deletions(-) >> >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh >> index dd484e92752e..84be8d7c361d 100755 >> --- a/scripts/link-vmlinux.sh >> +++ b/scripts/link-vmlinux.sh >> @@ -120,18 +120,9 @@ gen_btf() >> >> info "BTF" ${2} >> vmlinux_link ${1} >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} > >Is it really tested? Seems like you just dropped .BTF generation step >completely... Sorry, dropped the whole line:/ I don't know how to test .BTF . I can only check readelf -S... Attached the new patch. From 02afb9417d4f0f8d2175c94fc3797a94a95cc248 Mon Sep 17 00:00:00 2001 From: Fangrui Song <maskray@google.com> Date: Mon, 16 Mar 2020 18:02:31 -0700 Subject: [PATCH bpf v2] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. We use a dd comment to change the e_type field in the ELF header from ET_EXEC to ET_REL so that .btf.vmlinux.bin.o can be accepted by lld. Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") Cc: Stanislav Fomichev <sdf@google.com> Cc: Nick Desaulniers <ndesaulniers@google.com> Reported-by: Nathan Chancellor <natechancellor@gmail.com> Link: https://github.com/ClangBuiltLinux/linux/issues/871 Signed-off-by: Fangrui Song <maskray@google.com> --- scripts/link-vmlinux.sh | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh index dd484e92752e..b23313944c89 100755 --- a/scripts/link-vmlinux.sh +++ b/scripts/link-vmlinux.sh @@ -120,18 +120,10 @@ gen_btf() info "BTF" ${2} vmlinux_link ${1} - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} + ${PAHOLE} -J ${1} - # dump .BTF section into raw binary file to link with final vmlinux - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ - cut -d, -f1 | cut -d' ' -f2) - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ - awk '{print $4}') - ${OBJCOPY} --change-section-address .BTF=0 \ - --set-section-flags .BTF=alloc -O binary \ - --only-section=.BTF ${1} .btf.vmlinux.bin - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ - --rename-section .data=.BTF .btf.vmlinux.bin ${2} + # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux + ${OBJCOPY} --only-section=.BTF ${1} ${2} && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 } # Create ${2} .o file with all symbols from the ${1} object file -- 2.25.1.481.gfbce0eb801-goog ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH bpf v2] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF 2020-03-17 3:37 ` [PATCH bpf v2] " Fangrui Song @ 2020-03-17 5:09 ` Andrii Nakryiko 2020-03-17 5:21 ` Fangrui Song 0 siblings, 1 reply; 17+ messages in thread From: Andrii Nakryiko @ 2020-03-17 5:09 UTC (permalink / raw) To: Fangrui Song Cc: bpf, Networking, Daniel Borkmann, Nick Desaulniers, Nathan Chancellor, clang-built-linux, Stanislav Fomichev, David S. Miller, Alexei Starovoitov On Mon, Mar 16, 2020 at 8:37 PM Fangrui Song <maskray@google.com> wrote: > > On 2020-03-16, Andrii Nakryiko wrote: > >On Mon, Mar 16, 2020 at 6:17 PM Fangrui Song <maskray@google.com> wrote: > >> > >> Simplify gen_btf logic to make it work with llvm-objcopy and > >> llvm-objdump. We just need to retain one section .BTF. To do so, we can > >> use a simple objcopy --only-section=.BTF instead of jumping all the > >> hoops via an architecture-less binary file. > >> > >> We use a dd comment to change the e_type field in the ELF header from > >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o will be accepted by lld. > >> > >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") > >> Cc: Stanislav Fomichev <sdf@google.com> > >> Cc: Nick Desaulniers <ndesaulniers@google.com> > >> Tested-by: Nick Desaulniers <ndesaulniers@google.com> > >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> > >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 > >> Signed-off-by: Fangrui Song <maskray@google.com> > >> --- > >> scripts/link-vmlinux.sh | 13 ++----------- > >> 1 file changed, 2 insertions(+), 11 deletions(-) > >> > >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > >> index dd484e92752e..84be8d7c361d 100755 > >> --- a/scripts/link-vmlinux.sh > >> +++ b/scripts/link-vmlinux.sh > >> @@ -120,18 +120,9 @@ gen_btf() > >> > >> info "BTF" ${2} > >> vmlinux_link ${1} > >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} > > > >Is it really tested? Seems like you just dropped .BTF generation step > >completely... > > Sorry, dropped the whole line:/ > I don't know how to test .BTF . I can only check readelf -S... > > Attached the new patch. > > > From 02afb9417d4f0f8d2175c94fc3797a94a95cc248 Mon Sep 17 00:00:00 2001 > From: Fangrui Song <maskray@google.com> > Date: Mon, 16 Mar 2020 18:02:31 -0700 > Subject: [PATCH bpf v2] bpf: Support llvm-objcopy and llvm-objdump for > vmlinux BTF > > Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. > We use a dd comment to change the e_type field in the ELF header from > ET_EXEC to ET_REL so that .btf.vmlinux.bin.o can be accepted by lld. > > Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") > Cc: Stanislav Fomichev <sdf@google.com> > Cc: Nick Desaulniers <ndesaulniers@google.com> > Reported-by: Nathan Chancellor <natechancellor@gmail.com> > Link: https://github.com/ClangBuiltLinux/linux/issues/871 > Signed-off-by: Fangrui Song <maskray@google.com> > --- > scripts/link-vmlinux.sh | 14 +++----------- > 1 file changed, 3 insertions(+), 11 deletions(-) > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > index dd484e92752e..b23313944c89 100755 > --- a/scripts/link-vmlinux.sh > +++ b/scripts/link-vmlinux.sh > @@ -120,18 +120,10 @@ gen_btf() > > info "BTF" ${2} > vmlinux_link ${1} > - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} > + ${PAHOLE} -J ${1} I'm not sure why you are touching this line at all. LLVM_OBJCOPY part is necessary, pahole assumes llvm-objcopy by default, but that can (and should for objcopy) be overridden with LLVM_OBJCOPY. > > - # dump .BTF section into raw binary file to link with final vmlinux > - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ > - cut -d, -f1 | cut -d' ' -f2) > - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ > - awk '{print $4}') > - ${OBJCOPY} --change-section-address .BTF=0 \ > - --set-section-flags .BTF=alloc -O binary \ > - --only-section=.BTF ${1} .btf.vmlinux.bin > - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ > - --rename-section .data=.BTF .btf.vmlinux.bin ${2} > + # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux > + ${OBJCOPY} --only-section=.BTF ${1} ${2} && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 > } > > # Create ${2} .o file with all symbols from the ${1} object file > -- > 2.25.1.481.gfbce0eb801-goog > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH bpf v2] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF 2020-03-17 5:09 ` Andrii Nakryiko @ 2020-03-17 5:21 ` Fangrui Song 2020-03-17 5:31 ` Andrii Nakryiko 0 siblings, 1 reply; 17+ messages in thread From: Fangrui Song @ 2020-03-17 5:21 UTC (permalink / raw) To: Andrii Nakryiko Cc: bpf, Networking, Daniel Borkmann, Nick Desaulniers, Nathan Chancellor, clang-built-linux, Stanislav Fomichev, David S. Miller, Alexei Starovoitov On 2020-03-16, Andrii Nakryiko wrote: >On Mon, Mar 16, 2020 at 8:37 PM Fangrui Song <maskray@google.com> wrote: >> >> On 2020-03-16, Andrii Nakryiko wrote: >> >On Mon, Mar 16, 2020 at 6:17 PM Fangrui Song <maskray@google.com> wrote: >> >> >> >> Simplify gen_btf logic to make it work with llvm-objcopy and >> >> llvm-objdump. We just need to retain one section .BTF. To do so, we can >> >> use a simple objcopy --only-section=.BTF instead of jumping all the >> >> hoops via an architecture-less binary file. >> >> >> >> We use a dd comment to change the e_type field in the ELF header from >> >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o will be accepted by lld. >> >> >> >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") >> >> Cc: Stanislav Fomichev <sdf@google.com> >> >> Cc: Nick Desaulniers <ndesaulniers@google.com> >> >> Tested-by: Nick Desaulniers <ndesaulniers@google.com> >> >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> >> >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 >> >> Signed-off-by: Fangrui Song <maskray@google.com> >> >> --- >> >> scripts/link-vmlinux.sh | 13 ++----------- >> >> 1 file changed, 2 insertions(+), 11 deletions(-) >> >> >> >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh >> >> index dd484e92752e..84be8d7c361d 100755 >> >> --- a/scripts/link-vmlinux.sh >> >> +++ b/scripts/link-vmlinux.sh >> >> @@ -120,18 +120,9 @@ gen_btf() >> >> >> >> info "BTF" ${2} >> >> vmlinux_link ${1} >> >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} >> > >> >Is it really tested? Seems like you just dropped .BTF generation step >> >completely... >> >> Sorry, dropped the whole line:/ >> I don't know how to test .BTF . I can only check readelf -S... >> >> Attached the new patch. >> >> >> From 02afb9417d4f0f8d2175c94fc3797a94a95cc248 Mon Sep 17 00:00:00 2001 >> From: Fangrui Song <maskray@google.com> >> Date: Mon, 16 Mar 2020 18:02:31 -0700 >> Subject: [PATCH bpf v2] bpf: Support llvm-objcopy and llvm-objdump for >> vmlinux BTF >> >> Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. >> We use a dd comment to change the e_type field in the ELF header from >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o can be accepted by lld. >> >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") >> Cc: Stanislav Fomichev <sdf@google.com> >> Cc: Nick Desaulniers <ndesaulniers@google.com> >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 >> Signed-off-by: Fangrui Song <maskray@google.com> >> --- >> scripts/link-vmlinux.sh | 14 +++----------- >> 1 file changed, 3 insertions(+), 11 deletions(-) >> >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh >> index dd484e92752e..b23313944c89 100755 >> --- a/scripts/link-vmlinux.sh >> +++ b/scripts/link-vmlinux.sh >> @@ -120,18 +120,10 @@ gen_btf() >> >> info "BTF" ${2} >> vmlinux_link ${1} >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} >> + ${PAHOLE} -J ${1} > >I'm not sure why you are touching this line at all. LLVM_OBJCOPY part >is necessary, pahole assumes llvm-objcopy by default, but that can >(and should for objcopy) be overridden with LLVM_OBJCOPY. Why is LLVM_OBJCOPY assumed? What if llvm-objcopy is not available? This is confusing that one tool assumes llvm-objcopy while the block below immediately uses GNU objcopy (without this patch). e83b9f55448afce3fe1abcd1d10db9584f8042a6 "kbuild: add ability to generate BTF type info for vmlinux" does not say why LLVM_OBJCOPY is set. >> >> - # dump .BTF section into raw binary file to link with final vmlinux >> - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ >> - cut -d, -f1 | cut -d' ' -f2) >> - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ >> - awk '{print $4}') >> - ${OBJCOPY} --change-section-address .BTF=0 \ >> - --set-section-flags .BTF=alloc -O binary \ >> - --only-section=.BTF ${1} .btf.vmlinux.bin >> - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ >> - --rename-section .data=.BTF .btf.vmlinux.bin ${2} >> + # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux >> + ${OBJCOPY} --only-section=.BTF ${1} ${2} && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 >> } >> >> # Create ${2} .o file with all symbols from the ${1} object file >> -- >> 2.25.1.481.gfbce0eb801-goog >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH bpf v2] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF 2020-03-17 5:21 ` Fangrui Song @ 2020-03-17 5:31 ` Andrii Nakryiko 2020-03-17 5:43 ` [PATCH bpf v3] " Fangrui Song 0 siblings, 1 reply; 17+ messages in thread From: Andrii Nakryiko @ 2020-03-17 5:31 UTC (permalink / raw) To: Fangrui Song Cc: bpf, Networking, Daniel Borkmann, Nick Desaulniers, Nathan Chancellor, clang-built-linux, Stanislav Fomichev, David S. Miller, Alexei Starovoitov On Mon, Mar 16, 2020 at 10:21 PM Fangrui Song <maskray@google.com> wrote: > > > On 2020-03-16, Andrii Nakryiko wrote: > >On Mon, Mar 16, 2020 at 8:37 PM Fangrui Song <maskray@google.com> wrote: > >> > >> On 2020-03-16, Andrii Nakryiko wrote: > >> >On Mon, Mar 16, 2020 at 6:17 PM Fangrui Song <maskray@google.com> wrote: > >> >> > >> >> Simplify gen_btf logic to make it work with llvm-objcopy and > >> >> llvm-objdump. We just need to retain one section .BTF. To do so, we can > >> >> use a simple objcopy --only-section=.BTF instead of jumping all the > >> >> hoops via an architecture-less binary file. > >> >> > >> >> We use a dd comment to change the e_type field in the ELF header from > >> >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o will be accepted by lld. > >> >> > >> >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") > >> >> Cc: Stanislav Fomichev <sdf@google.com> > >> >> Cc: Nick Desaulniers <ndesaulniers@google.com> > >> >> Tested-by: Nick Desaulniers <ndesaulniers@google.com> > >> >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> > >> >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 > >> >> Signed-off-by: Fangrui Song <maskray@google.com> > >> >> --- > >> >> scripts/link-vmlinux.sh | 13 ++----------- > >> >> 1 file changed, 2 insertions(+), 11 deletions(-) > >> >> > >> >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > >> >> index dd484e92752e..84be8d7c361d 100755 > >> >> --- a/scripts/link-vmlinux.sh > >> >> +++ b/scripts/link-vmlinux.sh > >> >> @@ -120,18 +120,9 @@ gen_btf() > >> >> > >> >> info "BTF" ${2} > >> >> vmlinux_link ${1} > >> >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} > >> > > >> >Is it really tested? Seems like you just dropped .BTF generation step > >> >completely... > >> > >> Sorry, dropped the whole line:/ > >> I don't know how to test .BTF . I can only check readelf -S... > >> > >> Attached the new patch. > >> > >> > >> From 02afb9417d4f0f8d2175c94fc3797a94a95cc248 Mon Sep 17 00:00:00 2001 > >> From: Fangrui Song <maskray@google.com> > >> Date: Mon, 16 Mar 2020 18:02:31 -0700 > >> Subject: [PATCH bpf v2] bpf: Support llvm-objcopy and llvm-objdump for > >> vmlinux BTF > >> > >> Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. > >> We use a dd comment to change the e_type field in the ELF header from > >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o can be accepted by lld. > >> > >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") > >> Cc: Stanislav Fomichev <sdf@google.com> > >> Cc: Nick Desaulniers <ndesaulniers@google.com> > >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> > >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 > >> Signed-off-by: Fangrui Song <maskray@google.com> > >> --- > >> scripts/link-vmlinux.sh | 14 +++----------- > >> 1 file changed, 3 insertions(+), 11 deletions(-) > >> > >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > >> index dd484e92752e..b23313944c89 100755 > >> --- a/scripts/link-vmlinux.sh > >> +++ b/scripts/link-vmlinux.sh > >> @@ -120,18 +120,10 @@ gen_btf() > >> > >> info "BTF" ${2} > >> vmlinux_link ${1} > >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} > >> + ${PAHOLE} -J ${1} > > > >I'm not sure why you are touching this line at all. LLVM_OBJCOPY part > >is necessary, pahole assumes llvm-objcopy by default, but that can > >(and should for objcopy) be overridden with LLVM_OBJCOPY. > > Why is LLVM_OBJCOPY assumed? What if llvm-objcopy is not available? It's pahole assumption that we have to live with. pahole assumes llvm-objcopy internally, unless it is overriden with LLVM_OBJCOPY env var. So please revert this line otherwise you are breaking it for GCC objcopy case. > This is confusing that one tool assumes llvm-objcopy while the block > below immediately uses GNU objcopy (without this patch). > > e83b9f55448afce3fe1abcd1d10db9584f8042a6 "kbuild: add ability to > generate BTF type info for vmlinux" does not say why LLVM_OBJCOPY is > set. > > >> > >> - # dump .BTF section into raw binary file to link with final vmlinux > >> - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ > >> - cut -d, -f1 | cut -d' ' -f2) > >> - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ > >> - awk '{print $4}') > >> - ${OBJCOPY} --change-section-address .BTF=0 \ > >> - --set-section-flags .BTF=alloc -O binary \ > >> - --only-section=.BTF ${1} .btf.vmlinux.bin > >> - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ > >> - --rename-section .data=.BTF .btf.vmlinux.bin ${2} > >> + # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux > >> + ${OBJCOPY} --only-section=.BTF ${1} ${2} && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 > >> } > >> > >> # Create ${2} .o file with all symbols from the ${1} object file > >> -- > >> 2.25.1.481.gfbce0eb801-goog > >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH bpf v3] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF 2020-03-17 5:31 ` Andrii Nakryiko @ 2020-03-17 5:43 ` Fangrui Song 2020-03-17 16:05 ` Stanislav Fomichev 2020-03-17 16:24 ` Stanislav Fomichev 0 siblings, 2 replies; 17+ messages in thread From: Fangrui Song @ 2020-03-17 5:43 UTC (permalink / raw) To: Andrii Nakryiko Cc: bpf, Networking, Daniel Borkmann, Nick Desaulniers, Nathan Chancellor, clang-built-linux, Stanislav Fomichev, David S. Miller, Alexei Starovoitov On 2020-03-16, Andrii Nakryiko wrote: >On Mon, Mar 16, 2020 at 10:21 PM Fangrui Song <maskray@google.com> wrote: >> >> >> On 2020-03-16, Andrii Nakryiko wrote: >> >On Mon, Mar 16, 2020 at 8:37 PM Fangrui Song <maskray@google.com> wrote: >> >> >> >> On 2020-03-16, Andrii Nakryiko wrote: >> >> >On Mon, Mar 16, 2020 at 6:17 PM Fangrui Song <maskray@google.com> wrote: >> >> >> >> >> >> Simplify gen_btf logic to make it work with llvm-objcopy and >> >> >> llvm-objdump. We just need to retain one section .BTF. To do so, we can >> >> >> use a simple objcopy --only-section=.BTF instead of jumping all the >> >> >> hoops via an architecture-less binary file. >> >> >> >> >> >> We use a dd comment to change the e_type field in the ELF header from >> >> >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o will be accepted by lld. >> >> >> >> >> >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") >> >> >> Cc: Stanislav Fomichev <sdf@google.com> >> >> >> Cc: Nick Desaulniers <ndesaulniers@google.com> >> >> >> Tested-by: Nick Desaulniers <ndesaulniers@google.com> >> >> >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> >> >> >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 >> >> >> Signed-off-by: Fangrui Song <maskray@google.com> >> >> >> --- >> >> >> scripts/link-vmlinux.sh | 13 ++----------- >> >> >> 1 file changed, 2 insertions(+), 11 deletions(-) >> >> >> >> >> >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh >> >> >> index dd484e92752e..84be8d7c361d 100755 >> >> >> --- a/scripts/link-vmlinux.sh >> >> >> +++ b/scripts/link-vmlinux.sh >> >> >> @@ -120,18 +120,9 @@ gen_btf() >> >> >> >> >> >> info "BTF" ${2} >> >> >> vmlinux_link ${1} >> >> >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} >> >> > >> >> >Is it really tested? Seems like you just dropped .BTF generation step >> >> >completely... >> >> >> >> Sorry, dropped the whole line:/ >> >> I don't know how to test .BTF . I can only check readelf -S... >> >> >> >> Attached the new patch. >> >> >> >> >> >> From 02afb9417d4f0f8d2175c94fc3797a94a95cc248 Mon Sep 17 00:00:00 2001 >> >> From: Fangrui Song <maskray@google.com> >> >> Date: Mon, 16 Mar 2020 18:02:31 -0700 >> >> Subject: [PATCH bpf v2] bpf: Support llvm-objcopy and llvm-objdump for >> >> vmlinux BTF >> >> >> >> Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. >> >> We use a dd comment to change the e_type field in the ELF header from >> >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o can be accepted by lld. >> >> >> >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") >> >> Cc: Stanislav Fomichev <sdf@google.com> >> >> Cc: Nick Desaulniers <ndesaulniers@google.com> >> >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> >> >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 >> >> Signed-off-by: Fangrui Song <maskray@google.com> >> >> --- >> >> scripts/link-vmlinux.sh | 14 +++----------- >> >> 1 file changed, 3 insertions(+), 11 deletions(-) >> >> >> >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh >> >> index dd484e92752e..b23313944c89 100755 >> >> --- a/scripts/link-vmlinux.sh >> >> +++ b/scripts/link-vmlinux.sh >> >> @@ -120,18 +120,10 @@ gen_btf() >> >> >> >> info "BTF" ${2} >> >> vmlinux_link ${1} >> >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} >> >> + ${PAHOLE} -J ${1} >> > >> >I'm not sure why you are touching this line at all. LLVM_OBJCOPY part >> >is necessary, pahole assumes llvm-objcopy by default, but that can >> >(and should for objcopy) be overridden with LLVM_OBJCOPY. >> >> Why is LLVM_OBJCOPY assumed? What if llvm-objcopy is not available? > >It's pahole assumption that we have to live with. pahole assumes >llvm-objcopy internally, unless it is overriden with LLVM_OBJCOPY env >var. So please revert this line otherwise you are breaking it for GCC >objcopy case. Acknowledged. Uploaded v3. I added back 2>/dev/null which was removed by a previous change, to suppress GNU objcopy warnings. The warnings could be annoying in V=1 output. >> This is confusing that one tool assumes llvm-objcopy while the block >> below immediately uses GNU objcopy (without this patch). >> >> e83b9f55448afce3fe1abcd1d10db9584f8042a6 "kbuild: add ability to >> generate BTF type info for vmlinux" does not say why LLVM_OBJCOPY is >> set. >> >> >> >> >> - # dump .BTF section into raw binary file to link with final vmlinux >> >> - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ >> >> - cut -d, -f1 | cut -d' ' -f2) >> >> - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ >> >> - awk '{print $4}') >> >> - ${OBJCOPY} --change-section-address .BTF=0 \ >> >> - --set-section-flags .BTF=alloc -O binary \ >> >> - --only-section=.BTF ${1} .btf.vmlinux.bin >> >> - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ >> >> - --rename-section .data=.BTF .btf.vmlinux.bin ${2} >> >> + # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux >> >> + ${OBJCOPY} --only-section=.BTF ${1} ${2} && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 >> >> } >> >> >> >> # Create ${2} .o file with all symbols from the ${1} object file >> >> -- >> >> 2.25.1.481.gfbce0eb801-goog >> >> From ca3597477542453e9f63185c27c162da081a4baf Mon Sep 17 00:00:00 2001 From: Fangrui Song <maskray@google.com> Date: Mon, 16 Mar 2020 22:38:23 -0700 Subject: [PATCH bpf v3] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. Add 2>/dev/null to suppress GNU objcopy (but not llvm-objcopy) warnings "empty loadable segment detected at vaddr=0xffffffff81000000, is this intentional?" Our use of --only-section drops many SHF_ALLOC sections which will essentially nullify program headers. When used as linker input, program headers are simply ignored. We use a dd command to change the e_type field in the ELF header from ET_EXEC to ET_REL so that .btf.vmlinux.bin.o can be accepted by lld. Accepting ET_EXEC as an input file is an extremely rare GNU ld feature that lld does not intend to support, because this is very error-prone. Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") Cc: Stanislav Fomichev <sdf@google.com> Cc: Nick Desaulniers <ndesaulniers@google.com> Reported-by: Nathan Chancellor <natechancellor@gmail.com> Link: https://github.com/ClangBuiltLinux/linux/issues/871 Signed-off-by: Fangrui Song <maskray@google.com> --- scripts/link-vmlinux.sh | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh index dd484e92752e..c3e808a89d4a 100755 --- a/scripts/link-vmlinux.sh +++ b/scripts/link-vmlinux.sh @@ -122,16 +122,8 @@ gen_btf() vmlinux_link ${1} LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} - # dump .BTF section into raw binary file to link with final vmlinux - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ - cut -d, -f1 | cut -d' ' -f2) - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ - awk '{print $4}') - ${OBJCOPY} --change-section-address .BTF=0 \ - --set-section-flags .BTF=alloc -O binary \ - --only-section=.BTF ${1} .btf.vmlinux.bin - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ - --rename-section .data=.BTF .btf.vmlinux.bin ${2} + # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux + ${OBJCOPY} --only-section=.BTF ${1} ${2} 2> /dev/null && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 } # Create ${2} .o file with all symbols from the ${1} object file -- 2.25.1.481.gfbce0eb801-goog ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH bpf v3] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF 2020-03-17 5:43 ` [PATCH bpf v3] " Fangrui Song @ 2020-03-17 16:05 ` Stanislav Fomichev 2020-03-17 16:24 ` Stanislav Fomichev 1 sibling, 0 replies; 17+ messages in thread From: Stanislav Fomichev @ 2020-03-17 16:05 UTC (permalink / raw) To: Fangrui Song Cc: Andrii Nakryiko, bpf, Networking, Daniel Borkmann, Nick Desaulniers, Nathan Chancellor, clang-built-linux, Stanislav Fomichev, David S. Miller, Alexei Starovoitov On 03/16, Fangrui Song wrote: > On 2020-03-16, Andrii Nakryiko wrote: > > On Mon, Mar 16, 2020 at 10:21 PM Fangrui Song <maskray@google.com> wrote: > > > > > > > > > On 2020-03-16, Andrii Nakryiko wrote: > > > >On Mon, Mar 16, 2020 at 8:37 PM Fangrui Song <maskray@google.com> wrote: > > > >> > > > >> On 2020-03-16, Andrii Nakryiko wrote: > > > >> >On Mon, Mar 16, 2020 at 6:17 PM Fangrui Song <maskray@google.com> wrote: > > > >> >> > > > >> >> Simplify gen_btf logic to make it work with llvm-objcopy and > > > >> >> llvm-objdump. We just need to retain one section .BTF. To do so, we can > > > >> >> use a simple objcopy --only-section=.BTF instead of jumping all the > > > >> >> hoops via an architecture-less binary file. > > > >> >> > > > >> >> We use a dd comment to change the e_type field in the ELF header from > > > >> >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o will be accepted by lld. > > > >> >> > > > >> >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") > > > >> >> Cc: Stanislav Fomichev <sdf@google.com> > > > >> >> Cc: Nick Desaulniers <ndesaulniers@google.com> > > > >> >> Tested-by: Nick Desaulniers <ndesaulniers@google.com> > > > >> >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> > > > >> >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 > > > >> >> Signed-off-by: Fangrui Song <maskray@google.com> > > > >> >> --- > > > >> >> scripts/link-vmlinux.sh | 13 ++----------- > > > >> >> 1 file changed, 2 insertions(+), 11 deletions(-) > > > >> >> > > > >> >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > > > >> >> index dd484e92752e..84be8d7c361d 100755 > > > >> >> --- a/scripts/link-vmlinux.sh > > > >> >> +++ b/scripts/link-vmlinux.sh > > > >> >> @@ -120,18 +120,9 @@ gen_btf() > > > >> >> > > > >> >> info "BTF" ${2} > > > >> >> vmlinux_link ${1} > > > >> >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} > > > >> > > > > >> >Is it really tested? Seems like you just dropped .BTF generation step > > > >> >completely... > > > >> > > > >> Sorry, dropped the whole line:/ > > > >> I don't know how to test .BTF . I can only check readelf -S... > > > >> > > > >> Attached the new patch. > > > >> > > > >> > > > >> From 02afb9417d4f0f8d2175c94fc3797a94a95cc248 Mon Sep 17 00:00:00 2001 > > > >> From: Fangrui Song <maskray@google.com> > > > >> Date: Mon, 16 Mar 2020 18:02:31 -0700 > > > >> Subject: [PATCH bpf v2] bpf: Support llvm-objcopy and llvm-objdump for > > > >> vmlinux BTF > > > >> > > > >> Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. > > > >> We use a dd comment to change the e_type field in the ELF header from > > > >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o can be accepted by lld. > > > >> > > > >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") > > > >> Cc: Stanislav Fomichev <sdf@google.com> > > > >> Cc: Nick Desaulniers <ndesaulniers@google.com> > > > >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> > > > >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 > > > >> Signed-off-by: Fangrui Song <maskray@google.com> > > > >> --- > > > >> scripts/link-vmlinux.sh | 14 +++----------- > > > >> 1 file changed, 3 insertions(+), 11 deletions(-) > > > >> > > > >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > > > >> index dd484e92752e..b23313944c89 100755 > > > >> --- a/scripts/link-vmlinux.sh > > > >> +++ b/scripts/link-vmlinux.sh > > > >> @@ -120,18 +120,10 @@ gen_btf() > > > >> > > > >> info "BTF" ${2} > > > >> vmlinux_link ${1} > > > >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} > > > >> + ${PAHOLE} -J ${1} > > > > > > > >I'm not sure why you are touching this line at all. LLVM_OBJCOPY part > > > >is necessary, pahole assumes llvm-objcopy by default, but that can > > > >(and should for objcopy) be overridden with LLVM_OBJCOPY. > > > > > > Why is LLVM_OBJCOPY assumed? What if llvm-objcopy is not available? > > > > It's pahole assumption that we have to live with. pahole assumes > > llvm-objcopy internally, unless it is overriden with LLVM_OBJCOPY env > > var. So please revert this line otherwise you are breaking it for GCC > > objcopy case. > > Acknowledged. Uploaded v3. > > I added back 2>/dev/null which was removed by a previous change, to > suppress GNU objcopy warnings. The warnings could be annoying in V=1 > output. > > > > This is confusing that one tool assumes llvm-objcopy while the block > > > below immediately uses GNU objcopy (without this patch). > > > > > > e83b9f55448afce3fe1abcd1d10db9584f8042a6 "kbuild: add ability to > > > generate BTF type info for vmlinux" does not say why LLVM_OBJCOPY is > > > set. > > > > > > >> > > > >> - # dump .BTF section into raw binary file to link with final vmlinux > > > >> - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ > > > >> - cut -d, -f1 | cut -d' ' -f2) > > > >> - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ > > > >> - awk '{print $4}') > > > >> - ${OBJCOPY} --change-section-address .BTF=0 \ > > > >> - --set-section-flags .BTF=alloc -O binary \ > > > >> - --only-section=.BTF ${1} .btf.vmlinux.bin > > > >> - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ > > > >> - --rename-section .data=.BTF .btf.vmlinux.bin ${2} > > > >> + # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux > > > >> + ${OBJCOPY} --only-section=.BTF ${1} ${2} && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 > > > >> } > > > >> > > > >> # Create ${2} .o file with all symbols from the ${1} object file > > > >> -- > > > >> 2.25.1.481.gfbce0eb801-goog > > > >> > > From ca3597477542453e9f63185c27c162da081a4baf Mon Sep 17 00:00:00 2001 > From: Fangrui Song <maskray@google.com> > Date: Mon, 16 Mar 2020 22:38:23 -0700 > Subject: [PATCH bpf v3] bpf: Support llvm-objcopy and llvm-objdump for > vmlinux BTF > > Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. > Add 2>/dev/null to suppress GNU objcopy (but not llvm-objcopy) warnings > "empty loadable segment detected at vaddr=0xffffffff81000000, is this intentional?" > Our use of --only-section drops many SHF_ALLOC sections which will essentially nullify > program headers. When used as linker input, program headers are simply > ignored. > > We use a dd command to change the e_type field in the ELF header from > ET_EXEC to ET_REL so that .btf.vmlinux.bin.o can be accepted by lld. > Accepting ET_EXEC as an input file is an extremely rare GNU ld feature > that lld does not intend to support, because this is very error-prone. I'm testing this with binutils objcopy, will update with the results. > Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") > Cc: Stanislav Fomichev <sdf@google.com> > Cc: Nick Desaulniers <ndesaulniers@google.com> > Reported-by: Nathan Chancellor <natechancellor@gmail.com> > Link: https://github.com/ClangBuiltLinux/linux/issues/871 > Signed-off-by: Fangrui Song <maskray@google.com> > --- > scripts/link-vmlinux.sh | 12 ++---------- > 1 file changed, 2 insertions(+), 10 deletions(-) > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > index dd484e92752e..c3e808a89d4a 100755 > --- a/scripts/link-vmlinux.sh > +++ b/scripts/link-vmlinux.sh > @@ -122,16 +122,8 @@ gen_btf() > vmlinux_link ${1} > LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} > - # dump .BTF section into raw binary file to link with final vmlinux > - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ > - cut -d, -f1 | cut -d' ' -f2) > - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ > - awk '{print $4}') Should you also remove 'local bin_arch' on top? > - ${OBJCOPY} --change-section-address .BTF=0 \ > - --set-section-flags .BTF=alloc -O binary \ > - --only-section=.BTF ${1} .btf.vmlinux.bin > - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ > - --rename-section .data=.BTF .btf.vmlinux.bin ${2} > + # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux > + ${OBJCOPY} --only-section=.BTF ${1} ${2} 2> /dev/null && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 nit: maybe split this into multiple lines? and drop space in '2>/dev/null'? ${OBJCOPY} .... 2>/dev/null && \ printf '\1' | dd .... ? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH bpf v3] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF 2020-03-17 5:43 ` [PATCH bpf v3] " Fangrui Song 2020-03-17 16:05 ` Stanislav Fomichev @ 2020-03-17 16:24 ` Stanislav Fomichev 2020-03-17 20:58 ` [PATCH bpf-next v4] " Fangrui Song 1 sibling, 1 reply; 17+ messages in thread From: Stanislav Fomichev @ 2020-03-17 16:24 UTC (permalink / raw) To: Fangrui Song Cc: Andrii Nakryiko, bpf, Networking, Daniel Borkmann, Nick Desaulniers, Nathan Chancellor, clang-built-linux, Stanislav Fomichev, David S. Miller, Alexei Starovoitov On 03/16, Fangrui Song wrote: > On 2020-03-16, Andrii Nakryiko wrote: > > On Mon, Mar 16, 2020 at 10:21 PM Fangrui Song <maskray@google.com> wrote: > > > > > > > > > On 2020-03-16, Andrii Nakryiko wrote: > > > >On Mon, Mar 16, 2020 at 8:37 PM Fangrui Song <maskray@google.com> wrote: > > > >> > > > >> On 2020-03-16, Andrii Nakryiko wrote: > > > >> >On Mon, Mar 16, 2020 at 6:17 PM Fangrui Song <maskray@google.com> wrote: > > > >> >> > > > >> >> Simplify gen_btf logic to make it work with llvm-objcopy and > > > >> >> llvm-objdump. We just need to retain one section .BTF. To do so, we can > > > >> >> use a simple objcopy --only-section=.BTF instead of jumping all the > > > >> >> hoops via an architecture-less binary file. > > > >> >> > > > >> >> We use a dd comment to change the e_type field in the ELF header from > > > >> >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o will be accepted by lld. > > > >> >> > > > >> >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") > > > >> >> Cc: Stanislav Fomichev <sdf@google.com> > > > >> >> Cc: Nick Desaulniers <ndesaulniers@google.com> > > > >> >> Tested-by: Nick Desaulniers <ndesaulniers@google.com> > > > >> >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> > > > >> >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 > > > >> >> Signed-off-by: Fangrui Song <maskray@google.com> > > > >> >> --- > > > >> >> scripts/link-vmlinux.sh | 13 ++----------- > > > >> >> 1 file changed, 2 insertions(+), 11 deletions(-) > > > >> >> > > > >> >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > > > >> >> index dd484e92752e..84be8d7c361d 100755 > > > >> >> --- a/scripts/link-vmlinux.sh > > > >> >> +++ b/scripts/link-vmlinux.sh > > > >> >> @@ -120,18 +120,9 @@ gen_btf() > > > >> >> > > > >> >> info "BTF" ${2} > > > >> >> vmlinux_link ${1} > > > >> >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} > > > >> > > > > >> >Is it really tested? Seems like you just dropped .BTF generation step > > > >> >completely... > > > >> > > > >> Sorry, dropped the whole line:/ > > > >> I don't know how to test .BTF . I can only check readelf -S... > > > >> > > > >> Attached the new patch. > > > >> > > > >> > > > >> From 02afb9417d4f0f8d2175c94fc3797a94a95cc248 Mon Sep 17 00:00:00 2001 > > > >> From: Fangrui Song <maskray@google.com> > > > >> Date: Mon, 16 Mar 2020 18:02:31 -0700 > > > >> Subject: [PATCH bpf v2] bpf: Support llvm-objcopy and llvm-objdump for > > > >> vmlinux BTF > > > >> > > > >> Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. > > > >> We use a dd comment to change the e_type field in the ELF header from > > > >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o can be accepted by lld. > > > >> > > > >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") > > > >> Cc: Stanislav Fomichev <sdf@google.com> > > > >> Cc: Nick Desaulniers <ndesaulniers@google.com> > > > >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> > > > >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 > > > >> Signed-off-by: Fangrui Song <maskray@google.com> > > > >> --- > > > >> scripts/link-vmlinux.sh | 14 +++----------- > > > >> 1 file changed, 3 insertions(+), 11 deletions(-) > > > >> > > > >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > > > >> index dd484e92752e..b23313944c89 100755 > > > >> --- a/scripts/link-vmlinux.sh > > > >> +++ b/scripts/link-vmlinux.sh > > > >> @@ -120,18 +120,10 @@ gen_btf() > > > >> > > > >> info "BTF" ${2} > > > >> vmlinux_link ${1} > > > >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} > > > >> + ${PAHOLE} -J ${1} > > > > > > > >I'm not sure why you are touching this line at all. LLVM_OBJCOPY part > > > >is necessary, pahole assumes llvm-objcopy by default, but that can > > > >(and should for objcopy) be overridden with LLVM_OBJCOPY. > > > > > > Why is LLVM_OBJCOPY assumed? What if llvm-objcopy is not available? > > > > It's pahole assumption that we have to live with. pahole assumes > > llvm-objcopy internally, unless it is overriden with LLVM_OBJCOPY env > > var. So please revert this line otherwise you are breaking it for GCC > > objcopy case. > > Acknowledged. Uploaded v3. > > I added back 2>/dev/null which was removed by a previous change, to > suppress GNU objcopy warnings. The warnings could be annoying in V=1 > output. > > > > This is confusing that one tool assumes llvm-objcopy while the block > > > below immediately uses GNU objcopy (without this patch). > > > > > > e83b9f55448afce3fe1abcd1d10db9584f8042a6 "kbuild: add ability to > > > generate BTF type info for vmlinux" does not say why LLVM_OBJCOPY is > > > set. > > > > > > >> > > > >> - # dump .BTF section into raw binary file to link with final vmlinux > > > >> - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ > > > >> - cut -d, -f1 | cut -d' ' -f2) > > > >> - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ > > > >> - awk '{print $4}') > > > >> - ${OBJCOPY} --change-section-address .BTF=0 \ > > > >> - --set-section-flags .BTF=alloc -O binary \ > > > >> - --only-section=.BTF ${1} .btf.vmlinux.bin > > > >> - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ > > > >> - --rename-section .data=.BTF .btf.vmlinux.bin ${2} > > > >> + # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux > > > >> + ${OBJCOPY} --only-section=.BTF ${1} ${2} && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 > > > >> } > > > >> > > > >> # Create ${2} .o file with all symbols from the ${1} object file > > > >> -- > > > >> 2.25.1.481.gfbce0eb801-goog > > > >> > > From ca3597477542453e9f63185c27c162da081a4baf Mon Sep 17 00:00:00 2001 > From: Fangrui Song <maskray@google.com> > Date: Mon, 16 Mar 2020 22:38:23 -0700 > Subject: [PATCH bpf v3] bpf: Support llvm-objcopy and llvm-objdump for > vmlinux BTF > > Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. > Add 2>/dev/null to suppress GNU objcopy (but not llvm-objcopy) warnings > "empty loadable segment detected at vaddr=0xffffffff81000000, is this intentional?" > Our use of --only-section drops many SHF_ALLOC sections which will essentially nullify > program headers. When used as linker input, program headers are simply > ignored. > > We use a dd command to change the e_type field in the ELF header from > ET_EXEC to ET_REL so that .btf.vmlinux.bin.o can be accepted by lld. > Accepting ET_EXEC as an input file is an extremely rare GNU ld feature > that lld does not intend to support, because this is very error-prone. > > Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") > Cc: Stanislav Fomichev <sdf@google.com> > Cc: Nick Desaulniers <ndesaulniers@google.com> > Reported-by: Nathan Chancellor <natechancellor@gmail.com> > Link: https://github.com/ClangBuiltLinux/linux/issues/871 > Signed-off-by: Fangrui Song <maskray@google.com> > --- > scripts/link-vmlinux.sh | 12 ++---------- > 1 file changed, 2 insertions(+), 10 deletions(-) > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > index dd484e92752e..c3e808a89d4a 100755 > --- a/scripts/link-vmlinux.sh > +++ b/scripts/link-vmlinux.sh > @@ -122,16 +122,8 @@ gen_btf() > vmlinux_link ${1} > LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} > - # dump .BTF section into raw binary file to link with final vmlinux > - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ > - cut -d, -f1 | cut -d' ' -f2) > - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ > - awk '{print $4}') > - ${OBJCOPY} --change-section-address .BTF=0 \ > - --set-section-flags .BTF=alloc -O binary \ > - --only-section=.BTF ${1} .btf.vmlinux.bin > - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ > - --rename-section .data=.BTF .btf.vmlinux.bin ${2} > + # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux > + ${OBJCOPY} --only-section=.BTF ${1} ${2} 2> /dev/null && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 No, it doesn't work unfortunately, I get "in-kernel BTF is malformed" from the kernel. I think that's because -O binary adds the following: $ nm .btf.vmxlinux.bin 00000000002f7bc9 D _binary__btf_vmlinux_bin_end 00000000002f7bc9 A _binary__btf_vmlinux_bin_size 0000000000000000 D _binary__btf_vmlinux_bin_start While non-binary mode doesn't: $ nm .btf.vmxlinux.bin We don't add them manually in the linker map and expect objcopy to add them, see: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/kernel/bpf/btf.c#n3480 > } > # Create ${2} .o file with all symbols from the ${1} object file > -- > 2.25.1.481.gfbce0eb801-goog > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH bpf-next v4] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF 2020-03-17 16:24 ` Stanislav Fomichev @ 2020-03-17 20:58 ` Fangrui Song 2020-03-17 23:37 ` Andrii Nakryiko 0 siblings, 1 reply; 17+ messages in thread From: Fangrui Song @ 2020-03-17 20:58 UTC (permalink / raw) To: bpf, Networking Cc: Andrii Nakryiko, Daniel Borkmann, Nick Desaulniers, Nathan Chancellor, clang-built-linux, Stanislav Fomichev, David S. Miller, Alexei Starovoitov, Kees Cook On 2020-03-17, Stanislav Fomichev wrote: >On 03/16, Fangrui Song wrote: >> On 2020-03-16, Andrii Nakryiko wrote: >> > On Mon, Mar 16, 2020 at 10:21 PM Fangrui Song <maskray@google.com> wrote: >> > > >> > > >> > > On 2020-03-16, Andrii Nakryiko wrote: >> > > >On Mon, Mar 16, 2020 at 8:37 PM Fangrui Song <maskray@google.com> wrote: >> > > >> >> > > >> On 2020-03-16, Andrii Nakryiko wrote: >> > > >> >On Mon, Mar 16, 2020 at 6:17 PM Fangrui Song <maskray@google.com> wrote: >> > > >> >> >> > > >> >> Simplify gen_btf logic to make it work with llvm-objcopy and >> > > >> >> llvm-objdump. We just need to retain one section .BTF. To do so, we can >> > > >> >> use a simple objcopy --only-section=.BTF instead of jumping all the >> > > >> >> hoops via an architecture-less binary file. >> > > >> >> >> > > >> >> We use a dd comment to change the e_type field in the ELF header from >> > > >> >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o will be accepted by lld. >> > > >> >> >> > > >> >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") >> > > >> >> Cc: Stanislav Fomichev <sdf@google.com> >> > > >> >> Cc: Nick Desaulniers <ndesaulniers@google.com> >> > > >> >> Tested-by: Nick Desaulniers <ndesaulniers@google.com> >> > > >> >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> >> > > >> >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 >> > > >> >> Signed-off-by: Fangrui Song <maskray@google.com> >> > > >> >> --- >> > > >> >> scripts/link-vmlinux.sh | 13 ++----------- >> > > >> >> 1 file changed, 2 insertions(+), 11 deletions(-) >> > > >> >> >> > > >> >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh >> > > >> >> index dd484e92752e..84be8d7c361d 100755 >> > > >> >> --- a/scripts/link-vmlinux.sh >> > > >> >> +++ b/scripts/link-vmlinux.sh >> > > >> >> @@ -120,18 +120,9 @@ gen_btf() >> > > >> >> >> > > >> >> info "BTF" ${2} >> > > >> >> vmlinux_link ${1} >> > > >> >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} >> > > >> > >> > > >> >Is it really tested? Seems like you just dropped .BTF generation step >> > > >> >completely... >> > > >> >> > > >> Sorry, dropped the whole line:/ >> > > >> I don't know how to test .BTF . I can only check readelf -S... >> > > >> >> > > >> Attached the new patch. >> > > >> >> > > >> >> > > >> From 02afb9417d4f0f8d2175c94fc3797a94a95cc248 Mon Sep 17 00:00:00 2001 >> > > >> From: Fangrui Song <maskray@google.com> >> > > >> Date: Mon, 16 Mar 2020 18:02:31 -0700 >> > > >> Subject: [PATCH bpf v2] bpf: Support llvm-objcopy and llvm-objdump for >> > > >> vmlinux BTF >> > > >> >> > > >> Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. >> > > >> We use a dd comment to change the e_type field in the ELF header from >> > > >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o can be accepted by lld. >> > > >> >> > > >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") >> > > >> Cc: Stanislav Fomichev <sdf@google.com> >> > > >> Cc: Nick Desaulniers <ndesaulniers@google.com> >> > > >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> >> > > >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 >> > > >> Signed-off-by: Fangrui Song <maskray@google.com> >> > > >> --- >> > > >> scripts/link-vmlinux.sh | 14 +++----------- >> > > >> 1 file changed, 3 insertions(+), 11 deletions(-) >> > > >> >> > > >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh >> > > >> index dd484e92752e..b23313944c89 100755 >> > > >> --- a/scripts/link-vmlinux.sh >> > > >> +++ b/scripts/link-vmlinux.sh >> > > >> @@ -120,18 +120,10 @@ gen_btf() >> > > >> >> > > >> info "BTF" ${2} >> > > >> vmlinux_link ${1} >> > > >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} >> > > >> + ${PAHOLE} -J ${1} >> > > > >> > > >I'm not sure why you are touching this line at all. LLVM_OBJCOPY part >> > > >is necessary, pahole assumes llvm-objcopy by default, but that can >> > > >(and should for objcopy) be overridden with LLVM_OBJCOPY. >> > > >> > > Why is LLVM_OBJCOPY assumed? What if llvm-objcopy is not available? >> > >> > It's pahole assumption that we have to live with. pahole assumes >> > llvm-objcopy internally, unless it is overriden with LLVM_OBJCOPY env >> > var. So please revert this line otherwise you are breaking it for GCC >> > objcopy case. >> >> Acknowledged. Uploaded v3. >> >> I added back 2>/dev/null which was removed by a previous change, to >> suppress GNU objcopy warnings. The warnings could be annoying in V=1 >> output. >> >> > > This is confusing that one tool assumes llvm-objcopy while the block >> > > below immediately uses GNU objcopy (without this patch). >> > > >> > > e83b9f55448afce3fe1abcd1d10db9584f8042a6 "kbuild: add ability to >> > > generate BTF type info for vmlinux" does not say why LLVM_OBJCOPY is >> > > set. >> > > >> > > >> >> > > >> - # dump .BTF section into raw binary file to link with final vmlinux >> > > >> - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ >> > > >> - cut -d, -f1 | cut -d' ' -f2) >> > > >> - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ >> > > >> - awk '{print $4}') >> > > >> - ${OBJCOPY} --change-section-address .BTF=0 \ >> > > >> - --set-section-flags .BTF=alloc -O binary \ >> > > >> - --only-section=.BTF ${1} .btf.vmlinux.bin >> > > >> - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ >> > > >> - --rename-section .data=.BTF .btf.vmlinux.bin ${2} >> > > >> + # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux >> > > >> + ${OBJCOPY} --only-section=.BTF ${1} ${2} && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 >> > > >> } >> > > >> >> > > >> # Create ${2} .o file with all symbols from the ${1} object file >> > > >> -- >> > > >> 2.25.1.481.gfbce0eb801-goog >> > > >> >> >> From ca3597477542453e9f63185c27c162da081a4baf Mon Sep 17 00:00:00 2001 >> From: Fangrui Song <maskray@google.com> >> Date: Mon, 16 Mar 2020 22:38:23 -0700 >> Subject: [PATCH bpf v3] bpf: Support llvm-objcopy and llvm-objdump for >> vmlinux BTF >> >> Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. >> Add 2>/dev/null to suppress GNU objcopy (but not llvm-objcopy) warnings >> "empty loadable segment detected at vaddr=0xffffffff81000000, is this intentional?" >> Our use of --only-section drops many SHF_ALLOC sections which will essentially nullify >> program headers. When used as linker input, program headers are simply >> ignored. >> >> We use a dd command to change the e_type field in the ELF header from >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o can be accepted by lld. >> Accepting ET_EXEC as an input file is an extremely rare GNU ld feature >> that lld does not intend to support, because this is very error-prone. >> >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") >> Cc: Stanislav Fomichev <sdf@google.com> >> Cc: Nick Desaulniers <ndesaulniers@google.com> >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 >> Signed-off-by: Fangrui Song <maskray@google.com> >> --- >> scripts/link-vmlinux.sh | 12 ++---------- >> 1 file changed, 2 insertions(+), 10 deletions(-) >> >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh >> index dd484e92752e..c3e808a89d4a 100755 >> --- a/scripts/link-vmlinux.sh >> +++ b/scripts/link-vmlinux.sh >> @@ -122,16 +122,8 @@ gen_btf() >> vmlinux_link ${1} >> LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} >> - # dump .BTF section into raw binary file to link with final vmlinux >> - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ >> - cut -d, -f1 | cut -d' ' -f2) >> - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ >> - awk '{print $4}') >> - ${OBJCOPY} --change-section-address .BTF=0 \ >> - --set-section-flags .BTF=alloc -O binary \ >> - --only-section=.BTF ${1} .btf.vmlinux.bin >> - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ >> - --rename-section .data=.BTF .btf.vmlinux.bin ${2} >> + # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux >> + ${OBJCOPY} --only-section=.BTF ${1} ${2} 2> /dev/null && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 >No, it doesn't work unfortunately, I get "in-kernel BTF is malformed" >from the kernel. > >I think that's because -O binary adds the following: >$ nm .btf.vmxlinux.bin >00000000002f7bc9 D _binary__btf_vmlinux_bin_end >00000000002f7bc9 A _binary__btf_vmlinux_bin_size >0000000000000000 D _binary__btf_vmlinux_bin_start > >While non-binary mode doesn't: >$ nm .btf.vmxlinux.bin > >We don't add them manually in the linker map and expect objcopy to add >them, see: >https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/kernel/bpf/btf.c#n3480 Attached v4. * Added status=none to the dd command to suppress stderr output. * `objcopy -I binary` synthesized symbols are only used in BTF, not elsewhere. I think we can replace it with a more common trick, __start_$sectionname __stop_$sectionname. * GNU ld<2.23 can define __start_BTF and __stop_BTF as SHN_ABS. I think it is totally fine for a SHN_ABS symbol to be referenced by an R_X86_64_32S (absolute relocation), but arch/x86/tools/relocs.c contains an unnecessarily rigid check that rejects this. ... Invalid absolute R_X86_64_32S relocation: __start_BTF make[3]: *** [arch/x86/boot/compressed/Makefile:123: arch/x86/boot/compressed/vmlinux.relocs] Error 1 Since we are going to bump binutils version requirement to 2.23, which will completely avoid the issue. I will not mention it again. https://lore.kernel.org/lkml/202003161354.538479F16@keescook/ * I should mention that an orphan BTF (previously .BTF) could trigger a --orphan-handling=warn warning. So eventually we might need to add an output section description BTF : { *(BTF) } to the vmlinux linker script for every arch. I'll not do that in this patch, though. > >> } >> # Create ${2} .o file with all symbols from the ${1} object file >> -- >> 2.25.1.481.gfbce0eb801-goog >> From 9b694d68fefe041464eccb948f6d246fab67942d Mon Sep 17 00:00:00 2001 From: Fangrui Song <maskray@google.com> Date: Tue, 17 Mar 2020 13:51:04 -0700 Subject: [PATCH bpf-next v4] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. The existing 'file format' and 'architecture' parsing logic is brittle and does not work with llvm-objcopy/llvm-objdump. .BTF in .tmp_vmlinux.btf is non-SHF_ALLOC. Add the SHF_ALLOC flag and rename .BTF to BTF so that C code can reference the section via linker synthesized __start_BTF and __stop_BTF. This fixes a small problem that previous .BTF had the SHF_WRITE flag. Additionally, `objcopy -I binary` synthesized symbols _binary__btf_vmlinux_bin_start and _binary__btf_vmlinux_bin_start (not used elsewhere) are replaced with more common __start_BTF and __stop_BTF. Add 2>/dev/null because GNU objcopy (but not llvm-objcopy) warns "empty loadable segment detected at vaddr=0xffffffff81000000, is this intentional?" We use a dd command to change the e_type field in the ELF header from ET_EXEC to ET_REL so that lld will accept .btf.vmlinux.bin.o. Accepting ET_EXEC as an input file is an extremely rare GNU ld feature that lld does not intend to support, because this is error-prone. Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") Cc: Stanislav Fomichev <sdf@google.com> Cc: Nick Desaulniers <ndesaulniers@google.com> Reported-by: Nathan Chancellor <natechancellor@gmail.com> Link: https://github.com/ClangBuiltLinux/linux/issues/871 Signed-off-by: Fangrui Song <maskray@google.com> --- kernel/bpf/btf.c | 9 ++++----- kernel/bpf/sysfs_btf.c | 11 +++++------ scripts/link-vmlinux.sh | 16 ++++++---------- 3 files changed, 15 insertions(+), 21 deletions(-) diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 787140095e58..51fff49de561 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -3477,8 +3477,8 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size, return ERR_PTR(err); } -extern char __weak _binary__btf_vmlinux_bin_start[]; -extern char __weak _binary__btf_vmlinux_bin_end[]; +extern char __weak __start_BTF[]; +extern char __weak __stop_BTF[]; extern struct btf *btf_vmlinux; #define BPF_MAP_TYPE(_id, _ops) @@ -3605,9 +3605,8 @@ struct btf *btf_parse_vmlinux(void) } env->btf = btf; - btf->data = _binary__btf_vmlinux_bin_start; - btf->data_size = _binary__btf_vmlinux_bin_end - - _binary__btf_vmlinux_bin_start; + btf->data = __start_BTF; + btf->data_size = __stop_BTF - __start_BTF; err = btf_parse_hdr(env); if (err) diff --git a/kernel/bpf/sysfs_btf.c b/kernel/bpf/sysfs_btf.c index 7ae5dddd1fe6..3b495773de5a 100644 --- a/kernel/bpf/sysfs_btf.c +++ b/kernel/bpf/sysfs_btf.c @@ -9,15 +9,15 @@ #include <linux/sysfs.h> /* See scripts/link-vmlinux.sh, gen_btf() func for details */ -extern char __weak _binary__btf_vmlinux_bin_start[]; -extern char __weak _binary__btf_vmlinux_bin_end[]; +extern char __weak __start_BTF[]; +extern char __weak __stop_BTF[]; static ssize_t btf_vmlinux_read(struct file *file, struct kobject *kobj, struct bin_attribute *bin_attr, char *buf, loff_t off, size_t len) { - memcpy(buf, _binary__btf_vmlinux_bin_start + off, len); + memcpy(buf, __start_BTF + off, len); return len; } @@ -30,15 +30,14 @@ static struct kobject *btf_kobj; static int __init btf_vmlinux_init(void) { - if (!_binary__btf_vmlinux_bin_start) + if (!__start_BTF) return 0; btf_kobj = kobject_create_and_add("btf", kernel_kobj); if (!btf_kobj) return -ENOMEM; - bin_attr_btf_vmlinux.size = _binary__btf_vmlinux_bin_end - - _binary__btf_vmlinux_bin_start; + bin_attr_btf_vmlinux.size = __stop_BTF - __start_BTF; return sysfs_create_bin_file(btf_kobj, &bin_attr_btf_vmlinux); } diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh index dd484e92752e..c0d2ecf1bff7 100755 --- a/scripts/link-vmlinux.sh +++ b/scripts/link-vmlinux.sh @@ -122,16 +122,12 @@ gen_btf() vmlinux_link ${1} LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} - # dump .BTF section into raw binary file to link with final vmlinux - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ - cut -d, -f1 | cut -d' ' -f2) - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ - awk '{print $4}') - ${OBJCOPY} --change-section-address .BTF=0 \ - --set-section-flags .BTF=alloc -O binary \ - --only-section=.BTF ${1} .btf.vmlinux.bin - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ - --rename-section .data=.BTF .btf.vmlinux.bin ${2} + # Extract .BTF, add SHF_ALLOC, rename to BTF so that we can reference + # it via linker synthesized __start_BTF and __stop_BTF. Change e_type + # to ET_REL so that it can be used to link final vmlinux. + ${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \ + --rename-section .BTF=BTF ${1} ${2} 2>/dev/null && \ + printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 status=none } # Create ${2} .o file with all symbols from the ${1} object file -- 2.25.1.481.gfbce0eb801-goog ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH bpf-next v4] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF 2020-03-17 20:58 ` [PATCH bpf-next v4] " Fangrui Song @ 2020-03-17 23:37 ` Andrii Nakryiko 2020-03-18 17:59 ` Fangrui Song 0 siblings, 1 reply; 17+ messages in thread From: Andrii Nakryiko @ 2020-03-17 23:37 UTC (permalink / raw) To: Fangrui Song Cc: bpf, Networking, Daniel Borkmann, Nick Desaulniers, Nathan Chancellor, clang-built-linux, Stanislav Fomichev, David S. Miller, Alexei Starovoitov, Kees Cook On Tue, Mar 17, 2020 at 1:58 PM Fangrui Song <maskray@google.com> wrote: > > > On 2020-03-17, Stanislav Fomichev wrote: > >On 03/16, Fangrui Song wrote: > >> On 2020-03-16, Andrii Nakryiko wrote: > >> > On Mon, Mar 16, 2020 at 10:21 PM Fangrui Song <maskray@google.com> wrote: > >> > > > >> > > > >> > > On 2020-03-16, Andrii Nakryiko wrote: > >> > > >On Mon, Mar 16, 2020 at 8:37 PM Fangrui Song <maskray@google.com> wrote: > >> > > >> > >> > > >> On 2020-03-16, Andrii Nakryiko wrote: > >> > > >> >On Mon, Mar 16, 2020 at 6:17 PM Fangrui Song <maskray@google.com> wrote: > >> > > >> >> > >> > > >> >> Simplify gen_btf logic to make it work with llvm-objcopy and > >> > > >> >> llvm-objdump. We just need to retain one section .BTF. To do so, we can > >> > > >> >> use a simple objcopy --only-section=.BTF instead of jumping all the > >> > > >> >> hoops via an architecture-less binary file. > >> > > >> >> > >> > > >> >> We use a dd comment to change the e_type field in the ELF header from > >> > > >> >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o will be accepted by lld. > >> > > >> >> > >> > > >> >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") > >> > > >> >> Cc: Stanislav Fomichev <sdf@google.com> > >> > > >> >> Cc: Nick Desaulniers <ndesaulniers@google.com> > >> > > >> >> Tested-by: Nick Desaulniers <ndesaulniers@google.com> > >> > > >> >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> > >> > > >> >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 > >> > > >> >> Signed-off-by: Fangrui Song <maskray@google.com> > >> > > >> >> --- > >> > > >> >> scripts/link-vmlinux.sh | 13 ++----------- > >> > > >> >> 1 file changed, 2 insertions(+), 11 deletions(-) > >> > > >> >> > >> > > >> >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > >> > > >> >> index dd484e92752e..84be8d7c361d 100755 > >> > > >> >> --- a/scripts/link-vmlinux.sh > >> > > >> >> +++ b/scripts/link-vmlinux.sh > >> > > >> >> @@ -120,18 +120,9 @@ gen_btf() > >> > > >> >> > >> > > >> >> info "BTF" ${2} > >> > > >> >> vmlinux_link ${1} > >> > > >> >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} > >> > > >> > > >> > > >> >Is it really tested? Seems like you just dropped .BTF generation step > >> > > >> >completely... > >> > > >> > >> > > >> Sorry, dropped the whole line:/ > >> > > >> I don't know how to test .BTF . I can only check readelf -S... > >> > > >> > >> > > >> Attached the new patch. > >> > > >> > >> > > >> > >> > > >> From 02afb9417d4f0f8d2175c94fc3797a94a95cc248 Mon Sep 17 00:00:00 2001 > >> > > >> From: Fangrui Song <maskray@google.com> > >> > > >> Date: Mon, 16 Mar 2020 18:02:31 -0700 > >> > > >> Subject: [PATCH bpf v2] bpf: Support llvm-objcopy and llvm-objdump for > >> > > >> vmlinux BTF > >> > > >> > >> > > >> Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. > >> > > >> We use a dd comment to change the e_type field in the ELF header from > >> > > >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o can be accepted by lld. > >> > > >> > >> > > >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") > >> > > >> Cc: Stanislav Fomichev <sdf@google.com> > >> > > >> Cc: Nick Desaulniers <ndesaulniers@google.com> > >> > > >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> > >> > > >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 > >> > > >> Signed-off-by: Fangrui Song <maskray@google.com> > >> > > >> --- > >> > > >> scripts/link-vmlinux.sh | 14 +++----------- > >> > > >> 1 file changed, 3 insertions(+), 11 deletions(-) > >> > > >> > >> > > >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > >> > > >> index dd484e92752e..b23313944c89 100755 > >> > > >> --- a/scripts/link-vmlinux.sh > >> > > >> +++ b/scripts/link-vmlinux.sh > >> > > >> @@ -120,18 +120,10 @@ gen_btf() > >> > > >> > >> > > >> info "BTF" ${2} > >> > > >> vmlinux_link ${1} > >> > > >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} > >> > > >> + ${PAHOLE} -J ${1} > >> > > > > >> > > >I'm not sure why you are touching this line at all. LLVM_OBJCOPY part > >> > > >is necessary, pahole assumes llvm-objcopy by default, but that can > >> > > >(and should for objcopy) be overridden with LLVM_OBJCOPY. > >> > > > >> > > Why is LLVM_OBJCOPY assumed? What if llvm-objcopy is not available? > >> > > >> > It's pahole assumption that we have to live with. pahole assumes > >> > llvm-objcopy internally, unless it is overriden with LLVM_OBJCOPY env > >> > var. So please revert this line otherwise you are breaking it for GCC > >> > objcopy case. > >> > >> Acknowledged. Uploaded v3. > >> > >> I added back 2>/dev/null which was removed by a previous change, to > >> suppress GNU objcopy warnings. The warnings could be annoying in V=1 > >> output. > >> > >> > > This is confusing that one tool assumes llvm-objcopy while the block > >> > > below immediately uses GNU objcopy (without this patch). > >> > > > >> > > e83b9f55448afce3fe1abcd1d10db9584f8042a6 "kbuild: add ability to > >> > > generate BTF type info for vmlinux" does not say why LLVM_OBJCOPY is > >> > > set. > >> > > > >> > > >> > >> > > >> - # dump .BTF section into raw binary file to link with final vmlinux > >> > > >> - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ > >> > > >> - cut -d, -f1 | cut -d' ' -f2) > >> > > >> - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ > >> > > >> - awk '{print $4}') > >> > > >> - ${OBJCOPY} --change-section-address .BTF=0 \ > >> > > >> - --set-section-flags .BTF=alloc -O binary \ > >> > > >> - --only-section=.BTF ${1} .btf.vmlinux.bin > >> > > >> - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ > >> > > >> - --rename-section .data=.BTF .btf.vmlinux.bin ${2} > >> > > >> + # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux > >> > > >> + ${OBJCOPY} --only-section=.BTF ${1} ${2} && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 > >> > > >> } > >> > > >> > >> > > >> # Create ${2} .o file with all symbols from the ${1} object file > >> > > >> -- > >> > > >> 2.25.1.481.gfbce0eb801-goog > >> > > >> > >> > >> From ca3597477542453e9f63185c27c162da081a4baf Mon Sep 17 00:00:00 2001 > >> From: Fangrui Song <maskray@google.com> > >> Date: Mon, 16 Mar 2020 22:38:23 -0700 > >> Subject: [PATCH bpf v3] bpf: Support llvm-objcopy and llvm-objdump for > >> vmlinux BTF > >> > >> Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. > >> Add 2>/dev/null to suppress GNU objcopy (but not llvm-objcopy) warnings > >> "empty loadable segment detected at vaddr=0xffffffff81000000, is this intentional?" > >> Our use of --only-section drops many SHF_ALLOC sections which will essentially nullify > >> program headers. When used as linker input, program headers are simply > >> ignored. > >> > >> We use a dd command to change the e_type field in the ELF header from > >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o can be accepted by lld. > >> Accepting ET_EXEC as an input file is an extremely rare GNU ld feature > >> that lld does not intend to support, because this is very error-prone. > >> > >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") > >> Cc: Stanislav Fomichev <sdf@google.com> > >> Cc: Nick Desaulniers <ndesaulniers@google.com> > >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> > >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 > >> Signed-off-by: Fangrui Song <maskray@google.com> > >> --- > >> scripts/link-vmlinux.sh | 12 ++---------- > >> 1 file changed, 2 insertions(+), 10 deletions(-) > >> > >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > >> index dd484e92752e..c3e808a89d4a 100755 > >> --- a/scripts/link-vmlinux.sh > >> +++ b/scripts/link-vmlinux.sh > >> @@ -122,16 +122,8 @@ gen_btf() > >> vmlinux_link ${1} > >> LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} > >> - # dump .BTF section into raw binary file to link with final vmlinux > >> - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ > >> - cut -d, -f1 | cut -d' ' -f2) > >> - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ > >> - awk '{print $4}') > >> - ${OBJCOPY} --change-section-address .BTF=0 \ > >> - --set-section-flags .BTF=alloc -O binary \ > >> - --only-section=.BTF ${1} .btf.vmlinux.bin > >> - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ > >> - --rename-section .data=.BTF .btf.vmlinux.bin ${2} > >> + # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux > >> + ${OBJCOPY} --only-section=.BTF ${1} ${2} 2> /dev/null && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 > >No, it doesn't work unfortunately, I get "in-kernel BTF is malformed" > >from the kernel. > > > >I think that's because -O binary adds the following: > >$ nm .btf.vmxlinux.bin > >00000000002f7bc9 D _binary__btf_vmlinux_bin_end > >00000000002f7bc9 A _binary__btf_vmlinux_bin_size > >0000000000000000 D _binary__btf_vmlinux_bin_start > > > >While non-binary mode doesn't: > >$ nm .btf.vmxlinux.bin > > > >We don't add them manually in the linker map and expect objcopy to add > >them, see: > >https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/kernel/bpf/btf.c#n3480 > > Attached v4. > > * Added status=none to the dd command to suppress stderr output. > * `objcopy -I binary` synthesized symbols are only used in BTF, not > elsewhere. I think we can replace it with a more common trick, > __start_$sectionname __stop_$sectionname. > * GNU ld<2.23 can define __start_BTF and __stop_BTF as SHN_ABS. > I think it is totally fine for a SHN_ABS symbol to be referenced by an > R_X86_64_32S (absolute relocation), but arch/x86/tools/relocs.c > contains an unnecessarily rigid check that rejects this. > > ... > Invalid absolute R_X86_64_32S relocation: __start_BTF > make[3]: *** [arch/x86/boot/compressed/Makefile:123: > arch/x86/boot/compressed/vmlinux.relocs] Error 1 > > Since we are going to bump binutils version requirement to 2.23, which > will completely avoid the issue. I will not mention it again. > https://lore.kernel.org/lkml/202003161354.538479F16@keescook/ > > * I should mention that an orphan BTF (previously .BTF) could trigger > a --orphan-handling=warn warning. So eventually we might need to > add an output section description > > BTF : { *(BTF) } > > to the vmlinux linker script for every arch. > I'll not do that in this patch, though. > > > > >> } > >> # Create ${2} .o file with all symbols from the ${1} object file > >> -- > >> 2.25.1.481.gfbce0eb801-goog > >> > > From 9b694d68fefe041464eccb948f6d246fab67942d Mon Sep 17 00:00:00 2001 > From: Fangrui Song <maskray@google.com> > Date: Tue, 17 Mar 2020 13:51:04 -0700 > Subject: [PATCH bpf-next v4] bpf: Support llvm-objcopy and > llvm-objdump for vmlinux BTF > > Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. > The existing 'file format' and 'architecture' parsing logic is brittle > and does not work with llvm-objcopy/llvm-objdump. > > .BTF in .tmp_vmlinux.btf is non-SHF_ALLOC. Add the SHF_ALLOC flag and > rename .BTF to BTF so that C code can reference the section via linker > synthesized __start_BTF and __stop_BTF. This fixes a small problem that > previous .BTF had the SHF_WRITE flag. Additionally, `objcopy -I binary` > synthesized symbols _binary__btf_vmlinux_bin_start and > _binary__btf_vmlinux_bin_start (not used elsewhere) are replaced with > more common __start_BTF and __stop_BTF. > > Add 2>/dev/null because GNU objcopy (but not llvm-objcopy) warns > "empty loadable segment detected at vaddr=0xffffffff81000000, is this intentional?" > > We use a dd command to change the e_type field in the ELF header from > ET_EXEC to ET_REL so that lld will accept .btf.vmlinux.bin.o. Accepting > ET_EXEC as an input file is an extremely rare GNU ld feature that lld > does not intend to support, because this is error-prone. > > Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") > Cc: Stanislav Fomichev <sdf@google.com> > Cc: Nick Desaulniers <ndesaulniers@google.com> > Reported-by: Nathan Chancellor <natechancellor@gmail.com> > Link: https://github.com/ClangBuiltLinux/linux/issues/871 > Signed-off-by: Fangrui Song <maskray@google.com> > --- > kernel/bpf/btf.c | 9 ++++----- > kernel/bpf/sysfs_btf.c | 11 +++++------ > scripts/link-vmlinux.sh | 16 ++++++---------- > 3 files changed, 15 insertions(+), 21 deletions(-) > > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c > index 787140095e58..51fff49de561 100644 > --- a/kernel/bpf/btf.c > +++ b/kernel/bpf/btf.c > @@ -3477,8 +3477,8 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size, > return ERR_PTR(err); > } > > -extern char __weak _binary__btf_vmlinux_bin_start[]; > -extern char __weak _binary__btf_vmlinux_bin_end[]; > +extern char __weak __start_BTF[]; > +extern char __weak __stop_BTF[]; > extern struct btf *btf_vmlinux; > > #define BPF_MAP_TYPE(_id, _ops) > @@ -3605,9 +3605,8 @@ struct btf *btf_parse_vmlinux(void) > } > env->btf = btf; > > - btf->data = _binary__btf_vmlinux_bin_start; > - btf->data_size = _binary__btf_vmlinux_bin_end - > - _binary__btf_vmlinux_bin_start; > + btf->data = __start_BTF; > + btf->data_size = __stop_BTF - __start_BTF; > > err = btf_parse_hdr(env); > if (err) > diff --git a/kernel/bpf/sysfs_btf.c b/kernel/bpf/sysfs_btf.c > index 7ae5dddd1fe6..3b495773de5a 100644 > --- a/kernel/bpf/sysfs_btf.c > +++ b/kernel/bpf/sysfs_btf.c > @@ -9,15 +9,15 @@ > #include <linux/sysfs.h> > > /* See scripts/link-vmlinux.sh, gen_btf() func for details */ > -extern char __weak _binary__btf_vmlinux_bin_start[]; > -extern char __weak _binary__btf_vmlinux_bin_end[]; > +extern char __weak __start_BTF[]; > +extern char __weak __stop_BTF[]; > > static ssize_t > btf_vmlinux_read(struct file *file, struct kobject *kobj, > struct bin_attribute *bin_attr, > char *buf, loff_t off, size_t len) > { > - memcpy(buf, _binary__btf_vmlinux_bin_start + off, len); > + memcpy(buf, __start_BTF + off, len); > return len; > } > > @@ -30,15 +30,14 @@ static struct kobject *btf_kobj; > > static int __init btf_vmlinux_init(void) > { > - if (!_binary__btf_vmlinux_bin_start) > + if (!__start_BTF) > return 0; > > btf_kobj = kobject_create_and_add("btf", kernel_kobj); > if (!btf_kobj) > return -ENOMEM; > > - bin_attr_btf_vmlinux.size = _binary__btf_vmlinux_bin_end - > - _binary__btf_vmlinux_bin_start; > + bin_attr_btf_vmlinux.size = __stop_BTF - __start_BTF; > > return sysfs_create_bin_file(btf_kobj, &bin_attr_btf_vmlinux); > } > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > index dd484e92752e..c0d2ecf1bff7 100755 > --- a/scripts/link-vmlinux.sh > +++ b/scripts/link-vmlinux.sh > @@ -122,16 +122,12 @@ gen_btf() > vmlinux_link ${1} > LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} > > - # dump .BTF section into raw binary file to link with final vmlinux > - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ > - cut -d, -f1 | cut -d' ' -f2) > - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ > - awk '{print $4}') > - ${OBJCOPY} --change-section-address .BTF=0 \ > - --set-section-flags .BTF=alloc -O binary \ > - --only-section=.BTF ${1} .btf.vmlinux.bin > - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ > - --rename-section .data=.BTF .btf.vmlinux.bin ${2} > + # Extract .BTF, add SHF_ALLOC, rename to BTF so that we can reference > + # it via linker synthesized __start_BTF and __stop_BTF. Change e_type > + # to ET_REL so that it can be used to link final vmlinux. > + ${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \ > + --rename-section .BTF=BTF ${1} ${2} 2>/dev/null && \ You can't just rename .BTF into BTF. ".BTF" is part of BTF specification, tools like bpftool rely on that specific name. Libbpf relies on this name as well. It cannot be changed. Please stop making arbitrary changes and breaking stuff, please. > + printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 status=none I wonder if there is any less hacky and more obvious way to achieve this?.. Also, this script has `set -e` at the top, so you don't have to chain all command with &&, you put this on separate line with the same effect, but it will look less confusing. > } > > # Create ${2} .o file with all symbols from the ${1} object file > -- > 2.25.1.481.gfbce0eb801-goog > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH bpf-next v4] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF 2020-03-17 23:37 ` Andrii Nakryiko @ 2020-03-18 17:59 ` Fangrui Song 2020-03-18 18:30 ` Andrii Nakryiko 0 siblings, 1 reply; 17+ messages in thread From: Fangrui Song @ 2020-03-18 17:59 UTC (permalink / raw) To: Andrii Nakryiko Cc: bpf, Networking, Daniel Borkmann, Nick Desaulniers, Nathan Chancellor, clang-built-linux, Stanislav Fomichev, David S. Miller, Alexei Starovoitov, Kees Cook On 2020-03-17, Andrii Nakryiko wrote: >On Tue, Mar 17, 2020 at 1:58 PM Fangrui Song <maskray@google.com> wrote: >> >> >> On 2020-03-17, Stanislav Fomichev wrote: >> >On 03/16, Fangrui Song wrote: >> >> On 2020-03-16, Andrii Nakryiko wrote: >> >> > On Mon, Mar 16, 2020 at 10:21 PM Fangrui Song <maskray@google.com> wrote: >> >> > > >> >> > > >> >> > > On 2020-03-16, Andrii Nakryiko wrote: >> >> > > >On Mon, Mar 16, 2020 at 8:37 PM Fangrui Song <maskray@google.com> wrote: >> >> > > >> >> >> > > >> On 2020-03-16, Andrii Nakryiko wrote: >> >> > > >> >On Mon, Mar 16, 2020 at 6:17 PM Fangrui Song <maskray@google.com> wrote: >> >> > > >> >> >> >> > > >> >> Simplify gen_btf logic to make it work with llvm-objcopy and >> >> > > >> >> llvm-objdump. We just need to retain one section .BTF. To do so, we can >> >> > > >> >> use a simple objcopy --only-section=.BTF instead of jumping all the >> >> > > >> >> hoops via an architecture-less binary file. >> >> > > >> >> >> >> > > >> >> We use a dd comment to change the e_type field in the ELF header from >> >> > > >> >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o will be accepted by lld. >> >> > > >> >> >> >> > > >> >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") >> >> > > >> >> Cc: Stanislav Fomichev <sdf@google.com> >> >> > > >> >> Cc: Nick Desaulniers <ndesaulniers@google.com> >> >> > > >> >> Tested-by: Nick Desaulniers <ndesaulniers@google.com> >> >> > > >> >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> >> >> > > >> >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 >> >> > > >> >> Signed-off-by: Fangrui Song <maskray@google.com> >> >> > > >> >> --- >> >> > > >> >> scripts/link-vmlinux.sh | 13 ++----------- >> >> > > >> >> 1 file changed, 2 insertions(+), 11 deletions(-) >> >> > > >> >> >> >> > > >> >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh >> >> > > >> >> index dd484e92752e..84be8d7c361d 100755 >> >> > > >> >> --- a/scripts/link-vmlinux.sh >> >> > > >> >> +++ b/scripts/link-vmlinux.sh >> >> > > >> >> @@ -120,18 +120,9 @@ gen_btf() >> >> > > >> >> >> >> > > >> >> info "BTF" ${2} >> >> > > >> >> vmlinux_link ${1} >> >> > > >> >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} >> >> > > >> > >> >> > > >> >Is it really tested? Seems like you just dropped .BTF generation step >> >> > > >> >completely... >> >> > > >> >> >> > > >> Sorry, dropped the whole line:/ >> >> > > >> I don't know how to test .BTF . I can only check readelf -S... >> >> > > >> >> >> > > >> Attached the new patch. >> >> > > >> >> >> > > >> >> >> > > >> From 02afb9417d4f0f8d2175c94fc3797a94a95cc248 Mon Sep 17 00:00:00 2001 >> >> > > >> From: Fangrui Song <maskray@google.com> >> >> > > >> Date: Mon, 16 Mar 2020 18:02:31 -0700 >> >> > > >> Subject: [PATCH bpf v2] bpf: Support llvm-objcopy and llvm-objdump for >> >> > > >> vmlinux BTF >> >> > > >> >> >> > > >> Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. >> >> > > >> We use a dd comment to change the e_type field in the ELF header from >> >> > > >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o can be accepted by lld. >> >> > > >> >> >> > > >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") >> >> > > >> Cc: Stanislav Fomichev <sdf@google.com> >> >> > > >> Cc: Nick Desaulniers <ndesaulniers@google.com> >> >> > > >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> >> >> > > >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 >> >> > > >> Signed-off-by: Fangrui Song <maskray@google.com> >> >> > > >> --- >> >> > > >> scripts/link-vmlinux.sh | 14 +++----------- >> >> > > >> 1 file changed, 3 insertions(+), 11 deletions(-) >> >> > > >> >> >> > > >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh >> >> > > >> index dd484e92752e..b23313944c89 100755 >> >> > > >> --- a/scripts/link-vmlinux.sh >> >> > > >> +++ b/scripts/link-vmlinux.sh >> >> > > >> @@ -120,18 +120,10 @@ gen_btf() >> >> > > >> >> >> > > >> info "BTF" ${2} >> >> > > >> vmlinux_link ${1} >> >> > > >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} >> >> > > >> + ${PAHOLE} -J ${1} >> >> > > > >> >> > > >I'm not sure why you are touching this line at all. LLVM_OBJCOPY part >> >> > > >is necessary, pahole assumes llvm-objcopy by default, but that can >> >> > > >(and should for objcopy) be overridden with LLVM_OBJCOPY. >> >> > > >> >> > > Why is LLVM_OBJCOPY assumed? What if llvm-objcopy is not available? >> >> > >> >> > It's pahole assumption that we have to live with. pahole assumes >> >> > llvm-objcopy internally, unless it is overriden with LLVM_OBJCOPY env >> >> > var. So please revert this line otherwise you are breaking it for GCC >> >> > objcopy case. >> >> >> >> Acknowledged. Uploaded v3. >> >> >> >> I added back 2>/dev/null which was removed by a previous change, to >> >> suppress GNU objcopy warnings. The warnings could be annoying in V=1 >> >> output. >> >> >> >> > > This is confusing that one tool assumes llvm-objcopy while the block >> >> > > below immediately uses GNU objcopy (without this patch). >> >> > > >> >> > > e83b9f55448afce3fe1abcd1d10db9584f8042a6 "kbuild: add ability to >> >> > > generate BTF type info for vmlinux" does not say why LLVM_OBJCOPY is >> >> > > set. >> >> > > >> >> > > >> >> >> > > >> - # dump .BTF section into raw binary file to link with final vmlinux >> >> > > >> - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ >> >> > > >> - cut -d, -f1 | cut -d' ' -f2) >> >> > > >> - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ >> >> > > >> - awk '{print $4}') >> >> > > >> - ${OBJCOPY} --change-section-address .BTF=0 \ >> >> > > >> - --set-section-flags .BTF=alloc -O binary \ >> >> > > >> - --only-section=.BTF ${1} .btf.vmlinux.bin >> >> > > >> - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ >> >> > > >> - --rename-section .data=.BTF .btf.vmlinux.bin ${2} >> >> > > >> + # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux >> >> > > >> + ${OBJCOPY} --only-section=.BTF ${1} ${2} && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 >> >> > > >> } >> >> > > >> >> >> > > >> # Create ${2} .o file with all symbols from the ${1} object file >> >> > > >> -- >> >> > > >> 2.25.1.481.gfbce0eb801-goog >> >> > > >> >> >> >> >> From ca3597477542453e9f63185c27c162da081a4baf Mon Sep 17 00:00:00 2001 >> >> From: Fangrui Song <maskray@google.com> >> >> Date: Mon, 16 Mar 2020 22:38:23 -0700 >> >> Subject: [PATCH bpf v3] bpf: Support llvm-objcopy and llvm-objdump for >> >> vmlinux BTF >> >> >> >> Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. >> >> Add 2>/dev/null to suppress GNU objcopy (but not llvm-objcopy) warnings >> >> "empty loadable segment detected at vaddr=0xffffffff81000000, is this intentional?" >> >> Our use of --only-section drops many SHF_ALLOC sections which will essentially nullify >> >> program headers. When used as linker input, program headers are simply >> >> ignored. >> >> >> >> We use a dd command to change the e_type field in the ELF header from >> >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o can be accepted by lld. >> >> Accepting ET_EXEC as an input file is an extremely rare GNU ld feature >> >> that lld does not intend to support, because this is very error-prone. >> >> >> >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") >> >> Cc: Stanislav Fomichev <sdf@google.com> >> >> Cc: Nick Desaulniers <ndesaulniers@google.com> >> >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> >> >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 >> >> Signed-off-by: Fangrui Song <maskray@google.com> >> >> --- >> >> scripts/link-vmlinux.sh | 12 ++---------- >> >> 1 file changed, 2 insertions(+), 10 deletions(-) >> >> >> >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh >> >> index dd484e92752e..c3e808a89d4a 100755 >> >> --- a/scripts/link-vmlinux.sh >> >> +++ b/scripts/link-vmlinux.sh >> >> @@ -122,16 +122,8 @@ gen_btf() >> >> vmlinux_link ${1} >> >> LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} >> >> - # dump .BTF section into raw binary file to link with final vmlinux >> >> - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ >> >> - cut -d, -f1 | cut -d' ' -f2) >> >> - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ >> >> - awk '{print $4}') >> >> - ${OBJCOPY} --change-section-address .BTF=0 \ >> >> - --set-section-flags .BTF=alloc -O binary \ >> >> - --only-section=.BTF ${1} .btf.vmlinux.bin >> >> - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ >> >> - --rename-section .data=.BTF .btf.vmlinux.bin ${2} >> >> + # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux >> >> + ${OBJCOPY} --only-section=.BTF ${1} ${2} 2> /dev/null && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 >> >No, it doesn't work unfortunately, I get "in-kernel BTF is malformed" >> >from the kernel. >> > >> >I think that's because -O binary adds the following: >> >$ nm .btf.vmxlinux.bin >> >00000000002f7bc9 D _binary__btf_vmlinux_bin_end >> >00000000002f7bc9 A _binary__btf_vmlinux_bin_size >> >0000000000000000 D _binary__btf_vmlinux_bin_start >> > >> >While non-binary mode doesn't: >> >$ nm .btf.vmxlinux.bin >> > >> >We don't add them manually in the linker map and expect objcopy to add >> >them, see: >> >https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/kernel/bpf/btf.c#n3480 >> >> Attached v4. >> >> * Added status=none to the dd command to suppress stderr output. >> * `objcopy -I binary` synthesized symbols are only used in BTF, not >> elsewhere. I think we can replace it with a more common trick, >> __start_$sectionname __stop_$sectionname. >> * GNU ld<2.23 can define __start_BTF and __stop_BTF as SHN_ABS. >> I think it is totally fine for a SHN_ABS symbol to be referenced by an >> R_X86_64_32S (absolute relocation), but arch/x86/tools/relocs.c >> contains an unnecessarily rigid check that rejects this. >> >> ... >> Invalid absolute R_X86_64_32S relocation: __start_BTF >> make[3]: *** [arch/x86/boot/compressed/Makefile:123: >> arch/x86/boot/compressed/vmlinux.relocs] Error 1 >> >> Since we are going to bump binutils version requirement to 2.23, which >> will completely avoid the issue. I will not mention it again. >> https://lore.kernel.org/lkml/202003161354.538479F16@keescook/ >> >> * I should mention that an orphan BTF (previously .BTF) could trigger >> a --orphan-handling=warn warning. So eventually we might need to >> add an output section description >> >> BTF : { *(BTF) } >> >> to the vmlinux linker script for every arch. >> I'll not do that in this patch, though. >> >> > >> >> } >> >> # Create ${2} .o file with all symbols from the ${1} object file >> >> -- >> >> 2.25.1.481.gfbce0eb801-goog >> >> >> >> From 9b694d68fefe041464eccb948f6d246fab67942d Mon Sep 17 00:00:00 2001 >> From: Fangrui Song <maskray@google.com> >> Date: Tue, 17 Mar 2020 13:51:04 -0700 >> Subject: [PATCH bpf-next v4] bpf: Support llvm-objcopy and >> llvm-objdump for vmlinux BTF >> >> Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. >> The existing 'file format' and 'architecture' parsing logic is brittle >> and does not work with llvm-objcopy/llvm-objdump. >> >> .BTF in .tmp_vmlinux.btf is non-SHF_ALLOC. Add the SHF_ALLOC flag and >> rename .BTF to BTF so that C code can reference the section via linker >> synthesized __start_BTF and __stop_BTF. This fixes a small problem that >> previous .BTF had the SHF_WRITE flag. Additionally, `objcopy -I binary` >> synthesized symbols _binary__btf_vmlinux_bin_start and >> _binary__btf_vmlinux_bin_start (not used elsewhere) are replaced with >> more common __start_BTF and __stop_BTF. >> >> Add 2>/dev/null because GNU objcopy (but not llvm-objcopy) warns >> "empty loadable segment detected at vaddr=0xffffffff81000000, is this intentional?" >> >> We use a dd command to change the e_type field in the ELF header from >> ET_EXEC to ET_REL so that lld will accept .btf.vmlinux.bin.o. Accepting >> ET_EXEC as an input file is an extremely rare GNU ld feature that lld >> does not intend to support, because this is error-prone. >> >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") >> Cc: Stanislav Fomichev <sdf@google.com> >> Cc: Nick Desaulniers <ndesaulniers@google.com> >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 >> Signed-off-by: Fangrui Song <maskray@google.com> >> --- >> kernel/bpf/btf.c | 9 ++++----- >> kernel/bpf/sysfs_btf.c | 11 +++++------ >> scripts/link-vmlinux.sh | 16 ++++++---------- >> 3 files changed, 15 insertions(+), 21 deletions(-) >> >> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c >> index 787140095e58..51fff49de561 100644 >> --- a/kernel/bpf/btf.c >> +++ b/kernel/bpf/btf.c >> @@ -3477,8 +3477,8 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size, >> return ERR_PTR(err); >> } >> >> -extern char __weak _binary__btf_vmlinux_bin_start[]; >> -extern char __weak _binary__btf_vmlinux_bin_end[]; >> +extern char __weak __start_BTF[]; >> +extern char __weak __stop_BTF[]; >> extern struct btf *btf_vmlinux; >> >> #define BPF_MAP_TYPE(_id, _ops) >> @@ -3605,9 +3605,8 @@ struct btf *btf_parse_vmlinux(void) >> } >> env->btf = btf; >> >> - btf->data = _binary__btf_vmlinux_bin_start; >> - btf->data_size = _binary__btf_vmlinux_bin_end - >> - _binary__btf_vmlinux_bin_start; >> + btf->data = __start_BTF; >> + btf->data_size = __stop_BTF - __start_BTF; >> >> err = btf_parse_hdr(env); >> if (err) >> diff --git a/kernel/bpf/sysfs_btf.c b/kernel/bpf/sysfs_btf.c >> index 7ae5dddd1fe6..3b495773de5a 100644 >> --- a/kernel/bpf/sysfs_btf.c >> +++ b/kernel/bpf/sysfs_btf.c >> @@ -9,15 +9,15 @@ >> #include <linux/sysfs.h> >> >> /* See scripts/link-vmlinux.sh, gen_btf() func for details */ >> -extern char __weak _binary__btf_vmlinux_bin_start[]; >> -extern char __weak _binary__btf_vmlinux_bin_end[]; >> +extern char __weak __start_BTF[]; >> +extern char __weak __stop_BTF[]; >> >> static ssize_t >> btf_vmlinux_read(struct file *file, struct kobject *kobj, >> struct bin_attribute *bin_attr, >> char *buf, loff_t off, size_t len) >> { >> - memcpy(buf, _binary__btf_vmlinux_bin_start + off, len); >> + memcpy(buf, __start_BTF + off, len); >> return len; >> } >> >> @@ -30,15 +30,14 @@ static struct kobject *btf_kobj; >> >> static int __init btf_vmlinux_init(void) >> { >> - if (!_binary__btf_vmlinux_bin_start) >> + if (!__start_BTF) >> return 0; >> >> btf_kobj = kobject_create_and_add("btf", kernel_kobj); >> if (!btf_kobj) >> return -ENOMEM; >> >> - bin_attr_btf_vmlinux.size = _binary__btf_vmlinux_bin_end - >> - _binary__btf_vmlinux_bin_start; >> + bin_attr_btf_vmlinux.size = __stop_BTF - __start_BTF; >> >> return sysfs_create_bin_file(btf_kobj, &bin_attr_btf_vmlinux); >> } >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh >> index dd484e92752e..c0d2ecf1bff7 100755 >> --- a/scripts/link-vmlinux.sh >> +++ b/scripts/link-vmlinux.sh >> @@ -122,16 +122,12 @@ gen_btf() >> vmlinux_link ${1} >> LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} >> >> - # dump .BTF section into raw binary file to link with final vmlinux >> - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ >> - cut -d, -f1 | cut -d' ' -f2) >> - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ >> - awk '{print $4}') >> - ${OBJCOPY} --change-section-address .BTF=0 \ >> - --set-section-flags .BTF=alloc -O binary \ >> - --only-section=.BTF ${1} .btf.vmlinux.bin >> - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ >> - --rename-section .data=.BTF .btf.vmlinux.bin ${2} >> + # Extract .BTF, add SHF_ALLOC, rename to BTF so that we can reference >> + # it via linker synthesized __start_BTF and __stop_BTF. Change e_type >> + # to ET_REL so that it can be used to link final vmlinux. >> + ${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \ >> + --rename-section .BTF=BTF ${1} ${2} 2>/dev/null && \ > >You can't just rename .BTF into BTF. ".BTF" is part of BTF >specification, tools like bpftool rely on that specific name. Libbpf >relies on this name as well. It cannot be changed. Please stop making >arbitrary changes and breaking stuff, please. I can't find anything which really assumes ".BTF" under tools/bpf/bpftool % grep -r '\.BTF' Documentation/bpftool-btf.rst: .BTF section with well-defined BTF binary format data, > >> + printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 status=none > >I wonder if there is any less hacky and more obvious way to achieve this?.. No. As a maintainer of lld/ELF, I abandoned https://reviews.llvm.org/D76174 . Also as a maintainer of LLVM binary utilities, I have to complain the whole scheme is really hacky and did not get enough scrutiny when they were merged. A previous comment said pahole somehow relied on llvm-objcopy so LLVM_OBJCOPY is used, while llvm-objcopy/llvm-objdump are not really supported... Note, on March 16, I just pushed https://reviews.llvm.org/D76046 to make llvm-objdump print bfdnames to some part of the existing hacks happy... I am trying my best to make this stuff better. BTF, when merged into LLVM in December 2018, was not really the best example demonstrating how a subproject should be merged... OK, I'll stop complaining here. commit cb0cc635c7a9fa8a3a0f75d4d896721819c63add "powerpc: Include .BTF section" places .BTF somewhere in the writable section area, so if you insist on ".BTF", I'll make a similar change and add some stuff to arch/x86/kernel/vmlinux.lds.S >Also, this script has `set -e` at the top, so you don't have to chain >all command with &&, you put this on separate line with the same >effect, but it will look less confusing. Not using section "BTF" has the downside that we have to add .BTF : { PROVIDE(__start_BTF = .); *(.BTF) PROVIDE(__stop_BTF = .); } to every arch/*/kernel/vmlinux.lds.S which can use BTF. >> } >> >> # Create ${2} .o file with all symbols from the ${1} object file >> -- >> 2.25.1.481.gfbce0eb801-goog >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH bpf-next v4] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF 2020-03-18 17:59 ` Fangrui Song @ 2020-03-18 18:30 ` Andrii Nakryiko 2020-03-18 18:53 ` Fangrui Song 0 siblings, 1 reply; 17+ messages in thread From: Andrii Nakryiko @ 2020-03-18 18:30 UTC (permalink / raw) To: Fangrui Song Cc: bpf, Networking, Daniel Borkmann, Nick Desaulniers, Nathan Chancellor, clang-built-linux, Stanislav Fomichev, David S. Miller, Alexei Starovoitov, Kees Cook On Wed, Mar 18, 2020 at 10:59 AM Fangrui Song <maskray@google.com> wrote: > > > On 2020-03-17, Andrii Nakryiko wrote: > >On Tue, Mar 17, 2020 at 1:58 PM Fangrui Song <maskray@google.com> wrote: > >> > >> > >> On 2020-03-17, Stanislav Fomichev wrote: > >> >On 03/16, Fangrui Song wrote: > >> >> On 2020-03-16, Andrii Nakryiko wrote: > >> >> > On Mon, Mar 16, 2020 at 10:21 PM Fangrui Song <maskray@google.com> wrote: > >> >> > > > >> >> > > > >> >> > > On 2020-03-16, Andrii Nakryiko wrote: > >> >> > > >On Mon, Mar 16, 2020 at 8:37 PM Fangrui Song <maskray@google.com> wrote: > >> >> > > >> > >> >> > > >> On 2020-03-16, Andrii Nakryiko wrote: > >> >> > > >> >On Mon, Mar 16, 2020 at 6:17 PM Fangrui Song <maskray@google.com> wrote: > >> >> > > >> >> > >> >> > > >> >> Simplify gen_btf logic to make it work with llvm-objcopy and > >> >> > > >> >> llvm-objdump. We just need to retain one section .BTF. To do so, we can > >> >> > > >> >> use a simple objcopy --only-section=.BTF instead of jumping all the > >> >> > > >> >> hoops via an architecture-less binary file. > >> >> > > >> >> > >> >> > > >> >> We use a dd comment to change the e_type field in the ELF header from > >> >> > > >> >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o will be accepted by lld. > >> >> > > >> >> > >> >> > > >> >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") > >> >> > > >> >> Cc: Stanislav Fomichev <sdf@google.com> > >> >> > > >> >> Cc: Nick Desaulniers <ndesaulniers@google.com> > >> >> > > >> >> Tested-by: Nick Desaulniers <ndesaulniers@google.com> > >> >> > > >> >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> > >> >> > > >> >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 > >> >> > > >> >> Signed-off-by: Fangrui Song <maskray@google.com> > >> >> > > >> >> --- > >> >> > > >> >> scripts/link-vmlinux.sh | 13 ++----------- > >> >> > > >> >> 1 file changed, 2 insertions(+), 11 deletions(-) > >> >> > > >> >> > >> >> > > >> >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > >> >> > > >> >> index dd484e92752e..84be8d7c361d 100755 > >> >> > > >> >> --- a/scripts/link-vmlinux.sh > >> >> > > >> >> +++ b/scripts/link-vmlinux.sh > >> >> > > >> >> @@ -120,18 +120,9 @@ gen_btf() > >> >> > > >> >> > >> >> > > >> >> info "BTF" ${2} > >> >> > > >> >> vmlinux_link ${1} > >> >> > > >> >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} > >> >> > > >> > > >> >> > > >> >Is it really tested? Seems like you just dropped .BTF generation step > >> >> > > >> >completely... > >> >> > > >> > >> >> > > >> Sorry, dropped the whole line:/ > >> >> > > >> I don't know how to test .BTF . I can only check readelf -S... > >> >> > > >> > >> >> > > >> Attached the new patch. > >> >> > > >> > >> >> > > >> > >> >> > > >> From 02afb9417d4f0f8d2175c94fc3797a94a95cc248 Mon Sep 17 00:00:00 2001 > >> >> > > >> From: Fangrui Song <maskray@google.com> > >> >> > > >> Date: Mon, 16 Mar 2020 18:02:31 -0700 > >> >> > > >> Subject: [PATCH bpf v2] bpf: Support llvm-objcopy and llvm-objdump for > >> >> > > >> vmlinux BTF > >> >> > > >> > >> >> > > >> Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. > >> >> > > >> We use a dd comment to change the e_type field in the ELF header from > >> >> > > >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o can be accepted by lld. > >> >> > > >> > >> >> > > >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") > >> >> > > >> Cc: Stanislav Fomichev <sdf@google.com> > >> >> > > >> Cc: Nick Desaulniers <ndesaulniers@google.com> > >> >> > > >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> > >> >> > > >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 > >> >> > > >> Signed-off-by: Fangrui Song <maskray@google.com> > >> >> > > >> --- > >> >> > > >> scripts/link-vmlinux.sh | 14 +++----------- > >> >> > > >> 1 file changed, 3 insertions(+), 11 deletions(-) > >> >> > > >> > >> >> > > >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > >> >> > > >> index dd484e92752e..b23313944c89 100755 > >> >> > > >> --- a/scripts/link-vmlinux.sh > >> >> > > >> +++ b/scripts/link-vmlinux.sh > >> >> > > >> @@ -120,18 +120,10 @@ gen_btf() > >> >> > > >> > >> >> > > >> info "BTF" ${2} > >> >> > > >> vmlinux_link ${1} > >> >> > > >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} > >> >> > > >> + ${PAHOLE} -J ${1} > >> >> > > > > >> >> > > >I'm not sure why you are touching this line at all. LLVM_OBJCOPY part > >> >> > > >is necessary, pahole assumes llvm-objcopy by default, but that can > >> >> > > >(and should for objcopy) be overridden with LLVM_OBJCOPY. > >> >> > > > >> >> > > Why is LLVM_OBJCOPY assumed? What if llvm-objcopy is not available? > >> >> > > >> >> > It's pahole assumption that we have to live with. pahole assumes > >> >> > llvm-objcopy internally, unless it is overriden with LLVM_OBJCOPY env > >> >> > var. So please revert this line otherwise you are breaking it for GCC > >> >> > objcopy case. > >> >> > >> >> Acknowledged. Uploaded v3. > >> >> > >> >> I added back 2>/dev/null which was removed by a previous change, to > >> >> suppress GNU objcopy warnings. The warnings could be annoying in V=1 > >> >> output. > >> >> > >> >> > > This is confusing that one tool assumes llvm-objcopy while the block > >> >> > > below immediately uses GNU objcopy (without this patch). > >> >> > > > >> >> > > e83b9f55448afce3fe1abcd1d10db9584f8042a6 "kbuild: add ability to > >> >> > > generate BTF type info for vmlinux" does not say why LLVM_OBJCOPY is > >> >> > > set. > >> >> > > > >> >> > > >> > >> >> > > >> - # dump .BTF section into raw binary file to link with final vmlinux > >> >> > > >> - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ > >> >> > > >> - cut -d, -f1 | cut -d' ' -f2) > >> >> > > >> - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ > >> >> > > >> - awk '{print $4}') > >> >> > > >> - ${OBJCOPY} --change-section-address .BTF=0 \ > >> >> > > >> - --set-section-flags .BTF=alloc -O binary \ > >> >> > > >> - --only-section=.BTF ${1} .btf.vmlinux.bin > >> >> > > >> - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ > >> >> > > >> - --rename-section .data=.BTF .btf.vmlinux.bin ${2} > >> >> > > >> + # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux > >> >> > > >> + ${OBJCOPY} --only-section=.BTF ${1} ${2} && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 > >> >> > > >> } > >> >> > > >> > >> >> > > >> # Create ${2} .o file with all symbols from the ${1} object file > >> >> > > >> -- > >> >> > > >> 2.25.1.481.gfbce0eb801-goog > >> >> > > >> > >> >> > >> >> From ca3597477542453e9f63185c27c162da081a4baf Mon Sep 17 00:00:00 2001 > >> >> From: Fangrui Song <maskray@google.com> > >> >> Date: Mon, 16 Mar 2020 22:38:23 -0700 > >> >> Subject: [PATCH bpf v3] bpf: Support llvm-objcopy and llvm-objdump for > >> >> vmlinux BTF > >> >> > >> >> Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. > >> >> Add 2>/dev/null to suppress GNU objcopy (but not llvm-objcopy) warnings > >> >> "empty loadable segment detected at vaddr=0xffffffff81000000, is this intentional?" > >> >> Our use of --only-section drops many SHF_ALLOC sections which will essentially nullify > >> >> program headers. When used as linker input, program headers are simply > >> >> ignored. > >> >> > >> >> We use a dd command to change the e_type field in the ELF header from > >> >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o can be accepted by lld. > >> >> Accepting ET_EXEC as an input file is an extremely rare GNU ld feature > >> >> that lld does not intend to support, because this is very error-prone. > >> >> > >> >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") > >> >> Cc: Stanislav Fomichev <sdf@google.com> > >> >> Cc: Nick Desaulniers <ndesaulniers@google.com> > >> >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> > >> >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 > >> >> Signed-off-by: Fangrui Song <maskray@google.com> > >> >> --- > >> >> scripts/link-vmlinux.sh | 12 ++---------- > >> >> 1 file changed, 2 insertions(+), 10 deletions(-) > >> >> > >> >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > >> >> index dd484e92752e..c3e808a89d4a 100755 > >> >> --- a/scripts/link-vmlinux.sh > >> >> +++ b/scripts/link-vmlinux.sh > >> >> @@ -122,16 +122,8 @@ gen_btf() > >> >> vmlinux_link ${1} > >> >> LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} > >> >> - # dump .BTF section into raw binary file to link with final vmlinux > >> >> - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ > >> >> - cut -d, -f1 | cut -d' ' -f2) > >> >> - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ > >> >> - awk '{print $4}') > >> >> - ${OBJCOPY} --change-section-address .BTF=0 \ > >> >> - --set-section-flags .BTF=alloc -O binary \ > >> >> - --only-section=.BTF ${1} .btf.vmlinux.bin > >> >> - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ > >> >> - --rename-section .data=.BTF .btf.vmlinux.bin ${2} > >> >> + # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux > >> >> + ${OBJCOPY} --only-section=.BTF ${1} ${2} 2> /dev/null && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 > >> >No, it doesn't work unfortunately, I get "in-kernel BTF is malformed" > >> >from the kernel. > >> > > >> >I think that's because -O binary adds the following: > >> >$ nm .btf.vmxlinux.bin > >> >00000000002f7bc9 D _binary__btf_vmlinux_bin_end > >> >00000000002f7bc9 A _binary__btf_vmlinux_bin_size > >> >0000000000000000 D _binary__btf_vmlinux_bin_start > >> > > >> >While non-binary mode doesn't: > >> >$ nm .btf.vmxlinux.bin > >> > > >> >We don't add them manually in the linker map and expect objcopy to add > >> >them, see: > >> >https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/kernel/bpf/btf.c#n3480 > >> > >> Attached v4. > >> > >> * Added status=none to the dd command to suppress stderr output. > >> * `objcopy -I binary` synthesized symbols are only used in BTF, not > >> elsewhere. I think we can replace it with a more common trick, > >> __start_$sectionname __stop_$sectionname. > >> * GNU ld<2.23 can define __start_BTF and __stop_BTF as SHN_ABS. > >> I think it is totally fine for a SHN_ABS symbol to be referenced by an > >> R_X86_64_32S (absolute relocation), but arch/x86/tools/relocs.c > >> contains an unnecessarily rigid check that rejects this. > >> > >> ... > >> Invalid absolute R_X86_64_32S relocation: __start_BTF > >> make[3]: *** [arch/x86/boot/compressed/Makefile:123: > >> arch/x86/boot/compressed/vmlinux.relocs] Error 1 > >> > >> Since we are going to bump binutils version requirement to 2.23, which > >> will completely avoid the issue. I will not mention it again. > >> https://lore.kernel.org/lkml/202003161354.538479F16@keescook/ > >> > >> * I should mention that an orphan BTF (previously .BTF) could trigger > >> a --orphan-handling=warn warning. So eventually we might need to > >> add an output section description > >> > >> BTF : { *(BTF) } > >> > >> to the vmlinux linker script for every arch. > >> I'll not do that in this patch, though. > >> > >> > > >> >> } > >> >> # Create ${2} .o file with all symbols from the ${1} object file > >> >> -- > >> >> 2.25.1.481.gfbce0eb801-goog > >> >> > >> > >> From 9b694d68fefe041464eccb948f6d246fab67942d Mon Sep 17 00:00:00 2001 > >> From: Fangrui Song <maskray@google.com> > >> Date: Tue, 17 Mar 2020 13:51:04 -0700 > >> Subject: [PATCH bpf-next v4] bpf: Support llvm-objcopy and > >> llvm-objdump for vmlinux BTF > >> > >> Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. > >> The existing 'file format' and 'architecture' parsing logic is brittle > >> and does not work with llvm-objcopy/llvm-objdump. > >> > >> .BTF in .tmp_vmlinux.btf is non-SHF_ALLOC. Add the SHF_ALLOC flag and > >> rename .BTF to BTF so that C code can reference the section via linker > >> synthesized __start_BTF and __stop_BTF. This fixes a small problem that > >> previous .BTF had the SHF_WRITE flag. Additionally, `objcopy -I binary` > >> synthesized symbols _binary__btf_vmlinux_bin_start and > >> _binary__btf_vmlinux_bin_start (not used elsewhere) are replaced with > >> more common __start_BTF and __stop_BTF. > >> > >> Add 2>/dev/null because GNU objcopy (but not llvm-objcopy) warns > >> "empty loadable segment detected at vaddr=0xffffffff81000000, is this intentional?" > >> > >> We use a dd command to change the e_type field in the ELF header from > >> ET_EXEC to ET_REL so that lld will accept .btf.vmlinux.bin.o. Accepting > >> ET_EXEC as an input file is an extremely rare GNU ld feature that lld > >> does not intend to support, because this is error-prone. > >> > >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") > >> Cc: Stanislav Fomichev <sdf@google.com> > >> Cc: Nick Desaulniers <ndesaulniers@google.com> > >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> > >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 > >> Signed-off-by: Fangrui Song <maskray@google.com> > >> --- > >> kernel/bpf/btf.c | 9 ++++----- > >> kernel/bpf/sysfs_btf.c | 11 +++++------ > >> scripts/link-vmlinux.sh | 16 ++++++---------- > >> 3 files changed, 15 insertions(+), 21 deletions(-) > >> > >> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c > >> index 787140095e58..51fff49de561 100644 > >> --- a/kernel/bpf/btf.c > >> +++ b/kernel/bpf/btf.c > >> @@ -3477,8 +3477,8 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size, > >> return ERR_PTR(err); > >> } > >> > >> -extern char __weak _binary__btf_vmlinux_bin_start[]; > >> -extern char __weak _binary__btf_vmlinux_bin_end[]; > >> +extern char __weak __start_BTF[]; > >> +extern char __weak __stop_BTF[]; > >> extern struct btf *btf_vmlinux; > >> > >> #define BPF_MAP_TYPE(_id, _ops) > >> @@ -3605,9 +3605,8 @@ struct btf *btf_parse_vmlinux(void) > >> } > >> env->btf = btf; > >> > >> - btf->data = _binary__btf_vmlinux_bin_start; > >> - btf->data_size = _binary__btf_vmlinux_bin_end - > >> - _binary__btf_vmlinux_bin_start; > >> + btf->data = __start_BTF; > >> + btf->data_size = __stop_BTF - __start_BTF; > >> > >> err = btf_parse_hdr(env); > >> if (err) > >> diff --git a/kernel/bpf/sysfs_btf.c b/kernel/bpf/sysfs_btf.c > >> index 7ae5dddd1fe6..3b495773de5a 100644 > >> --- a/kernel/bpf/sysfs_btf.c > >> +++ b/kernel/bpf/sysfs_btf.c > >> @@ -9,15 +9,15 @@ > >> #include <linux/sysfs.h> > >> > >> /* See scripts/link-vmlinux.sh, gen_btf() func for details */ > >> -extern char __weak _binary__btf_vmlinux_bin_start[]; > >> -extern char __weak _binary__btf_vmlinux_bin_end[]; > >> +extern char __weak __start_BTF[]; > >> +extern char __weak __stop_BTF[]; > >> > >> static ssize_t > >> btf_vmlinux_read(struct file *file, struct kobject *kobj, > >> struct bin_attribute *bin_attr, > >> char *buf, loff_t off, size_t len) > >> { > >> - memcpy(buf, _binary__btf_vmlinux_bin_start + off, len); > >> + memcpy(buf, __start_BTF + off, len); > >> return len; > >> } > >> > >> @@ -30,15 +30,14 @@ static struct kobject *btf_kobj; > >> > >> static int __init btf_vmlinux_init(void) > >> { > >> - if (!_binary__btf_vmlinux_bin_start) > >> + if (!__start_BTF) > >> return 0; > >> > >> btf_kobj = kobject_create_and_add("btf", kernel_kobj); > >> if (!btf_kobj) > >> return -ENOMEM; > >> > >> - bin_attr_btf_vmlinux.size = _binary__btf_vmlinux_bin_end - > >> - _binary__btf_vmlinux_bin_start; > >> + bin_attr_btf_vmlinux.size = __stop_BTF - __start_BTF; > >> > >> return sysfs_create_bin_file(btf_kobj, &bin_attr_btf_vmlinux); > >> } > >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > >> index dd484e92752e..c0d2ecf1bff7 100755 > >> --- a/scripts/link-vmlinux.sh > >> +++ b/scripts/link-vmlinux.sh > >> @@ -122,16 +122,12 @@ gen_btf() > >> vmlinux_link ${1} > >> LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} > >> > >> - # dump .BTF section into raw binary file to link with final vmlinux > >> - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ > >> - cut -d, -f1 | cut -d' ' -f2) > >> - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ > >> - awk '{print $4}') > >> - ${OBJCOPY} --change-section-address .BTF=0 \ > >> - --set-section-flags .BTF=alloc -O binary \ > >> - --only-section=.BTF ${1} .btf.vmlinux.bin > >> - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ > >> - --rename-section .data=.BTF .btf.vmlinux.bin ${2} > >> + # Extract .BTF, add SHF_ALLOC, rename to BTF so that we can reference > >> + # it via linker synthesized __start_BTF and __stop_BTF. Change e_type > >> + # to ET_REL so that it can be used to link final vmlinux. > >> + ${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \ > >> + --rename-section .BTF=BTF ${1} ${2} 2>/dev/null && \ > > > >You can't just rename .BTF into BTF. ".BTF" is part of BTF > >specification, tools like bpftool rely on that specific name. Libbpf > >relies on this name as well. It cannot be changed. Please stop making > >arbitrary changes and breaking stuff, please. > > I can't find anything which really assumes ".BTF" under tools/bpf/bpftool It's in libbpf, which bpftool uses to load and work with BTF. See tools/lib/bpf/btf.{c,h}. And at this point there are other tools and apps that rely on .BTF section, so there is absolutely no way we are going to rename this, I'm sorry. > > % grep -r '\.BTF' > Documentation/bpftool-btf.rst: .BTF section with well-defined BTF binary format data, > > > > >> + printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 status=none > > > >I wonder if there is any less hacky and more obvious way to achieve this?.. > > No. As a maintainer of lld/ELF, I abandoned https://reviews.llvm.org/D76174 . > Also as a maintainer of LLVM binary utilities, I have to complain the whole > scheme is really hacky and did not get enough scrutiny when they were merged. > > A previous comment said pahole somehow relied on llvm-objcopy so LLVM_OBJCOPY > is used, while llvm-objcopy/llvm-objdump are not really supported... Note, on pahole relies on llvm-objcopy internally to add/replace .BTF section. Instead of doubting everything I'm saying, you could just have grepped for 'llvm-objcopy' in pahole sources. It can be done differently, I'm sure, but we'll need to support old versions of pahole anyways, so LLVM_OBJCOPY=${OBJCOPY} parts stays, however confusing it might look. > March 16, I just pushed https://reviews.llvm.org/D76046 to make llvm-objdump > print bfdnames to some part of the existing hacks happy... > > I am trying my best to make this stuff better. > BTF, when merged into LLVM in December 2018, was not really the best example demonstrating how a subproject should be merged... > OK, I'll stop complaining here. We should make sure that we are not making it worse first, don't you agree? I think it's more important, because there are many happy users already and breaking them is not an option. It's v5 of your patch and every single revision is broken, so I'm not sure you are the one to complain here. The least you can do to test this is to build kernel with BTF and run selftests/bpf, not just throw your patch out in hopes that others will point out all the issues. > > commit cb0cc635c7a9fa8a3a0f75d4d896721819c63add "powerpc: Include .BTF section" > places .BTF somewhere in the writable section area, so if you insist on ".BTF", > I'll make a similar change and add some stuff to arch/x86/kernel/vmlinux.lds.S > > >Also, this script has `set -e` at the top, so you don't have to chain > >all command with &&, you put this on separate line with the same > >effect, but it will look less confusing. > > Not using section "BTF" has the downside that we have to add > > .BTF : { > PROVIDE(__start_BTF = .); > *(.BTF) > PROVIDE(__stop_BTF = .); > } > > to every arch/*/kernel/vmlinux.lds.S which can use BTF. It doesn't look all that terrible, so why not, I guess? But just curious, why this is not required for "BTF", but required for ".BTF"? Not very familiar with linker scripts setup in kernel. > > >> } > >> > >> # Create ${2} .o file with all symbols from the ${1} object file > >> -- > >> 2.25.1.481.gfbce0eb801-goog > >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH bpf-next v4] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF 2020-03-18 18:30 ` Andrii Nakryiko @ 2020-03-18 18:53 ` Fangrui Song 2020-03-18 19:23 ` Andrii Nakryiko 0 siblings, 1 reply; 17+ messages in thread From: Fangrui Song @ 2020-03-18 18:53 UTC (permalink / raw) To: Andrii Nakryiko Cc: bpf, Networking, Daniel Borkmann, Nick Desaulniers, Nathan Chancellor, clang-built-linux, Stanislav Fomichev, David S. Miller, Alexei Starovoitov, Kees Cook On 2020-03-18, Andrii Nakryiko wrote: >On Wed, Mar 18, 2020 at 10:59 AM Fangrui Song <maskray@google.com> wrote: >> >> >> On 2020-03-17, Andrii Nakryiko wrote: >> >On Tue, Mar 17, 2020 at 1:58 PM Fangrui Song <maskray@google.com> wrote: >> >> >> >> >> >> On 2020-03-17, Stanislav Fomichev wrote: >> >> >On 03/16, Fangrui Song wrote: >> >> >> On 2020-03-16, Andrii Nakryiko wrote: >> >> >> > On Mon, Mar 16, 2020 at 10:21 PM Fangrui Song <maskray@google.com> wrote: >> >> >> > > >> >> >> > > >> >> >> > > On 2020-03-16, Andrii Nakryiko wrote: >> >> >> > > >On Mon, Mar 16, 2020 at 8:37 PM Fangrui Song <maskray@google.com> wrote: >> >> >> > > >> >> >> >> > > >> On 2020-03-16, Andrii Nakryiko wrote: >> >> >> > > >> >On Mon, Mar 16, 2020 at 6:17 PM Fangrui Song <maskray@google.com> wrote: >> >> >> > > >> >> >> >> >> > > >> >> Simplify gen_btf logic to make it work with llvm-objcopy and >> >> >> > > >> >> llvm-objdump. We just need to retain one section .BTF. To do so, we can >> >> >> > > >> >> use a simple objcopy --only-section=.BTF instead of jumping all the >> >> >> > > >> >> hoops via an architecture-less binary file. >> >> >> > > >> >> >> >> >> > > >> >> We use a dd comment to change the e_type field in the ELF header from >> >> >> > > >> >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o will be accepted by lld. >> >> >> > > >> >> >> >> >> > > >> >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") >> >> >> > > >> >> Cc: Stanislav Fomichev <sdf@google.com> >> >> >> > > >> >> Cc: Nick Desaulniers <ndesaulniers@google.com> >> >> >> > > >> >> Tested-by: Nick Desaulniers <ndesaulniers@google.com> >> >> >> > > >> >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> >> >> >> > > >> >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 >> >> >> > > >> >> Signed-off-by: Fangrui Song <maskray@google.com> >> >> >> > > >> >> --- >> >> >> > > >> >> scripts/link-vmlinux.sh | 13 ++----------- >> >> >> > > >> >> 1 file changed, 2 insertions(+), 11 deletions(-) >> >> >> > > >> >> >> >> >> > > >> >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh >> >> >> > > >> >> index dd484e92752e..84be8d7c361d 100755 >> >> >> > > >> >> --- a/scripts/link-vmlinux.sh >> >> >> > > >> >> +++ b/scripts/link-vmlinux.sh >> >> >> > > >> >> @@ -120,18 +120,9 @@ gen_btf() >> >> >> > > >> >> >> >> >> > > >> >> info "BTF" ${2} >> >> >> > > >> >> vmlinux_link ${1} >> >> >> > > >> >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} >> >> >> > > >> > >> >> >> > > >> >Is it really tested? Seems like you just dropped .BTF generation step >> >> >> > > >> >completely... >> >> >> > > >> >> >> >> > > >> Sorry, dropped the whole line:/ >> >> >> > > >> I don't know how to test .BTF . I can only check readelf -S... >> >> >> > > >> >> >> >> > > >> Attached the new patch. >> >> >> > > >> >> >> >> > > >> >> >> >> > > >> From 02afb9417d4f0f8d2175c94fc3797a94a95cc248 Mon Sep 17 00:00:00 2001 >> >> >> > > >> From: Fangrui Song <maskray@google.com> >> >> >> > > >> Date: Mon, 16 Mar 2020 18:02:31 -0700 >> >> >> > > >> Subject: [PATCH bpf v2] bpf: Support llvm-objcopy and llvm-objdump for >> >> >> > > >> vmlinux BTF >> >> >> > > >> >> >> >> > > >> Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. >> >> >> > > >> We use a dd comment to change the e_type field in the ELF header from >> >> >> > > >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o can be accepted by lld. >> >> >> > > >> >> >> >> > > >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") >> >> >> > > >> Cc: Stanislav Fomichev <sdf@google.com> >> >> >> > > >> Cc: Nick Desaulniers <ndesaulniers@google.com> >> >> >> > > >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> >> >> >> > > >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 >> >> >> > > >> Signed-off-by: Fangrui Song <maskray@google.com> >> >> >> > > >> --- >> >> >> > > >> scripts/link-vmlinux.sh | 14 +++----------- >> >> >> > > >> 1 file changed, 3 insertions(+), 11 deletions(-) >> >> >> > > >> >> >> >> > > >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh >> >> >> > > >> index dd484e92752e..b23313944c89 100755 >> >> >> > > >> --- a/scripts/link-vmlinux.sh >> >> >> > > >> +++ b/scripts/link-vmlinux.sh >> >> >> > > >> @@ -120,18 +120,10 @@ gen_btf() >> >> >> > > >> >> >> >> > > >> info "BTF" ${2} >> >> >> > > >> vmlinux_link ${1} >> >> >> > > >> - LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} >> >> >> > > >> + ${PAHOLE} -J ${1} >> >> >> > > > >> >> >> > > >I'm not sure why you are touching this line at all. LLVM_OBJCOPY part >> >> >> > > >is necessary, pahole assumes llvm-objcopy by default, but that can >> >> >> > > >(and should for objcopy) be overridden with LLVM_OBJCOPY. >> >> >> > > >> >> >> > > Why is LLVM_OBJCOPY assumed? What if llvm-objcopy is not available? >> >> >> > >> >> >> > It's pahole assumption that we have to live with. pahole assumes >> >> >> > llvm-objcopy internally, unless it is overriden with LLVM_OBJCOPY env >> >> >> > var. So please revert this line otherwise you are breaking it for GCC >> >> >> > objcopy case. >> >> >> >> >> >> Acknowledged. Uploaded v3. >> >> >> >> >> >> I added back 2>/dev/null which was removed by a previous change, to >> >> >> suppress GNU objcopy warnings. The warnings could be annoying in V=1 >> >> >> output. >> >> >> >> >> >> > > This is confusing that one tool assumes llvm-objcopy while the block >> >> >> > > below immediately uses GNU objcopy (without this patch). >> >> >> > > >> >> >> > > e83b9f55448afce3fe1abcd1d10db9584f8042a6 "kbuild: add ability to >> >> >> > > generate BTF type info for vmlinux" does not say why LLVM_OBJCOPY is >> >> >> > > set. >> >> >> > > >> >> >> > > >> >> >> >> > > >> - # dump .BTF section into raw binary file to link with final vmlinux >> >> >> > > >> - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ >> >> >> > > >> - cut -d, -f1 | cut -d' ' -f2) >> >> >> > > >> - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ >> >> >> > > >> - awk '{print $4}') >> >> >> > > >> - ${OBJCOPY} --change-section-address .BTF=0 \ >> >> >> > > >> - --set-section-flags .BTF=alloc -O binary \ >> >> >> > > >> - --only-section=.BTF ${1} .btf.vmlinux.bin >> >> >> > > >> - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ >> >> >> > > >> - --rename-section .data=.BTF .btf.vmlinux.bin ${2} >> >> >> > > >> + # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux >> >> >> > > >> + ${OBJCOPY} --only-section=.BTF ${1} ${2} && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 >> >> >> > > >> } >> >> >> > > >> >> >> >> > > >> # Create ${2} .o file with all symbols from the ${1} object file >> >> >> > > >> -- >> >> >> > > >> 2.25.1.481.gfbce0eb801-goog >> >> >> > > >> >> >> >> >> >> >> From ca3597477542453e9f63185c27c162da081a4baf Mon Sep 17 00:00:00 2001 >> >> >> From: Fangrui Song <maskray@google.com> >> >> >> Date: Mon, 16 Mar 2020 22:38:23 -0700 >> >> >> Subject: [PATCH bpf v3] bpf: Support llvm-objcopy and llvm-objdump for >> >> >> vmlinux BTF >> >> >> >> >> >> Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. >> >> >> Add 2>/dev/null to suppress GNU objcopy (but not llvm-objcopy) warnings >> >> >> "empty loadable segment detected at vaddr=0xffffffff81000000, is this intentional?" >> >> >> Our use of --only-section drops many SHF_ALLOC sections which will essentially nullify >> >> >> program headers. When used as linker input, program headers are simply >> >> >> ignored. >> >> >> >> >> >> We use a dd command to change the e_type field in the ELF header from >> >> >> ET_EXEC to ET_REL so that .btf.vmlinux.bin.o can be accepted by lld. >> >> >> Accepting ET_EXEC as an input file is an extremely rare GNU ld feature >> >> >> that lld does not intend to support, because this is very error-prone. >> >> >> >> >> >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") >> >> >> Cc: Stanislav Fomichev <sdf@google.com> >> >> >> Cc: Nick Desaulniers <ndesaulniers@google.com> >> >> >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> >> >> >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 >> >> >> Signed-off-by: Fangrui Song <maskray@google.com> >> >> >> --- >> >> >> scripts/link-vmlinux.sh | 12 ++---------- >> >> >> 1 file changed, 2 insertions(+), 10 deletions(-) >> >> >> >> >> >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh >> >> >> index dd484e92752e..c3e808a89d4a 100755 >> >> >> --- a/scripts/link-vmlinux.sh >> >> >> +++ b/scripts/link-vmlinux.sh >> >> >> @@ -122,16 +122,8 @@ gen_btf() >> >> >> vmlinux_link ${1} >> >> >> LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} >> >> >> - # dump .BTF section into raw binary file to link with final vmlinux >> >> >> - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ >> >> >> - cut -d, -f1 | cut -d' ' -f2) >> >> >> - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ >> >> >> - awk '{print $4}') >> >> >> - ${OBJCOPY} --change-section-address .BTF=0 \ >> >> >> - --set-section-flags .BTF=alloc -O binary \ >> >> >> - --only-section=.BTF ${1} .btf.vmlinux.bin >> >> >> - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ >> >> >> - --rename-section .data=.BTF .btf.vmlinux.bin ${2} >> >> >> + # Extract .BTF section, change e_type to ET_REL, to link with final vmlinux >> >> >> + ${OBJCOPY} --only-section=.BTF ${1} ${2} 2> /dev/null && printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 >> >> >No, it doesn't work unfortunately, I get "in-kernel BTF is malformed" >> >> >from the kernel. >> >> > >> >> >I think that's because -O binary adds the following: >> >> >$ nm .btf.vmxlinux.bin >> >> >00000000002f7bc9 D _binary__btf_vmlinux_bin_end >> >> >00000000002f7bc9 A _binary__btf_vmlinux_bin_size >> >> >0000000000000000 D _binary__btf_vmlinux_bin_start >> >> > >> >> >While non-binary mode doesn't: >> >> >$ nm .btf.vmxlinux.bin >> >> > >> >> >We don't add them manually in the linker map and expect objcopy to add >> >> >them, see: >> >> >https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/kernel/bpf/btf.c#n3480 >> >> >> >> Attached v4. >> >> >> >> * Added status=none to the dd command to suppress stderr output. >> >> * `objcopy -I binary` synthesized symbols are only used in BTF, not >> >> elsewhere. I think we can replace it with a more common trick, >> >> __start_$sectionname __stop_$sectionname. >> >> * GNU ld<2.23 can define __start_BTF and __stop_BTF as SHN_ABS. >> >> I think it is totally fine for a SHN_ABS symbol to be referenced by an >> >> R_X86_64_32S (absolute relocation), but arch/x86/tools/relocs.c >> >> contains an unnecessarily rigid check that rejects this. >> >> >> >> ... >> >> Invalid absolute R_X86_64_32S relocation: __start_BTF >> >> make[3]: *** [arch/x86/boot/compressed/Makefile:123: >> >> arch/x86/boot/compressed/vmlinux.relocs] Error 1 >> >> >> >> Since we are going to bump binutils version requirement to 2.23, which >> >> will completely avoid the issue. I will not mention it again. >> >> https://lore.kernel.org/lkml/202003161354.538479F16@keescook/ >> >> >> >> * I should mention that an orphan BTF (previously .BTF) could trigger >> >> a --orphan-handling=warn warning. So eventually we might need to >> >> add an output section description >> >> >> >> BTF : { *(BTF) } >> >> >> >> to the vmlinux linker script for every arch. >> >> I'll not do that in this patch, though. >> >> >> >> > >> >> >> } >> >> >> # Create ${2} .o file with all symbols from the ${1} object file >> >> >> -- >> >> >> 2.25.1.481.gfbce0eb801-goog >> >> >> >> >> >> >> From 9b694d68fefe041464eccb948f6d246fab67942d Mon Sep 17 00:00:00 2001 >> >> From: Fangrui Song <maskray@google.com> >> >> Date: Tue, 17 Mar 2020 13:51:04 -0700 >> >> Subject: [PATCH bpf-next v4] bpf: Support llvm-objcopy and >> >> llvm-objdump for vmlinux BTF >> >> >> >> Simplify gen_btf logic to make it work with llvm-objcopy and llvm-objdump. >> >> The existing 'file format' and 'architecture' parsing logic is brittle >> >> and does not work with llvm-objcopy/llvm-objdump. >> >> >> >> .BTF in .tmp_vmlinux.btf is non-SHF_ALLOC. Add the SHF_ALLOC flag and >> >> rename .BTF to BTF so that C code can reference the section via linker >> >> synthesized __start_BTF and __stop_BTF. This fixes a small problem that >> >> previous .BTF had the SHF_WRITE flag. Additionally, `objcopy -I binary` >> >> synthesized symbols _binary__btf_vmlinux_bin_start and >> >> _binary__btf_vmlinux_bin_start (not used elsewhere) are replaced with >> >> more common __start_BTF and __stop_BTF. >> >> >> >> Add 2>/dev/null because GNU objcopy (but not llvm-objcopy) warns >> >> "empty loadable segment detected at vaddr=0xffffffff81000000, is this intentional?" >> >> >> >> We use a dd command to change the e_type field in the ELF header from >> >> ET_EXEC to ET_REL so that lld will accept .btf.vmlinux.bin.o. Accepting >> >> ET_EXEC as an input file is an extremely rare GNU ld feature that lld >> >> does not intend to support, because this is error-prone. >> >> >> >> Fixes: df786c9b9476 ("bpf: Force .BTF section start to zero when dumping from vmlinux") >> >> Cc: Stanislav Fomichev <sdf@google.com> >> >> Cc: Nick Desaulniers <ndesaulniers@google.com> >> >> Reported-by: Nathan Chancellor <natechancellor@gmail.com> >> >> Link: https://github.com/ClangBuiltLinux/linux/issues/871 >> >> Signed-off-by: Fangrui Song <maskray@google.com> >> >> --- >> >> kernel/bpf/btf.c | 9 ++++----- >> >> kernel/bpf/sysfs_btf.c | 11 +++++------ >> >> scripts/link-vmlinux.sh | 16 ++++++---------- >> >> 3 files changed, 15 insertions(+), 21 deletions(-) >> >> >> >> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c >> >> index 787140095e58..51fff49de561 100644 >> >> --- a/kernel/bpf/btf.c >> >> +++ b/kernel/bpf/btf.c >> >> @@ -3477,8 +3477,8 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size, >> >> return ERR_PTR(err); >> >> } >> >> >> >> -extern char __weak _binary__btf_vmlinux_bin_start[]; >> >> -extern char __weak _binary__btf_vmlinux_bin_end[]; >> >> +extern char __weak __start_BTF[]; >> >> +extern char __weak __stop_BTF[]; >> >> extern struct btf *btf_vmlinux; >> >> >> >> #define BPF_MAP_TYPE(_id, _ops) >> >> @@ -3605,9 +3605,8 @@ struct btf *btf_parse_vmlinux(void) >> >> } >> >> env->btf = btf; >> >> >> >> - btf->data = _binary__btf_vmlinux_bin_start; >> >> - btf->data_size = _binary__btf_vmlinux_bin_end - >> >> - _binary__btf_vmlinux_bin_start; >> >> + btf->data = __start_BTF; >> >> + btf->data_size = __stop_BTF - __start_BTF; >> >> >> >> err = btf_parse_hdr(env); >> >> if (err) >> >> diff --git a/kernel/bpf/sysfs_btf.c b/kernel/bpf/sysfs_btf.c >> >> index 7ae5dddd1fe6..3b495773de5a 100644 >> >> --- a/kernel/bpf/sysfs_btf.c >> >> +++ b/kernel/bpf/sysfs_btf.c >> >> @@ -9,15 +9,15 @@ >> >> #include <linux/sysfs.h> >> >> >> >> /* See scripts/link-vmlinux.sh, gen_btf() func for details */ >> >> -extern char __weak _binary__btf_vmlinux_bin_start[]; >> >> -extern char __weak _binary__btf_vmlinux_bin_end[]; >> >> +extern char __weak __start_BTF[]; >> >> +extern char __weak __stop_BTF[]; >> >> >> >> static ssize_t >> >> btf_vmlinux_read(struct file *file, struct kobject *kobj, >> >> struct bin_attribute *bin_attr, >> >> char *buf, loff_t off, size_t len) >> >> { >> >> - memcpy(buf, _binary__btf_vmlinux_bin_start + off, len); >> >> + memcpy(buf, __start_BTF + off, len); >> >> return len; >> >> } >> >> >> >> @@ -30,15 +30,14 @@ static struct kobject *btf_kobj; >> >> >> >> static int __init btf_vmlinux_init(void) >> >> { >> >> - if (!_binary__btf_vmlinux_bin_start) >> >> + if (!__start_BTF) >> >> return 0; >> >> >> >> btf_kobj = kobject_create_and_add("btf", kernel_kobj); >> >> if (!btf_kobj) >> >> return -ENOMEM; >> >> >> >> - bin_attr_btf_vmlinux.size = _binary__btf_vmlinux_bin_end - >> >> - _binary__btf_vmlinux_bin_start; >> >> + bin_attr_btf_vmlinux.size = __stop_BTF - __start_BTF; >> >> >> >> return sysfs_create_bin_file(btf_kobj, &bin_attr_btf_vmlinux); >> >> } >> >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh >> >> index dd484e92752e..c0d2ecf1bff7 100755 >> >> --- a/scripts/link-vmlinux.sh >> >> +++ b/scripts/link-vmlinux.sh >> >> @@ -122,16 +122,12 @@ gen_btf() >> >> vmlinux_link ${1} >> >> LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} >> >> >> >> - # dump .BTF section into raw binary file to link with final vmlinux >> >> - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ >> >> - cut -d, -f1 | cut -d' ' -f2) >> >> - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ >> >> - awk '{print $4}') >> >> - ${OBJCOPY} --change-section-address .BTF=0 \ >> >> - --set-section-flags .BTF=alloc -O binary \ >> >> - --only-section=.BTF ${1} .btf.vmlinux.bin >> >> - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ >> >> - --rename-section .data=.BTF .btf.vmlinux.bin ${2} >> >> + # Extract .BTF, add SHF_ALLOC, rename to BTF so that we can reference >> >> + # it via linker synthesized __start_BTF and __stop_BTF. Change e_type >> >> + # to ET_REL so that it can be used to link final vmlinux. >> >> + ${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \ >> >> + --rename-section .BTF=BTF ${1} ${2} 2>/dev/null && \ >> > >> >You can't just rename .BTF into BTF. ".BTF" is part of BTF >> >specification, tools like bpftool rely on that specific name. Libbpf >> >relies on this name as well. It cannot be changed. Please stop making >> >arbitrary changes and breaking stuff, please. >> >> I can't find anything which really assumes ".BTF" under tools/bpf/bpftool > >It's in libbpf, which bpftool uses to load and work with BTF. See >tools/lib/bpf/btf.{c,h}. And at this point there are other tools and >apps that rely on .BTF section, so there is absolutely no way we are >going to rename this, I'm sorry. Stanislav informed me of the same. Thanks for the reminder. >> >> % grep -r '\.BTF' >> Documentation/bpftool-btf.rst: .BTF section with well-defined BTF binary format data, >> >> > >> >> + printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 status=none >> > >> >I wonder if there is any less hacky and more obvious way to achieve this?.. >> >> No. As a maintainer of lld/ELF, I abandoned https://reviews.llvm.org/D76174 . >> Also as a maintainer of LLVM binary utilities, I have to complain the whole >> scheme is really hacky and did not get enough scrutiny when they were merged. >> >> A previous comment said pahole somehow relied on llvm-objcopy so LLVM_OBJCOPY >> is used, while llvm-objcopy/llvm-objdump are not really supported... Note, on > >pahole relies on llvm-objcopy internally to add/replace .BTF section. >Instead of doubting everything I'm saying, you could just have grepped >for 'llvm-objcopy' in pahole sources. It can be done differently, I'm >sure, but we'll need to support old versions of pahole anyways, so >LLVM_OBJCOPY=${OBJCOPY} parts stays, however confusing it might look. > >> March 16, I just pushed https://reviews.llvm.org/D76046 to make llvm-objdump >> print bfdnames to some part of the existing hacks happy... >> >> I am trying my best to make this stuff better. >> BTF, when merged into LLVM in December 2018, was not really the best example demonstrating how a subproject should be merged... >> OK, I'll stop complaining here. > >We should make sure that we are not making it worse first, don't you >agree? I think it's more important, because there are many happy users >already and breaking them is not an option. It's v5 of your patch and >every single revision is broken, so I'm not sure you are the one to >complain here. The least you can do to test this is to build kernel >with BTF and run selftests/bpf, not just throw your patch out in hopes >that others will point out all the issues. I am late to the party. I am sorry that it has been v5 of my patch. There are plenty of my own reasons: I am a kernel newbie. So I apologize for that. Though, it should still be brought up that the various previous fixes touching this area suggest that the whole scheme makes any adjustment really really difficult. There are many subtle things affecting the best strategy here: * Before GNU ld 2.23 (more precisely, commit d9476c5a34043d72dd855cb03d124d4052b190ce), __start_foo has the st_shndx field of SHN_ABS. arch/x86/tools/relocs.c can be paranoid and reject it. * Before GNU objcopy 2.25, --dump-section was not available. * llvm-objdump<11 does not print bfdnames. I fixed it just a few days ago. * GNU objcopy<2.34 required useless -B for -I binary: https://sourceware.org/bugzilla/show_bug.cgi?id=24968 * GNU ld makes use of an error-prone feature: linkable ET_EXEC. I rejected it as an lld feature a few days ago. * We need PROVIDE in lld to avoid unnecessary symbols, or we can use #ifdef CONFIG_DEBUG_INFO_BTF * ... An experienced kernel developer who does not play enoguh linker scripts and objcopy commands may not do better than I do. >> >> commit cb0cc635c7a9fa8a3a0f75d4d896721819c63add "powerpc: Include .BTF section" >> places .BTF somewhere in the writable section area, so if you insist on ".BTF", >> I'll make a similar change and add some stuff to arch/x86/kernel/vmlinux.lds.S >> >> >Also, this script has `set -e` at the top, so you don't have to chain >> >all command with &&, you put this on separate line with the same >> >effect, but it will look less confusing. >> >> Not using section "BTF" has the downside that we have to add >> >> .BTF : { >> PROVIDE(__start_BTF = .); >> *(.BTF) >> PROVIDE(__stop_BTF = .); >> } >> >> to every arch/*/kernel/vmlinux.lds.S which can use BTF. > >It doesn't look all that terrible, so why not, I guess? But just >curious, why this is not required for "BTF", but required for ".BTF"? >Not very familiar with linker scripts setup in kernel. If every extension needing a section adds an output section description to vmlinux.lds.S, the linker script will be pretty much unmanageable. Note also that arch/powerpc/kernel/vmlinux.lds.S places .BTF in an undesired area (writable). We will fix it later. If a section name is a valid C identifier (not precisely, for example, Unicode is not really supported) will have magic __start_foo __stop_foo defined. _binary_* was never used before BTF added them. >> >> >> } >> >> >> >> # Create ${2} .o file with all symbols from the ${1} object file >> >> -- >> >> 2.25.1.481.gfbce0eb801-goog >> >> I am preparing a v6 and I need to take all the stuff into account. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH bpf-next v4] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF 2020-03-18 18:53 ` Fangrui Song @ 2020-03-18 19:23 ` Andrii Nakryiko 0 siblings, 0 replies; 17+ messages in thread From: Andrii Nakryiko @ 2020-03-18 19:23 UTC (permalink / raw) To: Fangrui Song Cc: bpf, Networking, Daniel Borkmann, Nick Desaulniers, Nathan Chancellor, clang-built-linux, Stanislav Fomichev, David S. Miller, Alexei Starovoitov, Kees Cook On Wed, Mar 18, 2020 at 11:53 AM Fangrui Song <maskray@google.com> wrote: > > On 2020-03-18, Andrii Nakryiko wrote: > >On Wed, Mar 18, 2020 at 10:59 AM Fangrui Song <maskray@google.com> wrote: > >> > >> > >> On 2020-03-17, Andrii Nakryiko wrote: > >> >On Tue, Mar 17, 2020 at 1:58 PM Fangrui Song <maskray@google.com> wrote: > >> >> > >> >> > >> >> On 2020-03-17, Stanislav Fomichev wrote: > >> >> >On 03/16, Fangrui Song wrote: > >> >> >> On 2020-03-16, Andrii Nakryiko wrote: > >> >> >> > On Mon, Mar 16, 2020 at 10:21 PM Fangrui Song <maskray@google.com> wrote: > >> >> >> > > > >> >> >> > > > >> >> >> > > On 2020-03-16, Andrii Nakryiko wrote: > >> >> >> > > >On Mon, Mar 16, 2020 at 8:37 PM Fangrui Song <maskray@google.com> wrote: > >> >> >> > > >> > >> >> >> > > >> On 2020-03-16, Andrii Nakryiko wrote: > >> >> >> > > >> >On Mon, Mar 16, 2020 at 6:17 PM Fangrui Song <maskray@google.com> wrote: > >> >> >> > > >> >> [...] > >> >> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > >> >> index dd484e92752e..c0d2ecf1bff7 100755 > >> >> --- a/scripts/link-vmlinux.sh > >> >> +++ b/scripts/link-vmlinux.sh > >> >> @@ -122,16 +122,12 @@ gen_btf() > >> >> vmlinux_link ${1} > >> >> LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} > >> >> > >> >> - # dump .BTF section into raw binary file to link with final vmlinux > >> >> - bin_arch=$(LANG=C ${OBJDUMP} -f ${1} | grep architecture | \ > >> >> - cut -d, -f1 | cut -d' ' -f2) > >> >> - bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ > >> >> - awk '{print $4}') > >> >> - ${OBJCOPY} --change-section-address .BTF=0 \ > >> >> - --set-section-flags .BTF=alloc -O binary \ > >> >> - --only-section=.BTF ${1} .btf.vmlinux.bin > >> >> - ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ > >> >> - --rename-section .data=.BTF .btf.vmlinux.bin ${2} > >> >> + # Extract .BTF, add SHF_ALLOC, rename to BTF so that we can reference > >> >> + # it via linker synthesized __start_BTF and __stop_BTF. Change e_type > >> >> + # to ET_REL so that it can be used to link final vmlinux. > >> >> + ${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \ > >> >> + --rename-section .BTF=BTF ${1} ${2} 2>/dev/null && \ > >> > > >> >You can't just rename .BTF into BTF. ".BTF" is part of BTF > >> >specification, tools like bpftool rely on that specific name. Libbpf > >> >relies on this name as well. It cannot be changed. Please stop making > >> >arbitrary changes and breaking stuff, please. > >> > >> I can't find anything which really assumes ".BTF" under tools/bpf/bpftool > > > >It's in libbpf, which bpftool uses to load and work with BTF. See > >tools/lib/bpf/btf.{c,h}. And at this point there are other tools and > >apps that rely on .BTF section, so there is absolutely no way we are > >going to rename this, I'm sorry. > > Stanislav informed me of the same. Thanks for the reminder. Good, glad we figured that out. > > >> > >> % grep -r '\.BTF' > >> Documentation/bpftool-btf.rst: .BTF section with well-defined BTF binary format data, > >> > >> > > >> >> + printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 status=none > >> > > >> >I wonder if there is any less hacky and more obvious way to achieve this?.. > >> > >> No. As a maintainer of lld/ELF, I abandoned https://reviews.llvm.org/D76174 . > >> Also as a maintainer of LLVM binary utilities, I have to complain the whole > >> scheme is really hacky and did not get enough scrutiny when they were merged. > >> > >> A previous comment said pahole somehow relied on llvm-objcopy so LLVM_OBJCOPY > >> is used, while llvm-objcopy/llvm-objdump are not really supported... Note, on > > > >pahole relies on llvm-objcopy internally to add/replace .BTF section. > >Instead of doubting everything I'm saying, you could just have grepped > >for 'llvm-objcopy' in pahole sources. It can be done differently, I'm > >sure, but we'll need to support old versions of pahole anyways, so > >LLVM_OBJCOPY=${OBJCOPY} parts stays, however confusing it might look. > > > >> March 16, I just pushed https://reviews.llvm.org/D76046 to make llvm-objdump > >> print bfdnames to some part of the existing hacks happy... > >> > >> I am trying my best to make this stuff better. > >> BTF, when merged into LLVM in December 2018, was not really the best example demonstrating how a subproject should be merged... > >> OK, I'll stop complaining here. > > > >We should make sure that we are not making it worse first, don't you > >agree? I think it's more important, because there are many happy users > >already and breaking them is not an option. It's v5 of your patch and > >every single revision is broken, so I'm not sure you are the one to > >complain here. The least you can do to test this is to build kernel > >with BTF and run selftests/bpf, not just throw your patch out in hopes > >that others will point out all the issues. > > I am late to the party. I am sorry that it has been v5 of my patch. > There are plenty of my own reasons: I am a kernel newbie. So I > apologize for that. Though, it should still be brought up that the > various previous fixes touching this area suggest that the whole scheme > makes any adjustment really really difficult. It certainly is tricky, no doubt about that. But coming in changing things drastically without understanding all the consequences and no proper testing (and building kernel is not proper testing), is not a great approach. And in addition to that complaining how everything is terrible, even though it's been working fine for quite some time for a lot of use cases (even if it doesn't work for llvm-objcopy) :) Anyways, let's stick to more productive approach. There are really two hard requirements, as far as I'm concerned: .BTF section name and that section being allocatable, so that kernel can use that data in runtime. Everything else is pretty flexible. > > There are many subtle things affecting the best strategy here: > > * Before GNU ld 2.23 (more precisely, commit d9476c5a34043d72dd855cb03d124d4052b190ce), > __start_foo has the st_shndx field of SHN_ABS. arch/x86/tools/relocs.c > can be paranoid and reject it. > * Before GNU objcopy 2.25, --dump-section was not available. > * llvm-objdump<11 does not print bfdnames. I fixed it just a few days ago. > * GNU objcopy<2.34 required useless -B for -I binary: https://sourceware.org/bugzilla/show_bug.cgi?id=24968 > * GNU ld makes use of an error-prone feature: linkable ET_EXEC. I rejected it as an lld feature a few days ago. > * We need PROVIDE in lld to avoid unnecessary symbols, or we can use #ifdef CONFIG_DEBUG_INFO_BTF > * ... > > An experienced kernel developer who does not play enoguh linker scripts > and objcopy commands may not do better than I do. I agree completely. My concern was with attitude of making changes and just building kernel to verify everything still works. BTF is a critical piece in many applications, tools and various kernel features, so running selftests/bpf (test_progs and test_verifier would be the minimum) is mandatory to ensure stuff still works. > > > >> > >> commit cb0cc635c7a9fa8a3a0f75d4d896721819c63add "powerpc: Include .BTF section" > >> places .BTF somewhere in the writable section area, so if you insist on ".BTF", > >> I'll make a similar change and add some stuff to arch/x86/kernel/vmlinux.lds.S > >> > >> >Also, this script has `set -e` at the top, so you don't have to chain > >> >all command with &&, you put this on separate line with the same > >> >effect, but it will look less confusing. > >> > >> Not using section "BTF" has the downside that we have to add > >> > >> .BTF : { > >> PROVIDE(__start_BTF = .); > >> *(.BTF) > >> PROVIDE(__stop_BTF = .); > >> } > >> > >> to every arch/*/kernel/vmlinux.lds.S which can use BTF. > > > >It doesn't look all that terrible, so why not, I guess? But just > >curious, why this is not required for "BTF", but required for ".BTF"? > >Not very familiar with linker scripts setup in kernel. > > If every extension needing a section adds an output section description > to vmlinux.lds.S, the linker script will be pretty much unmanageable. > > Note also that arch/powerpc/kernel/vmlinux.lds.S places .BTF in an > undesired area (writable). We will fix it later. > > If a section name is a valid C identifier (not precisely, for example, > Unicode is not really supported) will have magic __start_foo __stop_foo > defined. Yeah, so that's what I was curious about. But anyways, it has to be .BTF, so maybe adding special instructions for .BTF isn't such a big deal. Not sure what others think. > > _binary_* was never used before BTF added them. > __binary_* symbols are the ones produced by objcopy automatically, so that's what was used initially. If we need to change that, it's not a problem, because it's internal to kernel itself. > >> > >> >> } > >> >> > >> >> # Create ${2} .o file with all symbols from the ${1} object file > >> >> -- > >> >> 2.25.1.481.gfbce0eb801-goog > >> >> > > I am preparing a v6 and I need to take all the stuff into account. Sure, just make sure to run selftest, thanks! ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH bpf] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF 2020-03-17 1:16 [PATCH bpf] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF Fangrui Song 2020-03-17 3:10 ` Andrii Nakryiko @ 2020-03-17 17:02 ` kbuild test robot 2020-03-17 18:34 ` kbuild test robot 2 siblings, 0 replies; 17+ messages in thread From: kbuild test robot @ 2020-03-17 17:02 UTC (permalink / raw) To: Fangrui Song Cc: kbuild-all, bpf, netdev, daniel, Nick Desaulniers, Nathan Chancellor, clang-built-linux, Stanislav Fomichev, davem, ast [-- Attachment #1: Type: text/plain, Size: 2219 bytes --] Hi Fangrui, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on bpf/master] [also build test WARNING on net/master linus/master v5.6-rc6] [cannot apply to net-next/master next-20200317] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Fangrui-Song/bpf-Support-llvm-objcopy-and-llvm-objdump-for-vmlinux-BTF/20200317-125132 base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git master config: h8300-randconfig-a001-20200317 (attached as .config) compiler: h8300-linux-gcc (GCC) 9.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=9.2.0 make.cross ARCH=h8300 If you fix the issue, kindly add following tag Reported-by: kbuild test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> h8300-linux-objcopy: .tmp_vmlinux.btf: warning: empty loadable segment detected at vaddr=0x3ffeac, is this intentional? h8300-linux-objcopy: .tmp_vmlinux.btf: warning: empty loadable segment detected at vaddr=0x400048, is this intentional? h8300-linux-objcopy: .tmp_vmlinux.btf: warning: empty loadable segment detected at vaddr=0xc72720, is this intentional? h8300-linux-objcopy: .tmp_vmlinux.btf: warning: empty loadable segment detected at vaddr=0x119cf5c, is this intentional? h8300-linux-objcopy: .tmp_vmlinux.btf: warning: empty loadable segment detected at vaddr=0x119e2d0, is this intentional? h8300-linux-objcopy: .tmp_vmlinux.btf: warning: empty loadable segment detected at vaddr=0x119f000, is this intentional? h8300-linux-objcopy: .tmp_vmlinux.btf: warning: empty loadable segment detected at vaddr=0x122b300, is this intentional? 1+0 records in 1+0 records out 1 byte copied, 6.8066e-05 s, 14.7 kB/s --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 22086 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH bpf] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF 2020-03-17 1:16 [PATCH bpf] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF Fangrui Song 2020-03-17 3:10 ` Andrii Nakryiko 2020-03-17 17:02 ` [PATCH bpf] " kbuild test robot @ 2020-03-17 18:34 ` kbuild test robot 2 siblings, 0 replies; 17+ messages in thread From: kbuild test robot @ 2020-03-17 18:34 UTC (permalink / raw) To: Fangrui Song Cc: kbuild-all, bpf, netdev, daniel, Nick Desaulniers, Nathan Chancellor, clang-built-linux, Stanislav Fomichev, davem, ast [-- Attachment #1: Type: text/plain, Size: 1413 bytes --] Hi Fangrui, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on bpf/master] [also build test WARNING on net/master linus/master v5.6-rc6] [cannot apply to net-next/master next-20200317] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Fangrui-Song/bpf-Support-llvm-objcopy-and-llvm-objdump-for-vmlinux-BTF/20200317-125132 base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git master config: i386-randconfig-g003-20200317 (attached as .config) compiler: gcc-7 (Debian 7.5.0-5) 7.5.0 reproduce: # save the attached .config to linux build tree make ARCH=i386 If you fix the issue, kindly add following tag Reported-by: kbuild test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> objcopy: .tmp_vmlinux.btf: warning: empty loadable segment detected at vaddr=0xc1000000, is this intentional? objcopy: .tmp_vmlinux.btf: warning: empty loadable segment detected at vaddr=0xc1aae000, is this intentional? 1+0 records in 1+0 records out 1 byte copied, 8.22e-05 s, 12.2 kB/s --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 33334 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-03-18 19:23 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-17 1:16 [PATCH bpf] bpf: Support llvm-objcopy and llvm-objdump for vmlinux BTF Fangrui Song 2020-03-17 3:10 ` Andrii Nakryiko 2020-03-17 3:37 ` [PATCH bpf v2] " Fangrui Song 2020-03-17 5:09 ` Andrii Nakryiko 2020-03-17 5:21 ` Fangrui Song 2020-03-17 5:31 ` Andrii Nakryiko 2020-03-17 5:43 ` [PATCH bpf v3] " Fangrui Song 2020-03-17 16:05 ` Stanislav Fomichev 2020-03-17 16:24 ` Stanislav Fomichev 2020-03-17 20:58 ` [PATCH bpf-next v4] " Fangrui Song 2020-03-17 23:37 ` Andrii Nakryiko 2020-03-18 17:59 ` Fangrui Song 2020-03-18 18:30 ` Andrii Nakryiko 2020-03-18 18:53 ` Fangrui Song 2020-03-18 19:23 ` Andrii Nakryiko 2020-03-17 17:02 ` [PATCH bpf] " kbuild test robot 2020-03-17 18:34 ` kbuild test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).