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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.03.2021 10:50, Roger Pau Monné wrote:
> On Mon, Mar 01, 2021 at 10:17:32AM +0100, Jan Beulich wrote:
>> 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?
> 
> Sorry, I'm also confused.
> 
> It's my understanding that the point of using -ffreestanding is that
> the compiler will set __STDC_HOSTED__ == 0, and then the built in
> compiler headers will be used to provide a freestanding environment
> instead of the libc ones.
> 
> However if -nostdinc is used the header search path becomes:
> 
> #include <...> search starts here:
> End of search list.
> 
> At which point setting __STDC_HOSTED__ == 0 is pointless as the built
> in compiler headers are not used, and hence the compiler will always
> resort to the stand alone environment provided in this patch.
> 
> -ffreestanding also allows the program to have a non-standard main,
> but I don't think we care much about that since we already use a custom
> linker script.

While indeed we don't care about this specific last aspect, we
do e.g. care about the implied -fno-builtin (which currently we
specify explicitly, yes). So while with -nostdinc added we
_might_ indeed be fine already, I think we're better off going
the full step and specify what we mean, even if - right now -
we're unaware of further effects which are relevant to us. (For
example, I don't see why in principle we couldn't ourselves
grow a use of __STDC_HOSTED__ somewhere.)

Jan