Re: [tip:x86/urgent] x86/mm/kaslr: Use the _ASM_MUL macro for multiplication to work around Clang incompatibility

[hpa]

On May 5, 2017 11:44:05 AM PDT, Matthias Kaehlcke <mka@xxxxxxxxxxxx> wrote:
>El Fri, May 05, 2017 at 07:50:39PM +0200 Ingo Molnar ha dit:
>
>> 
>> * Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>> 
>> > On Fri, May 05, 2017 at 01:11:47AM -0700, tip-bot for Matthias
>Kaehlcke wrote:
>> > > Commit-ID:  121843eb02a6e2fa30aefab64bfe183c97230c75
>> > > Gitweb:    
>http://git.kernel.org/tip/121843eb02a6e2fa30aefab64bfe183c97230c75
>> > > Author:     Matthias Kaehlcke <mka@xxxxxxxxxxxx>
>> > > AuthorDate: Mon, 1 May 2017 15:47:41 -0700
>> > > Committer:  Ingo Molnar <mingo@xxxxxxxxxx>
>> > > CommitDate: Fri, 5 May 2017 08:31:05 +0200
>> > > 
>> > > x86/mm/kaslr: Use the _ASM_MUL macro for multiplication to work
>around Clang incompatibility
>> > > 
>> > > The constraint "rm" allows the compiler to put mix_const into
>memory.
>> > > When the input operand is a memory location then MUL needs an
>operand
>> > > size suffix, since Clang can't infer the multiplication width
>from the
>> > > operand.
>> > 
>> > *sigh*, this is another shining example of how LLVM is a better,
>faster
>> > moving compiler?
>> 
>> Well, I don't like it - but we already have similar patterns to cover
>some asm 
>> complications so I didn't mind. Apparently Clang is very close to
>being able to 
>> build a working Linux kernel, right?
>
>Indeed, I expect 4.12 (with this patch ...) to build with Clang for a
>x86 defconfig (with tons of warnings). ARM64 is very close.
>
>> In that sense it would be unfair to expect it to not have various
>legacies, 
>> missing features and quirks - just like the kernel has dozens of GCC
>related 
>> workarounds.
>
>Also my understanding is that this isn't really a clang issue. In the
>context of this code gcc apparently chooses to use a register for
>'mix_const', for memory locations it also needs a suffix.
>
>Actually I just tried to build this code from a single C file:
>
>void test() {
>  unsigned long raw, random;
>  const unsigned long mix_const = 0x3f39e593UL;
>
>  asm("MUL %3"
>    : "=a" (random), "=d" (raw)
>    : "a" (random), "rm" (mix_const));
>}
>
>gcc -c /tmp/test.c
>/tmp/test.c: Assembler messages:
>/tmp/test.c:6: Error: no instruction mnemonic suffix given and no
>  register operands; can't size instruction
>
>gcc version 4.9.x 20150123
>
>Cheers
>
>Matthias

Yes, this its a bug regardless of clang or not. It just happens to be hittin by the particular optimization choice is the current versions of gcc makes.

I asked that that be clear in the commit message, but by the time it was merged it has gotten muddled again.

However, the clang team also need to accept that they can't do arbitrary hacks in the kernel when their compiler is inadequate.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.