linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] modpost: skip ELF local symbols during section mismatch check
@ 2018-11-21 21:14 Paul Walmsley
  2018-11-22 11:44 ` Masahiro Yamada
  0 siblings, 1 reply; 2+ messages in thread
From: Paul Walmsley @ 2018-11-21 21:14 UTC (permalink / raw)
  To: linux-kernel, linux-kbuild
  Cc: Russell King, jimw, Michal Marek, Masahiro Yamada, Sam Ravnborg,
	Paul Walmsley


During development of a serial console driver with a gcc 8.2.0
toolchain for RISC-V, the following modpost warning appeared:

----
WARNING: vmlinux.o(.data+0x19b10): Section mismatch in reference from the variable .LANCHOR1 to the function .init.text:sifive_serial_console_setup()
The variable .LANCHOR1 references
the function __init sifive_serial_console_setup()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
----

".LANCHOR1" is an ELF local symbol, automatically created by gcc's section
anchor generation code:

https://gcc.gnu.org/onlinedocs/gccint/Anchored-Addresses.html

https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/varasm.c;h=cd9591a45617464946dcf9a126dde277d9de9804;hb=9fb89fa845c1b2e0a18d85ada0b077c84508ab78#l7473

This was verified by compiling the kernel with -fno-section-anchors
and observing that the ".LANCHOR1" ELF local symbol disappeared, and
modpost no longer warned about the section mismatch.  The serial
driver code idiom triggering the warning is standard Linux serial
driver practice that has a specific whitelist inclusion in modpost.c.

I'm neither a modpost nor an ELF expert, but naively, it doesn't seem
useful for modpost to report section mismatch warnings caused by ELF
local symbols by default.  Local symbols have compiler-generated
names, and thus bypass modpost's whitelisting algorithm, which relies
on the presence of a non-autogenerated symbol name.  This increases
the likelihood that false positive warnings will be generated (as in
the above case).

Thus, disable section mismatch reporting on ELF local symbols.  The
rationale here is similar to that of commit 2e3a10a1551d ("ARM: avoid
ARM binutils leaking ELF local symbols") and of similar code already
present in modpost.c:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/mod/modpost.c?h=v4.19-rc4&id=7876320f88802b22d4e2daf7eb027dd14175a0f8#n1256

This third version of the patch implements a suggestion from Masahiro
Yamada <yamada.masahiro@socionext.com> to restructure the code as an
additional pattern matching step inside secref_whitelist(), and
further improves the patch description.

Cc: Russell King <linux@armlinux.org.uk>
Cc: Jim Wilson <jimw@sifive.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Michal Marek <michal.lkml@markovi.net>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: linux-kbuild@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
  scripts/mod/modpost.c | 12 ++++++++++++
  1 file changed, 12 insertions(+)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 0d998c54564d..90bb04b4e166 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1163,6 +1163,14 @@ static const struct sectioncheck *section_mismatch(
   *   fromsec = text section
   *   refsymname = *.constprop.*
   *
+ * Pattern 6:
+ *   Hide section mismatch warnings for ELF local symbols.  The goal
+ *   is to eliminate false positive modpost warnings caused by
+ *   compiler-generated ELF local symbol names such as ".LANCHOR1".
+ *   Autogenerated symbol names bypass modpost's "Pattern 2"
+ *   whitelisting, which relies on pattern-matching against symbol
+ *   names to work.  (One situation where gcc can autogenerate ELF
+ *   local symbols is when "-fsection-anchors" is used.)
   **/
  static int secref_whitelist(const struct sectioncheck *mismatch,
  			    const char *fromsec, const char *fromsym,
@@ -1201,6 +1209,10 @@ static int secref_whitelist(const struct sectioncheck *mismatch,
  	    match(fromsym, optim_symbols))
  		return 0;

+	/* Check for pattern 6 */
+	if (strstarts(fromsym, ".L"))
+		return 0;
+
  	return 1;
  }

-- 
2.19.1



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] modpost: skip ELF local symbols during section mismatch check
  2018-11-21 21:14 [PATCH] modpost: skip ELF local symbols during section mismatch check Paul Walmsley
@ 2018-11-22 11:44 ` Masahiro Yamada
  0 siblings, 0 replies; 2+ messages in thread
From: Masahiro Yamada @ 2018-11-22 11:44 UTC (permalink / raw)
  To: paul.walmsley
  Cc: Linux Kernel Mailing List, Linux Kbuild mailing list,
	Russell King, jimw, Michal Marek, Sam Ravnborg, Paul Walmsley

On Thu, Nov 22, 2018 at 4:11 PM Paul Walmsley <paul.walmsley@sifive.com> wrote:
>
>
> During development of a serial console driver with a gcc 8.2.0
> toolchain for RISC-V, the following modpost warning appeared:
>
> ----
> WARNING: vmlinux.o(.data+0x19b10): Section mismatch in reference from the variable .LANCHOR1 to the function .init.text:sifive_serial_console_setup()
> The variable .LANCHOR1 references
> the function __init sifive_serial_console_setup()
> If the reference is valid then annotate the
> variable with __init* or __refdata (see linux/init.h) or name the variable:
> *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
> ----
>
> ".LANCHOR1" is an ELF local symbol, automatically created by gcc's section
> anchor generation code:
>
> https://gcc.gnu.org/onlinedocs/gccint/Anchored-Addresses.html
>
> https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/varasm.c;h=cd9591a45617464946dcf9a126dde277d9de9804;hb=9fb89fa845c1b2e0a18d85ada0b077c84508ab78#l7473
>
> This was verified by compiling the kernel with -fno-section-anchors
> and observing that the ".LANCHOR1" ELF local symbol disappeared, and
> modpost no longer warned about the section mismatch.  The serial
> driver code idiom triggering the warning is standard Linux serial
> driver practice that has a specific whitelist inclusion in modpost.c.
>
> I'm neither a modpost nor an ELF expert, but naively, it doesn't seem
> useful for modpost to report section mismatch warnings caused by ELF
> local symbols by default.  Local symbols have compiler-generated
> names, and thus bypass modpost's whitelisting algorithm, which relies
> on the presence of a non-autogenerated symbol name.  This increases
> the likelihood that false positive warnings will be generated (as in
> the above case).
>
> Thus, disable section mismatch reporting on ELF local symbols.  The
> rationale here is similar to that of commit 2e3a10a1551d ("ARM: avoid
> ARM binutils leaking ELF local symbols") and of similar code already
> present in modpost.c:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/mod/modpost.c?h=v4.19-rc4&id=7876320f88802b22d4e2daf7eb027dd14175a0f8#n1256
>
> This third version of the patch implements a suggestion from Masahiro
> Yamada <yamada.masahiro@socionext.com> to restructure the code as an
> additional pattern matching step inside secref_whitelist(), and
> further improves the patch description.
>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Jim Wilson <jimw@sifive.com>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Michal Marek <michal.lkml@markovi.net>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: linux-kbuild@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> ---

Applied with Sam's Ack.

Thanks!


>   scripts/mod/modpost.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 0d998c54564d..90bb04b4e166 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1163,6 +1163,14 @@ static const struct sectioncheck *section_mismatch(
>    *   fromsec = text section
>    *   refsymname = *.constprop.*
>    *
> + * Pattern 6:
> + *   Hide section mismatch warnings for ELF local symbols.  The goal
> + *   is to eliminate false positive modpost warnings caused by
> + *   compiler-generated ELF local symbol names such as ".LANCHOR1".
> + *   Autogenerated symbol names bypass modpost's "Pattern 2"
> + *   whitelisting, which relies on pattern-matching against symbol
> + *   names to work.  (One situation where gcc can autogenerate ELF
> + *   local symbols is when "-fsection-anchors" is used.)
>    **/
>   static int secref_whitelist(const struct sectioncheck *mismatch,
>                             const char *fromsec, const char *fromsym,
> @@ -1201,6 +1209,10 @@ static int secref_whitelist(const struct sectioncheck *mismatch,
>             match(fromsym, optim_symbols))
>                 return 0;
>
> +       /* Check for pattern 6 */
> +       if (strstarts(fromsym, ".L"))
> +               return 0;
> +
>         return 1;
>   }
>
> --
> 2.19.1
>
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-11-22 11:44 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-21 21:14 [PATCH] modpost: skip ELF local symbols during section mismatch check Paul Walmsley
2018-11-22 11:44 ` Masahiro Yamada

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).