Re: [PATCH v3] x86/vmlinux: Fix vmlinux.lds.S with pre-2.23 binutils

Tom Lendacky
2020-01-13T21:46:12Z

On 1/13/20 1:53 PM, Arvind Sankar wrote:
> Prior to binutils-2.23, ld treats the location counter as absolute if
> used outside an output section definition. From version 2.23 onwards,
> the location counter is treated as relative to an adjacent output
> section (usually the previous one, unless there isn't one or the
> location counter has been assigned to previously, in which case the next
> one).
>
> The result is that a symbol definition in the linker script, such as
> _etext = .;
> that appears outside an output section definition makes _etext an
> absolute symbol prior to binutils-2.23 and a relative symbol from
> version 2.23 onwards. So when using a 2.21 or 2.22 vintage linker, the
> build fails with
> Invalid absolute R_X86_64_32S relocation: _etext
> for x86-64, and a similar message with R_386_32 for x86-32.
>
> This can be reproduced with the official 2.21.1 and 2.22 binutils
> releases.
>
> Commit b907693883fd ("x86/vmlinux: Actually use _etext for the end of
> the text segment") moved _etext out of the .text section to place it
> after the exception table, however since commit f0d7ee17d57c
> ("x86/vmlinux: Move EXCEPTION_TABLE to RO_DATA segment") this is no
> longer needed. Move _etext back inside .text to make it relative even
> with older linkers.
>
> Commit c603a309cc75 ("x86/mm: Identify the end of the kernel area to be
> reserved") defines __end_of_kernel_reserve using the location counter
> outside an output section definition. Use __bss_stop instead of the
> location counter for the definition to make it relative with older
> linkers.
>
> Fixes: b907693883fd ("x86/vmlinux: Actually use _etext for the end of the text segment")
> Fixes: c603a309cc75 ("x86/mm: Identify the end of the kernel area to be reserved")
> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> ---
> v3: Modify vmlinux.lds.S instead of adding more workarounds to tools/relocs.c
>
> arch/x86/kernel/vmlinux.lds.S | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> index 3a1a819da137..bad4e22384dc 100644
> --- a/arch/x86/kernel/vmlinux.lds.S
> +++ b/arch/x86/kernel/vmlinux.lds.S
> @@ -144,10 +144,12 @@ SECTIONS
> *(.text.__x86.indirect_thunk)
> __indirect_thunk_end = .;
> #endif
> +
> + /* End of text section */
> + _etext = .;
> } :text =0xcccc
>
> - /* End of text section, which should occupy whole number of pages */
> - _etext = .;
> + /* .text should occupy whole number of pages */
> . = ALIGN(PAGE_SIZE);
>
> X86_ALIGN_RODATA_BEGIN
> @@ -372,7 +374,7 @@ SECTIONS
> * explicitly reserved using memblock_reserve() or it will be discarded
> * and treated as available memory.
> */
> - __end_of_kernel_reserve = .;
> + __end_of_kernel_reserve = __bss_stop;

The only thing I worry about is if someone inserts a section between
the bss section and here and doesn't update this value.

Thanks,
Tom

>
> . = ALIGN(PAGE_SIZE);
> .brk : AT(ADDR(.brk) - LOAD_OFFSET) {
>