Thank you for your contribution! Sashiko AI review found 3 potential issue(s) 
to consider:
- [High] The linker script fix is wrapped in an inverted configuration 
conditional (`#ifndef CONFIG_ARM_UNWIND`), applying the fix only when unwinding 
is disabled.
- [High] The wildcard patterns are enclosed in double quotes, which disables 
wildcard matching in GNU ld.
- [Low] Non-standard linker script syntax with a space between the wildcard 
asterisk and the section name list.
--

commit 165574ad9cf92e77eac788d811caa72c990e0b3d
Author: Xiao Junzhe <[email protected]>
Subject: ARM: module.lds: fix unwind metadata for merged .text sections

This commit attempts to fix a module stack unwinding failure on ARM by
coalescing .ARM.exidx and .ARM.extab input sections that belong to merged
.text.* sections. This is intended to keep the output names in sync with
what the ARM module unwind code expects.

> diff --git a/arch/arm/include/asm/module.lds.h 
> b/arch/arm/include/asm/module.lds.h
> --- a/arch/arm/include/asm/module.lds.h
> +++ b/arch/arm/include/asm/module.lds.h
> @@ -1,4 +1,20 @@
>  /* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef CONFIG_ARM_UNWIND

[Severity: High]
Does this exclude the fix in the exact configuration it is intended for?

The patch aims to fix module stack unwinding, which is controlled by
CONFIG_ARM_UNWIND. Because this block is wrapped in an #ifndef, the section
coalescing will be skipped when unwinding is enabled, leaving the unwind
tables unmerged and the issue unresolved.

> +SECTIONS {
> +     .ARM.extab              0 : {
> +             *(.ARM.extab)
> +             * (".ARM.extab.text")
> +             * (".ARM.extab.text.[0-9a-zA-Z_]*")

[Severity: Low]
This isn't a bug, but there is a space between the asterisk wildcard and
the section name list. While GNU ld ignores this whitespace, standard kernel
linker script conventions typically omit it.

[Severity: High]
Will GNU ld expand the wildcard patterns when they are enclosed in double
quotes?

Enclosing a section name in double quotes causes wildcard characters like
the asterisk and brackets to lose their special meaning and be treated as
literal characters. The linker will try to match a section literally named
".ARM.extab.text.[0-9a-zA-Z_]*" instead of expanding the glob to match
sections like ".ARM.extab.text.unlikely".

> +     }
> +     .ARM.exidx              0 : {
> +             *(.ARM.exidx)
> +             * (".ARM.exidx.text")
> +             * (".ARM.exidx.text.[0-9a-zA-Z_]*")
> +     }
> +}
> +#endif

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to