[U-Boot] [PATCH] tools/Makefile: fix HOSTCFLAGS with CROSS_BUILD_TOOLS

[Jan Kiszka]

On 25.08.19 17:13, Fabrice Fontaine wrote:
> Le dim. 25 août 2019 à 16:11, Jan Kiszka <jan.kiszka at web.de> a écrit :
>>
>> On 25.08.19 15:43, Fabrice Fontaine wrote:
>>> Hello,
>>> Le dim. 25 août 2019 à 13:44, Jan Kiszka <jan.kiszka at web.de> a écrit :
>>>>
>>>> On 01.05.19 15:08, Fabrice Fontaine wrote:
>>>>> When CROSS_BUILD_TOOLS is set, set HOSTCFLAGS to CFLAGS otherwise CC
>>>>> will be used with HOSTCFLAGS which seems wrong
>>>>>
>>>>> Signed-off-by: Fabrice Fontaine <fontaine.fabrice at gmail.com>
>>>>> ---
>>>>>     tools/Makefile | 1 +
>>>>>     1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/tools/Makefile b/tools/Makefile
>>>>> index 12a3027e23..eadeba417d 100644
>>>>> --- a/tools/Makefile
>>>>> +++ b/tools/Makefile
>>>>> @@ -272,6 +272,7 @@ subdir- += env
>>>>>
>>>>>     ifneq ($(CROSS_BUILD_TOOLS),)
>>>>>     override HOSTCC = $(CC)
>>>>> +override HOSTCFLAGS = $(CFLAGS)
>>>>>
>>>>>     quiet_cmd_crosstools_strip = STRIP   $^
>>>>>           cmd_crosstools_strip = $(STRIP) $^; touch $@
>>>>>
>>>>
>>>> This eats - among other things - -O2, normally set in /Makefile. And that breaks
>>>> CROSS_BUILD_TOOLS=y with CONFIG_FIT but without CONFIG_FIT_SIGNATURE because "if
>>>> (!IMAGE_ENABLE_SIGN)" is no longer optimized. I tend to believe this should
>>>> simply be reverted (which is what I'm doing locally in order to fix my builds).
>>> I don't think this patch should be reverted, I sent it to fix a
>>> cross-compilation build issue with host-openssl.
>>>
>>> Indeed, without this patch, with CROSS_BUILD_TOOLS=y, tools/Makefile
>>> sets HOSTCC=$(CC) but it forgets
>>> to override HOSTCFLAGS, though, so the tools are built with target
>>> compiler but host CFLAGS.
>>>
>>> This raises a build failure if host-openssl is built before uboot-tools because
>>> uboot-tools links with openssl headers from host which depends on pthread.h
>>>
>>> More information can be found on buildroot's patchwork here:
>>> - https://patchwork.ozlabs.org/patch/1092227
>>> - https://patchwork.ozlabs.org/patch/1093385
>>>>
>>>> Jan
>>>
>>> Best Regards,
>>>
>>> Fabrice
>>>
>>
>> So what is your suggestion to fix the O2-regression best?
> I'm far from being an expert in uboot so please excuse me if my answer
> contains some mistakes.
> However, IMHO, we should not pass -O2 or any other optimization flags.
>  From my understanding, CFLAGS (and so HOSTCFLAGS when building tools
> for the target through cross-tools) will (or should?) contain -O2 or
> -Os depending on CONFIG_CC_OPTIMIZE_FOR_SIZE value.
> However, I do understand that this is a change in the former behavior
> that was always passing -O2 in HOSTCFLAGS.
> So, if you really want to pass -O2, I would add it to HOSTCFLAGS in
> tools/Makefile (after the override).

This is a hack. U-boot relies on >= -O2 for proper build, so it must inject this
setting on its own - which it did prior to your change. I think top-level
HOSTCFLAGS belongs to the build, irrespective of native vs. cross. You may allow
appending (and, thus, overwriting) settings but you cannot remove mandatory ones.

> However, I have to confess that I don't understand why the build
> breaks "if (!IMAGE_ENABLE_SIGN)" is no longer optimized."

IMAGE_ENABLE_SIGN is 0 when CONFIG_FIT_SIGNATURE is not set. That will remove
all code from tools/image-host.c which is not reachable then. When it's not
removed because of default optimization settings, we get linker errors because
common/image.sig.c is not built without CONFIG_FIT_SIGNATURE. I assume this way
was preferred over #ifdef'ery, but it's broken now.

Jan