The last build tagged `eln` is still grub2-2.04-26.eln103, which fails to rebuild.  I can see newer builds of grub2 but they failed to be tagged properly:

    https://koji.fedoraproject.org/koji/taskinfo?taskID=54070009


Troy, could you please check what happened?  Does grub2 need to be tagged manually?

I see that this was built by bpeck/jenkins-continuous-infra.apps.ci.centos.org, does this user have permissions to build grub2 in one of the signers?

There is another build by Troy Dawson that failed in the same way:

    https://koji.fedoraproject.org/koji/taskinfo?taskID=51541349

Yes, grub2 is one of the handful of packages that have to be tagged manually, and only a small set of people can do it, and I am not part of that set.

Stephen Gallagher is either the person who can tag it in, or knows who it is.

Sorry for the long delay on this; I was trying to see if we would get a grub2 build with GCC 11 completed. This should now be working its way through signing.

Thanks for the update!  Unfortunately, the latest eln build of grub2 still fails to rebuild on Intel:

$ koji latest-build eln grub2
Build                                     Tag                   Built by
----------------------------------------  --------------------  ----------------
grub2-2.04-34.eln108                      eln                   bpeck/jenkins-continuous-infra.apps.ci.centos.org

$ koji buildinfo grub2-2.04-34.eln108 | grep ^Task                                  
Task: 59530293 build (eln, /rpms/grub2.git:f9736ec08517869c0e2d4e3491da8abe09ab1d60)

$ koji build --scratch eln "git+https://src.fedoraproject.org/rpms/grub2.git#f9736ec08517869c0e2d4e3491da8abe09ab1d60"
Created task: 60225867
Task info: https://koji.fedoraproject.org/koji/taskinfo?taskID=60225867
Watching tasks (this may be safely interrupted)...
[...]  0 free  0 open  3 done  3 failed

