#37 AARCH64 (and possibly other archs) generation is broken

[Pete Batard]

It appears that commit f7b2e1ff6d6b8db119a9d7ec8406eb491004a16d (Merge /u/gmbr3/gnu-efi/ branch master into master) completely broke AARCH64 EFI binary generation.

If you look at the generated binaries past this commit, they are missing the MZ signature and other PE fields, and trying to run them, both in QEMU and on real ARM64 hardware (such as Microsoft's ARM64 2023 Dev Kit) will fail.

You can test this with a simple "Hello World" project such as https://github.com/pbatard/uefi-simple which I just did on an up to date Debian 11.7 system with the latest gcc toolchains. Prior to f7b2e1ff, issuing make ARCH=aa64 CROSS_COMPILE=aarch64-linux-gnu- qemu from this project would build a working ARM64 binary, that would then run in QEMU and display information about the ARM64 system. But after f7b2e1ff, even trying to run the gnu-efi based executable from UEFI Shell results in Command Error Status: Unsupported, meaning that UEFI is unable to identify the binary as a EFI executable. And I also validated the same on real Qualcomm based ARM64 hardware.

Furthermore, these new changes appear to have greatly increased the size of the executable, which went from 44.3 KB (prior to f7b2e1ff) to 112 KB (after), and while this size has decreased with later commits, it currently still produces a 72.6 KB executable, i.e. near to double the size of the binaries prior to these new changes...

I strongly suspect that the removal of the MZ section from crt0-efi-aarch64.S (in commit 593e13937c340944ec91e7c8962e8baaaeff7d5 from the branch that was merge) is at the origin of the problem, so could this please be restored as a matter of urgency?

I feel like I must also ask if anybody has tested gnu-efi build ARM64 binaries (and other binaries, as I am seeing a similar issue with RISC-V) on actual or virtual hardware after these fairly heavy recent changes?

Note that, if gnu-efi is willing to move to GitHub (so that we can use GitHub Actions), I may contribute a test-suite leveraging qemu for all of x64, ia32, arm, aa64 and riscv64, so that these kind of breaking changes can be detected as soon as they are introduced, because, unless I am doing something wrong, I believe something major like this is something that should have been spotted and reported some time ago, and the fact that it wasn't is deeply concerning...

Thanks,

/Pete

[CALLUM JAMES FARMER]

Requires binutils 2.38+, do not use earlier. If you have a binutils <2.38, don't upgrade gnu-efi

[CALLUM JAMES FARMER]

and make sure you use the correct objcopy command (target not binary) otherwise you'll get no header

[Nigel Croxon]

On 5/18/23 10:43 AM, Pete Batard wrote:

---

[bugs:#37] https://sourceforge.net/p/gnu-efi/bugs/37/ AARCH64 (and
possibly other archs) generation is broken

Status: open
Group: v1.0_(example)
Created: Thu May 18, 2023 02:43 PM UTC by Pete Batard
Last Updated: Thu May 18, 2023 02:43 PM UTC
Owner: nobody

It appears that commit f7b2e1ff6d6b8db119a9d7ec8406eb491004a16d (Merge
/u/gmbr3/gnu-efi/ branch master into master) completely broke AARCH64
EFI binary generation.

If you look at the generated binaries past this commit, they are
missing the |MZ| signature and other PE fields, and trying to run
them, both in QEMU and on real ARM64 hardware (such as Microsoft's
ARM64 2023 Dev Kit) will fail.

You can test this with a simple "Hello World" project such as
https://github.com/pbatard/uefi-simple which I just did on an up to
date Debian 11.7 system with the latest gcc toolchains. Prior to
|f7b2e1ff|, issuing |make ARCH=aa64 CROSS_COMPILE=aarch64-linux-gnu-
qemu| from this project would build a working ARM64 binary, that would
then run in QEMU and display information about the ARM64 system. But
after |f7b2e1ff|, even trying to run the gnu-efi based executable from
UEFI Shell results in |Command Error Status: Unsupported|, meaning
that UEFI is unable to identify the binary as a EFI executable. And I
also validated the same on real Qualcomm based ARM64 hardware.

Furthermore, these new changes appear to have greatly increased the
size of the executable, which went from 44.3 KB (prior to |f7b2e1ff|)
to 112 KB (after), and while this size has decreased with later
commits, it currently still produces a 72.6 KB executable, i.e. near
to double the size of the binaries prior to these new changes...

I strongly suspect that the removal of the MZ section from
crt0-efi-aarch64.S (in commit 593e13937c340944ec91e7c8962e8baaaeff7d5
from the branch that was merge) is at the origin of the problem, so
could this please be restored as a matter of urgency?

I feel like I must also ask if anybody has tested gnu-efi build ARM64
binaries (and other binaries, as I am seeing a similar issue with
RISC-V) on actual or virtual hardware after these fairly heavy recent
changes?

Note that, if gnu-efi is willing to move to GitHub (so that we can use
GitHub Actions), I /may/ contribute a test-suite leveraging qemu for
all of x64, ia32, arm, aa64 and riscv64, so that these kind of
breaking changes can be detected as soon as they are introduced,
because, unless I am doing something wrong, I believe something major
like this is something that should have been spotted and reported some
time ago, and the fact that it wasn't is deeply concerning...

Thanks,

/Pete

---

Sent from sourceforge.net because you indicated interest in
https://sourceforge.net/p/gnu-efi/bugs/37/

To unsubscribe from further messages, please visit
https://sourceforge.net/auth/subscriptions/

Those commits are way back in time...   might be better to fix the current.

[Pete Batard]

I'm sorry, but you just broke current Debian, so I don't see how we can go with this approach.

Current up to date Debian uses binutils 2.35, not 2.38, and you can't just go around breaking other up to date distros, just because OpenSUSE happens to be using a super recent binutils.

Heck, even if Debian or Ubuntu had just upgraded to 2.38 I would tell you that this is a poor practice, because you'd leave plenty of people stranded (sorry but the, "don't upgrade... and don't benefit from important fixes" approach is not a viable one) on account that they, for whatever reasons (and there are PLENTY), happen to run on a slightly obsolete platform.

And that is not counting the size of the binary that exploded.

Please bear in mind that, when you are dealing with a project like gnu-efi, you are NOT applying patches and fixes just for your platform of choice, but for everybody, and by requiring binutils 2.38 (that was only released one year ago --- that's not nearly long enough to decide to "obsolete" something), you have made sure that loads of people can either no longer use this project and can not benefit from any of its improvements.

Nigel, I would strongly suggest that you create a separate branch for the openSUSE "improvements" (or better, let them maintain their own branch, so that we can cherry pick from it ONCE binutils 2.38 has become commonplace for long enough for ALL the major distros) and revert the recent changes. Otherwise, you are going to find that way too many people, who were using or are planning to use gnu-efi, have been left stranded by the project. Personally, I will add that, if the project starts to have little to no regard to current or slightly obsolete distros, I'm going to stop bothering with it altogether, and recommend that everyone switches to EDK2.

So please heed this warning: Unless one is providing something that is a critical fix or must have (which I am not seeing here, especially with the major increase in binary size), you DON'T break compatibility with current major distros, period. And if you do happen to have broken compatibility for whatever reason, you fix it.

Regards,

/Pete

[CALLUM JAMES FARMER]

I'm going to be polite (but only this once) even though your previous statement is very imposing.

1) It is policy among ALL Linux distros to not mix new and old software if there is a function incompatibility. Going against this will cause issues, blaming other people for someone not following basic policy is just bad conduct.
2) This was not done for benefiting me or the openSUSE Project only, this was done because fixes were required for all distros with the new software as it overlaps once binutils 2.38+ is used.
3) You'll have seen a number of major patches not from be but from Richard Hughes (of RedHat), that is clearing the long mess of gnu-efi in Fedora which broke gnu-efi for a while due to an issue with ctors/dtors which I fixed.
4) Me (of openSUSE) , Richard Hughes (of RedHat) and Mario Limoncello (of AMD) have been clearing up fwupd-efi with this and other issues which needed patches to be brought here which to be honest it is improving massively.
5) Debian/Ubuntu have a notably poor control of all UEFI software and if the issue is an old binutils take it up with them not me, I've rolled the software forwards not backward.
6) Issues were seen with all rolling distros not just openSUSE
7) Bigger size due to data being placed into the correct location, like WTF did you think I did

