X Tutup
Skip to content
/ scummvm Public

CONFIGURE: Don't use the Gold linker on i386/ppc (unless using --enable-gold)#6285

Merged
bluegr merged 1 commit intoscummvm:masterfrom
dwatteau:feat/configure-disable-gold-linker-default-on-i386-and-ppc
Jan 30, 2025
Merged

CONFIGURE: Don't use the Gold linker on i386/ppc (unless using --enable-gold)#6285
bluegr merged 1 commit intoscummvm:masterfrom
dwatteau:feat/configure-disable-gold-linker-default-on-i386-and-ppc

Conversation

@dwatteau
Copy link
Contributor

@dwatteau dwatteau commented Dec 3, 2024 •
edited
Loading

Debian and Fedora have been building their official ScummVM packages with --disable-gold for a while, because of build failures on (at least) i386: [1], [2].

I saw a similar issue with Gold and the ppc32 QEMU VM myself (internal error in relocate, at ../../gold/powerpc.cc:6773 when doing a full build with all engines), and both Gentoo and Fedora think of Gold as a deprecated linker nowadays, anyway: [1], [2], [3].

So this PR disables the Gold linker on i386 and ppc. People really wanting to use it there can still use it with an explicit --enable-gold.

Alternative linkers exist (mold, lld...) and are already supported, and otherwise ld.bfd often is a safe default (and AFAICS it's not as slow at it used to be).

Any objection to this? The idea is to take care, by default, of the build issues that Fedora and Debian have been having with Gold. It looks like (at least) the master-debian-i686 buildbot will be impacted by this change. (By the way, since it appears that this buildbot target never had an issue wild Gold, maybe we could disable its usage for older Gold releases only. But I can't say where to draw the line. Fedora/Debian hit the bug in 2020/2021, and Gold didn't receive many commits since then.)

EDIT: the issue was also present in early 2021 Slackware: [1]

@bluegr
Copy link
Member

bluegr commented Dec 3, 2024

Thanks! Both Debian and Fedora use bfd instead of gold, but we opt for mold instead. Perhaps we should also use bfd as well, when disabling gold in these cases? Or just simply replace gold with bfd altogether?

@lephilousophe
Copy link
Member

lephilousophe commented Dec 3, 2024

Mold is not a linker installed everywhere, bfd is a good default.

@dwatteau
Copy link
Contributor Author

dwatteau commented Dec 3, 2024

Thanks! Both Debian and Fedora use bfd instead of gold, but we opt for mold instead. Perhaps we should also use bfd as well, when disabling gold in these cases? Or just simply replace gold with bfd altogether?

Yes, bfd often already is the default ld binary on ELF systems.

@bluegr
Copy link
Member

bluegr commented Dec 3, 2024 •
edited
Loading

Thanks! Both Debian and Fedora use bfd instead of gold, but we opt for mold instead. Perhaps we should also use bfd as well, when disabling gold in these cases? Or just simply replace gold with bfd altogether?

Yes, bfd often already is the default ld binary on ELF systems.

Yeah, but for some reason, they've hardcoded bfd in the Fedora patch. We could assume that bfd is always available on Fedora, but if this is true, why would they hardcode it in their patch?

Here's the Fedora patch, for reference:

+# used bdf linker on x86 as FTBFS workaround            
+if test "$_have_x86" = yes ; then            
+    set_var LDFLAGS "-fuse-ld=bfd"            
+fi

@dwatteau
Copy link
Contributor Author

dwatteau commented Dec 3, 2024

Yeah, but for some reason, they've hardcoded bfd in the Fedora patch. We could assume that bfd is always available on Fedora, but if this is true, why would they hardcode it in their patch?

Here's the Fedora patch, for reference:

+# used bdf linker on x86 as FTBFS workaround            
+if test "$_have_x86" = yes ; then            
+    set_var LDFLAGS "-fuse-ld=bfd"            
+fi

Yes, as discussed on Discord, I thought that they did that just because, in their case, such a big override was always guaranteed to work (instead of analyzing what we're doing with the _gold variable in the configure script, and so on).

We can't just hardcode -fuse-ld=bfd as they do, because the linker on macOS isn't bfd for example.

I'll put the PR on hold until I'm done with some tests on Fedora, to be sure. Maybe they did set up gold as the default ld in some releases, and then yes, this diff would have to be done differently.

@dwatteau dwatteau marked this pull request as draft December 3, 2024 19:18
@lephilousophe
Copy link
Member

lephilousophe commented Dec 8, 2024

The patch was added by Fedora in 2020.
The last movements about Gold for Fedora are circa 2010 (https://fedoraproject.org/wiki/Features/GoldLinkerDefault).
So I would say this is quite safe although I am not even sure the bug is still around.

@dwatteau
CONFIGURE: Avoid using the Gold linker on non-mainstream archs, unles…
c4bbe74 
…s using --enable-gold

Debian, Fedora and Slackware have been building their official ScummVM
packages with --disable-gold for a while, because of build failures on
(at least) i386:

https://salsa.debian.org/games-team/scummvm/-/commit/a6afd58cf8cb8bb39ee1b1fd764ff1721b1001f4
https://src.fedoraproject.org/rpms/scummvm/c/e737fe0c41f2c9cff8400defaab10908b553db18?branch=rawhide
https://git.slackbuilds.org/slackbuilds/commit/games/scummvm?id=d3ce31e0215d7d6fdf7bdecac1bad499a0e0167d

I saw a similar issue on Linux ppc32 myself (linker internal error when
doing a full `-Og -g` build with all engines), and both Gentoo and Fedora
treat Gold as a deprecated linker nowadays, anyway.

Alternative linkers exist (mold, lld...), and ld.bfd often is a safe
default (and AFAICS it's not as slow at it used to be).

People really wanting to use Gold on other archs can still ask for it
with --enable-gold.

Out of simplicity, this patch doesn't handle the case where Gold would
be the default OS linker. We hope no system is doing that, if its linker
is known to have this kind of issues.
@dwatteau dwatteau force-pushed the feat/configure-disable-gold-linker-default-on-i386-and-ppc branch from d406401 to c4bbe74 Compare January 30, 2025 15:18
@dwatteau dwatteau marked this pull request as ready for review January 30, 2025 15:19
@dwatteau
Copy link
Contributor Author

dwatteau commented Jan 30, 2025 •
edited
Loading

Thanks.

I've spent some time trying to reproduce the original Gold issues with x86 VMs of Fedora 34 and Slackware 14.2 (whose release dates could match the date of the patches added to these distributions).

I couldn't reproduce the problems there, though. The only environment where I could reproduce this is the (old) ppc32 VM I've described above.

Anyway, I've updated the PR with some recent feedback over Discord.

Now this PR does this:

  • Avoid picking up -fuse-ld=gold on non-mainstream archs ("mainstream" = arm64, x86_64, where gold is more stable), unless one gives an explicit --enable-gold
  • Keep it simple, and don't try to handle the case where Gold would be the default linker. It's theoretically possible (there's an /etc/default mechanism allowing it in some distributions), but as lephilousophe said, systems providing a broken linker by default are broken anyway, and possibly don't exist anymore.

If this simpler approach is still an issue, let's wait for people to report it. So far, we've only seen some distros fixed that on their sides, but that was some years ago, and no end-user (or distro maintainer) reported a real problem for that.

So I'd say the updated PR should be simple enough…

@bluegr
Copy link
Member

bluegr commented Jan 30, 2025 •
edited
Loading

Much simpler and cleaner approach.
This should work in normal configurations. If there's any exotic case where the default linker (usually bfd) doesn't work, it can be checked if there's a relevant user report with information about that particular environment.

Thanks!

@bluegr bluegr merged commit eb904f6 into scummvm:master Jan 30, 2025
8 checks passed
@dwatteau dwatteau deleted the feat/configure-disable-gold-linker-default-on-i386-and-ppc branch January 30, 2025 17:54
@lephilousophe
Copy link
Member

lephilousophe commented Feb 8, 2025

@dwatteau Guess what... gold linker is now deprecated by GNU.
From https://lists.gnu.org/archive/html/info-gnu/2025-02/msg00001.html:

In a change to our previous practice, in this release the
binutils-2.44.tar tarball does not contain the sources for the gold
linker. This is because the gold linker is now deprecated and will
eventually be removed unless volunteers step forward and offer to
continue development and maintenance.

I think we should not make use of it by default anymore.

@dwatteau dwatteau mentioned this pull request Feb 12, 2025
Merged
@dwatteau
Copy link
Contributor Author

dwatteau commented Feb 12, 2025

@dwatteau Guess what... gold linker is now deprecated by GNU. From https://lists.gnu.org/archive/html/info-gnu/2025-02/msg00001.html:

In a change to our previous practice, in this release the
binutils-2.44.tar tarball does not contain the sources for the gold
linker. This is because the gold linker is now deprecated and will
eventually be removed unless volunteers step forward and offer to
continue development and maintenance.

I think we should not make use of it by default anymore.

Ha, nice timing 🙃 Thanks for the info!

I've submitted a PR for that, then: #6431

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dwatteau @bluegr @lephilousophe
X Tutup