(In reply to Kamil Dudka from comment #7)
> Thanks for the update!  Unfortunately, the latest eln build of grub2 still
> fails to rebuild on Intel:
> 
> $ koji latest-build eln grub2
> Build                                     Tag                   Built by
> ----------------------------------------  -------------------- 
> ----------------
> grub2-2.04-34.eln108                      eln                  
> bpeck/jenkins-continuous-infra.apps.ci.centos.org
> 
> $ koji buildinfo grub2-2.04-34.eln108 | grep ^Task                          
> 
> Task: 59530293 build (eln,
> /rpms/grub2.git:f9736ec08517869c0e2d4e3491da8abe09ab1d60)
> 
> $ koji build --scratch eln
> "git+https://src.fedoraproject.org/rpms/grub2.
> git#f9736ec08517869c0e2d4e3491da8abe09ab1d60"
> Created task: 60225867
> Task info: https://koji.fedoraproject.org/koji/taskinfo?taskID=60225867
> Watching tasks (this may be safely interrupted)...
> [...]  0 free  0 open  3 done  3 failed

"configure:32014: error: no suitable link format found"

This looks likely to be a toolchain bug.

This looks like someone added -march=x86-64-v2 to %optflags, and then -fcf-protection got confused because it effectively got "-march=x86-64-v2 -mtune=generic -fstack-clash-protection -fcf-protection -march=i386 -m32" when we try to build i386 binaries.

Adding -march=x86-64-v2 unconditionally to optflags is very much not okay.  If we need to change the arch from -march=x86-64 to -march=x86-64-v2, it probably needs to be conditionalized like -fPIE is, and adding that in something similar to "-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1".

I don't know where you're changing optflags, so I'm assigning this back to gcc, as it doesn't appear to be a grub2 bug at all.

This has nothing to do with -march=x86-64-v2.
All that matters is -fcf-protection -march=i386 -m32, that is the combination of flags that are rejected.
-fcf-protection isn in %{optflags} intentionally set, because we do want the distro protected from attacks on CET enabled hardware.
So, grub (and maybe 1-2 other packages, I doubt it affects more than that) when it wants to compile something with -march=i386 needs to remove -fcf-protection, or add -fcf-protection=none or similar.

Okay, but that seems to be ignoring two things.  One is that adding -march=x86-64-v2 this way means the command line looks ambiguous to me, and now I'm forced to wildly guess what the compiler is actually going to do and what's it's saying is unsupported.  This is clearly not great, because it's exactly what you're saying I've misunderstood.  I'm not going to be the only one misunderstanding it.  Additionally, it depends on me getting lucky - if I'd stuck our arguments on the other side of %optflags, presumably I'd just get the wrong arch in my builds, but it would work fine?  This isn't good.

The other thing is that it doesn't change my original point.  We have a mechanism for making these things default but automatically disabled when conflicting flags are provided, why can't we use it?  Just putting stuff that only works sometimes in optflags seems like it's really abusing that interface.

I can sed things out of optflags if I need to - sure we do it for features that are default but we just don't want.  But this seems like it's really doing it wrong.

For -march=, we've been adding -march= in %{optflags} for decade or two, there is nothing new about it.
gcc command line behavior is always that last occurrence of an option (or one of a set of mutually exclusive options) wins, so it always have been the case that overriding those needs to be done after %{optflags}, not before.
And, the additional -specs= hacks enlarge command lines and slow down the argument processing, so it is something that we do when we really have to, when it affects hundreds of packages.
In this case it would affect a single one.
Plus it is unclear why is grub using -march=i386 at all, neither Fedora nor RHEL supports any pre-i686 CPUs, so can't you use -march=i686 instead?  I'd understand wanting -mno-mmx -mno-sse if the bootloader code doesn't have
the vector unit initialized at some point, but don't really see why say conditional moves or atomic instructions couldn't be used in the bootloader code.

(In reply to Jakub Jelinek from comment #12)

> For -march=, we've been adding -march= in %{optflags} for decade or two,
> there is nothing new about it.

Right - the ambiguity isn't an issue until you have to dig in to why gcc is complaining about its invocation, and once you do, you can't tell which target it's complaining about, because it doesn't tell you.  It just says "cc1: error: '-fcf-protection' is not compatible with this target".

Even though it would still be hard to read the command line and get a sense of what it's doing, making the error message say "with target i386..." would resolve that part well enough - it would allow me to know which thing I need to bodge away with sed without having to go through a series of guess-and-test builds.

> gcc command line behavior is always that last occurrence of an option (or
> one of a set of mutually exclusive options) wins, so it always have been the
> case that overriding those needs to be done after %{optflags}, not before.

When I look at the "invoking GCC" / "GCC Command Options" section in the info page, it doesn't obviously say that *anywhere*.  In fact, the most relevant paragraph I can find seems to deny it:

   You can mix options and other arguments.  For the most part, the order
  you use doesn't matter.  Order does matter when you use several options
  of the same kind; for example, if you specify '-L' more than once, the
  directories are searched in the order specified.  Also, the placement of
  the '-l' option is significant.

Again, my point here is that it's unclear how to interpret the error message because of this, but that's a separate from the original point about actually enabling the option.

> And, the additional -specs= hacks enlarge command lines and slow down the
> argument processing, so it is something that we do when we really have to,
> when it affects hundreds of packages.

That seems like a condemnation of how this invocation works.  It's weird to see it as an argument in favor of unconditionally enlarging the command line.

> In this case it would affect a single one.

Well, I think it'll be more like 5-10 when it's all said and done, but I get your point.  Again, we can swizzle this out with sed if we have to, but it seems like it should be part of _hardened_cflags at the very least, even if it's not using -specs=... .

> Plus it is unclear why is grub using -march=i386 at all, neither Fedora nor
> RHEL supports any pre-i686 CPUs, so can't you use -march=i686 instead?

I'm not 100% sure we actually have a preference between -march=i386 and -march=i686, but we've always built the BIOS code for i386; I don't know whether or not it will actually work correctly with -march=i686.  It seems likely that it would, but there's never been any reason to bother changing it.

> I'd understand wanting -mno-mmx -mno-sse if the bootloader code doesn't have
> the vector unit initialized at some point, but don't really see why say
> conditional moves or atomic instructions couldn't be used in the bootloader
> code.

Yeah, they will probably work well enough, we've just never had a reason to change it from what was supported years ago.  It's one piece of code that runs once, loads another piece of code, and gets thrown away, so e.g. cmov optimizations have never really mattered at all.  Changing it has just always been extra work (though *possibly* very little extra work) for no clear benefit.

I'll note that the documentation for -fcf-protection doesn't say *anything at all* about which x86 targets support CET.  Does -march=i686 work with it?  It would seem surprising if it did, but I certainly don't know.  I also have no idea if special setup is required for it to work, either.  But I think all of this is essentially unrelated to what we're currently discussing.

Elsewhere, Carlos has asked me to list concrete requests for what difference I'd like to see, and keep them up to date based on our various discussions, so I'll do that here.  I think our concerns would be minimally addressed by the following:

1) the -fcf-protection error message specified which target it was incompatible with (this is really true of any errors of this form...)
2) -fcf-protection were part of _hardened_cflags (whether or not that means being inside -specs=...)

