Re: [RFC PATCH v2 2/8] tools/nolibc: Remove .global _start from the entry point code

[Nick Desaulniers <ndesaulniers@google.com>]

On Tue, Mar 22, 2022 at 3:21 AM Ammar Faizi <ammarfaizi2@gnuweeb.org> wrote:
>
> Building with clang yields the following error:
> ```
>   <inline asm>:3:1: error: _start changed binding to STB_GLOBAL
>   .global _start
>   ^
>   1 error generated.
> ```
> Make sure only specify one between `.global _start` and `.weak _start`.
> Removing `.global _start`.

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

Yes, symbols should either be `.weak` or `.global`. The warning from
Clang's integrated assembler is meant to flush out funny business.

I assume there's a good reason _why_ _start is weak and not strong?
Then again, I'm not familiar with nolibc.

>
> Cc: llvm@lists.linux.dev
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
> ---
>
> @@ Changelog:
>
>    Link RFC v1: https://lore.kernel.org/llvm/20220320093750.159991-3-ammarfaizi2@gnuweeb.org
>    RFC v1 -> RFC v2:
>     - Remove all `.global _start` for all build (GCC and Clang) instead of
>       removing all `.weak _start` for clang build (Comment from Willy).
> ---
>  tools/include/nolibc/arch-aarch64.h | 1 -
>  tools/include/nolibc/arch-arm.h     | 1 -
>  tools/include/nolibc/arch-i386.h    | 1 -
>  tools/include/nolibc/arch-mips.h    | 1 -
>  tools/include/nolibc/arch-riscv.h   | 1 -
>  tools/include/nolibc/arch-x86_64.h  | 1 -
>  6 files changed, 6 deletions(-)
>
> diff --git a/tools/include/nolibc/arch-aarch64.h b/tools/include/nolibc/arch-aarch64.h
> index 87d9e434820c..2dbd80d633cb 100644
> --- a/tools/include/nolibc/arch-aarch64.h
> +++ b/tools/include/nolibc/arch-aarch64.h
> @@ -184,7 +184,6 @@ struct sys_stat_struct {
>  /* startup code */
>  asm(".section .text\n"
>      ".weak _start\n"
> -    ".global _start\n"
>      "_start:\n"
>      "ldr x0, [sp]\n"              // argc (x0) was in the stack
>      "add x1, sp, 8\n"             // argv (x1) = sp
> diff --git a/tools/include/nolibc/arch-arm.h b/tools/include/nolibc/arch-arm.h
> index 001a3c8c9ad5..1191395b5acd 100644
> --- a/tools/include/nolibc/arch-arm.h
> +++ b/tools/include/nolibc/arch-arm.h
> @@ -177,7 +177,6 @@ struct sys_stat_struct {
>  /* startup code */
>  asm(".section .text\n"
>      ".weak _start\n"
> -    ".global _start\n"
>      "_start:\n"
>  #if defined(__THUMBEB__) || defined(__THUMBEL__)
>      /* We enter here in 32-bit mode but if some previous functions were in
> diff --git a/tools/include/nolibc/arch-i386.h b/tools/include/nolibc/arch-i386.h
> index d7e4d53325a3..125a691fc631 100644
> --- a/tools/include/nolibc/arch-i386.h
> +++ b/tools/include/nolibc/arch-i386.h
> @@ -176,7 +176,6 @@ struct sys_stat_struct {
>   */
>  asm(".section .text\n"
>      ".weak _start\n"
> -    ".global _start\n"
>      "_start:\n"
>      "pop %eax\n"                // argc   (first arg, %eax)
>      "mov %esp, %ebx\n"          // argv[] (second arg, %ebx)
> diff --git a/tools/include/nolibc/arch-mips.h b/tools/include/nolibc/arch-mips.h
> index c9a6aac87c6d..1a124790c99f 100644
> --- a/tools/include/nolibc/arch-mips.h
> +++ b/tools/include/nolibc/arch-mips.h
> @@ -192,7 +192,6 @@ struct sys_stat_struct {
>  asm(".section .text\n"
>      ".weak __start\n"
>      ".set nomips16\n"
> -    ".global __start\n"
>      ".set    noreorder\n"
>      ".option pic0\n"
>      ".ent __start\n"
> diff --git a/tools/include/nolibc/arch-riscv.h b/tools/include/nolibc/arch-riscv.h
> index bc10b7b5706d..511d67fc534e 100644
> --- a/tools/include/nolibc/arch-riscv.h
> +++ b/tools/include/nolibc/arch-riscv.h
> @@ -185,7 +185,6 @@ struct sys_stat_struct {
>  /* startup code */
>  asm(".section .text\n"
>      ".weak _start\n"
> -    ".global _start\n"
>      "_start:\n"
>      ".option push\n"
>      ".option norelax\n"
> diff --git a/tools/include/nolibc/arch-x86_64.h b/tools/include/nolibc/arch-x86_64.h
> index a7b70ea51b68..84c174181425 100644
> --- a/tools/include/nolibc/arch-x86_64.h
> +++ b/tools/include/nolibc/arch-x86_64.h
> @@ -199,7 +199,6 @@ struct sys_stat_struct {
>   */
>  asm(".section .text\n"
>      ".weak _start\n"
> -    ".global _start\n"
>      "_start:\n"
>      "pop %rdi\n"                // argc   (first arg, %rdi)
>      "mov %rsp, %rsi\n"          // argv[] (second arg, %rsi)
> --
> Ammar Faizi
>


-- 
Thanks,
~Nick Desaulniers