I decline to comment further as your tone is genuinely appalling and you'll notice it says openSUSE not SUSE so I don't get paid to deal with things like this.

Callum Farmer
gmbr3@opensuse.org

[Pete Batard]

Yes, my statement is imposing. First of all, you are being singled out here because you are the author of the commit that broke compatibility. And I'm also seeing two major things that one should fully expect to have seen happening from a BREAKING change (or at least one that you rightfully expected/knew would break compatibility with older platforms):

1. Trying to detect the binutils version of support for whatever newer option you need as part of the build process, and only use the new approach in that case. Or at the very least, notify on the incompatible version. The current Makefile has a section that try to detect if someone is using a gcc that is too old to be compatible (see check_gcc:). I fail to see how/why doing something similar for binutils was not considered.
2. No update to the README or any notes to mention the new requirement for binutils 2.38 either. Once again, shouldn't a change that multiple people know is likely to be breaking be mentioned somewhere explicit?

I am genuinely concerned that I'm not seeing any hints that you (or the other people who pushed for the 2.38+ way of doing things) stopped to think about how you could make this more palatable for end users when it seems pretty obvious that not everyone is going to run with a <1 year old binutils.

As to your items:

1) It is also policy to try to detect breakage and report it to the user. Not doing so, when you know things are going to break, especially when you do have an example of how to do it in the project, is actually what I would call "bad conduct".
2) Again, you are the person who introduced the regression in 593e13937c340944ec91e7c8962e8baaaeff7d57. That commit clearly requires the use of binutils 2.38 or later, and therefore implies breakage for anything earlier. Yet this was casually introduced as if people using an older version were either non-existent or irrelevant.
3) I fail to see how you fixing something that somebody else broke is relevant.
4) I am positive that the changes that have been applied will help in the long run. But again, this should not come at the detriment of current users, when you had means to notify them of the fact that what you were introducing required a specific version of binutils. Again, I am mostly reiterating the same point, but it doesn't look to me like you went out of your way to consider that you did have an alternative to "eh, users/distros with older binutils will just sort themselves out on their own", which you seem to be further illustrated on the next point where...
5) ...you imply that trying to go even slightly out of your way to cater for distros that haven't yet upgraded to a version of binutils that was released less than 1 year prior to your patch shouldn't be your job (and this is the part where, if that is your stance, we will have a massive disagreement, because if you do contribute to a project, then, yes, it is your job to handle breakage in a manner that doesn't keep users in the dark, regardless of how much you want to congratulate yourself on having rolled the software forward). Also, as you know, binutils applies to more than UEFI, whereas you seem to imply that Debian/Ubuntu haven't upgraded because they don't care about UEFI and that valuing stability (i.e. being slow to introduce potentially breaking changes rather than rushing into them) is a negative trait.
6) Once more, my issue is that this could be addressed much better than "use binutils 2.38+ or be left to wonder why it broke". Unless this was a security vulnerability that needed to be rushed, out, it very much looks to me like your patch chose to completely disregarded a whole slew of users and distros, that it could easily have tried to cater for.
7) Well, from what I can see, the size did come down with the subsequent patches, so there was room for improvements, though I don't think I'd care at all if silent breakage hadn't been introduced for non binutils 2.38+ users.

