diff mbox series

rust: Respect HOSTCC when linking for host

Message ID 20230915172900.3784163-1-mmaurer@google.com (mailing list archive)
State New, archived
Headers show
Series rust: Respect HOSTCC when linking for host | expand

Commit Message

Matthew Maurer Sept. 15, 2023, 5:28 p.m. UTC
Currently, rustc defaults to invoking `cc`, even if `HOSTCC` is defined,
resulting in build failures in hermetic environments where `cc` does not
exist. This includes both hostprogs and proc-macros.

Since we are setting the linker to `HOSTCC`, we set the linker flavor to
`gcc` explicitly.

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
 rust/Makefile         | 1 +
 scripts/Makefile.host | 1 +
 2 files changed, 2 insertions(+)

Comments

Martin Rodriguez Reboredo Sept. 15, 2023, 10:51 p.m. UTC | #1
On 9/15/23 14:28, Matthew Maurer wrote:
> Currently, rustc defaults to invoking `cc`, even if `HOSTCC` is defined,
> resulting in build failures in hermetic environments where `cc` does not
> exist. This includes both hostprogs and proc-macros.
> 
> Since we are setting the linker to `HOSTCC`, we set the linker flavor to
> `gcc` explicitly.
> 
> Signed-off-by: Matthew Maurer <mmaurer@google.com>
> ---
> [...]

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
Fiona Behrens Sept. 16, 2023, 4:52 p.m. UTC | #2
On 15 Sep 2023, at 19:28, Matthew Maurer wrote:

> Currently, rustc defaults to invoking `cc`, even if `HOSTCC` is defined,
> resulting in build failures in hermetic environments where `cc` does not
> exist. This includes both hostprogs and proc-macros.
>
> Since we are setting the linker to `HOSTCC`, we set the linker flavor to
> `gcc` explicitly.
But as `HOSTCC` could also be clang, the linker flavor would then be wrong, would that create a problem?
>
> Signed-off-by: Matthew Maurer <mmaurer@google.com>
> ---
>  rust/Makefile         | 1 +
>  scripts/Makefile.host | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/rust/Makefile b/rust/Makefile
> index 87958e864be0..2a2352638f11 100644
> --- a/rust/Makefile
> +++ b/rust/Makefile
> @@ -383,6 +383,7 @@ $(obj)/exports_kernel_generated.h: $(obj)/kernel.o FORCE
>  quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
>        cmd_rustc_procmacro = \
>  	$(RUSTC_OR_CLIPPY) $(rust_common_flags) \
> +		-C linker-flavor=gcc -C linker=$(HOSTCC) \
>  		--emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \
>  		--crate-type proc-macro \
>  		--crate-name $(patsubst lib%.so,%,$(notdir $@)) $<
> diff --git a/scripts/Makefile.host b/scripts/Makefile.host
> index 8f7f842b54f9..0aa95a3af1c4 100644
> --- a/scripts/Makefile.host
> +++ b/scripts/Makefile.host
> @@ -91,6 +91,7 @@ hostcxx_flags  = -Wp,-MMD,$(depfile) \
>  # current working directory, which may be not accessible in the out-of-tree
>  # modules case.
>  hostrust_flags = --out-dir $(dir $@) --emit=dep-info=$(depfile) \
> +		 -C linker-flavor=gcc -C linker=$(HOSTCC) \
>                   $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
>                   $(HOSTRUSTFLAGS_$(target-stem))
>
> -- 
> 2.42.0.459.ge4e396fd5e-goog
Björn Roy Baron Sept. 16, 2023, 5:53 p.m. UTC | #3
On Saturday, September 16th, 2023 at 18:52, Finn Behrens <me@kloenk.dev> wrote:

> 
> On 15 Sep 2023, at 19:28, Matthew Maurer wrote:
> 
> > Currently, rustc defaults to invoking `cc`, even if `HOSTCC` is defined,
> > resulting in build failures in hermetic environments where `cc` does not
> > exist. This includes both hostprogs and proc-macros.
> >
> > Since we are setting the linker to `HOSTCC`, we set the linker flavor to
> > `gcc` explicitly.
> But as `HOSTCC` could also be clang, the linker flavor would then be wrong, would that create a problem?

Rustc uses the gcc linker flavor for clang too. There has been a proposal to split it up, but I'm not sure of the status of that. In any case clang's cli is similar enough to gcc that it works fine to use the gcc linker flavor.

> >
> > Signed-off-by: Matthew Maurer <mmaurer@google.com>
> > ---
> >  rust/Makefile         | 1 +
> >  scripts/Makefile.host | 1 +
> >  2 files changed, 2 insertions(+)
> >
> > diff --git a/rust/Makefile b/rust/Makefile
> > index 87958e864be0..2a2352638f11 100644
> > --- a/rust/Makefile
> > +++ b/rust/Makefile
> > @@ -383,6 +383,7 @@ $(obj)/exports_kernel_generated.h: $(obj)/kernel.o FORCE
> >  quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
> >        cmd_rustc_procmacro = \
> >  	$(RUSTC_OR_CLIPPY) $(rust_common_flags) \
> > +		-C linker-flavor=gcc -C linker=$(HOSTCC) \
> >  		--emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \
> >  		--crate-type proc-macro \
> >  		--crate-name $(patsubst lib%.so,%,$(notdir $@)) $<
> > diff --git a/scripts/Makefile.host b/scripts/Makefile.host
> > index 8f7f842b54f9..0aa95a3af1c4 100644
> > --- a/scripts/Makefile.host
> > +++ b/scripts/Makefile.host
> > @@ -91,6 +91,7 @@ hostcxx_flags  = -Wp,-MMD,$(depfile) \
> >  # current working directory, which may be not accessible in the out-of-tree
> >  # modules case.
> >  hostrust_flags = --out-dir $(dir $@) --emit=dep-info=$(depfile) \
> > +		 -C linker-flavor=gcc -C linker=$(HOSTCC) \
> >                   $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
> >                   $(HOSTRUSTFLAGS_$(target-stem))
> >
> > --
> > 2.42.0.459.ge4e396fd5e-goog
Fiona Behrens Sept. 16, 2023, 6:06 p.m. UTC | #4
On 16 Sep 2023, at 19:53, Björn Roy Baron wrote:

