Re: [PATCH for-4.15 2/3] firmware: provide a stand alone set of headers

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.03.2021 10:07, Roger Pau Monné wrote:
> On Fri, Feb 26, 2021 at 02:24:43PM +0100, Jan Beulich wrote:
>> On 26.02.2021 09:59, Roger Pau Monne wrote:
>>> The current build of the firmware relies on having 32bit compatible
>>> headers installed in order to build some of the 32bit firmware, but
>>> that usually requires multilib support and installing a i386 libc when
>>> building from an amd64 environment which is cumbersome just to get
>>> some headers.
>>>
>>> Usually this could be solved by using the -ffreestanding compiler
>>> option which drops the usage of the system headers in favor of a
>>> private set of freestanding headers provided by the compiler itself
>>> that are not tied to libc. However such option is broken at least
>>> in the gcc compiler provided in Alpine Linux, as the system include
>>> path (ie: /usr/include) takes precedence over the gcc private include
>>> path:
>>>
>>> #include <...> search starts here:
>>>  /usr/include
>>>  /usr/lib/gcc/x86_64-alpine-linux-musl/10.2.1/include
>>>
>>> Since -ffreestanding is currently broken on at least that distro, and
>>> for resilience against future compilers also having the option broken
>>> provide a set of stand alone 32bit headers required for the firmware
>>> build.
>>>
>>> This allows to drop the build time dependency on having a i386
>>> compatible set of libc headers on amd64.
>>>
>>> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>> with possibly small adjustments:
>>
>>> ---
>>> There's the argument of fixing gcc in Alpine and instead just use
>>> -ffreestanding. I think that's more fragile than providing our own set
>>> of stand alone headers for the firmware bits. Having the include paths
>>> wrongly sorted can easily make the system headers being picked up
>>> instead of the gcc ones, and then building can randomly fail because
>>> the system headers could be amd64 only (like the musl ones).
>>>
>>> I've also seen clang-9 on Debian with the following include paths:
>>>
>>> #include "..." search starts here:
>>> #include <...> search starts here:
>>>  /usr/local/include
>>>  /usr/lib/llvm-9/lib/clang/9.0.1/include
>>>  /usr/include/x86_64-linux-gnu
>>>  /usr/include
>>>
>>> Which also seems slightly dangerous as local comes before the compiler
>>> private path.
>>>
>>> IMO using our own set of stand alone headers is more resilient.
>>
>> I agree (in particular given the observations), but I don't view
>> this as an argument against use of -ffreestanding. In fact I'd
>> rather see this change re-based on top of Andrew's changes. Then ...
> 
> But doesn't using -nostdinc kind of defeats the purpose of
> -ffreestanding, as it would remove all default paths from the include
> search, and thus prevent using the gcc private headers?

I guess I don't understand: It is the purpose of this change here to
not use compiler provided headers (nor libc provided ones), so why
would it matter to retain any kind of built-in include paths?

>>> --- /dev/null
>>> +++ b/tools/firmware/include/stdint.h
>>> @@ -0,0 +1,39 @@
>>> +#ifndef _STDINT_H_
>>> +#define _STDINT_H_
>>> +
>>> +#ifdef __LP64__
>>> +#error "32bit only header"
>>> +#endif
>>
>> Could I talk you into extending this to also cover __P64__? (The
>> alternative I see would be to omit this altogether.)
> 
> Sure. I'm having a hard time finding any documentation for __P64__
> however. Does it stand for pointers are 64 bits, while longs are
> 32bits?

Yeah, it's uncommon in Linux/Unix, but it's the model Windows uses
for 64-bit environments.

Jan