All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jessica Yu <jeyu@kernel.org>
To: Marco Elver <elver@google.com>
Cc: Fangrui Song <maskray@google.com>,
	LKML <linux-kernel@vger.kernel.org>,
	clang-built-linux <clang-built-linux@googlegroups.com>,
	Sam Ravnborg <sam@ravnborg.org>
Subject: Re: [PATCH v2] module: Ignore _GLOBAL_OFFSET_TABLE_ when warning for undefined symbols
Date: Fri, 15 Jan 2021 09:14:42 +0100	[thread overview]
Message-ID: <20210115081442.GB9637@linux-8ccs> (raw)
In-Reply-To: <CANpmjNOrD76O2_Zpwo5RJ2d12gczuQpG9bJkXYLY_sOVeEVZGQ@mail.gmail.com>

+++ Marco Elver [15/01/21 08:03 +0100]:
>On Thu, 14 Jan 2021 at 22:54, Fangrui Song <maskray@google.com> wrote:
>> clang-12 -fno-pic (since
>> https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de0083333232da3f1d6)
>> can emit `call __stack_chk_fail@PLT` instead of `call __stack_chk_fail`
>> on x86.  The two forms should have identical behaviors on x86-64 but the
>> former causes GNU as<2.37 to produce an unreferenced undefined symbol
>> _GLOBAL_OFFSET_TABLE_.
>>
>> (On x86-32, there is an R_386_PC32 vs R_386_PLT32 difference but the
>> linker behavior is identical as far as Linux kernel is concerned.)
>>
>> Simply ignore _GLOBAL_OFFSET_TABLE_ for now, like what
>> scripts/mod/modpost.c:ignore_undef_symbol does. This also fixes the
>> problem for gcc/clang -fpie and -fpic, which may emit `call foo@PLT` for
>> external function calls on x86.
>>
>> Note: ld -z defs and dynamic loaders do not error for unreferenced
>> undefined symbols so the module loader is reading too much.  If we ever
>> need to ignore more symbols, the code should be refactored to ignore
>> unreferenced symbols.
>>
>> Reported-by: Marco Elver <elver@google.com>
>> Link: https://github.com/ClangBuiltLinux/linux/issues/1250
>> Signed-off-by: Fangrui Song <maskray@google.com>
>
>Tested-by: Marco Elver <elver@google.com>
>
>Thank you for the patch!
>
>> ---
>>  kernel/module.c | 20 ++++++++++++++++++--
>>  1 file changed, 18 insertions(+), 2 deletions(-)
>> ---
>> Changes in v2:
>> * Fix Marco's email address
>> * Add a function ignore_undef_symbol similar to scripts/mod/modpost.c:ignore_undef_symbol
>>
>> diff --git a/kernel/module.c b/kernel/module.c
>> index 4bf30e4b3eaa..278f5129bde2 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -2348,6 +2348,20 @@ static int verify_exported_symbols(struct module *mod)
>>         return 0;
>>  }
>>
>> +static int ignore_undef_symbol(Elf_Half emachine, const char *name)
>
>Why not 'bool' return-type?
>
>> +{
>> +       /* On x86, PIC code and Clang non-PIC code may have call foo@PLT. GNU as
>
>Not sure if checkpatch.pl warns about this, but this multi-line
>comment does not follow the normal kernel-style (see elsewhere in
>file):
>
>/*
> * ...
> */

+1 to Marco's comments. Otherwise, patch looks good to me.

Thanks Fangrui!

Jessica


  reply	other threads:[~2021-01-15  8:15 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-14  5:48 [PATCH] module: Ignore _GLOBAL_OFFSET_TABLE_ when warning for undefined symbols Fangrui Song
2021-01-14 14:06 ` Jessica Yu
2021-01-14 16:57   ` Fāng-ruì Sòng
2021-01-14 21:18     ` Jessica Yu
2021-01-14 21:54       ` [PATCH v2] " Fangrui Song
2021-01-14 22:01         ` Nick Desaulniers
2021-01-15  8:09           ` Jessica Yu
2021-01-15  7:03         ` Marco Elver
2021-01-15  8:14           ` Jessica Yu [this message]
2021-01-15 19:50           ` Fāng-ruì Sòng
2021-01-15 19:52       ` [PATCH v3] " Fangrui Song
2021-01-15 19:55         ` Nathan Chancellor
2021-01-18 10:14         ` Jessica Yu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210115081442.GB9637@linux-8ccs \
    --to=jeyu@kernel.org \
    --cc=clang-built-linux@googlegroups.com \
    --cc=elver@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maskray@google.com \
    --cc=sam@ravnborg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.