Re: [PATCH v5] x86: use builtins to read eflags

[Andrew Cooper]

On 17/03/2022 18:52, Linus Torvalds wrote:
> On Thu, Mar 17, 2022 at 11:00 AM Nick Desaulniers
> <ndesaulniers@xxxxxxxxxx> wrote:
>>>> One change you may see due to this patch is the compiler moving the
>>>> "pop %..." instruction away from the "pushf" instruction. This could
>>>> happen if the compiler determines that it could produce better code by
>>>> doing so---e.g. to reduce register pressure. The "gcc -O2" example
>>>> below shows this code movement.
> Honestly, that part worries me a _lot_.
>
> Why?
>
> Because clang in particular has already messed up eflags handling
> once, by spilling condition codes (in the form of eflags) onto the
> stack, and then loading them later with a "popf".
>
> And it did so across a function call THAT MODIFIED 'IF'. This was a
> major bug in clang back in 2015 or so, and made it unusable for the
> kernel.
>
> See for example
>
>     https://lore.kernel.org/all/CA+icZUU7y5ATSLV_0TGzi5m5deWADLmAMBkAT32FKGyUWNSJSA@xxxxxxxxxxxxxx/
>
> for some kernel discussion, and separately
>
>     https://lists.llvm.org/pipermail/llvm-dev/2015-July/088774.html
>
> for just llvm discussions.
>
> It is perhaps telling that the LLVM discussion I found seems to talk
> more about the performance impact, not about the fact that THE
> GENERATED CODE WAS WRONG.
>
> That compiler bug would basically enable or disable interrupts in
> random places - because clang developers thought that 'eflags' is only
> about the condition codes.

I was idly playing with this builtin in response to the thread, and
found that GCC has a far more serious bug than Clang had.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104971

If the optimiser concluded that the result of the readflags builtin is
unused, it generates:

broken:
    pushf
    ret

~Andrew