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

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



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.


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.


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.


Ian.

[1] In particular ISTM that on systems with unsigned pointers,
pointers around the 0x8bazillion boundary cause trouble.

Say, for any reason someone were to use the linker machinery to
produce two pointer constants 0x7f00.... and 0x8100.... .  Then that's
the two signed numbers 7f00.._16 and -7f00.._16 and the difference is
fe00.._16 which is not representable in the same sized signed integer.
But of course what the programmer wanted was to treat that mod 2^n and
get 200_16 ie 0x0200.. and that is what using unsigned arithmetic,
with last-minute conversion to signed, does.

To use signed arithmetic it would be necessary to somehow exclude such
uses.  Maybe they are fanciful because the two linker symbols would
often be in different notional segments but ISTM that this argument
depends on assumptions about the addressing layout and is in general
rather weak.  So a BUILD_BUG_ON would be needed.

All of this seems much more easily avoided by using uintptr_t.

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