On Fri, Oct 21, 2022 at 08:05:19PM -0700, Xin Li wrote:
> Andrew Cooper suggested upgrading the orphan section warning to a hard link
> error. However Nathan Chancellor said outright turning the warning into an
> error with no escape hatch might be too aggressive, as we have had these
> warnings triggered by new compiler generated sections, and suggested turning
> orphan sections into an error only if CONFIG_WERROR is set. Kees Cook echoed
> and emphasized that the mandate from Linus is that we should avoid breaking
> builds. It wrecks bisection, it causes problems across compiler versions, etc.
>
> Thus upgrade the orphan section warning to a hard link error only if
> CONFIG_WERROR is set.
>
> Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Suggested-by: Nathan Chancellor <nathan@xxxxxxxxxx>
> Signed-off-by: Xin Li <xin3.li@xxxxxxxxx>
Thanks for the patch!
Reviewed-by: Nathan Chancellor <nathan@xxxxxxxxxx>
Tested-by: Nathan Chancellor <nathan@xxxxxxxxxx>
We could deduplicate the '$(if $(CONFIG_WERROR),error,warn)' logic if we
hoisted it into Kconfig by having something like
config LD_ORPHAN_WARN_LEVEL
string
depends on LD_ORPHAN_WARN
default "error" if WERROR
default "warn"
in init/Kconfig then using it everywhere like
--orphan-handling=$(CONFIG_LD_ORPHAN_WARN_LEVEL)
but I will let others decide if they would prefer that over the
direction we went here.
Cheers,
Nathan
> ---
> Makefile | 2 +-
> arch/arm/boot/compressed/Makefile | 2 +-
> arch/arm64/kernel/vdso/Makefile | 2 +-
> arch/arm64/kernel/vdso32/Makefile | 2 +-
> arch/x86/boot/compressed/Makefile | 2 +-
> init/Kconfig | 9 ++++++---
> 6 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index f41ec8c8426b..b6716a64519f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1118,7 +1118,7 @@ endif
> # We never want expected sections to be placed heuristically by the
> # linker. All sections should be explicitly named in the linker script.
> ifdef CONFIG_LD_ORPHAN_WARN
> -LDFLAGS_vmlinux += --orphan-handling=warn
> +LDFLAGS_vmlinux += --orphan-handling=$(if $(CONFIG_WERROR),error,warn)
> endif
>
> # Align the bit size of userspace programs with the kernel
> diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
> index 41bcbb460fac..c97db8b14c4f 100644
> --- a/arch/arm/boot/compressed/Makefile
> +++ b/arch/arm/boot/compressed/Makefile
> @@ -123,7 +123,7 @@ LDFLAGS_vmlinux += --no-undefined
> LDFLAGS_vmlinux += -X
> # Report orphan sections
> ifdef CONFIG_LD_ORPHAN_WARN
> -LDFLAGS_vmlinux += --orphan-handling=warn
> +LDFLAGS_vmlinux += --orphan-handling=$(if $(CONFIG_WERROR),error,warn)
> endif
> # Next argument is a linker script
> LDFLAGS_vmlinux += -T
> diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
> index 619e2dc7ee14..c8fcc06b5037 100644
> --- a/arch/arm64/kernel/vdso/Makefile
> +++ b/arch/arm64/kernel/vdso/Makefile
> @@ -27,7 +27,7 @@ ldflags-y := -shared -soname=linux-vdso.so.1 --hash-style=sysv \
> -Bsymbolic --build-id=sha1 -n $(btildflags-y)
>
> ifdef CONFIG_LD_ORPHAN_WARN
> - ldflags-y += --orphan-handling=warn
> + ldflags-y += --orphan-handling=$(if $(CONFIG_WERROR),error,warn)
> endif
>
> ldflags-y += -T
> diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
> index 36c8f66cad25..fa157366e814 100644
> --- a/arch/arm64/kernel/vdso32/Makefile
> +++ b/arch/arm64/kernel/vdso32/Makefile
> @@ -104,7 +104,7 @@ VDSO_AFLAGS += -D__ASSEMBLY__
> VDSO_LDFLAGS += -Bsymbolic --no-undefined -soname=linux-vdso.so.1
> VDSO_LDFLAGS += -z max-page-size=4096 -z common-page-size=4096
> VDSO_LDFLAGS += -shared --hash-style=sysv --build-id=sha1
> -VDSO_LDFLAGS += --orphan-handling=warn
> +VDSO_LDFLAGS += --orphan-handling=$(if $(CONFIG_WERROR),error,warn)
>
>
> # Borrow vdsomunge.c from the arm vDSO
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index 3a261abb6d15..a1a8bec61c10 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -68,7 +68,7 @@ KBUILD_LDFLAGS += $(call ld-option,--no-ld-generated-unwind-info)
> # address by the bootloader.
> LDFLAGS_vmlinux := -pie $(call ld-option, --no-dynamic-linker)
> ifdef CONFIG_LD_ORPHAN_WARN
> -LDFLAGS_vmlinux += --orphan-handling=warn
> +LDFLAGS_vmlinux += --orphan-handling=$(if $(CONFIG_WERROR),error,warn)
> endif
> LDFLAGS_vmlinux += -z noexecstack
> ifeq ($(CONFIG_LD_IS_BFD),y)
> diff --git a/init/Kconfig b/init/Kconfig
> index abf65098f1b6..8f4b838ece47 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -159,10 +159,12 @@ config WERROR
> help
> A kernel build should not cause any compiler warnings, and this
> enables the '-Werror' (for C) and '-Dwarnings' (for Rust) flags
> - to enforce that rule by default.
> + to enforce that rule by default. Certain warnings from other tools
> + such as the linker may be upgraded to errors with this option as
> + well.
>
> - However, if you have a new (or very old) compiler with odd and
> - unusual warnings, or you have some architecture with problems,
> + However, if you have a new (or very old) compiler or linker with odd
> + and unusual warnings, or you have some architecture with problems,
> you may need to disable this config option in order to
> successfully build the kernel.
>
> @@ -1454,6 +1456,7 @@ config LD_ORPHAN_WARN
> def_bool y
> depends on ARCH_WANT_LD_ORPHAN_WARN
> depends on $(ld-option,--orphan-handling=warn)
> + depends on $(ld-option,--orphan-handling=error)
>
> config SYSCTL
> bool
> --
> 2.34.1
>
>
