Re: [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw

[Ian Jackson <Ian.Jackson@eu.citrix.com>]

Julien Grall writes ("Re: [PATCH 4/4] xen/public: arm: rework the macro set_xen_guest_handle_raw"):
> On 03/11/15 14:18, Stefano Stabellini wrote:
> >> +#define set_xen_guest_handle_raw(hnd, val)                              \
> >> +    do {                                                                \
> >> +        /* Check if the handle is 64-bit (i.e 8-byte) */                \
> >> +        (void) sizeof(struct { int : -!!(sizeof (hnd) != 8); });        \
> >> +        /* Check if the type of val is compatible with the handle */    \
> >> +        (void) sizeof((val) != (hnd).p);                                \
> >> +        (hnd).q = (uint64_t)(uintptr_t)(val);                           \
> >>      } while ( 0 )
> >>  #define set_xen_guest_handle(hnd, val) set_xen_guest_handle_raw(hnd, val)

I hate to throw yet more variables into this, but I had a discussion
with some C experts in the pub about this problem.  Further thought
(including a red herring where someone suggested that memcpy would
"launder" the types) results in this proposal:


union xen_guest_handle_TYPE {
  TYPE *p;
  uint64_t align;
};

struct xen_guest_handle__pointer {
  uint8_t p0,p1,p2,p3;
};

/*
 * void set_xen_guest_handle_raw(xen_guest_handle_TYPE *hnd, TYPE *val);
 */
#define set_xen_guest_handle_raw(hnd, val)
  do {
     void *copy = (val);
     struct xen_guest_handle__pointer *src = &(hnd);
     struct xen_guest_handle__pointer *dst = &(xen_xgh_copy);
     dst->p0 = src->p0;
     dst->p1 = src->p1;
     dst->p2 = src->p2;
     dst->p3 = src->p3;
     dst[1]->p0 = 0;
     dst[1]->p1 = 0;
     dst[1]->p2 = 0;
     dst[1]->p3 = 0;
     sizeof((hnd).p == (val)); /* typecheck */
  }

#define get_xen_guest_handle(hnd) ((hnd).p)


The above is legal for the following reasons (C99 6.5):

Considering the accesses to src and dst.  These are legal according to
(7) point 5.  (Accesses via a character type are always legal.)

After the above macro has been used to set, what is the effective type
of hnd for a subsequent get_xen_guest_handle ?

If hnd is an object with a declared type (ie, an actual variable,
function parameter, or whatever, rather than from malloc), then the
effective type is that of hnd, so the effective type is the union, and
the effective type of hnd.p is TYPE*.

If hnd is not a declared object, then the effective type might be set
by one of the stores in (6): store through an non-character lvalue;
copy via memcpy or memmove; copy as `an array of character type'.

But we store only through character lvalues.  We do not store with
memcpy or memmove.  We do not copy as an array, but as a struct.

So none of the rules defining the effective type apply.  We are left
with the fallback: the effective type of the subsequent read is the
type used for access.

So a subsequent get's read of hnd.p is legal.


With luck the compiler's optimiser will eliminate the temporaries and
aggregate the byte-wide stores.


In the actual macro all of the struct and union fields and all of the
temporary variables should be prefixed with `xen_xgh_' or some such,
in case the calling code has (arguably foolishly) #defined src or dst
or p or something.

Ian.