[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v9 1/5] xen: introduce ptrdiff_t, fix uintptr_t



>>> On 12.02.19 at 16:26, <ian.jackson@xxxxxxxxxx> wrote:
> Jan Beulich writes ("Re: [PATCH v9 1/5] xen: introduce ptrdiff_t, fix 
> uintptr_t"):
>> On 12.02.19 at 02:13, <sstabellini@xxxxxxxxxx> wrote:
>> > -typedef unsigned int __attribute__((__mode__(__pointer__))) uintptr_t;
>> > +typedef unsigned long __attribute__((__mode__(__pointer__))) uintptr_t;
>> 
>> I don't understand this change: The mode attribute discards any width
>> association derived from the specified base type.
> 
> I looked for the reference documentation for the semantics of
> __attribute__((__mode__(__pointer__))).  I found this in my GCC 6
> manual:
> 
>   'mode (MODE)'
>        This attribute specifies the data type for the
>        declaration--whichever type corresponds to the mode MODE.  This in
>        effect lets you request an integer or floating-point type according
>        to its width.
> 
>        You may also specify a mode of 'byte' or '__byte__' to indicate the
>        mode corresponding to a one-byte integer, 'word' or '__word__' for
>        the mode of a one-word integer, and 'pointer' or '__pointer__' for
>        the mode used to represent pointers.
> 
> This wording is extremely obscure, especially when read in the context
> of C's insane pointer provenance rules and compiler authors' insane
> interpretations of those rules.
> 
> If our goal is to avoid malicious compiler optimisation nonsense we
> should not use a feature like this whose semantics might be taken to
> imply that the things are really pointers.

I can understand your reservations, albeit I don't share them. But
this is why I've suggested other options.

>> > +typedef long ptrdiff_t;
>> 
>> FTR I'm unconvinced of this, or the need to use uintptr_t in the first
>> place in the macros you introduce: The entire UNIX world implies
>> sizeof(long) == sizeof(void*) afaik.
> 
> Please provide a reference for this assertion about sizeof(long).

Well, my (limited) familiarity with UNIX in general (hence the "afaik")
goes back to the times when the IA64 ABI was formulated, and
when (just like a few years later for x86-64) this was called out as
the main difference to the Windows world. I don't think I can
provide anything that would come close to a "reference", other
than the processor specific ABIs that have emerged from that
discussion.

What's clear though is that both Linux and Xen (having inherited
this from Linux) make this assumption in numerous places (sadly
in Linux this also gets violated every now and then by people
introducing casts between pointers and uint64_t, but once the
warnings get noticed in 32-bit builds things usually get fixed).

>> But if we really want to have ptrdiff_t, then I think we should either
>> follow the uintptr_t model and use attribute((mode())), or we should
>> leverage the compiler's __PTRDIFF_TYPE__ (and then also
>> __UINTPTR_TYPE__ for uintptr_t, at least if available - not sure what
>> its availability depends on, but it's conditional in gcc's
>> c_stddef_cpp_builtins()).
> 
> It is not unusual for porting something like Xen to a new architecture
> to involve writing a short header file with these kind of type
> definitions.  I don't know why we couldn't take that approach.

If we think we need the extra types, I'd be fine going that
route (as much as I would be going the other one; the only
thing I'm not fine with is a mix of approaches). But as with
Stefano's changes, for the moment I'm unconvinced we
actually need uintptr_t or ptrdiff_t here, when - as said - we
already heavily rely on long <-> pointer conversions anyway.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.