[0/2] modpost: skip section mismatch warnings on ELF local symbols by default
mbox series

Message ID 20181020131911.22443-1-paul.walmsley@sifive.com
Headers show
Series
  • modpost: skip section mismatch warnings on ELF local symbols by default
Related show

Message

Paul Walmsley Oct. 20, 2018, 1:19 p.m. UTC
modpost: skip section mismatch warnings on ELF local symbols by default

modpost, by default, reports section mismatch warnings on ELF local
symbols.  This caused false positive warnings to be reported for a
local symbol name that would otherwise be elided by matching against a
name pattern.  This was observed using a RISC-V toolchain that generates
section anchors.

To avoid this noise in the common case, this patch series disables
section mismatch warnings on ELF local symbols by default.  It also
adds a modpost command line switch to re-enable these warnings, since
I wasn't able to convince myself that section mismatch warnings on ELF
local symbols were completely useless; just mostly useless :-)

I'm neither a modpost nor an ELF expert, so careful review of this
series is appreciated.

This modpost series can also be found at:

https://github.com/sifive/riscv-linux/tree/dev/paulw/modpost-elf-local-symbols-v4.19-rc7

The series was tested on a RISC-V build of the SiFive UART serial
driver, at:

https://github.com/sifive/riscv-linux/tree/dev/paulw/serial-v4.19-rc7

Paul Walmsley (2):
  modpost: add switch to skip symbol exclusions likely to generate false
    positives
  modpost: skip ELF local symbols by default during section mismatch
    check

 scripts/mod/modpost.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

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: linux-kbuild@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Comments

Sam Ravnborg Oct. 20, 2018, 2:08 p.m. UTC | #1
Hi Paul.

> modpost: skip section mismatch warnings on ELF local symbols by default
> 
> modpost, by default, reports section mismatch warnings on ELF local
> symbols.  This caused false positive warnings to be reported for a
> local symbol name that would otherwise be elided by matching against a
> name pattern.  This was observed using a RISC-V toolchain that generates
> section anchors.
> 
> To avoid this noise in the common case, this patch series disables
> section mismatch warnings on ELF local symbols by default.
This part is fine.

> It also
> adds a modpost command line switch to re-enable these warnings, since
> I wasn't able to convince myself that section mismatch warnings on ELF
> local symbols were completely useless; just mostly useless :-)
modpost is not supposed to be used outside the kernel build.
And therefore if we introduce a new option then the infrastructure
to enable that option should also be in place.
In this particular case I cannot see why we should add the possibility
to include local symbols, in other words do not add the option.
Wait a few days before you kill it, maybe others see the usefulness of it.

I checked if there were any options supported by modpost that
was not configurable in makefile.modpost.
And I could see that the -M and -K options in getopt() was leftovers.
The code that used these option was was dropped in:
a8773769d1a1e08d0ca15f890515401ab3860637 ("Kbuild: clear marker out of modpost")

Could you add a patch that delete these on top of what you already have.

	Sam
Paul Walmsley Nov. 15, 2018, 12:48 a.m. UTC | #2
Hi Sam,

On Sat, 20 Oct 2018, Sam Ravnborg wrote:

> modpost is not supposed to be used outside the kernel build.
> And therefore if we introduce a new option then the infrastructure
> to enable that option should also be in place.
> In this particular case I cannot see why we should add the possibility
> to include local symbols, in other words do not add the option.
> Wait a few days before you kill it, maybe others see the usefulness of it.

I've dropped the optionality from the patch set.

> I checked if there were any options supported by modpost that
> was not configurable in makefile.modpost.
> And I could see that the -M and -K options in getopt() was leftovers.
> The code that used these option was was dropped in:
> a8773769d1a1e08d0ca15f890515401ab3860637 ("Kbuild: clear marker out of modpost")
>
> Could you add a patch that delete these on top of what you already have.

Done.  This patch is now in the v2 set which should appear shortly.

- Paul