All of lore.kernel.org
 help / color / mirror / Atom feed
* Clang and X86-EFlags (was Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning)
@ 2018-04-24 13:28 Sedat Dilek
  2018-04-24 13:46 ` Sedat Dilek
  2018-04-24 15:22 ` Linus Torvalds
  0 siblings, 2 replies; 4+ messages in thread
From: Sedat Dilek @ 2018-04-24 13:28 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Alan Stern, David Laight, Jiri Kosina, Steven Rostedt, Tejun Heo,
	Lai Jiangshan, Benjamin Tissoires, Paul McKenney,
	Andy Lutomirski, LKML, USB list, Greg Kroah-Hartman,
	Peter Zijlstra, Ingo Molnar, Matthias Kaehlcke, Michael Hordijk,
	chandlerc

On Mon, Jun 27, 2016 at 10:14 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Mon, Jun 27, 2016 at 12:50 PM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
>>
>> $ objdump -S clang-eflag.o
>>
>> clang-eflag.o:     file format elf64-x86-64
>>
>>
>> Disassembly of section .text:
>>
>> 0000000000000000 <bar>:
>>    0:   55                      push   %rbp
>>    1:   48 89 e5                mov    %rsp,%rbp
>>    4:   53                      push   %rbx
>>    5:   50                      push   %rax
>>    6:   e8 00 00 00 00          callq  b <bar+0xb>
>>    b:   ff 0d 00 00 00 00       decl   0x0(%rip)        # 11 <bar+0x11>
>>   11:   9c                      pushfq
>>   12:   5b                      pop    %rbx
>>   13:   e8 00 00 00 00          callq  18 <bar+0x18>
>>   18:   b8 01 00 00 00          mov    $0x1,%eax
>>   1d:   53                      push   %rbx
>>   1e:   9d                      popfq
>>   1f:   75 07                   jne    28 <bar+0x28>
>
>
> Yeah, the above is pure garbage.
>
>> So, the issue is still alive.
>>
>> What do you mean by "for the kernel we at a minimum need a way to
>> disable that code generation"?
>> Can this be fixed in the Linux-kernel?
>
> No. This will never be fixed in the kernel. It's a compiler bug.
>
> The compiler generates shit code. It's absolutely atrociously bad even
> if you ignore any kernel issues, because that kind of code just
> performs badly (the compiler should have used "setcc" or something
> similar to just set the comparison value, not save and restore eflags.
>
> And quite frankly, any compiler writer that thinks it is good code is
> not somebody I want touching a compiler that the kernel depends on
> anyway.
>
> But it is not just bad code for the kernel, it's actively buggy code,
> since it corrupts the IF.
>
> Until this gets fixed in LLVM, there's no way in hell that we will
> ever have a kernel compiled with that piece of shit.
>
> Really. If the LLVM developers cannot fix their crap code generation,
> it's not worth touching that shit with a ten-foot pole.
>
> I'd love to be able to compile the kernel with LLVM, but the fact that
> the broken eflags code apparently _still_ hasn't been fixed makes me
> just go "not worth it".
>
> And if the LLVM developers don't see this as an obvious bug, it's even
> less worth it - because that shows not just that the compiler is
> broken, but that the developers involved with it are broken too.
>
>                   Linus

[ Changed Subject ]
[ CC Matthias ]
[ CC Michael test-case ]
[ CC Chandler ]

Hi Linus,

Matthias pointed me in [0] to [1] in the LLVM-BTS.

...and I tried again the test-case from Michael from [3] "[LLVMdev]
optimizer clobber EFLAGS"...

...with clang-7 (version:
7~svn330207-1~exp1+0~20180417201234.1709~1.gbp6fb10d) from
<https://apt.llvm.org/>.

[ TEST-CASE ]

[ clang-eflag.c ]
#include <stdlib.h>
#include <stdbool.h>

void foo(void);
int a;

int bar(void)
{
         foo();

         bool const zero = a -= 1;

         asm volatile ("" : : : "cc");
         foo();

         if (zero) {
                 return EXIT_FAILURE;
         }

         foo();

         return EXIT_SUCCESS;
}
- EOF -

$ clang-7 -O2 -c -o clang-eflag.o clang-eflag.c

[ OBJDUMP ]

$ objdump -S clang-eflag.o

clang-eflag.o:     file format elf64-x86-64


Disassembly of section .text:

0000000000000000 <bar>:
   0:   53                      push   %rbx
   1:   e8 00 00 00 00          callq  6 <bar+0x6>
   6:   83 05 00 00 00 00 ff    addl   $0xffffffff,0x0(%rip)        #
d <bar+0xd>
   d:   0f 95 c3                setne  %bl
  10:   e8 00 00 00 00          callq  15 <bar+0x15>
  15:   b8 01 00 00 00          mov    $0x1,%eax
  1a:   f6 c3 ff                test   $0xff,%bl
  1d:   74 02                   je     21 <bar+0x21>
  1f:   5b                      pop    %rbx
  20:   c3                      retq
  21:   e8 00 00 00 00          callq  26 <bar+0x26>
  26:   31 c0                   xor    %eax,%eax
  28:   5b                      pop    %rbx
  29:   c3                      retq

Does this now look good?

Thanks.

Kind regards,
- Sedat -

[0] https://marc.info/?l=linux-kernel&m=152450535720279&w=2
[1] https://bugs.llvm.org/show_bug.cgi?id=36028
[2] http://lists.llvm.org/pipermail/llvm-dev/2015-July/088766.html
[3] https://marc.info/?l=linux-kernel&m=152457089205170&w=2

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Clang and X86-EFlags (was Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning)
  2018-04-24 13:28 Clang and X86-EFlags (was Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning) Sedat Dilek
@ 2018-04-24 13:46 ` Sedat Dilek
  2018-04-24 15:22 ` Linus Torvalds
  1 sibling, 0 replies; 4+ messages in thread
From: Sedat Dilek @ 2018-04-24 13:46 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Alan Stern, David Laight, Jiri Kosina, Steven Rostedt, Tejun Heo,
	Lai Jiangshan, Benjamin Tissoires, Paul McKenney,
	Andy Lutomirski, LKML, USB list, Greg Kroah-Hartman,
	Peter Zijlstra, Ingo Molnar, Matthias Kaehlcke, Michael Hordijk,
	chandlerc

On Tue, Apr 24, 2018 at 3:28 PM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> On Mon, Jun 27, 2016 at 10:14 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>> On Mon, Jun 27, 2016 at 12:50 PM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
>>>
>>> $ objdump -S clang-eflag.o
>>>
>>> clang-eflag.o:     file format elf64-x86-64
>>>
>>>
>>> Disassembly of section .text:
>>>
>>> 0000000000000000 <bar>:
>>>    0:   55                      push   %rbp
>>>    1:   48 89 e5                mov    %rsp,%rbp
>>>    4:   53                      push   %rbx
>>>    5:   50                      push   %rax
>>>    6:   e8 00 00 00 00          callq  b <bar+0xb>
>>>    b:   ff 0d 00 00 00 00       decl   0x0(%rip)        # 11 <bar+0x11>
>>>   11:   9c                      pushfq
>>>   12:   5b                      pop    %rbx
>>>   13:   e8 00 00 00 00          callq  18 <bar+0x18>
>>>   18:   b8 01 00 00 00          mov    $0x1,%eax
>>>   1d:   53                      push   %rbx
>>>   1e:   9d                      popfq
>>>   1f:   75 07                   jne    28 <bar+0x28>
>>
>>
>> Yeah, the above is pure garbage.
>>
>>> So, the issue is still alive.
>>>
>>> What do you mean by "for the kernel we at a minimum need a way to
>>> disable that code generation"?
>>> Can this be fixed in the Linux-kernel?
>>
>> No. This will never be fixed in the kernel. It's a compiler bug.
>>
>> The compiler generates shit code. It's absolutely atrociously bad even
>> if you ignore any kernel issues, because that kind of code just
>> performs badly (the compiler should have used "setcc" or something
>> similar to just set the comparison value, not save and restore eflags.
>>
>> And quite frankly, any compiler writer that thinks it is good code is
>> not somebody I want touching a compiler that the kernel depends on
>> anyway.
>>
>> But it is not just bad code for the kernel, it's actively buggy code,
>> since it corrupts the IF.
>>
>> Until this gets fixed in LLVM, there's no way in hell that we will
>> ever have a kernel compiled with that piece of shit.
>>
>> Really. If the LLVM developers cannot fix their crap code generation,
>> it's not worth touching that shit with a ten-foot pole.
>>
>> I'd love to be able to compile the kernel with LLVM, but the fact that
>> the broken eflags code apparently _still_ hasn't been fixed makes me
>> just go "not worth it".
>>
>> And if the LLVM developers don't see this as an obvious bug, it's even
>> less worth it - because that shows not just that the compiler is
>> broken, but that the developers involved with it are broken too.
>>
>>                   Linus
>
> [ Changed Subject ]
> [ CC Matthias ]
> [ CC Michael test-case ]
> [ CC Chandler ]
>
> Hi Linus,
>
> Matthias pointed me in [0] to [1] in the LLVM-BTS.
>
> ...and I tried again the test-case from Michael from [3] "[LLVMdev]
> optimizer clobber EFLAGS"...
>
> ...with clang-7 (version:
> 7~svn330207-1~exp1+0~20180417201234.1709~1.gbp6fb10d) from
> <https://apt.llvm.org/>.
>
> [ TEST-CASE ]
>
> [ clang-eflag.c ]
> #include <stdlib.h>
> #include <stdbool.h>
>
> void foo(void);
> int a;
>
> int bar(void)
> {
>          foo();
>
>          bool const zero = a -= 1;
>
>          asm volatile ("" : : : "cc");
>          foo();
>
>          if (zero) {
>                  return EXIT_FAILURE;
>          }
>
>          foo();
>
>          return EXIT_SUCCESS;
> }
> - EOF -
>
> $ clang-7 -O2 -c -o clang-eflag.o clang-eflag.c
>
> [ OBJDUMP ]
>
> $ objdump -S clang-eflag.o
>
> clang-eflag.o:     file format elf64-x86-64
>
>
> Disassembly of section .text:
>
> 0000000000000000 <bar>:
>    0:   53                      push   %rbx
>    1:   e8 00 00 00 00          callq  6 <bar+0x6>
>    6:   83 05 00 00 00 00 ff    addl   $0xffffffff,0x0(%rip)        #
> d <bar+0xd>
>    d:   0f 95 c3                setne  %bl
>   10:   e8 00 00 00 00          callq  15 <bar+0x15>
>   15:   b8 01 00 00 00          mov    $0x1,%eax
>   1a:   f6 c3 ff                test   $0xff,%bl
>   1d:   74 02                   je     21 <bar+0x21>
>   1f:   5b                      pop    %rbx
>   20:   c3                      retq
>   21:   e8 00 00 00 00          callq  26 <bar+0x26>
>   26:   31 c0                   xor    %eax,%eax
>   28:   5b                      pop    %rbx
>   29:   c3                      retq
>
> Does this now look good?
>
> Thanks.
>
> Kind regards,
> - Sedat -
>
> [0] https://marc.info/?l=linux-kernel&m=152450535720279&w=2
> [1] https://bugs.llvm.org/show_bug.cgi?id=36028
> [2] http://lists.llvm.org/pipermail/llvm-dev/2015-July/088766.html
> [3] https://marc.info/?l=linux-kernel&m=152457089205170&w=2

For the sake of completeness the two fixes in LLVM upstream.

>From [4]:

Chandler Carruth 2018-04-09 23:41:58 PDT

This should finally be resolved in its entirety with r329657 (and r329673).

"[x86] Introduce a pass to begin more systematically fixing PR36028
and similar issues"
https://github.com/llvm-mirror/llvm/commit/21a0c18174343502c9f2b546a01333d1c351d9c0
svn -r329657

"[x86] Model the direction flag (DF) separately from the rest of EFLAGS."
https://github.com/llvm-mirror/llvm/commit/9f8f7c5e8ab12afcc92f51d0ed596ac0867eb0fa
svn-r329673

- Sedat -

[4] https://bugs.llvm.org/show_bug.cgi?id=36028#c2

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Clang and X86-EFlags (was Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning)
  2018-04-24 13:28 Clang and X86-EFlags (was Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning) Sedat Dilek
  2018-04-24 13:46 ` Sedat Dilek
@ 2018-04-24 15:22 ` Linus Torvalds
  2018-04-24 15:46   ` Sedat Dilek
  1 sibling, 1 reply; 4+ messages in thread
From: Linus Torvalds @ 2018-04-24 15:22 UTC (permalink / raw)
  To: Sedat Dilek
  Cc: Alan Stern, David Laight, Jiri Kosina, Steven Rostedt, Tejun Heo,
	Lai Jiangshan, Benjamin Tissoires, Paul McKenney,
	Andy Lutomirski, LKML, USB list, Greg Kroah-Hartman,
	Peter Zijlstra, Ingo Molnar, Matthias Kaehlcke, Michael Hordijk,
	Chandler Carruth

On Tue, Apr 24, 2018 at 6:28 AM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> $ objdump -S clang-eflag.o
>
> Does this now look good?

Looks fine to me. The instruction choice is still pretty odd:

  1a:   f6 c3 ff                test   $0xff,%bl
  1d:   74 02                   je     21 <bar+0x21>

that "test   $0xff,%bl" is a rather odd way of testing the byte for zero, since

  1a:   84 db                test   %bl,%bl
  1c:   74 02                   je     20 <bar+0x20>

would have been a byte shorter and is the canonical way on x86 to test
a register against zero.

But that's just a "looks odd to somebody who is used to x86 asm", not
a bug or anything really noticeable.

                  Linus

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Clang and X86-EFlags (was Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning)
  2018-04-24 15:22 ` Linus Torvalds
@ 2018-04-24 15:46   ` Sedat Dilek
  0 siblings, 0 replies; 4+ messages in thread
From: Sedat Dilek @ 2018-04-24 15:46 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Alan Stern, David Laight, Jiri Kosina, Steven Rostedt, Tejun Heo,
	Lai Jiangshan, Benjamin Tissoires, Paul McKenney,
	Andy Lutomirski, LKML, USB list, Greg Kroah-Hartman,
	Peter Zijlstra, Ingo Molnar, Matthias Kaehlcke, Michael Hordijk,
	Chandler Carruth

On Tue, Apr 24, 2018 at 5:22 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Tue, Apr 24, 2018 at 6:28 AM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
>>
>> $ objdump -S clang-eflag.o
>>
>> Does this now look good?
>
> Looks fine to me. The instruction choice is still pretty odd:
>
>   1a:   f6 c3 ff                test   $0xff,%bl
>   1d:   74 02                   je     21 <bar+0x21>
>
> that "test   $0xff,%bl" is a rather odd way of testing the byte for zero, since
>
>   1a:   84 db                test   %bl,%bl
>   1c:   74 02                   je     20 <bar+0x20>
>
> would have been a byte shorter and is the canonical way on x86 to test
> a register against zero.
>
> But that's just a "looks odd to somebody who is used to x86 asm", not
> a bug or anything really noticeable.
>
>                   Linus

Thanks for the quick reply and the confirmation.

Personally, I do not speak x86/asm.
I hope CCed LLVM folks know on how to improve things.

- Sedat -

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-04-24 15:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-24 13:28 Clang and X86-EFlags (was Re: [PATCH] usbhid: Fix lockdep unannotated irqs-off warning) Sedat Dilek
2018-04-24 13:46 ` Sedat Dilek
2018-04-24 15:22 ` Linus Torvalds
2018-04-24 15:46   ` Sedat Dilek

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.