On 20/10/2023 11.38, Richard Purdie wrote:
> On Fri, 2023-10-20 at 10:10 +0200, Rasmus Villemoes wrote:
>> On 19/10/2023 14.48, Richard Purdie wrote:

>>> The fact this works suggests perf is ignoring TARGET_CFLAGS. Is there
>>> anything in the perf build system where we should be passing in cflags
>>> we really want used?
>>
>> Well, IIUC, the normal way TARGET_CFLAGS would take effect is via the
>> 'export CFLAGS = "${TARGET_CFLAGS}'. But in the perf recipe, both
>> do_compile and do_install start with
>>
>>         # Linux kernel build system is expected to do the right thing
>>         unset CFLAGS
>>
>> And perf's build system does indeed add lots of flags of its own, at
>> least for most TUs, but nowhere is any -f(macro/debug/file)-prefix being
>> set.
>>
>> So I do think that TARGET_CC_ARCH is the best place for flags that we
>> really want used. At least as an initial step, because this is known to
>> work when using the security_flags.inc file. Maybe there's some cleaner
>> way where we don't unset CFLAGS, but that could be a giant can of worms.
>>
>> And yes, a comment should be added. Is something like
>>
>> # Perf's build system adds its own optimization flags for most TUs,
>> # overriding the flags included here. But for some, perf does not add
>> # any -O option, so ensure the distro's chosen optimization gets used
>> # for those. Since ${SELECTED_OPTIMIZATION} always includes
>> # ${DEBUG_FLAGS} which in turn includes ${DEBUG_PREFIX_MAP}, this also
>> # ensures perf is built with appropriate -f*-prefix-map options,
>> # avoiding the 'buildpaths' QA warning.
>>
>> too verbose?
> 
> If it were me I'd probably write:
> 
> # The perf makefile has "unset CFLAGS" as "Linux kernel build system is
> # expected to do the right thing". 

It's not the perf makefile, it's right there in our recipe itself. I
don't think upstream perf is to blame.

> It doesn't and we need our prefix 
> # map options 

Yes.

> and security flags amongst other things so force our 
> # cflags in.

This has nothing to do with the security flags. That file still contains

TARGET_CC_ARCH:append:class-target = " ${SECURITY_CFLAGS}"

and TARGET_CC_ARCH is included in the CC value we pass to perf's make,
with or without this patch. So if the distro includes
security_flags.inc, perf gets built with those flags as expected.

But when that file is _not_ included, we do miss the prefix-map flags,
which should be passed regardless of any security distro settings. I
think the line I'm moving out of security_flags.inc have never really
had any business being there in the first place, regardless of whether
adding ${SELECTED_OPTIMIZATION} to TARGET_CC_ARCH is the most
appropriate method to get those prefix-map options passed. But since
that's how the poky distro has achieved that until now, it seems to be
the least invasive and least risky fix for distros that don't use
security_flags.inc.

Rasmus

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#189513): 
https://lists.openembedded.org/g/openembedded-core/message/189513
Mute This Topic: https://lists.openembedded.org/mt/102058904/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to