> On Saturday, September 16th, 2023 at 18:52, Finn Behrens <me@kloenk.dev> wrote:
>
>>
>> On 15 Sep 2023, at 19:28, Matthew Maurer wrote:
>>
>>> Currently, rustc defaults to invoking `cc`, even if `HOSTCC` is defined,
>>> resulting in build failures in hermetic environments where `cc` does not
>>> exist. This includes both hostprogs and proc-macros.
>>>
>>> Since we are setting the linker to `HOSTCC`, we set the linker flavor to
>>> `gcc` explicitly.
>> But as `HOSTCC` could also be clang, the linker flavor would then be wrong, would that create a problem?
>
> Rustc uses the gcc linker flavor for clang too. There has been a proposal to split it up, but I'm not sure of the status of that. In any case clang's cli is similar enough to gcc that it works fine to use the gcc linker flavor.
>
In that case this looks very reasonable.

Second thing I noticed is that `HOSTCC` could be the wrong variable, and `HOSTLD` would make more sense as we look for the linker and not the general C compiler.

>>>
>>> Signed-off-by: Matthew Maurer <mmaurer@google.com>
>>> ---
>>>  rust/Makefile         | 1 +
>>>  scripts/Makefile.host | 1 +
>>>  2 files changed, 2 insertions(+)
>>>
>>> diff --git a/rust/Makefile b/rust/Makefile
>>> index 87958e864be0..2a2352638f11 100644
>>> --- a/rust/Makefile
>>> +++ b/rust/Makefile
>>> @@ -383,6 +383,7 @@ $(obj)/exports_kernel_generated.h: $(obj)/kernel.o FORCE
>>>  quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
>>>        cmd_rustc_procmacro = \
>>>  	$(RUSTC_OR_CLIPPY) $(rust_common_flags) \
>>> +		-C linker-flavor=gcc -C linker=$(HOSTCC) \
>>>  		--emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \
>>>  		--crate-type proc-macro \
>>>  		--crate-name $(patsubst lib%.so,%,$(notdir $@)) $<
>>> diff --git a/scripts/Makefile.host b/scripts/Makefile.host
>>> index 8f7f842b54f9..0aa95a3af1c4 100644
>>> --- a/scripts/Makefile.host
>>> +++ b/scripts/Makefile.host
>>> @@ -91,6 +91,7 @@ hostcxx_flags  = -Wp,-MMD,$(depfile) \
>>>  # current working directory, which may be not accessible in the out-of-tree
>>>  # modules case.
>>>  hostrust_flags = --out-dir $(dir $@) --emit=dep-info=$(depfile) \
>>> +		 -C linker-flavor=gcc -C linker=$(HOSTCC) \
>>>                   $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
>>>                   $(HOSTRUSTFLAGS_$(target-stem))
>>>
>>> --
>>> 2.42.0.459.ge4e396fd5e-goog
Matthew Maurer Sept. 16, 2023, 7:54 p.m. UTC | #5
On Sat, Sep 16, 2023 at 11:07 AM Finn Behrens <me@kloenk.dev> wrote:
>
>
>
> On 16 Sep 2023, at 19:53, Björn Roy Baron wrote:
>
> > On Saturday, September 16th, 2023 at 18:52, Finn Behrens <me@kloenk.dev> wrote:
> >
> >>
> >> On 15 Sep 2023, at 19:28, Matthew Maurer wrote:
> >>
> >>> Currently, rustc defaults to invoking `cc`, even if `HOSTCC` is defined,
> >>> resulting in build failures in hermetic environments where `cc` does not
> >>> exist. This includes both hostprogs and proc-macros.
> >>>
> >>> Since we are setting the linker to `HOSTCC`, we set the linker flavor to
> >>> `gcc` explicitly.
> >> But as `HOSTCC` could also be clang, the linker flavor would then be wrong, would that create a problem?
> >
> > Rustc uses the gcc linker flavor for clang too. There has been a proposal to split it up, but I'm not sure of the status of that. In any case clang's cli is similar enough to gcc that it works fine to use the gcc linker flavor.
> >
> In that case this looks very reasonable.
>
> Second thing I noticed is that `HOSTCC` could be the wrong variable, and `HOSTLD` would make more sense as we look for the linker and not the general C compiler.
>
Yes, thanks Bjorn - "gcc" is the linker flavor used for "Use the C
compiler as a linker".

With regards to HOSTLD, I was trying to make the minimum possible
change. Currently, it is using the command `cc` as a linker, so this
would preserve existing behavior when HOSTCC is unset.

If we would prefer `HOSTLD` instead we can do that, but we would need
to additionally inspect `LLVM` to set the linker flavor accordingly
(e.g. set ld vs ld.lld).

Do folks have strong opinions between these? My primary concern is to
avoid calling programs by foo when their HOSTFOO variable is set.

See https://doc.rust-lang.org/rustc/codegen-options/index.html#linker-flavor
for details on linker flavor settings.
> >>>
> >>> Signed-off-by: Matthew Maurer <mmaurer@google.com>
> >>> ---
> >>>  rust/Makefile         | 1 +
> >>>  scripts/Makefile.host | 1 +
> >>>  2 files changed, 2 insertions(+)
> >>>
> >>> diff --git a/rust/Makefile b/rust/Makefile
> >>> index 87958e864be0..2a2352638f11 100644
> >>> --- a/rust/Makefile
> >>> +++ b/rust/Makefile
> >>> @@ -383,6 +383,7 @@ $(obj)/exports_kernel_generated.h: $(obj)/kernel.o FORCE
> >>>  quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
> >>>        cmd_rustc_procmacro = \
> >>>     $(RUSTC_OR_CLIPPY) $(rust_common_flags) \
> >>> +           -C linker-flavor=gcc -C linker=$(HOSTCC) \
> >>>             --emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \
> >>>             --crate-type proc-macro \
> >>>             --crate-name $(patsubst lib%.so,%,$(notdir $@)) $<
> >>> diff --git a/scripts/Makefile.host b/scripts/Makefile.host
> >>> index 8f7f842b54f9..0aa95a3af1c4 100644
> >>> --- a/scripts/Makefile.host
> >>> +++ b/scripts/Makefile.host
> >>> @@ -91,6 +91,7 @@ hostcxx_flags  = -Wp,-MMD,$(depfile) \
> >>>  # current working directory, which may be not accessible in the out-of-tree
> >>>  # modules case.
> >>>  hostrust_flags = --out-dir $(dir $@) --emit=dep-info=$(depfile) \
> >>> +            -C linker-flavor=gcc -C linker=$(HOSTCC) \
> >>>                   $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
> >>>                   $(HOSTRUSTFLAGS_$(target-stem))
> >>>
> >>> --
> >>> 2.42.0.459.ge4e396fd5e-goog
Fiona Behrens Sept. 16, 2023, 10:39 p.m. UTC | #6
On 16 Sep 2023, at 21:54, Matthew Maurer wrote:

> On Sat, Sep 16, 2023 at 11:07 AM Finn Behrens <me@kloenk.dev> wrote:
>>
>>
>>
>> On 16 Sep 2023, at 19:53, Björn Roy Baron wrote:
>>
>>> On Saturday, September 16th, 2023 at 18:52, Finn Behrens <me@kloenk.dev> wrote:
>>>
>>>>
>>>> On 15 Sep 2023, at 19:28, Matthew Maurer wrote:
>>>>
>>>>> Currently, rustc defaults to invoking `cc`, even if `HOSTCC` is defined,
>>>>> resulting in build failures in hermetic environments where `cc` does not
>>>>> exist. This includes both hostprogs and proc-macros.
>>>>>
>>>>> Since we are setting the linker to `HOSTCC`, we set the linker flavor to
>>>>> `gcc` explicitly.
>>>> But as `HOSTCC` could also be clang, the linker flavor would then be wrong, would that create a problem?
>>>
>>> Rustc uses the gcc linker flavor for clang too. There has been a proposal to split it up, but I'm not sure of the status of that. In any case clang's cli is similar enough to gcc that it works fine to use the gcc linker flavor.
>>>
>> In that case this looks very reasonable.
>>
>> Second thing I noticed is that `HOSTCC` could be the wrong variable, and `HOSTLD` would make more sense as we look for the linker and not the general C compiler.
>>
> Yes, thanks Bjorn - "gcc" is the linker flavor used for "Use the C
> compiler as a linker".
>
> With regards to HOSTLD, I was trying to make the minimum possible
> change. Currently, it is using the command `cc` as a linker, so this
> would preserve existing behavior when HOSTCC is unset.
>
> If we would prefer `HOSTLD` instead we can do that, but we would need
> to additionally inspect `LLVM` to set the linker flavor accordingly
> (e.g. set ld vs ld.lld).
>
> Do folks have strong opinions between these? My primary concern is to
> avoid calling programs by foo when their HOSTFOO variable is set.
I don’t have an educated opinion at all, mainly nudged in the direction, so that the existence if `HOSTLD` does not get overlooked. If there is a valid reason to not use it, this patch looks good to me.
>
> See https://doc.rust-lang.org/rustc/codegen-options/index.html#linker-flavor
> for details on linker flavor settings.
>>>>>
>>>>> Signed-off-by: Matthew Maurer <mmaurer@google.com>
>>>>> ---
>>>>>  rust/Makefile         | 1 +
>>>>>  scripts/Makefile.host | 1 +
>>>>>  2 files changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/rust/Makefile b/rust/Makefile
>>>>> index 87958e864be0..2a2352638f11 100644
>>>>> --- a/rust/Makefile
>>>>> +++ b/rust/Makefile
>>>>> @@ -383,6 +383,7 @@ $(obj)/exports_kernel_generated.h: $(obj)/kernel.o FORCE
>>>>>  quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
>>>>>        cmd_rustc_procmacro = \
>>>>>     $(RUSTC_OR_CLIPPY) $(rust_common_flags) \
>>>>> +           -C linker-flavor=gcc -C linker=$(HOSTCC) \
>>>>>             --emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \
>>>>>             --crate-type proc-macro \
>>>>>             --crate-name $(patsubst lib%.so,%,$(notdir $@)) $<
>>>>> diff --git a/scripts/Makefile.host b/scripts/Makefile.host
>>>>> index 8f7f842b54f9..0aa95a3af1c4 100644
>>>>> --- a/scripts/Makefile.host
>>>>> +++ b/scripts/Makefile.host
>>>>> @@ -91,6 +91,7 @@ hostcxx_flags  = -Wp,-MMD,$(depfile) \
>>>>>  # current working directory, which may be not accessible in the out-of-tree
>>>>>  # modules case.
>>>>>  hostrust_flags = --out-dir $(dir $@) --emit=dep-info=$(depfile) \
>>>>> +            -C linker-flavor=gcc -C linker=$(HOSTCC) \
>>>>>                   $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
>>>>>                   $(HOSTRUSTFLAGS_$(target-stem))
>>>>>
>>>>> --
>>>>> 2.42.0.459.ge4e396fd5e-goog
Martin Rodriguez Reboredo Sept. 17, 2023, 1:24 p.m. UTC | #7
On 9/16/23 16:54, Matthew Maurer wrote:> Yes, thanks Bjorn - "gcc" is the linker flavor used for "Use the C
> compiler as a linker".
> 
> With regards to HOSTLD, I was trying to make the minimum possible
> change. Currently, it is using the command `cc` as a linker, so this
> would preserve existing behavior when HOSTCC is unset.
> 
> If we would prefer `HOSTLD` instead we can do that, but we would need
> to additionally inspect `LLVM` to set the linker flavor accordingly
> (e.g. set ld vs ld.lld).

LLVM can use all of bfd, gold, lld and mold plus each and one of them
support roughly the same set of flags, so we can kinda ignore the
differences between them.

> Do folks have strong opinions between these? My primary concern is to
> avoid calling programs by foo when their HOSTFOO variable is set.
> 
> See https://doc.rust-lang.org/rustc/codegen-options/index.html#linker-flavor
> for details on linker flavor settings.

If I were to talk about my experiences with CMake then I'd say that
build systems tend to kinda ignore your linker choice if you thought
that it was going to be used standalone. I think KBuild does indeed
honor it, so if we can go ahead with that then we could do so.
Although I don't know what else would bring upon the table.
Nick Desaulniers Sept. 18, 2023, 3:24 p.m. UTC | #8
On Sat, Sep 16, 2023 at 12:54 PM Matthew Maurer <mmaurer@google.com> wrote:
>
> On Sat, Sep 16, 2023 at 11:07 AM Finn Behrens <me@kloenk.dev> wrote:
> >
> >
> >
> > On 16 Sep 2023, at 19:53, Björn Roy Baron wrote:
> >
> > > On Saturday, September 16th, 2023 at 18:52, Finn Behrens <me@kloenk.dev> wrote:
> > >
> > >>
> > >> On 15 Sep 2023, at 19:28, Matthew Maurer wrote:
> > >>
> > >>> Currently, rustc defaults to invoking `cc`, even if `HOSTCC` is defined,
> > >>> resulting in build failures in hermetic environments where `cc` does not
> > >>> exist. This includes both hostprogs and proc-macros.
> > >>>
> > >>> Since we are setting the linker to `HOSTCC`, we set the linker flavor to
> > >>> `gcc` explicitly.
> > >> But as `HOSTCC` could also be clang, the linker flavor would then be wrong, would that create a problem?
> > >
> > > Rustc uses the gcc linker flavor for clang too. There has been a proposal to split it up, but I'm not sure of the status of that. In any case clang's cli is similar enough to gcc that it works fine to use the gcc linker flavor.
> > >
> > In that case this looks very reasonable.
> >
> > Second thing I noticed is that `HOSTCC` could be the wrong variable, and `HOSTLD` would make more sense as we look for the linker and not the general C compiler.
> >
> Yes, thanks Bjorn - "gcc" is the linker flavor used for "Use the C
> compiler as a linker".
>
> With regards to HOSTLD, I was trying to make the minimum possible
> change. Currently, it is using the command `cc` as a linker, so this
> would preserve existing behavior when HOSTCC is unset.

Hey! Isn't HOSTCC always set? Top level Makefile lines 442-445?

What happens if you invoke the linker directly?

Generally, the kernel either invokes the compiler or the linker
directly. (For assembler, it is typically preprocessed, as are linker
scripts!)  So invoking the linker directly is a common pattern in
kbuild.  It also makes me slightly sad if the rust compiler ends up
invoking a c compiler, even if it's simply to drive the linker.

I'm concerned that while this might invoke $HOSTCC, it probably won't
do so with any of the $KBUILD_HOSTCFLAGS set.  That's generally been a
problem in the past.

For example, Android carries a downstream patch to set `-fuse-ld=lld`
for $KBUILD_HOSTCFLAGS, because its build environment doesn't contain
GNU binutils ("guilty, officer").

So if you set `rustc` to use `clang` as the linker, how do you
guarantee that `-fuse-ld=lld` or any of the upstream
$KBUILD_HOSTCFLAGS get used?

Android also sets `--sysroot` for the $KBUILD_HOSTCFLAGS to guarantee
that the UAPI header tests can be built against bionic (Android's
libc).

Or is that handled elsewhere in rust/Makefile already?

Also, if this is your first kernel patch, nice job! Welcome!

>
> If we would prefer `HOSTLD` instead we can do that, but we would need
> to additionally inspect `LLVM` to set the linker flavor accordingly
> (e.g. set ld vs ld.lld).
>
> Do folks have strong opinions between these? My primary concern is to
> avoid calling programs by foo when their HOSTFOO variable is set.
>
> See https://doc.rust-lang.org/rustc/codegen-options/index.html#linker-flavor
> for details on linker flavor settings.
> > >>>
> > >>> Signed-off-by: Matthew Maurer <mmaurer@google.com>
> > >>> ---
> > >>>  rust/Makefile         | 1 +
> > >>>  scripts/Makefile.host | 1 +
> > >>>  2 files changed, 2 insertions(+)
> > >>>
> > >>> diff --git a/rust/Makefile b/rust/Makefile
> > >>> index 87958e864be0..2a2352638f11 100644
> > >>> --- a/rust/Makefile
> > >>> +++ b/rust/Makefile
> > >>> @@ -383,6 +383,7 @@ $(obj)/exports_kernel_generated.h: $(obj)/kernel.o FORCE
> > >>>  quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
> > >>>        cmd_rustc_procmacro = \
> > >>>     $(RUSTC_OR_CLIPPY) $(rust_common_flags) \
> > >>> +           -C linker-flavor=gcc -C linker=$(HOSTCC) \
> > >>>             --emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \
> > >>>             --crate-type proc-macro \
> > >>>             --crate-name $(patsubst lib%.so,%,$(notdir $@)) $<
> > >>> diff --git a/scripts/Makefile.host b/scripts/Makefile.host
> > >>> index 8f7f842b54f9..0aa95a3af1c4 100644
> > >>> --- a/scripts/Makefile.host
> > >>> +++ b/scripts/Makefile.host
> > >>> @@ -91,6 +91,7 @@ hostcxx_flags  = -Wp,-MMD,$(depfile) \
> > >>>  # current working directory, which may be not accessible in the out-of-tree
> > >>>  # modules case.
> > >>>  hostrust_flags = --out-dir $(dir $@) --emit=dep-info=$(depfile) \
> > >>> +            -C linker-flavor=gcc -C linker=$(HOSTCC) \
> > >>>                   $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
> > >>>                   $(HOSTRUSTFLAGS_$(target-stem))
> > >>>
> > >>> --
> > >>> 2.42.0.459.ge4e396fd5e-goog
Matthew Maurer Sept. 18, 2023, 4:38 p.m. UTC | #9
On Mon, Sep 18, 2023 at 8:25 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Sat, Sep 16, 2023 at 12:54 PM Matthew Maurer <mmaurer@google.com> wrote:
> >
> > On Sat, Sep 16, 2023 at 11:07 AM Finn Behrens <me@kloenk.dev> wrote:
> > >
> > >
> > >
> > > On 16 Sep 2023, at 19:53, Björn Roy Baron wrote:
> > >
> > > > On Saturday, September 16th, 2023 at 18:52, Finn Behrens <me@kloenk.dev> wrote:
> > > >
> > > >>
> > > >> On 15 Sep 2023, at 19:28, Matthew Maurer wrote:
> > > >>
> > > >>> Currently, rustc defaults to invoking `cc`, even if `HOSTCC` is defined,
> > > >>> resulting in build failures in hermetic environments where `cc` does not
> > > >>> exist. This includes both hostprogs and proc-macros.
> > > >>>
> > > >>> Since we are setting the linker to `HOSTCC`, we set the linker flavor to
> > > >>> `gcc` explicitly.
> > > >> But as `HOSTCC` could also be clang, the linker flavor would then be wrong, would that create a problem?
> > > >
> > > > Rustc uses the gcc linker flavor for clang too. There has been a proposal to split it up, but I'm not sure of the status of that. In any case clang's cli is similar enough to gcc that it works fine to use the gcc linker flavor.
> > > >
> > > In that case this looks very reasonable.
> > >
> > > Second thing I noticed is that `HOSTCC` could be the wrong variable, and `HOSTLD` would make more sense as we look for the linker and not the general C compiler.
> > >
> > Yes, thanks Bjorn - "gcc" is the linker flavor used for "Use the C
> > compiler as a linker".
> >
> > With regards to HOSTLD, I was trying to make the minimum possible
> > change. Currently, it is using the command `cc` as a linker, so this
> > would preserve existing behavior when HOSTCC is unset.
>
> Hey! Isn't HOSTCC always set? Top level Makefile lines 442-445?
Yes, you're correct. What I was trying to express was that this keeps
the build process the same as when `cc=HOSTCC` today, which while not
required is frequently true.
>
> What happens if you invoke the linker directly?
Rust unfortunately currently doesn't support invoking the linker
directly: https://github.com/rust-lang/rust/issues/73632
We work around this for kernel level code by manually providing
expected symbols/stubs normally injected by rustc during linking
because it is critical to use the kernel linking infrastructure there,
but this doesn't seem as important on host unless I've missed
something. While we could do that, it would add nontrivial complexity
for minimal gain.
A nontrivial chunk of rust/Makefile is currently spent working around
this lack of support for non-rustc-managed link stages.
>
> Generally, the kernel either invokes the compiler or the linker
> directly. (For assembler, it is typically preprocessed, as are linker
> scripts!)  So invoking the linker directly is a common pattern in
> kbuild.  It also makes me slightly sad if the rust compiler ends up
> invoking a c compiler, even if it's simply to drive the linker.
As mentioned earlier, we could pass $HOSTLD, but if the linker isn't
named something accurate (e.g. if the linker is not named lld, but is
lld), we need to know how to pass a flavor:
https://doc.rust-lang.org/rustc/codegen-options/index.html#linker-flavor
Would it be appropriate to just assume the linker is named correctly?
>
> I'm concerned that while this might invoke $HOSTCC, it probably won't
> do so with any of the $KBUILD_HOSTCFLAGS set.  That's generally been a
> problem in the past.
This would not work correctly with my patch. Once we decide which of
the two linkage strategies we want (using $HOSTCC or $HOSTLD) I'll
send a new patch that forwards the flags along.
>
> For example, Android carries a downstream patch to set `-fuse-ld=lld`
> for $KBUILD_HOSTCFLAGS, because its build environment doesn't contain
> GNU binutils ("guilty, officer").
Oddly, the Android kernel environment (Kleaf) is the one that I needed
this patch to build in, but it seemed to be working without a manual
KBUILD_HOSTCFLAGS forwarding.
>
> So if you set `rustc` to use `clang` as the linker, how do you
> guarantee that `-fuse-ld=lld` or any of the upstream
> $KBUILD_HOSTCFLAGS get used?
>
> Android also sets `--sysroot` for the $KBUILD_HOSTCFLAGS to guarantee
> that the UAPI header tests can be built against bionic (Android's
> libc).
>
> Or is that handled elsewhere in rust/Makefile already?
>
> Also, if this is your first kernel patch, nice job! Welcome!
>
Overall, it sounds like you'd prefer if I set this to use
`KBUILD_HOSTLD` and `KBUILD_HOSTLDFLAGS`, and leave the linker flavor
to autodetect?
> >
> > If we would prefer `HOSTLD` instead we can do that, but we would need
> > to additionally inspect `LLVM` to set the linker flavor accordingly
> > (e.g. set ld vs ld.lld).
> >
> > Do folks have strong opinions between these? My primary concern is to
> > avoid calling programs by foo when their HOSTFOO variable is set.
> >
> > See https://doc.rust-lang.org/rustc/codegen-options/index.html#linker-flavor
> > for details on linker flavor settings.
> > > >>>
> > > >>> Signed-off-by: Matthew Maurer <mmaurer@google.com>
> > > >>> ---
> > > >>>  rust/Makefile         | 1 +
> > > >>>  scripts/Makefile.host | 1 +
> > > >>>  2 files changed, 2 insertions(+)
> > > >>>
> > > >>> diff --git a/rust/Makefile b/rust/Makefile
> > > >>> index 87958e864be0..2a2352638f11 100644
> > > >>> --- a/rust/Makefile
> > > >>> +++ b/rust/Makefile
> > > >>> @@ -383,6 +383,7 @@ $(obj)/exports_kernel_generated.h: $(obj)/kernel.o FORCE
> > > >>>  quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
> > > >>>        cmd_rustc_procmacro = \
> > > >>>     $(RUSTC_OR_CLIPPY) $(rust_common_flags) \
> > > >>> +           -C linker-flavor=gcc -C linker=$(HOSTCC) \
> > > >>>             --emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \
> > > >>>             --crate-type proc-macro \
> > > >>>             --crate-name $(patsubst lib%.so,%,$(notdir $@)) $<
> > > >>> diff --git a/scripts/Makefile.host b/scripts/Makefile.host
> > > >>> index 8f7f842b54f9..0aa95a3af1c4 100644
> > > >>> --- a/scripts/Makefile.host
> > > >>> +++ b/scripts/Makefile.host
> > > >>> @@ -91,6 +91,7 @@ hostcxx_flags  = -Wp,-MMD,$(depfile) \
> > > >>>  # current working directory, which may be not accessible in the out-of-tree
> > > >>>  # modules case.
> > > >>>  hostrust_flags = --out-dir $(dir $@) --emit=dep-info=$(depfile) \
> > > >>> +            -C linker-flavor=gcc -C linker=$(HOSTCC) \
> > > >>>                   $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
> > > >>>                   $(HOSTRUSTFLAGS_$(target-stem))
> > > >>>
> > > >>> --
> > > >>> 2.42.0.459.ge4e396fd5e-goog
>
>
>
> --
> Thanks,
> ~Nick Desaulniers
Nick Desaulniers Sept. 18, 2023, 6:43 p.m. UTC | #10
On Mon, Sep 18, 2023 at 9:38 AM Matthew Maurer <mmaurer@google.com> wrote:
>
> On Mon, Sep 18, 2023 at 8:25 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > What happens if you invoke the linker directly?
> Rust unfortunately currently doesn't support invoking the linker
> directly: https://github.com/rust-lang/rust/issues/73632

Wait; does Rust have its own linker? It doesn't use the system linker?
 Perhaps that's necessary for the rust module format? If so, TIL.

> > Generally, the kernel either invokes the compiler or the linker
> > directly. (For assembler, it is typically preprocessed, as are linker
> > scripts!)  So invoking the linker directly is a common pattern in
> > kbuild.  It also makes me slightly sad if the rust compiler ends up
> > invoking a c compiler, even if it's simply to drive the linker.
> As mentioned earlier, we could pass $HOSTLD, but if the linker isn't
> named something accurate (e.g. if the linker is not named lld, but is
> lld), we need to know how to pass a flavor:
> https://doc.rust-lang.org/rustc/codegen-options/index.html#linker-flavor
> Would it be appropriate to just assume the linker is named correctly?

If it were not, what does failure look like?

command not found: asdfadfasdf

Seems fine to me. If the user mis-specifies HOSTLD=, then they will
get a similar error, which should be prescriptive enough for them to
figure out how exactly they're "holding it wrong."

> > For example, Android carries a downstream patch to set `-fuse-ld=lld`
> > for $KBUILD_HOSTCFLAGS, because its build environment doesn't contain
> > GNU binutils ("guilty, officer").
> Oddly, the Android kernel environment (Kleaf) is the one that I needed
> this patch to build in, but it seemed to be working without a manual
> KBUILD_HOSTCFLAGS forwarding.

Surprising that worked.

> Overall, it sounds like you'd prefer if I set this to use
> `KBUILD_HOSTLD` and `KBUILD_HOSTLDFLAGS`, and leave the linker flavor
> to autodetect?

Yes for the first two.

Dunno, what precisely is a linker flavor?  Is that like a flavor of ice cream?
Oh, right looking at your link:
https://doc.rust-lang.org/rustc/codegen-options/index.html#linker-flavor
Seems like if `LLVM=1` is set, or `LD=ld.lld`, or CONFIG_LD_IS_LLD, then
the flavor should be set to ld.lld, else ld.  Then the
KBUILD_HOSTLDFLAGS need to be passed, probably.

But how are there "linker flavors" like ld or ld.lld if you just said
"Rust unfortunately currently doesn't support invoking the linker
directly: https://github.com/rust-lang/rust/issues/73632".  I'm having
trouble reconciling the two.

Can we do something more like the below?

ifdef CONFIG_LD_IS_LLD
hostrust_flags += -C linker-flavor=ld.lld
else
hostrust_flags += -C linker-flavor=ld
endif
hostrust_flags += -C linker=$(HOSTLD) <todo: figure out how to pass
KBUILD_HOSTLDFLAGS>
Matthew Maurer Sept. 18, 2023, 6:55 p.m. UTC | #11
On Mon, Sep 18, 2023 at 11:43 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Mon, Sep 18, 2023 at 9:38 AM Matthew Maurer <mmaurer@google.com> wrote:
> >
> > On Mon, Sep 18, 2023 at 8:25 AM Nick Desaulniers
> > <ndesaulniers@google.com> wrote:
> > >
> > > What happens if you invoke the linker directly?
> > Rust unfortunately currently doesn't support invoking the linker
> > directly: https://github.com/rust-lang/rust/issues/73632
>
> Wait; does Rust have its own linker? It doesn't use the system linker?
>  Perhaps that's necessary for the rust module format? If so, TIL.
It does use the system linker (this is what -C linker is controlling),
but the command line passed to the linker may change, extra object
files may be added to the command line, etc.
>
> > > Generally, the kernel either invokes the compiler or the linker
> > > directly. (For assembler, it is typically preprocessed, as are linker
> > > scripts!)  So invoking the linker directly is a common pattern in
> > > kbuild.  It also makes me slightly sad if the rust compiler ends up
> > > invoking a c compiler, even if it's simply to drive the linker.
> > As mentioned earlier, we could pass $HOSTLD, but if the linker isn't
> > named something accurate (e.g. if the linker is not named lld, but is
> > lld), we need to know how to pass a flavor:
> > https://doc.rust-lang.org/rustc/codegen-options/index.html#linker-flavor
> > Would it be appropriate to just assume the linker is named correctly?
>
> If it were not, what does failure look like?
That depends. I think it will usually look like "unrecognized flag:
blah blah", but that's not guaranteed.
>
> command not found: asdfadfasdf
This isn't about command-not-found, it's about "I set
HOSTLD=foo/bar/weirdname, and it is an lld-like linker. rustc invoked
it assuming it is an ld-like linker."
>
> Seems fine to me. If the user mis-specifies HOSTLD=, then they will
> get a similar error, which should be prescriptive enough for them to
> figure out how exactly they're "holding it wrong."
>
> > > For example, Android carries a downstream patch to set `-fuse-ld=lld`
> > > for $KBUILD_HOSTCFLAGS, because its build environment doesn't contain
> > > GNU binutils ("guilty, officer").
> > Oddly, the Android kernel environment (Kleaf) is the one that I needed
> > this patch to build in, but it seemed to be working without a manual
> > KBUILD_HOSTCFLAGS forwarding.
>
> Surprising that worked.
>
> > Overall, it sounds like you'd prefer if I set this to use
> > `KBUILD_HOSTLD` and `KBUILD_HOSTLDFLAGS`, and leave the linker flavor
> > to autodetect?
>
> Yes for the first two.
>
> Dunno, what precisely is a linker flavor?  Is that like a flavor of ice cream?
> Oh, right looking at your link:
> https://doc.rust-lang.org/rustc/codegen-options/index.html#linker-flavor
> Seems like if `LLVM=1` is set, or `LD=ld.lld`, or CONFIG_LD_IS_LLD, then
> the flavor should be set to ld.lld, else ld.  Then the
> KBUILD_HOSTLDFLAGS need to be passed, probably.
>
> But how are there "linker flavors" like ld or ld.lld if you just said
> "Rust unfortunately currently doesn't support invoking the linker
> directly: https://github.com/rust-lang/rust/issues/73632".  I'm having
> trouble reconciling the two.
Yes, what I meant by that is that *rustc* wants to invoke the linker,
rather than having the surrounding build system invoke the linker. The
exact command line passed to the linker in the final link, including
potential synthetic objects, is considered an internal detail.
>
> Can we do something more like the below?
>
> ifdef CONFIG_LD_IS_LLD
> hostrust_flags += -C linker-flavor=ld.lld
> else
> hostrust_flags += -C linker-flavor=ld
> endif
> hostrust_flags += -C linker=$(HOSTLD) <todo: figure out how to pass
> KBUILD_HOSTLDFLAGS>
Yes, I can make host linking use `$(HOSTLD)` and pass the flavor based
on CONFIG_LD_IS_LLD. I'll send a variant that does that this
afternoon.
> --
> Thanks,
> ~Nick Desaulniers
Masahiro Yamada Sept. 26, 2023, 4:51 p.m. UTC | #12
On Tue, Sep 19, 2023 at 3:55 AM Matthew Maurer <mmaurer@google.com> wrote:
>
> On Mon, Sep 18, 2023 at 11:43 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > On Mon, Sep 18, 2023 at 9:38 AM Matthew Maurer <mmaurer@google.com> wrote:
> > >
> > > On Mon, Sep 18, 2023 at 8:25 AM Nick Desaulniers
> > > <ndesaulniers@google.com> wrote:
> > > >
> > > > What happens if you invoke the linker directly?
> > > Rust unfortunately currently doesn't support invoking the linker
> > > directly: https://github.com/rust-lang/rust/issues/73632
> >
> > Wait; does Rust have its own linker? It doesn't use the system linker?
> >  Perhaps that's necessary for the rust module format? If so, TIL.
> It does use the system linker (this is what -C linker is controlling),
> but the command line passed to the linker may change, extra object
> files may be added to the command line, etc.
> >
> > > > Generally, the kernel either invokes the compiler or the linker
> > > > directly. (For assembler, it is typically preprocessed, as are linker
> > > > scripts!)  So invoking the linker directly is a common pattern in
> > > > kbuild.  It also makes me slightly sad if the rust compiler ends up
> > > > invoking a c compiler, even if it's simply to drive the linker.
> > > As mentioned earlier, we could pass $HOSTLD, but if the linker isn't
> > > named something accurate (e.g. if the linker is not named lld, but is
> > > lld), we need to know how to pass a flavor:
> > > https://doc.rust-lang.org/rustc/codegen-options/index.html#linker-flavor
> > > Would it be appropriate to just assume the linker is named correctly?
> >
> > If it were not, what does failure look like?
> That depends. I think it will usually look like "unrecognized flag:
> blah blah", but that's not guaranteed.
> >
> > command not found: asdfadfasdf
> This isn't about command-not-found, it's about "I set
> HOSTLD=foo/bar/weirdname, and it is an lld-like linker. rustc invoked
> it assuming it is an ld-like linker."
> >
> > Seems fine to me. If the user mis-specifies HOSTLD=, then they will
> > get a similar error, which should be prescriptive enough for them to
> > figure out how exactly they're "holding it wrong."
> >
> > > > For example, Android carries a downstream patch to set `-fuse-ld=lld`
> > > > for $KBUILD_HOSTCFLAGS, because its build environment doesn't contain
> > > > GNU binutils ("guilty, officer").
> > > Oddly, the Android kernel environment (Kleaf) is the one that I needed
> > > this patch to build in, but it seemed to be working without a manual
> > > KBUILD_HOSTCFLAGS forwarding.
> >
> > Surprising that worked.
> >
> > > Overall, it sounds like you'd prefer if I set this to use
> > > `KBUILD_HOSTLD` and `KBUILD_HOSTLDFLAGS`, and leave the linker flavor
> > > to autodetect?
> >
> > Yes for the first two.
> >
> > Dunno, what precisely is a linker flavor?  Is that like a flavor of ice cream?
> > Oh, right looking at your link:
> > https://doc.rust-lang.org/rustc/codegen-options/index.html#linker-flavor
> > Seems like if `LLVM=1` is set, or `LD=ld.lld`, or CONFIG_LD_IS_LLD, then
> > the flavor should be set to ld.lld, else ld.  Then the
> > KBUILD_HOSTLDFLAGS need to be passed, probably.
> >
> > But how are there "linker flavors" like ld or ld.lld if you just said
> > "Rust unfortunately currently doesn't support invoking the linker
> > directly: https://github.com/rust-lang/rust/issues/73632".  I'm having
> > trouble reconciling the two.
> Yes, what I meant by that is that *rustc* wants to invoke the linker,
> rather than having the surrounding build system invoke the linker. The
> exact command line passed to the linker in the final link, including
> potential synthetic objects, is considered an internal detail.
> >
> > Can we do something more like the below?
> >
> > ifdef CONFIG_LD_IS_LLD
> > hostrust_flags += -C linker-flavor=ld.lld
> > else
> > hostrust_flags += -C linker-flavor=ld
> > endif
> > hostrust_flags += -C linker=$(HOSTLD) <todo: figure out how to pass
> > KBUILD_HOSTLDFLAGS>
> Yes, I can make host linking use `$(HOSTLD)` and pass the flavor based
> on CONFIG_LD_IS_LLD. I'll send a variant that does that this
> afternoon.


