On Fri, 2023-10-20 at 12:03 +0200, Rasmus Villemoes wrote:
> 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.

Sorry, I misunderstood. I thought this was the perf environment that
was doing this so in that case your comment does make much more sense
than mine.

Out of interest did you see what happens if you allow our CFLAGS in and
remove that unset? I also wondered if EXTRA_CFLAGS in EXTRA_OEMAKE
might be an option?

I'm a bit worried that the current situation all feels a little
fragile.

> > 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.

I agree. It was added as a "lets get security flags roughly working and
find out where the issues are" and has unfortunately stuck around and
not been properly fixed.

>  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.

Agreed, however I am interested to see if we can remove some of the
complexity instead as that would be better overall I think if we could
do it.

Cheers,

Richard
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#189514): 
https://lists.openembedded.org/g/openembedded-core/message/189514
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