I can't see how either of these would add *any* complexity on the order of even parsing a more complex command line.  I still don't think adding things to the command line by default which break if you build with a different supported target and don't strip them out is a very good implementation practice, but if doing better than that is unfeasible, the two changes above would still be a significant improvement.

CET works fine on i686 and newer CPUs, it is just the older ones (i[345]86) that didn't handle that.
The error has been added because somebody complained that his i486-DX2 stopped working.

So you make reasonable points Peter, but what matters right now is we need to get grub2 building again.  I see two viable paths forward given how grub2 is built.

First note that the redhat-rpm-config macros do not add -fcf-protection for the i386 architecture.  -fcf-protection is only added for i686 and x86_64 and that's not likely to change.

So that means that grub2 needs to change.

You might think that just changing target_cflags or legacy_target_cflags would be sufficient, but AFAICT it isn't as the grub2 build isn't fully set up to isolate the host from target environment -- host_cflags still gets used during configuration of the legacy and alternate targets.

So this is what works for me.  As the package maintainer you'll have to decide if its OK or if you want to fix the lack of host vs target separation fully and then twiddle target_cflags or legay_target_cflags.

diff --git a/grub.macros b/grub.macros
index 775787d..922b03e 100644
--- a/grub.macros
+++ b/grub.macros
@@ -24,6 +24,7 @@
                -e 's/--param=ssp-buffer-size=4//g'             \\\
                -e 's/-mregparm=3/-mregparm=4/g'                \\\
                -e 's/-fexceptions//g'                          \\\
+               -e 's/-fcf-protection//g'                               \\\
                -e 's/-fasynchronous-unwind-tables//g'          \\\
                -e 's/^/ -fno-strict-aliasing /'                \\\
                %{nil}

Created attachment 1755273 [details]
A patch to make -fcf-protection only show up if you've got _hardened_build set

(In reply to Jeff Law from comment #15)
> So you make reasonable points Peter, but what matters right now is we need
> to get grub2 building again.  I see two viable paths forward given how grub2
> is built.

We can easily get grub2 building again; the workaround is straightforward (as you note below).  But it *is* a workaround, and we shouldn't need to do it.  I think what's important right now is we learn to stop doing these things in a way that's so likely to have collateral damage.  Especially because *we've already got a model for how*.
 
> First note that the redhat-rpm-config macros do not add -fcf-protection for
> the i386 architecture.  -fcf-protection is only added for i686 and x86_64
> and that's not likely to change.

Right, but it evaluates this at the package builder level instead of at the compiler target level, and I can't make this an i686 build because of issues with the interaction of our composes and the way they do multiarch - part because we don't compose for ix86 arches, and also part because I've got two different non-64-bit targets on x86, and they both have 64-bit userland tools in the build.  It's complicated, and there are a lot of weird interaction between composes and errata building.

That aside, I still think this would be much cleaner and more reliable to do in redhat-hardened-cc1, so it can be tailored to the target arch for the compiler, because that's where we have the information.  But even without that, since Jakub has objected for performance reasons, as a compromise, consider something like the attached patch.

With this patch it would only be included when we're building with _hardened_build defined, and when we build stuff without that, we wouldn't get it.  That would at least put us back in a scenario where I can disable the automatic inclusion in a well understood way, and when I want those flags back (i.e. for userland) I can use %{_hardening_cflags} and get the obviously correct behavior.
 
> So that means that grub2 needs to change.

I agree that we can work around this in grub2, and it may well be expedient to do so.  But I also think yall have done this in a way that we should improve upon.

> You might think that just changing target_cflags or legacy_target_cflags
> would be sufficient, but AFAICT it isn't as the grub2 build isn't fully set
> up to isolate the host from target environment -- host_cflags still gets
> used during configuration of the legacy and alternate targets.

It is, /partly/ but that was a relatively new development in grub's configure scripts, so we might not have plumbed it out everywhere we should in the package just yet.

> So this is what works for me.  As the package maintainer you'll have to
> decide if its OK or if you want to fix the lack of host vs target separation
> fully and then twiddle target_cflags or legay_target_cflags.
> 
> diff --git a/grub.macros b/grub.macros
> index 775787d..922b03e 100644
> --- a/grub.macros
> +++ b/grub.macros
> @@ -24,6 +24,7 @@
>                 -e 's/--param=ssp-buffer-size=4//g'             \\\
>                 -e 's/-mregparm=3/-mregparm=4/g'                \\\
>                 -e 's/-fexceptions//g'                          \\\
> +               -e 's/-fcf-protection//g'                               \\\
>                 -e 's/-fasynchronous-unwind-tables//g'          \\\
>                 -e 's/^/ -fno-strict-aliasing /'                \\\
>                 %{nil}

As I said above in https://bugzilla.redhat.com/show_bug.cgi?id=1881331#c13 :

> Again, we can swizzle this out with sed if we have to, but it
> seems like it should be part of _hardened_cflags at the very least, even if
> it's not using -specs=... .

And I stick by that.  Anyway, please see the attached patch.

Note: I suspect my patch has a minor error, inasmuch as there's no need to make the change to %_hardening_ldflags as well, only to %_hardening_cflags .

*** Bug 1915452 has been marked as a duplicate of this bug. ***

According to the F34 schedule [0], tomorrow is the "Change Checkpoint: Completion deadline (testable)"
and so it would be good to have a resolution and push a fix today to get the package building again.

I agree with Peter that -fcf-protection is more suitable to be part of the hardening flags but we can
merge Jeff's proposed workaround while we have this discussion. What do you think?

[0]: https://fedorapeople.org/groups/schedule/f-34/f-34-all-milestones-tasks.html

I'd much rather apply the trivial patch to grub2 for F34.  I think we can discuss if we want to move the cf-protection stuff into hardened_cflags for F35.  Jakub has some concerns in that space that I think we need to still hash out.

I've pushed Jeff's patch [0] and did a grub2-2.04-35.fc34 [1] build.

I'm changing this bug summary and switching the component to gcc
again to keep the discussion about the possibility to move the flag
to %_hardening_cflags instead.

Hope that's Ok for everyone.

[0]: https://src.fedoraproject.org/rpms/grub2/c/3a8f1e293b78
[1]: https://koji.fedoraproject.org/koji/taskinfo?taskID=61631449

Thanks Javier.

This bug appears to have been reported against 'rawhide' during the Fedora 34 development cycle.
Changing version to 34.

(In reply to Jeff Law from comment #20)
> I'd much rather apply the trivial patch to grub2 for F34.  I think we can
> discuss if we want to move the cf-protection stuff into hardened_cflags for
> F35.  Jakub has some concerns in that space that I think we need to still
> hash out.

I'd like to be sure those get written down here, so I'm adding a NEEDINFO for Jakub.

Dear Maintainer,

your package has an open Fails To Build From Source bug for Fedora 34.
Action is required from you.

If you can fix your package to build, perform a build in koji, and either create
an update in bodhi, or close this bug without creating an update, if updating is
not appropriate [1]. If you are working on a fix, set the status to ASSIGNED to
acknowledge this. If you have already fixed this issue, please close this Bugzilla report.

Following the policy for such packages [2], your package will be orphaned if
this bug remains in NEW state more than 8 weeks (not sooner than 2020-11-17).

A week before the mass branching of Fedora 35 according to the schedule [3],
any packages not successfully rebuilt at least on Fedora 33 will be
retired regardless of the status of this bug.

[1] https://docs.fedoraproject.org/en-US/fesco/Updates_Policy/
[2] https://docs.fedoraproject.org/en-US/fesco/Fails_to_build_from_source_Fails_to_install/
[3] https://fedorapeople.org/groups/schedule/f-35/f-35-key-tasks.html

Dear Maintainer,

your package has an open Fails To Build From Source bug for Fedora 34.
Action is required from you.

If you can fix your package to build, perform a build in koji, and either create
an update in bodhi, or close this bug without creating an update, if updating is
not appropriate [1]. If you are working on a fix, set the status to ASSIGNED to
acknowledge this. If you have already fixed this issue, please close this Bugzilla report.

Following the policy for such packages [2], your package will be orphaned if
this bug remains in NEW state more than 8 weeks (not sooner than 2020-11-17).

A week before the mass branching of Fedora 35 according to the schedule [3],
any packages not successfully rebuilt at least on Fedora 33 will be
retired regardless of the status of this bug.

[1] https://docs.fedoraproject.org/en-US/fesco/Updates_Policy/
[2] https://docs.fedoraproject.org/en-US/fesco/Fails_to_build_from_source_Fails_to_install/
[3] https://fedorapeople.org/groups/schedule/f-35/f-35-key-tasks.html

Dear Maintainer,

your package has an open Fails To Build From Source bug for Fedora 34.
Action is required from you.

If you can fix your package to build, perform a build in koji, and either create
an update in bodhi, or close this bug without creating an update, if updating is
not appropriate [1]. If you are working on a fix, set the status to ASSIGNED to
acknowledge this. If you have already fixed this issue, please close this Bugzilla report.

Following the policy for such packages [2], your package will be orphaned if
this bug remains in NEW state more than 8 weeks (not sooner than 2020-11-17).

A week before the mass branching of Fedora 35 according to the schedule [3],
any packages not successfully rebuilt at least on Fedora 33 will be
retired regardless of the status of this bug.

[1] https://docs.fedoraproject.org/en-US/fesco/Updates_Policy/
[2] https://docs.fedoraproject.org/en-US/fesco/Fails_to_build_from_source_Fails_to_install/
[3] https://fedorapeople.org/groups/schedule/f-35/f-35-key-tasks.html

Changing this to Rawhide since the F34 package already builds for some time
with the mentioned workaround from Comment 15.

Dear Maintainer,

your package has an open Fails To Build From Source bug for Fedora 34.
Action is required from you.

If you can fix your package to build, perform a build in koji, and either create
an update in bodhi, or close this bug without creating an update, if updating is
not appropriate [1]. If you are working on a fix, set the status to ASSIGNED to
acknowledge this. If you have already fixed this issue, please close this Bugzilla report.

Following the policy for such packages [2], your package will be orphaned if
this bug remains in NEW state more than 8 weeks (not sooner than 2020-11-17).

A week before the mass branching of Fedora 35 according to the schedule [3],
any packages not successfully rebuilt at least on Fedora 33 will be
retired regardless of the status of this bug.

[1] https://docs.fedoraproject.org/en-US/fesco/Updates_Policy/
[2] https://docs.fedoraproject.org/en-US/fesco/Fails_to_build_from_source_Fails_to_install/
[3] https://fedorapeople.org/groups/schedule/f-35/f-35-key-tasks.html

Dear Maintainer,

your package has an open Fails To Build From Source bug for Fedora 34.
Action is required from you.

If you can fix your package to build, perform a build in koji, and either create
an update in bodhi, or close this bug without creating an update, if updating is
not appropriate [1]. If you are working on a fix, set the status to ASSIGNED to
acknowledge this. If you have already fixed this issue, please close this Bugzilla report.

Following the policy for such packages [2], your package will be orphaned if
this bug remains in NEW state more than 8 weeks (not sooner than 2020-11-17).

A week before the mass branching of Fedora 35 according to the schedule [3],
any packages not successfully rebuilt at least on Fedora 33 will be
retired regardless of the status of this bug.

[1] https://docs.fedoraproject.org/en-US/fesco/Updates_Policy/
[2] https://docs.fedoraproject.org/en-US/fesco/Fails_to_build_from_source_Fails_to_install/
[3] https://fedorapeople.org/groups/schedule/f-35/f-35-key-tasks.html

This package has changed maintainer in Fedora. Reassigning to the new maintainer of this component.

Dear Maintainer,

your package has an open Fails To Build From Source bug for Fedora 34.
Action is required from you.

If you can fix your package to build, perform a build in koji, and either create
an update in bodhi, or close this bug without creating an update, if updating is
not appropriate [1]. If you are working on a fix, set the status to ASSIGNED to
acknowledge this. If you have already fixed this issue, please close this Bugzilla report.

Following the policy for such packages [2], your package will be orphaned if
this bug remains in NEW state more than 8 weeks (not sooner than 2020-11-17).

A week before the mass branching of Fedora 35 according to the schedule [3],
any packages not successfully rebuilt at least on Fedora 33 will be
retired regardless of the status of this bug.

[1] https://docs.fedoraproject.org/en-US/fesco/Updates_Policy/
[2] https://docs.fedoraproject.org/en-US/fesco/Fails_to_build_from_source_Fails_to_install/
[3] https://fedorapeople.org/groups/schedule/f-35/f-35-key-tasks.html

Dear Maintainer,

your package has an open Fails To Build From Source bug for Fedora 34.
Action is required from you.

If you can fix your package to build, perform a build in koji, and either create
an update in bodhi, or close this bug without creating an update, if updating is
not appropriate [1]. If you are working on a fix, set the status to ASSIGNED to
acknowledge this. If you have already fixed this issue, please close this Bugzilla report.

Following the policy for such packages [2], your package will be orphaned if
this bug remains in NEW state more than 8 weeks (not sooner than 2020-11-17).

A week before the mass branching of Fedora 35 according to the schedule [3],
any packages not successfully rebuilt at least on Fedora 33 will be
retired regardless of the status of this bug.

[1] https://docs.fedoraproject.org/en-US/fesco/Updates_Policy/
[2] https://docs.fedoraproject.org/en-US/fesco/Fails_to_build_from_source_Fails_to_install/
[3] https://fedorapeople.org/groups/schedule/f-35/f-35-key-tasks.html

We used the workaround mentioned in Comment 15 but in theory the root cause should be fixed in gcc.
Now this BZ was assigned to grub2 again, so I'll just close it if they don't plan to do anything
about it.