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

[Peter Zijlstra]

On Fri, Nov 05, 2021 at 07:17:20PM +0000, Sean Christopherson wrote:
> On Fri, Nov 05, 2021, Peter Zijlstra wrote:
> > On Fri, Nov 05, 2021 at 05:32:14PM +0000, Sean Christopherson wrote:
> > 
> > > > +#define EX_IMM_MASK	0xFFFF0000
> > 
> > > > +	imm  = FIELD_GET(EX_IMM_MASK,  e->type);
> > > 
> > > FIELD_GET casts the result based on the type of the mask, but doesn't explicitly
> > > sign extend the masked field, i.e. there's no intermediate cast to tell the compiler
> > > that the imm is a 16-bit value that should be sign extended.
> > > 
> > > Modifying FIELD_GET to sign extended is probably a bad idea as I'm guessing the
> > > vast, vast majority of use cases don't want that behavior.  I'm not sure how that
> > > would even work with masks that are e.g. 5 bits or so.
> > 
> > So the way I was reading it was that typeof(_mask) is 'int', e->type is
> > also 'int', we mask out the top bits, and since it's all 'int' we do an
> > arith shift right (ie. preserves sign).
> > 
> > Where did that reading go wrong?
> 
> Hmm, C99 standard says that right shift with a negative value is implementation
> specific:

C99 is sodding daft wrt signed values. That's why we force -fwrapv and
say signed is 2s complement and expect sanity.

> 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...