Oh and none of us get paid to deal with things like this. However, you shouldn't be entirely surprised to be called out when you introduced a breaking patch seemingly without even remotely considering whether it could be feasible to add an option to maintain compatibility or at the very least, detect incompatible platforms and notify the user.

So let me be imposing yet again by restating that you should pay proper consideration to changes you introduce, when you know they are going to (potentially) break compatibility, and demonstrate that you have done your best effort to reduce the impact of said changes on users. Unfortunately your patch, that doesn't even attempt to detect if the platform is using binutils 2.38+, so that the modifications being applied can actually work, shows none of that...

/Pete

[Pete Batard]

Just going to add that on a vanilla Ubuntu 23.04 system (that was released a couple weeks ago and is recent enough to come with binutils 2.40), the default gnu-efi apps are still broken.

apt install git build-essential gcc-aarch64-linux-gnu
git clone https://git.code.sf.net/p/gnu-efi/code gnu-efi
cd gnu-efi
aarch64-linux-gnu-objcopy --version # reports: "GNU objcopy (GNU Binutils for Ubuntu) 2.40"
export CROSS_COMPILE=aarch64-linux-gnu-
make apps

Trying to run the resulting t.efi (a simple "Hello, World") under UEFI Shell 2.2 produces Command Error Status: Unsupported (tested on both Raspberry Pi 4 and Windows Dev Kit 2023).

Reverting to the commit prior to f7b2e1ff6d6b8db119a9d7ec8406eb491004a16d (and removing --fatal-warnings from Make.defaults since newer binutils appear to produce a aarch64-linux-gnu-ld: warning: t.so has a LOAD segment with RWX permissions warning otherwise) works.

Of course, it is very possible that this regression, that is not related to using binutil < 2.38, was introduced further down the line, as I only really tested 2 revs this time around. But the fact remains that, no matter what version of binutils you appear to use, the current gnu-efi no longer appears to be able to produce working UEFI AARCH64 binaries. I will therefore ask the people responsible for introducing what looks like a major regression to first validate that they can replicate the issue (if needed, you can find recent UEFI Shell binaries at https://github.com/pbatard/UEFI-Shell), and, once they have done so, fix it.

/Pete

[Pete Batard]

Well, considering that a fix for this issue has now "magically" appeared in https://github.com/rhboot/gnu-efi/commit/9e6cb2150bee08e83ec0cdfb8d6c2f83975dd3df, which has been integrated into gnu-efi, and that none of the people responsible for the regression bothered to notify about this fix in the original bug report, I guess I'm going to entitle myself to some further snide remarks by pointing out what a nice "rolling the software forwards" this has been, when it looks like none the 3+ developers involved into heavily reworking the way UEFI binary executables are generated with gnu-efi, appear to have bothered validating whether said executables could still run in a UEFI Shell...

Thus, from what I am seeing, it does look like the RH Bootloader team have been a lot more interested in getting gnu-efi to work in their very specific subcontext, without much regard for the consequences, than they have been into making sure they didn't break stuff for other folks, who also happen to depend on the same upstream project...

So, once again, I will call out on people, who I fully expect to know better, to consider more than just their immediate specific goal when applying heavy handed modifications to a project that they do share with everyone.