linux-kernel - Re: [PATCH v4 2/2] tools/resolve_btfids: Alter how HOSTCC is forced

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAP-5=fVvB7CO0qzO4rSJiS3EqpmBn2dCcCYE70zmAVnFa_zebA@mail.gmail.com>
Date:   Tue, 31 Jan 2023 18:00:49 -0800
From:   Ian Rogers <irogers@...gle.com>
To:     Jiri Olsa <olsajiri@...il.com>
Cc:     Nathan Chancellor <nathan@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <martin.lau@...ux.dev>,
        Song Liu <song@...nel.org>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        Stanislav Fomichev <sdf@...gle.com>,
        Hao Luo <haoluo@...gle.com>,
        Connor OBrien <connoro@...gle.com>,
        Kumar Kartikeya Dwivedi <memxor@...il.com>,
        bpf@...r.kernel.org, linux-kernel@...r.kernel.org,
        llvm@...ts.linux.dev
Subject: Re: [PATCH v4 2/2] tools/resolve_btfids: Alter how HOSTCC is forced

On Tue, Jan 31, 2023 at 2:50 PM Ian Rogers <irogers@...gle.com> wrote:
>
> On Tue, Jan 31, 2023 at 2:40 PM Jiri Olsa <olsajiri@...il.com> wrote:
> >
> > On Tue, Jan 31, 2023 at 02:17:08PM -0700, Nathan Chancellor wrote:
> > > On Tue, Jan 31, 2023 at 12:59:04PM -0800, Ian Rogers wrote:
> > > > On Tue, Jan 31, 2023 at 11:33 AM Nathan Chancellor <nathan@...nel.org> wrote:
> > > > >
> > > > > On Tue, Jan 31, 2023 at 11:25:38AM -0800, Ian Rogers wrote:
> > > > > > On Tue, Jan 31, 2023 at 10:08 AM Ian Rogers <irogers@...gle.com> wrote:
> > > > > > >
> > > > > > > On Tue, Jan 31, 2023 at 9:21 AM Nathan Chancellor <nathan@...nel.org> wrote:
> > > > > > > >
> > > > > > > > Hi Ian,
> > > > > > > >
> > > > > > > > On Mon, Jan 23, 2023 at 10:43:24PM -0800, Ian Rogers wrote:
> > > > > > > > > HOSTCC is always wanted when building. Setting CC to HOSTCC happens
> > > > > > > > > after tools/scripts/Makefile.include is included, meaning flags are
> > > > > > > > > set assuming say CC is gcc, but then it can be later set to HOSTCC
> > > > > > > > > which may be clang. tools/scripts/Makefile.include is needed for host
> > > > > > > > > set up and common macros in objtool's Makefile. Rather than override
> > > > > > > > > CC to HOSTCC, just pass CC as HOSTCC to Makefile.build, the libsubcmd
> > > > > > > > > builds and the linkage step. This means the Makefiles don't see things
> > > > > > > > > like CC changing and tool flag determination, and similar, work
> > > > > > > > > properly.
> > > > > > > > >
> > > > > > > > > Also, clear the passed subdir as otherwise an outer build may break by
> > > > > > > > > inadvertently passing an inappropriate value.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Ian Rogers <irogers@...gle.com>
> > > > > > > > > ---
> > > > > > > > >  tools/bpf/resolve_btfids/Makefile | 17 +++++++----------
> > > > > > > > >  1 file changed, 7 insertions(+), 10 deletions(-)
> > > > > > > > >
> > > > > > > > > diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> > > > > > > > > index 1fe0082b2ecc..daed388aa5d7 100644
> > > > > > > > > --- a/tools/bpf/resolve_btfids/Makefile
> > > > > > > > > +++ b/tools/bpf/resolve_btfids/Makefile
> > > > > > > > > @@ -18,14 +18,11 @@ else
> > > > > > > > >  endif
> > > > > > > > >
> > > > > > > > >  # always use the host compiler
> > > > > > > > > -AR       = $(HOSTAR)
> > > > > > > > > -CC       = $(HOSTCC)
> > > > > > > > > -LD       = $(HOSTLD)
> > > > > > > > > -ARCH     = $(HOSTARCH)
> > > > > > > > > +HOST_OVERRIDES := AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)" ARCH="$(HOSTARCH)" \
> > > > > > > > > +               EXTRA_CFLAGS="$(HOSTCFLAGS) $(KBUILD_HOSTCFLAGS)"
> > > > > > > > > +
> > > > > > > > >  RM      ?= rm
> > > > > > > > >  CROSS_COMPILE =
> > > > > > > > > -CFLAGS  := $(KBUILD_HOSTCFLAGS)
> > > > > > > > > -LDFLAGS := $(KBUILD_HOSTLDFLAGS)
> > > > > > > > >
> > > > > > > > >  OUTPUT ?= $(srctree)/tools/bpf/resolve_btfids/
> > > > > > > > >
> > > > > > > > > @@ -56,12 +53,12 @@ $(OUTPUT) $(OUTPUT)/libsubcmd $(LIBBPF_OUT):
> > > > > > > > >
> > > > > > > > >  $(SUBCMDOBJ): fixdep FORCE | $(OUTPUT)/libsubcmd
> > > > > > > > >       $(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(SUBCMD_OUT) \
> > > > > > > > > -                 DESTDIR=$(SUBCMD_DESTDIR) prefix= \
> > > > > > > > > +                 DESTDIR=$(SUBCMD_DESTDIR) $(HOST_OVERRIDES) prefix= subdir= \
> > > > > > > > >                   $(abspath $@) install_headers
> > > > > > > > >
> > > > > > > > >  $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OUT)
> > > > > > > > >       $(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC) OUTPUT=$(LIBBPF_OUT)    \
> > > > > > > > > -                 DESTDIR=$(LIBBPF_DESTDIR) prefix= EXTRA_CFLAGS="$(CFLAGS)" \
> > > > > > > > > +                 DESTDIR=$(LIBBPF_DESTDIR) $(HOST_OVERRIDES) prefix= subdir= \
> > > > > > > > >                   $(abspath $@) install_headers
> > > > > > > > >
> > > > > > > > >  LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
> > > > > > > > > @@ -80,11 +77,11 @@ export srctree OUTPUT CFLAGS Q
> > > > > > > > >  include $(srctree)/tools/build/Makefile.include
> > > > > > > > >
> > > > > > > > >  $(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)
> > > > > > > > > -     $(Q)$(MAKE) $(build)=resolve_btfids
> > > > > > > > > +     $(Q)$(MAKE) $(build)=resolve_btfids $(HOST_OVERRIDES)
> > > > > > > > >
> > > > > > > > >  $(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
> > > > > > > > >       $(call msg,LINK,$@)
> > > > > > > > > -     $(Q)$(CC) $(BINARY_IN) $(LDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
> > > > > > > > > +     $(Q)$(HOSTCC) $(BINARY_IN) $(KBUILD_HOSTLDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
> > > > > > > > >
> > > > > > > > >  clean_objects := $(wildcard $(OUTPUT)/*.o                \
> > > > > > > > >                              $(OUTPUT)/.*.o.cmd           \
> > > > > > > > > --
> > > > > > > > > 2.39.0.246.g2a6d74b583-goog
> > > > > > > > >
> > > > > > > >
> > > > > > > > I just bisected a linking failure when building resolve_btfids with
> > > > > > > > clang to this change as commit 13e07691a16f ("tools/resolve_btfids:
> > > > > > > > Alter how HOSTCC is forced") in the bpf-next tree.
> > > > > > > >
> > > > > > > > It appears to be related to whether or not CROSS_COMPILE is specified,
> > > > > > > > which we have to do for certain architectures and configurations still.
> > > > > > > > arm64 is not one of those but it helps demonstrate the issue.
> > > > > > > >
> > > > > > > >   # Turn off CONFIG_DEBUG_INFO_REDUCED and turn on CONFIG_DEBUG_INFO_BTF
> > > > > > > >   $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- HOSTLDFLAGS=-fuse-ld=lld LLVM=1 defconfig menuconfig
> > > > > > > >
> > > > > > > >   $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- HOSTLDFLAGS=-fuse-ld=lld LLVM=1 prepare
> > > > > > > >   ld.lld: error: $LINUX_SRC/tools/bpf/resolve_btfids//resolve_btfids-in.o is incompatible with elf64-x86-64
> > > > > > > >   clang-17: error: linker command failed with exit code 1 (use -v to see invocation)
> > > > > > > >   ...
> > > > > > > >
> > > > > > > > Before your change, with V=1, I see:
> > > > > > > >
> > > > > > > > clang -Wp,-MD,$LINUX_SRC/tools/bpf/resolve_btfids/.main.o.d -Wp,-MT,$LINUX_SRC/tools/bpf/resolve_btfids/main.o -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 -fomit-frame-pointer -std=gnu11 -Wdeclaration-after-statement -g -I$LINUX_SRC/tools/include -I$LINUX_SRC/tools/include/uapi -I$LINUX_SRC/tools/bpf/resolve_btfids/libbpf/include -I$LINUX_SRC/tools/bpf/resolve_btfids/libsubcmd/include -D"BUILD_STR(s)=#s" -c -o $LINUX_SRC/tools/bpf/resolve_btfids/main.o main.c
> > > > > > > >
> > > > > > > > After, I see:
> > > > > > > >
> > > > > > > > clang -Wp,-MD,$LINUX_SRC/tools/bpf/resolve_btfids/.main.o.d -Wp,-MT,$LINUX_SRC/tools/bpf/resolve_btfids/main.o --target=aarch64-linux-gnu -g -I$LINUX_SRC/tools/include -I$LINUX_SRC/tools/include/uapi -I$LINUX_SRC/tools/bpf/resolve_btfids/libbpf/include -I$LINUX_SRC/tools/bpf/resolve_btfids/libsubcmd/include -D"BUILD_STR(s)=#s" -c -o $LINUX_SRC/tools/bpf/resolve_btfids/main.o main.c
> > > > > > > >
> > > > > > > > We seem to have taken on a '--target=aarch64-linux-gnu' (changing the
> > > > > > > > target of resolve_btfids-in.o) and we dropped the warning flags.
> > > > > > > >
> > > > > > > > I think this comes from the clang block in
> > > > > > > > tools/scripts/Makefile.include, which is included into the
> > > > > > > > resolve_btfids Makefile via tools/lib/bpf/Makefile.
> > > > > > > >
> > > > > > > > I am not super familiar with the tools build system, otherwise I would
> > > > > > > > try to provide a patch. I tried moving CROSS_COMPILE from a recursive to
> > > > > > > > simple variable ('=' -> ':=') and moving it to HOST_OVERRIDES but those
> > > > > > > > did not appear to resolve it for me.
> > > > > > > >
> > > > > > > > If there is any other information I can provide or patches I can test,
> > > > > > > > please let me know.
> > > > > > > >
> > > > > > > > Cheers,
> > > > > > > > Nathan
> > > > > > >
> > > > > > > Thanks Nathan, and thanks for all the details in the bug report. I'm
> > > > > > > looking into this.
> > > > > > >
> > > > > > > Ian
> > > > > >
> > > > > > Given the somewhat complicated cross compile I wasn't able to get a
> > > > > > reproduction. Could you see if the following addresses the problem:
> > > > >
> > > > > As long as you have an LLVM toolchain that targets AArch64 and your
> > > > > host, you should be able to reproduce this issue with those commands
> > > > > verbatim, as that command should not use any GNU binutils. I am pretty
> > > > > sure I tried it in a fresh container before reporting it but it is
> > > > > possible that I did not.
> > > >
> > > > Thanks, do you have instructions on setting up the container?
> > >
> > > Sure thing, this worked for me in the Linux source you would like to
> > > build (I marked it readonly to make sure any modifications were done in
> > > my host environment, remove the ':ro' if you would like to make edits to
> > > the source within the container). Docker should work as well but I did
> > > not test it.
> > >
> > > $ podman run --rm -ti -v $PWD:/linux:ro docker.io/archlinux:base-devel
> > > # pacman -Syyu --noconfirm bc clang lib32-glibc lld llvm python
> > > ...
> > >
> > > # To turn on CONFIG_DEBUG_INFO_BTF
> > > # make -C /linux -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- HOSTLDFLAGS=-fuse-ld=lld LLVM=1 O=/build defconfig menuconfig
> > >
> > > # make -C /linux -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- HOSTLDFLAGS=-fuse-ld=lld LLVM=1 O=/build prepare
> > > ld.lld: error: /build/tools/bpf/resolve_btfids//resolve_btfids-in.o is incompatible with elf64-x86-64
> > > clang-15: error: linker command failed with exit code 1 (use -v to see invocation)
> > >
> > > > > > ```
> > > > > > diff --git a/tools/bpf/resolve_btfids/Makefile
> > > > > > b/tools/bpf/resolve_btfids/Makefile
> > > > > > index daed388aa5d7..a06966841df4 100644
> > > > > > --- a/tools/bpf/resolv
> > > >
> > > > Ian
> > > > e_btfids/Makefile
> > > > > > +++ b/tools/bpf/resolve_btfids/Makefile
> > > > > > @@ -19,10 +19,9 @@ endif
> > > > > >
> > > > > > # always use the host compiler
> > > > > > HOST_OVERRIDES := AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)"
> > > > > > ARCH="$(HOSTARCH)" \
> > > > > > -                 EXTRA_CFLAGS="$(HOSTCFLAGS) $(KBUILD_HOSTCFLAGS)"
> > > > > > +                 EXTRA_CFLAGS="$(HOSTCFLAGS) $(KBUILD_HOSTCFLAGS)"
> > > > > > CROSS_COMPILE=""
> > > > > >
> > > > > > RM      ?= rm
> > > > > > -CROSS_COMPILE =
> > > > > >
> > > > > > OUTPUT ?= $(srctree)/tools/bpf/resolve_btfids/
> > > > > > ```
> > > > > >
> > > > >
> > > > > Unfortunately, it does not. I still see '--target=' end up in the
> > > > > CFLAGS of those files.
> >
> > hi,
> > I can reproduce that and I think the reason is that the patch removes
> > CFLAGS setup
> >
> >   CFLAGS  := $(KBUILD_HOSTCFLAGS)
> >
> > so now the 'include ../../scripts/Makefile.include' will set CFLAGS
> > with the  --target=... and we fail
> >
> > I can compile with the change below.. we could also set CROSS_COMPILE
> > before including tools/scripts/Makefile.include ... not sure which
> > on is better
> >
> > also I need to check if we should keep LDFLAGS setup as well
> >
> > jirka
>
> Right, but the code shouldn't be doing that - it was the purpose of
> the patch to remove it. If you include tools/scripts/Makefile.include,
> which is done on line 2, then a bunch of warnings, cflags, etc. are
> set based on CC. To then overwrite those values with various HOST
> values is at best confusing - CC no longer being CC but HOSTCC, but
> other values may or may not have changed. As we build using submakes,
> the goal here was to push the problem into the submake which will only
> ever see CC and other values being passed in.
>
> So perhaps:
> HOST_OVERRIDES := AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)"
> ARCH="$(HOSTARCH)" \
>   EXTRA_CFLAGS="$(HOSTCFLAGS) $(KBUILD_HOSTCFLAGS)" CROSS_COMPILE=""
>
> should be:
> HOST_OVERRIDES := AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)"
> ARCH="$(HOSTARCH)" \
>   CFLAGS="$(HOSTCFLAGS) $(KBUILD_HOSTCFLAGS)" CROSS_COMPILE=""
>
> I'm still trying to get a reproduction.
>
> Thanks,
> Ian

So I got a reproduction from Nathan's steps and I believe:
https://lore.kernel.org/lkml/20230201015015.359535-1-irogers@google.com/
Fixes the issue and tidies the Makefile a little. I was a little
unclear on the target being built, and tested some with -C, etc. I'm
always quite capable of making more errors though, so please help me
to check.

Apologies again for the problem and hopefully this fixes it. Thanks,
Ian

> >
> > ---
> > diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> > index daed388aa5d7..875e3fcce7cd 100644
> > --- a/tools/bpf/resolve_btfids/Makefile
> > +++ b/tools/bpf/resolve_btfids/Makefile
> > @@ -64,6 +64,7 @@ $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
> >  LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
> >  LIBELF_LIBS  := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
> >
> > +CFLAGS := $(KBUILD_HOSTCFLAGS)
> >  CFLAGS += -g \
> >            -I$(srctree)/tools/include \
> >            -I$(srctree)/tools/include/uapi \
> >