Messages in this thread | | | From | Andrew Cooper <> | Subject | Re: [PATCH v5] x86: use builtins to read eflags | Date | Thu, 17 Mar 2022 21:05:52 +0000 |
| |
On 17/03/2022 18:52, Linus Torvalds wrote: > On Thu, Mar 17, 2022 at 11:00 AM Nick Desaulniers > <ndesaulniers@google.com> 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@mail.gmail.com/ > > 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
| |