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

Thu Nov 14 16:52:36 UTC 2019

On 14.11.19 17:41, Tom Rini wrote:
> On Thu, Nov 14, 2019 at 05:28:06PM +0100, Jan Kiszka wrote:
>> On 26.08.19 08:43, Fabrice Fontaine wrote:
>>> Le lun. 26 août 2019 à 07:57, Jan Kiszka <jan.kiszka at web.de> a écrit :
>>>>
>>>> On 25.08.19 21:11, Fabrice Fontaine wrote:
>>>>> Le dim. 25 août 2019 à 17:49, Jan Kiszka <jan.kiszka at web.de> a écrit :
>>>>>>
>>>>>> 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.
>>>>> Thanks a lot for your explanation.
>>>>> I was able to reproduce the issue and I think that
>>>>> https://github.com/ffontaine/u-boot/commit/61a162e5c0343fc55304902042f8c9adfe531f51
>>>>> should fixed it.
>>>>> Can you review it? I can also directly send a PR.
>>>>
>>>> I still think that dropping the content of HOSTCFLAGS is the actual bug. There
>>>> is more than -O2 that you lose. If you want your CFLAGS to be respected, append
>>>> them.
>>> I add Arnout to the list to get his feedback as he is the one that
>>> suggested this solution during review of
>>> https://patchwork.ozlabs.org/patch/1092227.
>>
>> Nothing happened, bug is still present in master. Any suggestions how to
>> resolve it?
>
> OK, I've looked over the thread again.  Where and how are you hitting a
> problem here again?  This area is indeed a bit funky and I see
> OpenEmbedded has long been doing "shove everything we need in via CC, do
> not use CFLAGS/etc".  Thanks!

This is how you can easily reproduce it:

make avnet_ultra96_rev1_defconfig
make CROSS_COMPILE=aarch64-linux-gnu- CROSS_BUILD_TOOLS=y

Jan