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

Re: [Xen-devel] [PATCH v11 3/9] xen: introduce DECLARE_BOUNDS



>>> Ian Jackson <ian.jackson@xxxxxxxxxx> 03/07/19 3:16 PM >>>
>Jan Beulich writes ("Re: [PATCH v11 3/9] xen: introduce DECLARE_BOUNDS"):
>> On 06.03.19 at 21:55, <sstabellini@xxxxxxxxxx> wrote:
>> > On Wed, 6 Mar 2019, Jan Beulich wrote:
>> > uintptr_t is the integer type corresponding to a pointer, so we should
>> > use uintptr_t first. ptrdiff_t is the difference type so we should cast
>> > to it afterwards. Specifically, uintptr_t is unsigned and ptrdiff_t is
>> > signed. So I don't think it would be correct to do:
>> > 
>> >   return (ptrdiff_t)((ptrdiff_t)s2 - (ptrdiff_t)s1);
>> > 
>> > Or am I missing your point?
>> 
>> Well, I really mean
>> 
>>    return (ptrdiff_t)s2 - (ptrdiff_t)s1;
>> 
>> But that aside - let's consider all three cases:
>> 
>> 1) sizeof(ptrdiff_t) == sizeof(void *)
>> 
>> All fine. And you'll have some difficulty finding a platform where this
>> isn't the case.
>
>No.  This is not fine.  Signed integer subtraction sometimes has UB.
>
>To use signed integer subtraction safely in C it is necessary to prove
>that it cannot overflow.  I doubt that such a proof would be a
>tractable thing to try to write for a general macro like this.  It
>would imply some complicated constraint on the arguments. [1]
>
>OTOH, unsigned integer subtraction is always defined; and the
>subsequent conversion to signed is also always defined.

I've spent an hour trying to find the relevant parts of the spec, but I'm
afraid I've once again failed at finding all necessary pieces. In the
Conversions section there is

"Otherwise, if the new type is unsigned, the value is converted by
 repeatedly adding or subtracting one more than the maximum value
 that can be represented in the new type until the value is in the
 range of the new type."


But I can't take this as applicable to other than conversions.

Whereas one of the general clauses in the Expressions section says

"If an exceptional condition occurs during the evaluation of an expression
 (that is, if the result is not mathematically defined or not in the range of
 representable values for its type), the behavior is undefined."

This does not make any provisions for unsigned types being special.

And there's nothing in the Additive operators section afaics.

As a result, I'd appreciate if you could help me out and point at what
exact sections you're referring to.

As to the unsigned -> signed conversion, according to, again in the
Conversions section,


"Otherwise, the new type is signed and the value cannot be represented
 in it; either the result is implementation-defined or an implementation-
 defined signal is raised."


this is implementation defined according to my understanding (but I take
it that we're fine with this).


>Jan, I appreciate you looking at this stuff in detail.
>
>But it is important tht these kind of review comments are actually
>correct.  This is not the first occasion in the discussion of this
>series where you have advocated a construct which contains a lurking
>possibility of UB, and argued against the corresponding construct
>which has no UB.

I'm sorry for this. I have to admit that (see above) the scattering around of
rules in the C spec isn't too helpful here.


>Conversely, I think the discussion of the sizes of these types is not
>really relevant.  To port Xen it is necessary to have an environment
>where
>sizeof(ptrdiff_t) == sizeof(uintptr_t)
>== sizeof(void*) == sizeof(struct maxalign*)
>and I think there is little harm in further baking in those
>assumptions.

Good (except that I don't understand the struct maxalign* part - neither
why you use a pointer there, nor - assuming there was no pointer - how
that would be in line with __{,u}int128_t or with vector types).


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