Hi,
My container failed to compile the latest v6.3, and bisecting shows that
commit 5d1dd961e74334a2178264193ea813d44ce5e725 ("x86/alternatives: Add
alt_instr.flags") introduces the regression.
```
./arch/x86/include/asm/uaccess_64.h:124: Error: found 'L', expected: ')'
./arch/x86/include/asm/uaccess_64.h:124: Error: found 'L', expected: ')'
./arch/x86/include/asm/uaccess_64.h:124: Error: found 'L', expected: ')'
./arch/x86/include/asm/uaccess_64.h:124: Error: found 'L', expected: ')'
./arch/x86/include/asm/uaccess_64.h:124: Error: found 'L', expected: ')'
./arch/x86/include/asm/uaccess_64.h:124: Error: found 'L', expected: ')'
./arch/x86/include/asm/uaccess_64.h:124: Error: junk at end of line,
first unrecognized character is `L'
```
The gcc inside container is:
gcc (GCC) 6.5.1 20181026 (Alibaba 6.5.1-1)
While the gcc on the host can compile it successfully:
gcc (GCC) 9.2.1 20200522 (Alibaba 9.2.1-3 2.17)
--
Thanks,
Jingbo
On Tue, Mar 14, 2023 at 11:12:13AM +0800, Jingbo Xu wrote:
> gcc (GCC) 6.5.1 20181026 (Alibaba 6.5.1-1)
Looks like you should complain to whoever patched this gcc and broke it
in the process. Unless you can reproduce the issue with an official
compiler...
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 3/14/23 6:23 PM, Borislav Petkov wrote:
> On Tue, Mar 14, 2023 at 11:12:13AM +0800, Jingbo Xu wrote:
>> gcc (GCC) 6.5.1 20181026 (Alibaba 6.5.1-1)
>
> Looks like you should complain to whoever patched this gcc and broke it
> in the process. Unless you can reproduce the issue with an official
> compiler...
>
Yeah, thanks for the reply. I will try if I have a chance, though I
doubt if I could compile the official compiler from the ground.
--
Thanks,
Jingbo
Hi Boris!
On Tue, Mar 14, 2023 at 11:23:16AM +0100, Borislav Petkov wrote:
> On Tue, Mar 14, 2023 at 11:12:13AM +0800, Jingbo Xu wrote:
> > gcc (GCC) 6.5.1 20181026 (Alibaba 6.5.1-1)
>
> Looks like you should complain to whoever patched this gcc and broke it
> in the process. Unless you can reproduce the issue with an official
> compiler...
I got the exact same issue today when building over distcc using locally-
built gcc-7.5 and gcc-6.5 here, though the one from kernel.org/crosstool
didn't have the problem. The only difference I found is the binutils that
the asm comes from. Mine rely on 2.27 while the one from kernel.org is on
2.32.
I produced the .s file then tried to build it and compared. Both .s files
are identical, and only one asm (2.32) assembles it fine. The error is
triggered here:
663:
.pushsection .altinstructions,"a"
.long 661b - .
.long 6641f - .
.4byte ((((((1UL))) << (0)) << 16) | ((18*32+ 4)))
^
This is the "L" the asm complains about. If I change it to "1U" it's
happy. Indeed, the difference is here in the introduction of the BIT()
macro in the definition of ALT_FLAG_NOT in your commit 5d1dd961e743
("x86/alternatives: Add alt_instr.flags") merged into 6.3-rc1:
-#define ALTINSTR_FLAG_INV (1 << 15)
-#define ALT_NOT(feat) ((feat) | ALTINSTR_FLAG_INV)
+#define ALT_FLAGS_SHIFT 16
+
+#define ALT_FLAG_NOT BIT(0)
+#define ALT_NOT(feature) ((ALT_FLAG_NOT << ALT_FLAGS_SHIFT) | (feature))
And I can confirm that the following patch fixes it for me, now the kernel
builds fine:
diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
index e2975a32d443..d7da28fada87 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -8,7 +8,7 @@
#define ALT_FLAGS_SHIFT 16
-#define ALT_FLAG_NOT BIT(0)
+#define ALT_FLAG_NOT (1 << 0)
#define ALT_NOT(feature) ((ALT_FLAG_NOT << ALT_FLAGS_SHIFT) | (feature))
#ifndef __ASSEMBLY__
May I send you a cleaner patch for this ?
Thanks!
Willy
On April 15, 2023 7:56:01 PM GMT+02:00, Willy Tarreau <[email protected]> wrote:
>May I send you a cleaner patch for this ?
Can you pls first send a minimal reproducer so that we can show it to gcc folks?
Thx.
--
Sent from a small device: formatting sucks and brevity is inevitable.
Hi Boris,
On Sat, Apr 15, 2023 at 08:56:47PM +0200, Borislav Petkov wrote:
> On April 15, 2023 7:56:01 PM GMT+02:00, Willy Tarreau <[email protected]> wrote:
> >May I send you a cleaner patch for this ?
>
> Can you pls first send a minimal reproducer so that we can show it to gcc folks?
Oh it's not even gcc, it's really just a matter of compatibility with
binutils. Documentation/Changes says binutils minimum is 2.25. This
toolchain I'm using in distcc was made against 2.27, I'm reproducing
the error with it:
$ cat repro.s
.4byte (1U)
.4byte (1UL)
$ /f/tc/x86_64-gcc75_glibc228-linux-gnu/bin/x86_64-gcc75_glibc228-linux-gnu-ld -v
GNU ld (crosstool-NG 1.24.0.500_645889f) 2.27
$ /f/tc/x86_64-gcc75_glibc228-linux-gnu/bin/x86_64-gcc75_glibc228-linux-gnu-as repro.s
repro.s: Assembler messages:
repro.s:2: Error: found 'L', expected: ')'
repro.s:2: Error: junk at end of line, first unrecognized character is `L'
This other one relying on 2.26 fails both on 1U and 1UL:
$ ld -v
GNU ld version 2.26.20160125
$ as repro.s
repro.s: Assembler messages:
repro.s:1: Error: missing ')'
repro.s:1: Error: junk at end of line, first unrecognized character is `U'
repro.s:2: Error: missing ')'
repro.s:2: Error: junk at end of line, first unrecognized character is `U'
And this one based on 2.29 works for both:
$ /dev/shm/gcc-5.5.0-nolibc/x86_64-linux/bin/x86_64-linux-ld -v
GNU ld (GNU Binutils) 2.29.1.20170915
$ /dev/shm/gcc-5.5.0-nolibc/x86_64-linux/bin/x86_64-linux-as repro.s
So it just means that the support for the "U" suffix on numbers was
added in binutils 2.27 and the "L" suffix on numbers was added somewhere
between 2.27 and 2.29.
And given that there's a single occurrence of all this in the whole tree,
that's why I'm proposing to just get back to the good old (1 << 0) instead
of BIT(0).
Thanks!
Willy
On Sun, Apr 16, 2023 at 10:36:36AM +0200, Willy Tarreau wrote:
> ...
> So it just means that the support for the "U" suffix on numbers was
> added in binutils 2.27 and the "L" suffix on numbers was added somewhere
> between 2.27 and 2.29.
Thanks for that - I'd like to document this once I've hashed out with
the toolchain person the proper binutils versions which got this
support. But that'll come later.
> And given that there's a single occurrence of all this in the whole tree,
> that's why I'm proposing to just get back to the good old (1 << 0) instead
> of BIT(0).
Yeah, we have those UC() macro things but they don't work in inline asm
in C code. So yeah, pls do the thing you're suggesting.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On April 17, 2023 11:14:17 AM PDT, Borislav Petkov <[email protected]> wrote:
>On Sun, Apr 16, 2023 at 10:36:36AM +0200, Willy Tarreau wrote:
>> ...
>> So it just means that the support for the "U" suffix on numbers was
>> added in binutils 2.27 and the "L" suffix on numbers was added somewhere
>> between 2.27 and 2.29.
>
>Thanks for that - I'd like to document this once I've hashed out with
>the toolchain person the proper binutils versions which got this
>support. But that'll come later.
>
>> And given that there's a single occurrence of all this in the whole tree,
>> that's why I'm proposing to just get back to the good old (1 << 0) instead
>> of BIT(0).
>
>Yeah, we have those UC() macro things but they don't work in inline asm
>in C code. So yeah, pls do the thing you're suggesting.
>
>Thx.
>
We do have assembly-aware macros for this; I believe they are called _UL() etc.
On Mon, Apr 17, 2023 at 11:32:19AM -0700, H. Peter Anvin wrote:
> We do have assembly-aware macros for this; I believe they are called _UL() etc.
It doesn't work even if I do:
#define ALT_FLAG_NOT BIT_MASK(0)
and that macro has the UL() wrappery for the __ASSEMBLY__ preprocessor case
because where it gets used, ALTERNATIVE_3() in __clear_user(),
__ASSEMBLY__ is there not defined, ofc - it is C code.
So it does:
"# ALT: oldinstr3\n" "661:\n\t" "rep stosb" "\n662:\n" "# ALT: padding3\n"...
" .4byte " "((((((1UL))) << ((0) % 64)) << 16) | ((18*32+ 4)))"
which those old gas things don't have support yet for.
And U and L suffixes are C-syntax, strictly speaking. So assembler
numbers don't need them. Even if binutils has support for them:
$ binutils-gdb> git tag --contains e140100a5da85568e83ffe8e77d3f5e4a59ddee8 | head
binutils-2_27
...
$ binutils-gdb> git tag --contains 86b80085c889cd388fa677a5ae9053fd4be3776c | head
binutils-2_28
...
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette