Re: [Xen-devel] [PATCH 1/2] fix locking in offline_page()

[Tim Deegan <tim@xxxxxxx>]

At 10:55 +0000 on 27 Nov (1385546118), Jan Beulich wrote:
> >>> On 27.11.13 at 11:48, Tim Deegan <tim@xxxxxxx> wrote:
> > At 10:43 +0000 on 27 Nov (1385545399), Jan Beulich wrote:
> >> >>> On 27.11.13 at 11:34, Tim Deegan <tim@xxxxxxx> wrote:
> >> > At 10:05 +0000 on 27 Nov (1385543154), Jan Beulich wrote:
> >> >> >>> On 27.11.13 at 11:01, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 
> >> >> >>> wrote:
> >> >> > On 27/11/13 08:25, Jan Beulich wrote:
> >> >> >>          else
> >> >> >>          {
> >> >> >>              *status = PG_OFFLINE_OWNED | PG_OFFLINE_PENDING |
> >> >> >> -              (owner->domain_id << PG_OFFLINE_OWNER_SHIFT);
> >> >> >> +                      (owner->domain_id << PG_OFFLINE_OWNER_SHIFT);
> >> >> > 
> >> >> > domain_id will be promoted from a uint16_t to an int32_t, then shifted
> >> >> > left by 16 bits which is undefined if the top of domain_id bit was 
> >> >> > set.
> >> >> 
> >> >> That promotion is done by zero extension, so I don't figure what
> >> >> you think is undefined here.
> >> > 
> >> > If the domid has bit 15 set, it will be shifted into the sign bit of
> >> > the promoted integer; it should be explicitly cast to an unsigned type
> >> > before the shift.
> >> 
> >> Again - I don't see this. Promotion happens before the shift,
> >> i.e. 0x8000 -> 0x00008000 -> 0x80000000.
> > 
> > AIUI the default promotion is to a signed integer if the value will
> > fit, i.e.:
> >              (unsigned short) 0x8000 
> > promoted     (signed int) 0x00008000 
> > shifted left (signed int) 0x80000000 (undefined behaviour)
> 
> Right - but the promotion (as you also show) is done via zero
> extension. Hence, plus because of left shifts being ignorant of
> signed-ness, no need for a cast.

No: left-shifting that set bit into the sign bit of the promoted value
is undefined behaviour.  I still don't have my standard reference to
hand, but e.g. http://blog.regehr.org/archives/738

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel