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

Re: [PATCH 2/2] hvmloader: do not include system headers for type declarations [and 1 more messages]

On 24.02.2021 15:33, Ian Jackson wrote:
> Andrew Cooper writes ("Re: [PATCH 2/2] hvmloader: do not include system 
> headers for type declarations"):
>> At what point do we just declare Alpine broken?  These are all
>> freestanding headers, an explicitly available for use, in the way we use
>> them.
> There is IMO nothing wrong with Alpine here.  Alpine amd64 simply does
> not support compilation of 32-bit x86 userland binaries.
> But that's OK for us.  Xen doe not require the execution of any 32-bit
> userland binaries.  hvmloader is not a userland binary.
> As Roger said on irc
> 13:35 <royger> but requiring a compiler that supports generating
>                i386 code doens't imply that we also have a libc for it?
>> There are substantial portability costs for making changes like this,
>> which takes us from standards compliant C to GCC-isms-only.
> Since we are defining our out standalone environment for hvmloader, we
> are in the position of the C *implementor*.  Compilers have features
> (like __builtin_va*) that are helpful for implementing standard C
> features like stdarg.h and indeed stdint.h.
> Or to put it another way, GCC does not, by itself, provide (in
> Standard C terms) a "freestanding implementation".  Arguably GCC ought
> to provide stdint.h et al but in practice it doing so causes more
> trouble as it gets in the way of the implentors of hosted
> implementations.

But gcc _does_ provide a stdint.h.

> Jan Beulich writes ("Re: [PATCH 2/2] hvmloader: do not include system headers 
> for type declarations"):
>> On 24.02.2021 12:07, Ian Jackson wrote:
>>> This code is only ever going to be for 32-bit x86, so I think the way
>>> Roger did it is fine.
>> It is technically correct at this point in time, from all we can
>> tell. I can't see any reason though why a compiler might not
>> support wider int or, in particular, long long.
> Our requirement for hvmloader is that we have an ILP32 LL64 compiler
> which generates 32-bit x86 machine code.  That is what "gcc -m32"
> means.

I'm not sure about the last statement; I'm pretty sure we don't
check that we have such a compiler (in tools/configure).

>  Whether future compiler(s) might exist which can provide ILP32
> LLP64 (and what type uint64_t is on such a compiler) is not relevant.
>>> Doing it the other way, to cope with this file being used with
>>> compiler settings where the above set of types is wrong, would also
>>> imply more complex definitions of INT32_MIN et al.
>> Well, that's only as far as the use of number suffixes goes. The
>> values used won't change, as these constants describe fixed width
>> types.
> So the definitions would need to contain casts.

Which they can't, as that would make them unusable in preprocessor

>>>> Like the hypervisor, we should prefer using __SIZE_TYPE__
>>>> when available.
>>> I disagree.
>> May I ask why? There is a reason providing of these types did get
>> added to (at least) gcc.
> __SIZE_TYPE__ is provided by the compiler to the libc implementor.  It
> is one of those facilities like __builtin_va*.  The bulk of the code
> in hvmloader should not use this kind of thing.  It should use plain
> size_t.
> As for the new header in hvmloader, it does not matter whether it uses
> __SIZE_TYPE__ or some other type which is known to be 32-bit, since
> this code is definitely only ever for 32-bit x86.

>From a compiler perspective, "32-bit" and "x86" needs further pairing
with an OS, as it's the OS which defines the ABI. This is why further
up I did say "It is technically correct at this point in time, from
all we can tell" - we imply that all OSes we want to be able to build
on provide a suitable ABI, so we can use their compilers.

>> One argument against this would be above mentioned independence
>> on any ABI the compiler would be built for, but I'd buy that only
>> if above we indeed used __attribute__((__mode__())), as that's
>> the only way to achieve such independence.
> We don't want or need to support building hvmloader with a differnet
> ABI.
>>>> Nit: Perhaps better omit the unnecessary inner parentheses?
>>> We should definitely keep the inner parentheses.  I don't want to
>>> start carefully reasoning about precisely which inner parentheses are
>>> necesary for macro argument parsing correctness.
>> Can you give me an example of when the inner parentheses would be
>> needed? I don't think they're needed no matter whether (taking the
>> example here) __builtin_va_...() were functions or macros. They
>> would of course be needed if the identifiers were part of
>> expressions beyond the mere function invocation.
> You mention the situation where the parentheses would be needed
> yourself.

Okay, if that would have been your example, then since there are
no further expressions involved here you agree parentheses aren't
needed here?




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