-
Notifications
You must be signed in to change notification settings - Fork 17.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gccgo: offical support for musl libc #51280
Comments
Hi there, I'm the original author of the patchset and I am happy to see this working its way upstream. Note that while it doesn't apply cleanly to GCC 11 any more (we ship GCC 8 in Adélie right now), these changes are needed to
Also, I don't see discussion of the Happy to do any forward-porting or testing required, including with GCC 11 or trunk if desired. cc @sroracle who did some work with GCC 10 last year. |
We have revised and updated many of the patches since, we now use them successfully with GCC 11 at Alpine (see my
The two patches you are mentioning for PPC are superseded by
Yeah, I missed that one. |
Thanks. Discussing here is fine. Ideally send patches to the gofrontend repo as described at https://go.dev/doc/contribute. For Detecting |
Right, but I didn't see the PPC regs patch - that patch is what no longer applies cleanly.
Ah, I missed that. |
I think it would. These conditions would be true on musl:
If those are true, we need to add |
I think this will be difficult in this case since the Alpine patchset is authored by multiple people, I can't sign the CLA for everyone (and also I don't have a Google account myself). @awilfox would you be interested in submitting your patches yourself as a starting point? Alternatively, maybe @ianlancetaylor could use our patches as a source of inspiration and commit modified version of them, i.e. treat this more as a bug report (as I mentioned most need some cleanup anyhow)? I also think that many of these 2-3 line changes aren't copyrightable in the first place. |
When cross-compiling we should not assume that links succeed in general. People need to be able to build GCC in order to build libc. So when cross-compiling a library we should be prepared to assume that all link tests will fail. For patches that are just a couple of lines I agree that they aren't copyrightable and it doesn't matter who sends them into Gerrit. Sending to gcc-patches is also OK but less convenient; be sure to CC me for any patches to libgo or gcc/go, as otherwise it's easy for me to miss them. It's unrealistic to wait for me to write my own version of the patches, I don't use musl and I don't have the time. Sorry. |
I'm willing to handle this, I can submit these patches on behalf of Alpine via Chainguard, who have a CLA on file with Google :) |
Assuming proper attribution for the patches I authored, I am completely fine with @kaniini handling the submissions of these patches. I have already signed the Google CLA individually for google/marl#7 which should still apply for the patches I wrote for GCC Go. |
With musl 1.2.3 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105225 also needs to be fixed (there is already a patch for it in Alpine as well). |
Over the past months, I have upstreamed many of other gcc-go patches. As part of this process, many patches were slightly improved and adjusted upstream. This commit replaces our downstream version of these patches with what has been accepted upstream. See: golang/go#51280
I am happy to report that the majority of the Alpine gccgo musl patches has been integrated upstream by now. Apart from the libucontext issues discussed above, the only remaining portability issue I am presently aware of is that Any idea how this could be fixed in a portable way upstream? GNU autoconf provides |
Moving from Go to C for something like this is fine. That is, we can fix this by testing for the functionality in libgo/configure.ac and writing the relevant code in C using |
The Also: Is there a CI for gofrontend/gccgo? Would there be an interested in adding Alpine to it? |
Using Note that on x86_64 we use custom written assembly code for Alas, there is no CI for gofrontend. |
The libucontext issue has been resolved as well. The only remaining problem I am presently aware of is that Thanks @ianlancetaylor for improving and integrating the patchset! 🎉 |
Thanks for all the work on this. |
tl;dr At Alpine Linux downstream, we have several patches to add support for musl libc to gccgo and gofrontend, I would like to see these patches integrated upstream. This is intended as a tracking issue to help with upstream integration.
I hope this is the right place to discuss this kind of issue. If gccgo-related stuff is better discussed on the gofrontend-dev ML please let me know. I am just opening this here because related gccgo issues were also discussed in this bug tracker.
What version of Go are you using (
go version
)?What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
I am a contributor to the Alpine Linux distribution. I recently became interested in bootstrapping our Google Go implementation using gccgo. While attempting to do so, I ran into several issues since musl doesn't seem to be supported by gccgo at the moment. To resolve these issues, several patches were written for gcc and the gofrontend. These patches are currently applied at Alpine Linux downstream. Ideally, I would like to see them integrated upstream to have official support for musl libc in gccgo. Some of the patches definitely require a cleanup, however, they are presently sufficient to bootstrap Go using gccgo on Alpine on all architectures supported by Alpine (aarch64, armhf, armv7, ppc64le, s390x, x86 and x86_64). The full list of patches we presently apply for gcc can be found here: https://gitlab.alpinelinux.org/alpine/aports/-/tree/da8ed6612f864a639e8f381ebae7dab7c20164a2/main/gcc patches specific to gofrontend/gccgo will be further discussed below.
Signal 34 is special on musl
musl uses signal 34 internally (similar to how glibc uses signal 32 and signal 33). For this reason, special handling is needed for this signal in the runtime. The Google Go implementation already handles signal 34 accordingly (see #39343 etc) but unfortunately gofrontend doesn't. At Alpine, we apply two patches to adjust the gofrontend signal handling code accordingly:
0033-gcc-go-Fix-handling-of-signal-34-on-musl.patch
0035-gcc-go-signal-34-is-special-on-musl-libc.patch
These two patches could IMHO be applied as is.
Definition of
off64_t
/loff_t
as a macromusl defines the non-standard
off64_t
andloff_t
types as a macro, not astypedef
. The gofrontend code uses these types unconditionally in a few places. Unfortunately,-fdump-go-spec
(andmksysinfo.sh
) are not able to correctly detect the presence of these types when they are defined as macros as not astypedef
. For this reason, they are absent from the Go type declaration file generated by-fdump-go-spec
and thus result in libgo compilation errors.At Alpine Linux downstream, we presently fix this via the
0041-libgo-Recognize-off64_t-and-loff_t-definitions-of-mu.patch
by#undef
'ing the macros and definingoff64_t
/loff_t
manually usingtypedef
. Alternatively, the libgo code could probably be rewritten to not rely on the presence of these types since they are IIRC non-standard (probably the cleaner solution).XSI-compliant
strerror_r()
When libgo is compiled on Linux, it is assumed that the GNU-extended version of
strerror_r(3)
is available for theErrstr()
implementation. However, musl only supports the standard XSI-compliant version ofstrerror_r(3)
. At Alpine Linux, we fix this via the0038-Use-generic-errstr.go-implementation-on-musl.patch
by unconditionally using the XSI-compliant version on Linux. If continued support for the GNU-extended version ofstrerror_r(3)
is desired it would probably be required to somehow detect musl in the Go//+ build
flags.Memory allocation issues on 32-bit arches
On 32-bit architectures gccgo's runtime has some weird memory allocation issues where it fails to allocate any memory at all with the following error message on musl at runtime:
I am presently not entirely sure what is causing this, but changing the
sysMmap
andmmap
offset function argument type fromuintptr
toint64
helps for some reason (see0044-gcc-go-Use-int64-type-as-offset-argument-for-mmap.patch
). This needs further investigation, any idea what might be causing this? On our 32-bit builders, we are also running into some out-of-memory issues when bootstrapping Go with gcc-go without settingGOMAXPROCS=1
which may or may not be related to this issue.libucontext related issues
musl itself does not support
swapcontext
,makecontext
, etc. These are instead provided by libucontext which requires a few minimal gccgo patches to link against libucontext and fix some minor quirks in this regard:Maybe it is possible to detect use of libucontext via GNU autotools and adjust the buildsystem configuration accordingly?
gcc issues
We also ran into two issues which are more closely related to gcc itself rather than gofrontend. These have already been fixed upstream. The remaining patches, which have not been integrated, are more closely related to gofrontend. According to my understanding of the gofrontend contribution guidelines I am hence opening this tracking issue here and not in the gcc bugtracker.
Would you be interested in including the discussed patches in gofrontend?
What did you expect to see?
Upstream gccgo musl support.
What did you see instead?
No support for musl libc in gccgo.
CC: @awilfox, @kaniini, @ianlancetaylor
The text was updated successfully, but these errors were encountered: