kbuild: fix linker feature test macros when cross compiling with Clang
diff mbox series

Message ID 20171026201744.89744-1-ndesaulniers@google.com
State New, archived
Headers show
Series
  • kbuild: fix linker feature test macros when cross compiling with Clang
Related show

Commit Message

Nick Desaulniers Oct. 26, 2017, 8:17 p.m. UTC
I was not seeing my linker flags getting added when using ld-option when
cross compiling with Clang. Upon investigation, this seems to be due to
a difference in how GCC vs Clang handle cross compilation.

GCC is configured at build time to support one backend, that is implicit
when compiling.  Clang is explicit via the use of `-target <triple>` and
ships with all supported backends by default.

GNU Make feature test macros that compile then link will always fail
when cross compiling with Clang unless Clang's triple is passed along to
the compiler. For example:

$ clang -x c /dev/null -c -o temp.o
$ aarch64-linux-android/bin/ld -E temp.o
aarch64-linux-android/bin/ld:
unknown architecture of input file `temp.o' is incompatible with
aarch64 output
aarch64-linux-android/bin/ld:
warning: cannot find entry symbol _start; defaulting to
0000000000400078
$ echo $?
1

$ clang -target aarch64-linux-android- -x c /dev/null -c -o temp.o
$ aarch64-linux-android/bin/ld -E temp.o
aarch64-linux-android/bin/ld:
warning: cannot find entry symbol _start; defaulting to 00000000004002e4
$ echo $?
0

This causes conditional checks that invoke $(CC) without the target
triple, then $(LD) on the result, to always fail.

Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
---
 scripts/Kbuild.include | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Masahiro Yamada Oct. 27, 2017, 11:20 a.m. UTC | #1
Hi Nick

2017-10-27 5:17 GMT+09:00 Nick Desaulniers <ndesaulniers@google.com>:
> I was not seeing my linker flags getting added when using ld-option when
> cross compiling with Clang. Upon investigation, this seems to be due to
> a difference in how GCC vs Clang handle cross compilation.
>
> GCC is configured at build time to support one backend, that is implicit
> when compiling.  Clang is explicit via the use of `-target <triple>` and
> ships with all supported backends by default.
>
> GNU Make feature test macros that compile then link will always fail
> when cross compiling with Clang unless Clang's triple is passed along to
> the compiler. For example:
>
> $ clang -x c /dev/null -c -o temp.o
> $ aarch64-linux-android/bin/ld -E temp.o
> aarch64-linux-android/bin/ld:
> unknown architecture of input file `temp.o' is incompatible with
> aarch64 output
> aarch64-linux-android/bin/ld:
> warning: cannot find entry symbol _start; defaulting to
> 0000000000400078
> $ echo $?
> 1
>
> $ clang -target aarch64-linux-android- -x c /dev/null -c -o temp.o
> $ aarch64-linux-android/bin/ld -E temp.o
> aarch64-linux-android/bin/ld:
> warning: cannot find entry symbol _start; defaulting to 00000000004002e4
> $ echo $?
> 0
>
> This causes conditional checks that invoke $(CC) without the target
> triple, then $(LD) on the result, to always fail.
>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  scripts/Kbuild.include | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 9ffd3dda3889..23c4df90e8ff 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -160,12 +160,12 @@ cc-if-fullversion = $(shell [ $(cc-fullversion) $(1) $(2) ] && echo $(3) || echo
>  # cc-ldoption
>  # Usage: ldflags += $(call cc-ldoption, -Wl$(comma)--hash-style=both)
>  cc-ldoption = $(call try-run,\
> -       $(CC) $(1) -nostdlib -x c /dev/null -o "$$TMP",$(1),$(2))
> +       $(CC) $(CLANG_TARGET) $(1) -nostdlib -x c /dev/null -o "$$TMP",$(1),$(2))
>
>  # ld-option
>  # Usage: LDFLAGS += $(call ld-option, -X)
>  ld-option = $(call try-run,\
> -       $(CC) -x c /dev/null -c -o "$$TMPO" ; $(LD) $(1) "$$TMPO" -o "$$TMP",$(1),$(2))
> +       $(CC) $(CLANG_TARGET) -x c /dev/null -c -o "$$TMPO" ; $(LD) $(1) "$$TMPO" -o "$$TMP",$(1),$(2))
>
>  # ar-option
>  # Usage: KBUILD_ARFLAGS := $(call ar-option,D)
> --

I do not like to add $(CLANG_TARGET) to a place for common helpers.


Instead of $(CLANG_TARGET), please add

$(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS)

to cc-ldoption and ld-option.



I have two requests next time:
  - please include  linux-kbuild@vger.kernel.org in your To list
  - please base your patch on linux-kbuild/kbuild branch


The URL of the tree is described in MAINTAINERS.


Thanks.
Nick Desaulniers Oct. 27, 2017, 6:28 p.m. UTC | #2
+ linux-kbuild@vger.kernel.org

On Fri, Oct 27, 2017 at 4:20 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> I do not like to add $(CLANG_TARGET) to a place for common helpers.
> Instead of $(CLANG_TARGET), please add
> $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS)
> to cc-ldoption and ld-option.

Thanks for the review.  I agree that sounds like a better option.

> I have two requests next time:
>   - please include  linux-kbuild@vger.kernel.org in your To list

Sure thing. scripts/get_maintainer.pl does not recommend that list for
this file; is there a way to explicitly add that list to the
recommendation for that source file?

>   - please base your patch on linux-kbuild/kbuild branch

Will do. Do I need to note it's based off that branch? Otherwise wont
0-day bot complain that my patch doesn't apply/build on torvalds/linux
?
Nick Desaulniers Oct. 27, 2017, 8:10 p.m. UTC | #3
On Fri, Oct 27, 2017 at 11:28 AM, Nick Desaulniers
<ndesaulniers@google.com> wrote:
> On Fri, Oct 27, 2017 at 4:20 AM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>>   - please base your patch on linux-kbuild/kbuild branch
>
> Will do. Do I need to note it's based off that branch? Otherwise wont
> 0-day bot complain that my patch doesn't apply/build on torvalds/linux
> ?

Talked to some teammates about this, sounds like it's not a problem,
so I'll just send v2 with a note to the reviewer.
Masahiro Yamada Oct. 28, 2017, 2:59 p.m. UTC | #4
2017-10-28 3:28 GMT+09:00 Nick Desaulniers <ndesaulniers@google.com>:
> + linux-kbuild@vger.kernel.org
>
> On Fri, Oct 27, 2017 at 4:20 AM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> I do not like to add $(CLANG_TARGET) to a place for common helpers.
>> Instead of $(CLANG_TARGET), please add
>> $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS)
>> to cc-ldoption and ld-option.
>
> Thanks for the review.  I agree that sounds like a better option.
>
>> I have two requests next time:
>>   - please include  linux-kbuild@vger.kernel.org in your To list
>
> Sure thing. scripts/get_maintainer.pl does not recommend that list for
> this file; is there a way to explicitly add that list to the
> recommendation for that source file?

Ah, I realized MAINTAINERS does not claim the maintainership
of this file.

There are so many misc scripts
in scripts/, so I hesitate to add
scripts/* pattern to MAINTAINERS.




>>   - please base your patch on linux-kbuild/kbuild branch
>
> Will do. Do I need to note it's based off that branch? Otherwise wont
> 0-day bot complain that my patch doesn't apply/build on torvalds/linux
> ?

I am not sure in which case the 0-day bot complain about it.

Now, I applied v2.
If the bot find a problem, it will send us a report.

Patch
diff mbox series

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 9ffd3dda3889..23c4df90e8ff 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -160,12 +160,12 @@  cc-if-fullversion = $(shell [ $(cc-fullversion) $(1) $(2) ] && echo $(3) || echo
 # cc-ldoption
 # Usage: ldflags += $(call cc-ldoption, -Wl$(comma)--hash-style=both)
 cc-ldoption = $(call try-run,\
-	$(CC) $(1) -nostdlib -x c /dev/null -o "$$TMP",$(1),$(2))
+	$(CC) $(CLANG_TARGET) $(1) -nostdlib -x c /dev/null -o "$$TMP",$(1),$(2))
 
 # ld-option
 # Usage: LDFLAGS += $(call ld-option, -X)
 ld-option = $(call try-run,\
-	$(CC) -x c /dev/null -c -o "$$TMPO" ; $(LD) $(1) "$$TMPO" -o "$$TMP",$(1),$(2))
+	$(CC) $(CLANG_TARGET) -x c /dev/null -c -o "$$TMPO" ; $(LD) $(1) "$$TMPO" -o "$$TMP",$(1),$(2))
 
 # ar-option
 # Usage: KBUILD_ARFLAGS := $(call ar-option,D)