Re: [PATCH v6 3/7] Adjust symbol ordering in text output section

[Kees Cook]

On Fri, Oct 25, 2024 at 10:14:05PM -0700, Rong Xu wrote:
> When the -ffunction-sections compiler option is enabled, each function
> is placed in a separate section named .text.function_name rather than
> putting all functions in a single .text section.
> 
> However, using -function-sections can cause problems with the
> linker script. The comments included in include/asm-generic/vmlinux.lds.h
> note these issues.:
>   “TEXT_MAIN here will match .text.fixup and .text.unlikely if dead
>    code elimination is enabled, so these sections should be converted
>    to use ".." first.”
> 
> It is unclear whether there is a straightforward method for converting
> a suffix to "..".

While I'm not a fan of the potential loss of a page, I'm not aware of
a way to do this renaming. If we want to keep these as "cold", then this
is probably the best way to do it.

> This patch modifies the order of subsections within the text output
> section. Specifically, it repositions sections with certain fixed patterns
> (for example .text.unlikely) before TEXT_MAIN, ensuring that they are
> grouped and matched together. It also places .text.hot section at the
> beginning of a page to help the TLB performance.
> 
> Note that the limitation arises because the linker script employs glob
> patterns instead of regular expressions for string matching. While there
> is a method to maintain the current order using complex patterns, this
> significantly complicates the pattern and increases the likelihood of
> errors.
> 
> This patch also changes vmlinux.lds.S for the sparc64 architecture to
> accommodate specific symbol placement requirements.
> 
> Co-developed-by: Han Shen <shenhan@xxxxxxxxxx>
> Signed-off-by: Han Shen <shenhan@xxxxxxxxxx>
> Signed-off-by: Rong Xu <xur@xxxxxxxxxx>
> Suggested-by: Sriraman Tallam <tmsriram@xxxxxxxxxx>
> Suggested-by: Krzysztof Pszeniczny <kpszeniczny@xxxxxxxxxx>
> Tested-by: Yonghong Song <yonghong.song@xxxxxxxxx>
> Tested-by: Yabin Cui <yabinc@xxxxxxxxxx>
> Change-Id: I5202d40bc7e24f93c2bfb2f0d987e9dc57dec1b1

Reviewed-by: Kees Cook <kees@xxxxxxxxxx>

-Kees

> ---
>  arch/sparc/kernel/vmlinux.lds.S   |  5 +++++
>  include/asm-generic/vmlinux.lds.h | 19 ++++++++++++-------
>  2 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/sparc/kernel/vmlinux.lds.S b/arch/sparc/kernel/vmlinux.lds.S
> index d317a843f7ea9..f1b86eb303404 100644
> --- a/arch/sparc/kernel/vmlinux.lds.S
> +++ b/arch/sparc/kernel/vmlinux.lds.S
> @@ -48,6 +48,11 @@ SECTIONS
>  	{
>  		_text = .;
>  		HEAD_TEXT
> +	        ALIGN_FUNCTION();
> +#ifdef CONFIG_SPARC64
> +	        /* Match text section symbols in head_64.S first */
> +	        *head_64.o(.text)
> +#endif
>  		TEXT_TEXT
>  		SCHED_TEXT
>  		LOCK_TEXT
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index eeadbaeccf88b..fd901951549c0 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -553,19 +553,24 @@
>   * .text section. Map to function alignment to avoid address changes
>   * during second ld run in second ld pass when generating System.map
>   *
> - * TEXT_MAIN here will match .text.fixup and .text.unlikely if dead
> - * code elimination is enabled, so these sections should be converted
> - * to use ".." first.
> + * TEXT_MAIN here will match symbols with a fixed pattern (for example,
> + * .text.hot or .text.unlikely) if dead code elimination or
> + * function-section is enabled. Match these symbols first before
> + * TEXT_MAIN to ensure they are grouped together.
> + *
> + * Also placing .text.hot section at the beginning of a page, this
> + * would help the TLB performance.
>   */
>  #define TEXT_TEXT							\
>  		ALIGN_FUNCTION();					\
> +		*(.text.asan.* .text.tsan.*)				\
> +		*(.text.unknown .text.unknown.*)			\
> +		*(.text.unlikely .text.unlikely.*)			\
> +		. = ALIGN(PAGE_SIZE);					\
>  		*(.text.hot .text.hot.*)				\
>  		*(TEXT_MAIN .text.fixup)				\
> -		*(.text.unlikely .text.unlikely.*)			\
> -		*(.text.unknown .text.unknown.*)			\
>  		NOINSTR_TEXT						\
> -		*(.ref.text)						\
> -		*(.text.asan.* .text.tsan.*)
> +		*(.ref.text)
>  
>  
>  /* sched.text is aling to function alignment to secure we have same
> -- 
> 2.47.0.163.g1226f6d8fa-goog
> 

-- 
Kees Cook