Re: [RFC][PATCH 07/22] x86,extable: Extend extable functionality

[Peter Zijlstra <peterz@infradead.org>]

On Fri, Nov 05, 2021 at 07:47:22PM +0000, Sean Christopherson wrote:
> On Fri, Nov 05, 2021, Peter Zijlstra wrote:
> > On Fri, Nov 05, 2021 at 07:17:20PM +0000, Sean Christopherson wrote:

> > C99 is sodding daft wrt signed values. That's why we force -fwrapv and
> > say signed is 2s complement and expect sanity.
> 
> FWIW, -fwrapv was supplanted by -fno-strict-overflow back in 2009 by commit
> a137802ee839 ("Don't use '-fwrapv' compiler option: it's buggy in gcc-4.1.x").
> But I don't think that matters because AFAICT both apply only to "addition,
> subtraction and multiplication".

Right... I think i remember running into that before. I think I yelled
at people at the time for that not being very consistent. If we
explicitly state we want 2s complement, it had damn well be everywhere.

> > > gcc-10 generates a bare "shr", i.e. doesn't special case negative values, and "shr"
> > > is explicitly defined as an unsigned divide.
> > 
> > We hard rely on signed shift right to preserve sign all over the place,
> > how come it goes sideways here? Lemme go stare at asm...
> 
> Huh.  TIL there's actually a use case for this.

Yeah, the canonical pattern for sign extending an n-bit int is:

	(int)(x << (32-n)) >> (32-n)

I think we have macros for that somehwere, but I can never remember what
they're called.

> Anyways, I think the issue is that EX_IMM_MASK is being interpreted as an unsigned
> value by the compiler.  Which I think is legal for a 64-bit kernel?  Because bit 63
> is the MSB, not bit 31.  E.g. this
> 
> 	FIELD_GET((int)EX_IMM_MASK, e->type)
> 
> generates SAR instead of SHR.

Bah, you're right. The compiler is free to interpret a hex constant as
either. At the same time there's only an unsigned suffix, so while we
can force it unsigned, we can't explicitly construct a signed hex
constant.

That's really unfortunate that is... 6.4.4.1 item 5 of the C99 spec
covers this gem :-( I suppose I'll go stick that (int) cast in the
EX_IMM_MASK definition or something.