Re: [PATCH v11 1/9] xen: use __UINTPTR_TYPE__ for uintptr_t

[Ian Jackson <ian.jackson@citrix.com>]

Andrew Cooper writes ("Re: [PATCH v11 1/9] xen: use __UINTPTR_TYPE__ for uintptr_t"):
> NACK.
> 
> Stop messing around with GCC-isms and use the spec-compliant way of
> getting uintptr_t (and others).
> 
> #include <stdint.h>

If everything is working correctly, stdint.h is provided by the
compiler (eg by libgcc) and this will DTRT.

However, if things are not working correctly, we will pick up the host
operating system <stdint.h>.  I consider it unacceptable that a build
system issue would be able to cause us to compile a hypervisor with a
busted uintptr_t.  (This is particularly bad given that the hypervisor
currently hardly uses uintptr_t at all, which means such a
miscompilation might even go largely undetected.)

I would tolerate this approach if it were accompanied by an
appropriate set of BUILD_BUG_ON which will detect if this has
occurred, for example by checking that sizeof(void*) ==
sizeof(uintptr_t).


> That way, *any* compiler will give you a working uintptr_t, not just
> those which are emulating GCC's internals.

It has been conventional for many years in embedded programming to,
effectively, briefly put on the hat of the C language implementor and
provide one's own uintptr_t et al.

For this reason all compilers have always provided compiler-defined
types like __UINTPTR_TYPE__.

It is not conceivable that we would succeed in porting Xen to a
compiler which was so strange that it did not provide any internal
type aliases for these standard types.  (Indeed such a compiler
authors would have to do more work to implement <stdint.h>!)  It is
highly unlikely that we would want to try porting Xen to a compiler
which didn't provide these with these now-conventional names, given
that any contemporary compiler author can trivially provide them.

And the (vanishingly unlikely) failure mode is a completely obvious
missing type error.

So __UINTPTR_TYPE__ is strictly superior: it is de jure correct
according to the manual for the compiler that invented the name; it is
currently implemented everywhere we care about (including, right now,
llvm), and will almost certainly remain available, and unlike
<stdint.h> there is absolutely no risk of it meaning anything else.


To put it another way, <stdint.h> is a layer of indirection to
__UINTPTR_TYPE__.  It is provided by libgcc (and other compilers'
equivalents) for the benefit of programs which need to run on much
wider variety of platforms than Xen, and to provide a set of more
convenient names which are not available by default (for compatibility
reasons).

We do need the convenient and standard names, but we can provide them
ourselves.  Relying on an external layer of indirection, however,
exposes us to needless additional risk that the indirection ends up
pointing to the wrong place.


> This isn't rocket science, and I know all of us have better things to be
> doing that wasting time arguing over aspects which are unrelated to
> actually fixing the problem.

I suggest we compromise on <stdint.h> + BUILD_BUG_ON since at least
two of us seem to be able to tolerate that.


Ian.

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