| Message ID | 20190618131048.543-1-will.deacon@arm.com |
|---|---|
| State | Accepted |
| Commit | a222061b85234d8a44486a46bd4df7e2cda52385 |
| Headers | show |
| Series | genksyms: Teach parser about 128-bit built-in types | expand |
On Tue, Jun 18, 2019 at 3:10 PM Will Deacon <will.deacon@arm.com> wrote: > > + { "__int128", BUILTIN_INT_KEYW }, > + { "__int128_t", BUILTIN_INT_KEYW }, > + { "__uint128_t", BUILTIN_INT_KEYW }, I wonder if it's safe to treat them as the same type, since __int128_t and __uint128_t differ in signedness. If someone exports a symbol with one and changes it to the other, they would appear to be the same type. Arnd
Hi Arnd, On Tue, Jun 18, 2019 at 04:17:35PM +0200, Arnd Bergmann wrote: > On Tue, Jun 18, 2019 at 3:10 PM Will Deacon <will.deacon@arm.com> wrote: > > > > + { "__int128", BUILTIN_INT_KEYW }, > > + { "__int128_t", BUILTIN_INT_KEYW }, > > + { "__uint128_t", BUILTIN_INT_KEYW }, > > I wonder if it's safe to treat them as the same type, since > __int128_t and __uint128_t differ in signedness. > > If someone exports a symbol with one and changes it to the other, they > would appear to be the same type. My understanding is that the actual CRC generation for normal symbols is based purely on the string-representation of the function signature, and so the underlying type information isn't relevant to that. I can also confirm that the CRC for an exported symbol that returns a __uint128_t is not the same if you change it to return a __int128_t instead. Will
On Wed, Jun 19, 2019 at 1:21 AM Will Deacon <will.deacon@arm.com> wrote: > > Hi Arnd, > > On Tue, Jun 18, 2019 at 04:17:35PM +0200, Arnd Bergmann wrote: > > On Tue, Jun 18, 2019 at 3:10 PM Will Deacon <will.deacon@arm.com> wrote: > > > > > > + { "__int128", BUILTIN_INT_KEYW }, > > > + { "__int128_t", BUILTIN_INT_KEYW }, > > > + { "__uint128_t", BUILTIN_INT_KEYW }, > > > > I wonder if it's safe to treat them as the same type, since > > __int128_t and __uint128_t differ in signedness. > > > > If someone exports a symbol with one and changes it to the other, they > > would appear to be the same type. > > My understanding is that the actual CRC generation for normal symbols is > based purely on the string-representation of the function signature, and > so the underlying type information isn't relevant to that. I can also > confirm that the CRC for an exported symbol that returns a __uint128_t > is not the same if you change it to return a __int128_t instead. Right. Applied to linux-kbuild. Thanks! -- Best Regards Masahiro Yamada
diff --git a/scripts/genksyms/keywords.c b/scripts/genksyms/keywords.c index 9f40bcd17d07..f6956aa41366 100644 --- a/scripts/genksyms/keywords.c +++ b/scripts/genksyms/keywords.c @@ -24,6 +24,10 @@ static struct resword { { "__volatile__", VOLATILE_KEYW }, { "__builtin_va_list", VA_LIST_KEYW }, + { "__int128", BUILTIN_INT_KEYW }, + { "__int128_t", BUILTIN_INT_KEYW }, + { "__uint128_t", BUILTIN_INT_KEYW }, + // According to rth, c99 defines "_Bool", __restrict", __restrict__", "restrict". KAO { "_Bool", BOOL_KEYW }, { "_restrict", RESTRICT_KEYW }, diff --git a/scripts/genksyms/parse.y b/scripts/genksyms/parse.y index 00a6d7e54971..1ebcf52cd0f9 100644 --- a/scripts/genksyms/parse.y +++ b/scripts/genksyms/parse.y @@ -76,6 +76,7 @@ static void record_compound(struct string_list **keyw, %token ATTRIBUTE_KEYW %token AUTO_KEYW %token BOOL_KEYW +%token BUILTIN_INT_KEYW %token CHAR_KEYW %token CONST_KEYW %token DOUBLE_KEYW @@ -263,6 +264,7 @@ simple_type_specifier: | VOID_KEYW | BOOL_KEYW | VA_LIST_KEYW + | BUILTIN_INT_KEYW | TYPE { (*$1)->tag = SYM_TYPEDEF; $$ = $1; } ;
__uint128_t crops up in a few files that export symbols to modules, so teach genksyms about it and the other GCC built-in 128-bit integer types so that we don't end up skipping the CRC generation for some symbols due to the parser failing to spot them: | WARNING: EXPORT symbol "kernel_neon_begin" [vmlinux] version | generation failed, symbol will not be versioned. | ld: arch/arm64/kernel/fpsimd.o: relocation R_AARCH64_ABS32 against | `__crc_kernel_neon_begin' can not be used when making a shared | object | ld: arch/arm64/kernel/fpsimd.o:(.data+0x0): dangerous relocation: | unsupported relocation Cc: Richard Henderson <rth@twiddle.net> Cc: Masahiro Yamada <yamada.masahiro@socionext.com> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reported-by: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Will Deacon <will.deacon@arm.com> --- Without this patch, we're seeing arm64 build breakage in linux-next under some configurations. scripts/genksyms/keywords.c | 4 ++++ scripts/genksyms/parse.y | 2 ++ 2 files changed, 6 insertions(+) -- 2.11.0