CONFIG_LD_IS_LLD=y states that the linker for the
kernel space is LLD.

Host programs should not be affected.
diff mbox series

Patch

diff --git a/rust/Makefile b/rust/Makefile
index 87958e864be0..2a2352638f11 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -383,6 +383,7 @@  $(obj)/exports_kernel_generated.h: $(obj)/kernel.o FORCE
 quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
       cmd_rustc_procmacro = \
 	$(RUSTC_OR_CLIPPY) $(rust_common_flags) \
+		-C linker-flavor=gcc -C linker=$(HOSTCC) \
 		--emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \
 		--crate-type proc-macro \
 		--crate-name $(patsubst lib%.so,%,$(notdir $@)) $<
diff --git a/scripts/Makefile.host b/scripts/Makefile.host
index 8f7f842b54f9..0aa95a3af1c4 100644
--- a/scripts/Makefile.host
+++ b/scripts/Makefile.host
@@ -91,6 +91,7 @@  hostcxx_flags  = -Wp,-MMD,$(depfile) \
 # current working directory, which may be not accessible in the out-of-tree
 # modules case.
 hostrust_flags = --out-dir $(dir $@) --emit=dep-info=$(depfile) \
+		 -C linker-flavor=gcc -C linker=$(HOSTCC) \
                  $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
                  $(HOSTRUSTFLAGS_$(target-stem))