RE: [PATCH v2] x86/asm: fix assembly constraints in bitops

[David Laight]

From: Ingo Molnar
> Sent: 05 April 2019 10:40
> 
> * Alexander Potapenko <glider@xxxxxxxxxx> wrote:
> 
> > 1. Use memory clobber in bitops that touch arbitrary memory
> >
> > Certain bit operations that read/write bits take a base pointer and an
> > arbitrarily large offset to address the bit relative to that base.
> > Inline assembly constraints aren't expressive enough to tell the
> > compiler that the assembly directive is going to touch a specific memory
> > location of unknown size, therefore we have to use the "memory" clobber
> > to indicate that the assembly is going to access memory locations other
> > than those listed in the inputs/outputs.
> > To indicate that BTR/BTS instructions don't necessarily touch the first
> > sizeof(long) bytes of the argument, we also move the address to assembly
> > inputs.
> >
> > This particular change leads to size increase of 124 kernel functions in
> > a defconfig build. For some of them the diff is in NOP operations, other
> > end up re-reading values from memory and may potentially slow down the
> > execution. But without these clobbers the compiler is free to cache
> > the contents of the bitmaps and use them as if they weren't changed by
> > the inline assembly.
> >
> > 2. Use byte-sized arguments for operations touching single bytes.
> >
> > Passing a long value to ANDB/ORB/XORB instructions makes the compiler
> > treat sizeof(long) bytes as being clobbered, which isn't the case. This
> > may theoretically lead to worse code in the case of heavy optimization.
> >
...
> 
> I'm wondering what the primary motivation for the patch is:
> 
>  - Does it fix an actual miscompilation, or only a theoretical miscompilation?
> 
>  - If it fixes an existing miscompilation:
> 
>    - Does it fix a miscompilation triggered by current/future versions of GCC?
>    - Does it fix a miscompilation triggered by current/future versions of Clang?
> 
>  - Also, is the miscompilation triggered by 'usual' kernel configs, or
>    does it require exotics such as weird debug options or GCC plugins,
>    etc?
> 
> I.e. a bit more context would be useful.

The missing memory clobber (change 1) can cause very difficult to debug bugs.
Simple things like gcc deciding to inline a function can change the order
of memory accesses.
Having the wrong just isn't worth the trouble it can cause.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)