Hello!
To clarify from the very beginning, I don't know how this part of the larger picture actually works. The end result is that seabios project, with the current source tarball available for download from https://www.seabios.org/downloads/seabios-1.13.0.tar.gz , can't be built using GNU ld as of version 2.34.90.20200706.
Here's the relevant output:
seabios-1.13.0 $ make silentoldconfig seabios-1.13.0 $ make V=1 PYTHON=python3 ... Linking out/rom16.o ld -T out/romlayout16.lds out/code16.o -o out/rom16.o Stripping out/rom16.strip.o strip out/rom16.o -o out/rom16.strip.o Linking out/rom.o ld -N -T out/romlayout32flat.lds out/rom16.strip.o out/rom32seg.strip.o out/code32flat.o -o out/rom.o ld: cannot use executable file 'out/rom16.strip.o' as input to a link make: *** [Makefile:187: out/rom.o] Error 1 seabios-1.13.0 $ _
Previous versions of ld accepted this file just fine and produced valid out/bios.bin in the end.
I dunno if this is a problem is seabios (which has actually been already reported to seabios by Paul Menzel today), or an issue in ld. These ROMs/firmwares are a bit tricky to build..
Thanks!
/mjt
On Thu, Jul 9, 2020 at 7:40 AM Michael Tokarev mjt@tls.msk.ru wrote:
Hello!
To clarify from the very beginning, I don't know how this part of the larger picture actually works. The end result is that seabios project, with the current source tarball available for download from https://www.seabios.org/downloads/seabios-1.13.0.tar.gz , can't be built using GNU ld as of version 2.34.90.20200706.
Here's the relevant output:
seabios-1.13.0 $ make silentoldconfig seabios-1.13.0 $ make V=1 PYTHON=python3 ... Linking out/rom16.o ld -T out/romlayout16.lds out/code16.o -o out/rom16.o Stripping out/rom16.strip.o strip out/rom16.o -o out/rom16.strip.o Linking out/rom.o ld -N -T out/romlayout32flat.lds out/rom16.strip.o out/rom32seg.strip.o out/code32flat.o -o out/rom.o ld: cannot use executable file 'out/rom16.strip.o' as input to a link make: *** [Makefile:187: out/rom.o] Error 1 seabios-1.13.0 $ _
Previous versions of ld accepted this file just fine and produced valid out/bios.bin in the end.
I dunno if this is a problem is seabios (which has actually been already reported to seabios by Paul Menzel today), or an issue in ld. These ROMs/firmwares are a bit tricky to build..
It was caused by:
commit a87e1817a435dab6c6c042f9306497c9f13d4236 Author: Nick Clifton nickc@redhat.com Date: Thu May 28 17:43:21 2020 +0100
Have the linker fail if any attempt to link in an executable is made.
PR 26047 * ldelf.c (ldelf_after_open): Fail if attempting to link one executable into another. Ensure that the test is made for all forms of linking.
Please open a binutils regression bug against 2.35.
[seabios only, - seabios list is apparently subscribers-only, so others can't post there]
09.07.2020 17:39, Michael Tokarev wrote:
Hello!
To clarify from the very beginning, I don't know how this part of the larger picture actually works. The end result is that seabios project, with the current source tarball available for download from https://www.seabios.org/downloads/seabios-1.13.0.tar.gz , can't be built using GNU ld as of version 2.34.90.20200706.
Created binutils bugreport, https://sourceware.org/bugzilla/show_bug.cgi?id=26223
/mjt
On Thu, Jul 09, 2020 at 05:39:48PM +0300, Michael Tokarev wrote:
Hello!
To clarify from the very beginning, I don't know how this part of the larger picture actually works. The end result is that seabios project, with the current source tarball available for download from https://www.seabios.org/downloads/seabios-1.13.0.tar.gz , can't be built using GNU ld as of version 2.34.90.20200706.
Here's the relevant output:
seabios-1.13.0 $ make silentoldconfig seabios-1.13.0 $ make V=1 PYTHON=python3 ... Linking out/rom16.o ld -T out/romlayout16.lds out/code16.o -o out/rom16.o Stripping out/rom16.strip.o strip out/rom16.o -o out/rom16.strip.o Linking out/rom.o ld -N -T out/romlayout32flat.lds out/rom16.strip.o out/rom32seg.strip.o out/code32flat.o -o out/rom.o ld: cannot use executable file 'out/rom16.strip.o' as input to a link make: *** [Makefile:187: out/rom.o] Error 1 seabios-1.13.0 $ _
Previous versions of ld accepted this file just fine and produced valid out/bios.bin in the end.
I dunno if this is a problem is seabios (which has actually been already reported to seabios by Paul Menzel today), or an issue in ld. These ROMs/firmwares are a bit tricky to build..
Some background links:
https://www.mail-archive.com/seabios@seabios.org/msg12275.html
https://sourceware.org/bugzilla/show_bug.cgi?id=26047
Nick - the original poster of PR 26047 was aware the binutils change would break software. Were you aware of that when you committed the change?
Thanks, -Kevin
09.07.2020 18:41, Kevin O'Connor wrote: ..
Some background links:
https://www.mail-archive.com/seabios@seabios.org/msg12275.html
Doh. This is a known issue, even the commit in question has been known to break seabios even before it were committed..
Nick - the original poster of PR 26047 was aware the binutils change would break software. Were you aware of that when you committed the change?
What's wrong with changing seabios as done by Fāng-ruì Sòng?
/mjt
On Thu, Jul 09, 2020 at 06:49:37PM +0300, Michael Tokarev wrote:
09.07.2020 18:41, Kevin O'Connor wrote: ..
Some background links:
https://www.mail-archive.com/seabios@seabios.org/msg12275.html
Doh. This is a known issue, even the commit in question has been known to break seabios even before it were committed..
Nick - the original poster of PR 26047 was aware the binutils change would break software. Were you aware of that when you committed the change?
What's wrong with changing seabios as done by Fāng-ruì Sòng?
My initial feedback was at:
https://www.mail-archive.com/seabios@seabios.org/msg12157.html
and later at (now as a "patch 3"):
https://www.mail-archive.com/seabios@seabios.org/msg12189.html
The proposed approach is to use a combination of printf and dd to binary modify the intermediate object file so as to clear the (now banned) linker flag. I did feel, and continue to feel, this is an ugly hack that is fragile.
-Kevin
On Jul 09 2020, Kevin O'Connor wrote:
The proposed approach is to use a combination of printf and dd to binary modify the intermediate object file so as to clear the (now banned) linker flag. I did feel, and continue to feel, this is an ugly hack that is fragile.
The easiest way to link a binary blob into an object is to encapsulate the blob into a rel object via the .incbin assembler directive.
Andreas.
09.07.2020 19:05, Kevin O'Connor wrote:
On Thu, Jul 09, 2020 at 06:49:37PM +0300, Michael Tokarev wrote:
..
What's wrong with changing seabios as done by Fāng-ruì Sòng?
..
https://www.mail-archive.com/seabios@seabios.org/msg12189.html
The proposed approach is to use a combination of printf and dd to binary modify the intermediate object file so as to clear the (now banned) linker flag. I did feel, and continue to feel, this is an ugly hack that is fragile.
Yes it smells ugly. ld doesn't have a way to specify e_type, does it?
Speaking of patching the object file, - it is always the same byte, since elf format specifies its placement, meaning and possible values, all utilities who understand elf know this place (byte #16).
While still being ugly, maybe it's more elegant to use a small python script instead of printf|dd, - a script that'll open file and write single byte at offset 16..
I dunno. To me the thing from ld PoV smells like seabios is using an ugly hack to link the binary..
/mjt
Hi Guys,
I have just updated PR 26047 with a suggested patch which I think could resolve this situation. (Patch attached here as well in order to save time). It adds a new linker command line option: -z allowexec which will disable the warnings about linking in executable files. The default is still to have these warnings as I think that in most cases this behaviour makes sense.
Will this solve the problem for you ?
Cheers Nick
On 2020-07-10, Nick Clifton via Binutils wrote:
Hi Guys,
I have just updated PR 26047 with a suggested patch which I think could resolve this situation. (Patch attached here as well in order to save time). It adds a new linker command line option: -z allowexec which will disable the warnings about linking in executable files. The default is still to have these warnings as I think that in most cases this behaviour makes sense.
Will this solve the problem for you ?
Cheers Nick
In the future there may be more ET_* types. I don't think linking them will be correct/intended
https://groups.google.com/forum/#!topic/generic-abi/tJq7anc6WKs "RFC: Add ET_DEBUG" (the RFC has not been accepted but OSes can freely use ET_GNU_* )
-----
For seabios, I sincerely wish Kevin can consider changing e_type instead of relying a new linker option which has no use case otherwise.
ET_EXEC works for seabios is just because seabios uses a very different link model. It ignores the symbol table and many sections which would normally appear in a normal ET_EXEC (.hash .gnu.hash .plt .dynstr .dynsym ...) I should add that rejecting ET_EXEC is also wish of a maintainer of the Arm proprietary linker.
On Fri, Jul 10, 2020 at 11:58:06AM +0100, Nick Clifton wrote:
Hi Guys,
I have just updated PR 26047 with a suggested patch which I think could resolve this situation. (Patch attached here as well in order to save time). It adds a new linker command line option: -z allowexec which will disable the warnings about linking in executable files. The default is still to have these warnings as I think that in most cases this behaviour makes sense.
Will this solve the problem for you ?
Hi Nick,
Thanks for looking at this.
I think the main issue is going to be the breaking of existing software builds. In particular, as distros pull in the new version of binutils, they'll run into errors building their current version of SeaBIOS. Even if we update SeaBIOS today, it will be some time before the distros will pull that change in. It'll also be a problem for those trying to build older versions of SeaBIOS on newer toolchains.
Unless I'm missing something, this will also be a problem for those building recent versions of the Linux kernel. It's build used this ability of ld up until a few months ago (see https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... ). Although the latest Linux has changed, building older versions is fairly common. It's unclear to me if there are other common packages that use this ability of ld.
Also, you mentioned "which will disable the warnings about linking in executable files". I thought PR 26047 created a fatal error in this situation. Is the proposed change to make the error a warning or am I missing something?
As for the particular workaround, "-z allowexec" generates a warning on current versions of ld. Given that, for SeaBIOS, my first reaction would probably be to change the SeaBIOS build to unconditionally clear the flag in the intermediate binary than to deploy a more complicated ld version check to set the given command-line flag. I'd like to give that some more thought though.
Thanks again, -Kevin
On 2020-07-10, Kevin O'Connor wrote:
On Fri, Jul 10, 2020 at 11:58:06AM +0100, Nick Clifton wrote:
Hi Guys,
I have just updated PR 26047 with a suggested patch which I think could resolve this situation. (Patch attached here as well in order to save time). It adds a new linker command line option: -z allowexec which will disable the warnings about linking in executable files. The default is still to have these warnings as I think that in most cases this behaviour makes sense.
Will this solve the problem for you ?
Hi Nick,
Thanks for looking at this.
I think the main issue is going to be the breaking of existing software builds. In particular, as distros pull in the new version of binutils, they'll run into errors building their current version of SeaBIOS. Even if we update SeaBIOS today, it will be some time before the distros will pull that change in. It'll also be a problem for those trying to build older versions of SeaBIOS on newer toolchains.
Unless I'm missing something, this will also be a problem for those building recent versions of the Linux kernel. It's build used this ability of ld up until a few months ago (see https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... ). Although the latest Linux has changed, building older versions is fairly common.
There is no problem (reject linking ET_EXEC as input) for 5.4, 5.6 and HEAD. The fixes are in stable trees.
[PATCH 5.4 007/134] bpf: Support llvm-objcopy for vmlinux BTF [PATCH 5.6 012/161] bpf: Support llvm-objcopy for vmlinux BTF
I haven't tested 5.2 (the first major release with potentially problematic BTF commit) but if someone worries about 5.2 linkability with GNU ld 2.35, the error can be downgraded to a warning.
It's unclear to me if there are other common packages that use this ability of ld.
I don't think so. There is no FreeBSD package which has been marked LLD_UNSAFE for the sole reason that older GNU ld accepts ET_EXEC while LLD rejects ET_EXEC.
Among the tens of thousands of ports,
% rg -l LLD_UNSAFE 75
In the future there may be more ET_* types. I don't think linking them will be correct/intended
https://groups.google.com/forum/#!topic/generic-abi/tJq7anc6WKs "RFC: Add ET_DEBUG" (the RFC has not been accepted but OSes can freely use ET_GNU_* )
The opinion in my other message (https://sourceware.org/pipermail/binutils/2020-July/112283.html ) stands. allowexec will not be a suitable option name.
For seabios and Linux 5.2 (this major release only), a linker warning should not hurt. In a future release of GNU ld, the warning can be upgraded to an error.
Hi Fangrui,
The opinion in my other message (https://sourceware.org/pipermail/binutils/2020-July/112283.html ) stands. allowexec will not be a suitable option name.
Agreed - thinking on it some more, if we do implement this option then a name like "allowall" or "allowany" might be better.
For seabios and Linux 5.2 (this major release only), a linker warning should not hurt. In a future release of GNU ld, the warning can be upgraded to an error.
This seems like a good compromise to me. I will add a patch to the 2.35 branch to change the error to a warning, along with a note that supporting this kind of linking is likely to be deprecated in the future.
Cheers Nick
On Mon, Jul 13, 2020 at 5:25 AM Nick Clifton nickc@redhat.com wrote:
Hi Fangrui,
The opinion in my other message (https://sourceware.org/pipermail/binutils/2020-July/112283.html ) stands. allowexec will not be a suitable option name.
Agreed - thinking on it some more, if we do implement this option then a name like "allowall" or "allowany" might be better.
I think we don't need an option then. The number of use cases is small enough.
For seabios and Linux 5.2 (this major release only), a linker warning should not hurt. In a future release of GNU ld, the warning can be upgraded to an error.
This seems like a good compromise to me. I will add a patch to the 2.35 branch to change the error to a warning, along with a note that supporting this kind of linking is likely to be deprecated in the future.
Cheers Nick
Sounds like a good plan. Thanks!
11.07.2020 03:08, Kevin O'Connor wrote: []
As for the particular workaround, "-z allowexec" generates a warning on current versions of ld. Given that, for SeaBIOS, my first reaction would probably be to change the SeaBIOS build to unconditionally clear the flag in the intermediate binary than to deploy a more complicated ld version check to set the given command-line flag. I'd like to give that some more thought though.
Can't seabios use the way suggested by Andreas Schwab, namely .incbin assembler instruction?
/mjt
On Sat, Jul 11, 2020 at 09:22:30AM +0300, Michael Tokarev wrote:
11.07.2020 03:08, Kevin O'Connor wrote: []
As for the particular workaround, "-z allowexec" generates a warning on current versions of ld. Given that, for SeaBIOS, my first reaction would probably be to change the SeaBIOS build to unconditionally clear the flag in the intermediate binary than to deploy a more complicated ld version check to set the given command-line flag. I'd like to give that some more thought though.
Can't seabios use the way suggested by Andreas Schwab, namely .incbin assembler instruction?
SeaBIOS has some funky linking requirements. It needs to link 16bit code, 32bit "segmented" code, and normal 32bit code. There's a document online describing a bit of this at: https://www.seabios.org/Linking_overview
The main limitation to simple ".incbin" usage is that SeaBIOS needs to put a variety of 16bit sections along with 32bit "segmented" sections at certain fixed memory locations. So, there is no one simple .incbin directive that would replace the final linking step. It's certainly possible to come up with an alternative linking mechanism (or to even manually assemble the final image), but I suspect the alternatives will come with further complexity. Certainly something to look at though.
Cheers, -Kevin
On Sat, 11 Jul 2020, Kevin O'Connor wrote:
The main limitation to simple ".incbin" usage is that SeaBIOS needs to put a variety of 16bit sections along with 32bit "segmented" sections at certain fixed memory locations. So, there is no one simple .incbin directive that would replace the final linking step. It's certainly possible to come up with an alternative linking mechanism (or to even manually assemble the final image), but I suspect the alternatives will come with further complexity. Certainly something to look at though.
The point is with `.incbin' you can effectively convert any piece of data back into the relocatable format without having to resort to undocumented hacks or plain bugs. And you have further control over this data by, say, putting it into an individual section, or defining symbols to delimit the beginning and/or the end. Then you can use the resulting object file as an ordinary input to the linker just as with your current recipe, and use a linker script to lay out individual inputs however you wish, including at fixed memory locations.
Maciej
Hi Kevin,
I think the main issue is going to be the breaking of existing software builds.
True - but the counter argument is that those builds were relying upon a mis-feature of the BFD linker, and so maybe the correct thing to do is to fix the builds.
Also, you mentioned "which will disable the warnings about linking in executable files". I thought PR 26047 created a fatal error in this situation. Is the proposed change to make the error a warning or am I missing something?
Sorry - I misstyped. I meant that the proposed patch would disable the fatal error, rather than a simple warning.
As for the particular workaround, "-z allowexec" generates a warning on current versions of ld.
Do you mean that using this option introduces a new warning message (presumably about the option not being recognised), or that it does not work and using it has no effect ? If the former then this means that the proposed patch has not been applied to the linker, or that the patched linker has not then been rebuilt. If it is the latter then I must have missed something in the creation of the patch.
Cheers Nick
On Mon, Jul 13, 2020 at 01:22:27PM +0100, Nick Clifton wrote:
Hi Kevin,
I think the main issue is going to be the breaking of existing software builds.
True - but the counter argument is that those builds were relying upon a mis-feature of the BFD linker, and so maybe the correct thing to do is to fix the builds.
Understood. I'm okay with a change to SeaBIOS.
As for the particular workaround, "-z allowexec" generates a warning on current versions of ld.
Do you mean that using this option introduces a new warning message (presumably about the option not being recognised), or that it does not work and using it has no effect ? If the former then this means that the proposed patch has not been applied to the linker, or that the patched linker has not then been rebuilt. If it is the latter then I must have missed something in the creation of the patch.
I mean the former - if I change current SeaBIOS to always include "-z allowexec" then currently deployed versions of ld (eg, v2.34) will report "ld: warning: -z allowexec ignored".
On Mon, Jul 13, 2020 at 01:25:12PM +0100, Nick Clifton wrote:
For seabios and Linux 5.2 (this major release only), a linker warning should not hurt. In a future release of GNU ld, the warning can be upgraded to an error.
This seems like a good compromise to me. I will add a patch to the 2.35 branch to change the error to a warning, along with a note that supporting this kind of linking is likely to be deprecated in the future.
Works for me.
Thanks, -Kevin
Dear SeaBIOS and Binutils folks,
Am 13.07.20 um 14:22 schrieb Nick Clifton:
I think the main issue is going to be the breaking of existing software builds.
True - but the counter argument is that those builds were relying upon a mis-feature of the BFD linker, and so maybe the correct thing to do is to fix the builds.
Also, you mentioned "which will disable the warnings about linking in executable files". I thought PR 26047 created a fatal error in this situation. Is the proposed change to make the error a warning or am I missing something?
Sorry - I misstyped. I meant that the proposed patch would disable the fatal error, rather than a simple warning.
The Debian Sid/unstable packages were just updated to the newly released version 2.35. With that, I am able to build SeaBIOS again, and, as expected, the two warnings below are shown.
ld: Using an executable file (out/rom16.strip.o) as input to a link is deprecated - support is likely to be removed in the future ld: Using an executable file (out/rom32seg.strip.o) as input to a link is deprecated - support is likely to be removed in the future
Thank you everyone involved.
Kind regards,
Paul
-
Andreas Schwab -
Fangrui Song -
Fangrui Song -
Fāng-ruì Sòng
-
H.J. Lu -
Kevin O'Connor -
Maciej W. Rozycki -
Michael Tokarev -
Nick Clifton -
Paul Menzel