[PATCH bpf] bpf: support pre-2.25-binutils objcopy for vmlinux BTF

[PATCH bpf] bpf: support pre-2.25-binutils objcopy for vmlinux BTF

[Stanislav Fomichev]

If vmlinux BTF generation fails, but CONFIG_DEBUG_INFO_BTF is set,
.BTF section of vmlinux is empty and kernel will prohibit
BPF loading and return "in-kernel BTF is malformed".

--dump-section argument to binutils' objcopy was added in version 2.25.
When using pre-2.25 binutils, BTF generation silently fails. Convert
to --only-section which is present on pre-2.25 binutils.

Documentation/process/changes.rst states that binutils 2.21+
is supported, not sure those standards apply to BPF subsystem.

Fixes: 341dfcf8d78ea ("btf: expose BTF info through sysfs")
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 scripts/link-vmlinux.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 06495379fcd8..c56ba91f52b0 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -127,7 +127,8 @@ gen_btf()
 		cut -d, -f1 | cut -d' ' -f2)
 	bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \
 		awk '{print $4}')
-	${OBJCOPY} --dump-section .BTF=.btf.vmlinux.bin ${1} 2>/dev/null
+	${OBJCOPY} --set-section-flags .BTF=alloc -O binary \
+		--only-section=.BTF ${1} .btf.vmlinux.bin 2>/dev/null
 	${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \
 		--rename-section .data=.BTF .btf.vmlinux.bin ${2}
 }
-- 
2.24.0.432.g9d3f5f5b63-goog

Re: [PATCH bpf] bpf: support pre-2.25-binutils objcopy for vmlinux BTF

[Andrii Nakryiko]

On Tue, Nov 26, 2019 at 9:56 AM Stanislav Fomichev <sdf@google.com> wrote:
>
> If vmlinux BTF generation fails, but CONFIG_DEBUG_INFO_BTF is set,
> .BTF section of vmlinux is empty and kernel will prohibit
> BPF loading and return "in-kernel BTF is malformed".
>
> --dump-section argument to binutils' objcopy was added in version 2.25.
> When using pre-2.25 binutils, BTF generation silently fails. Convert
> to --only-section which is present on pre-2.25 binutils.
>
> Documentation/process/changes.rst states that binutils 2.21+
> is supported, not sure those standards apply to BPF subsystem.
>
> Fixes: 341dfcf8d78ea ("btf: expose BTF info through sysfs")
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---

Looks good, still works for me :)

Tested-by: Andrii Nakryiko <andriin@fb.com>

>  scripts/link-vmlinux.sh | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index 06495379fcd8..c56ba91f52b0 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -127,7 +127,8 @@ gen_btf()
>                 cut -d, -f1 | cut -d' ' -f2)
>         bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \
>                 awk '{print $4}')
> -       ${OBJCOPY} --dump-section .BTF=.btf.vmlinux.bin ${1} 2>/dev/null
> +       ${OBJCOPY} --set-section-flags .BTF=alloc -O binary \
> +               --only-section=.BTF ${1} .btf.vmlinux.bin 2>/dev/null
>         ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \
>                 --rename-section .data=.BTF .btf.vmlinux.bin ${2}
>  }
> --
> 2.24.0.432.g9d3f5f5b63-goog
>

RE: [PATCH bpf] bpf: support pre-2.25-binutils objcopy for vmlinux BTF

[John Fastabend]

Stanislav Fomichev wrote:
> If vmlinux BTF generation fails, but CONFIG_DEBUG_INFO_BTF is set,
> .BTF section of vmlinux is empty and kernel will prohibit
> BPF loading and return "in-kernel BTF is malformed".
> 
> --dump-section argument to binutils' objcopy was added in version 2.25.
> When using pre-2.25 binutils, BTF generation silently fails. Convert
> to --only-section which is present on pre-2.25 binutils.

hmm I think we should fail hard if a feature explicitly asked for
in the .config is not able to be built due to tooling. Otherwise
users may later try to use a feature that can only be supported by
BTF and that will have to fail at runtime. The runtime failure
seems more likely to surprise users compared to the inconvience
of having a compile time error. I view this similar to how having
old ssl libs fails the build with the various signing options are
set.

Can we print a useful help message instead so users can disable
CONFIG_DEBUG_INFO_BTF or update binutils?

> 
> Documentation/process/changes.rst states that binutils 2.21+
> is supported, not sure those standards apply to BPF subsystem.
> 
> Fixes: 341dfcf8d78ea ("btf: expose BTF info through sysfs")
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
>  scripts/link-vmlinux.sh | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Re: [PATCH bpf] bpf: support pre-2.25-binutils objcopy for vmlinux BTF

[Stanislav Fomichev]

On 11/26, John Fastabend wrote:
> Stanislav Fomichev wrote:
> > If vmlinux BTF generation fails, but CONFIG_DEBUG_INFO_BTF is set,
> > .BTF section of vmlinux is empty and kernel will prohibit
> > BPF loading and return "in-kernel BTF is malformed".
> > 
> > --dump-section argument to binutils' objcopy was added in version 2.25.
> > When using pre-2.25 binutils, BTF generation silently fails. Convert
> > to --only-section which is present on pre-2.25 binutils.
> 
> hmm I think we should fail hard if a feature explicitly asked for
> in the .config is not able to be built due to tooling. Otherwise
> users may later try to use a feature that can only be supported by
> BTF and that will have to fail at runtime. The runtime failure
> seems more likely to surprise users compared to the inconvience
> of having a compile time error. I view this similar to how having
> old ssl libs fails the build with the various signing options are
> set.
I agree. This is what actually happened to me. At some point
all my selftests started to fail.

> Can we print a useful help message instead so users can disable
> CONFIG_DEBUG_INFO_BTF or update binutils?
I'm not sure objcopy returns with error if it fails to execute the command.
I guess we can query the size of .BTF section in vmlinux and print
an error if it's empty.

Another thing we can do is to add a special 'data_size == 0' to
btf_parse_vmlinux. That way, at least, kernel can fall-back to
pre-BTF world instead of assuming that BTF is malformed (it's not
malformed, it's just not there). But that's, again, a surprise
at runtime. Checking at build time seems like a better option.