[Bug go/79037] gccgo: Binaries crash with parforsetup: pos is not aligned on m68k
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79037 --- Comment #14 from ian at gcc dot gnu.org --- Author: ian Date: Wed Feb 1 23:35:59 2017 New Revision: 245110 URL: https://gcc.gnu.org/viewcvs?rev=245110=gcc=rev Log: PR go/79037 Backport from mainline: compiler, runtime: align gc data for m68k The current GC requires that the gc data be aligned to at least a 4 byte boundary, because it uses the lower two bits of the address for flags (see LOOP and PRECISE in runtime/mgc0.c). As the gc data is stored as a [...]uintptr, that is normally always true. However, on m68k, that only guarantees 2 byte alignment. Fix it by forcing the alignment. The parfor code used by the current GC requires that the parfor data be aligned to at least an 8 byte boundary. The code in parfor.c verifies this. This is normally true, as the data uses uint64_t values, but, again, this must be enforced explicitly on m68k. Fixes GCC PR 79037. Change-Id: Ifdf422db7b37e88f490e54c2f6d249117d359dd6 Modified: branches/gcc-6-branch/gcc/go/gofrontend/types.cc branches/gcc-6-branch/libgo/runtime/go-unsafe-pointer.c branches/gcc-6-branch/libgo/runtime/parfor.c branches/gcc-6-branch/libgo/runtime/runtime.h
[Bug go/79037] gccgo: Binaries crash with parforsetup: pos is not aligned on m68k
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79037 Ian Lance Taylor changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED --- Comment #13 from Ian Lance Taylor --- Fixed.
[Bug go/79037] gccgo: Binaries crash with parforsetup: pos is not aligned on m68k
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79037 --- Comment #12 from ian at gcc dot gnu.org --- Author: ian Date: Mon Jan 23 18:15:22 2017 New Revision: 244824 URL: https://gcc.gnu.org/viewcvs?rev=244824=gcc=rev Log: PR go/79037 compiler, runtime: align gc data for m68k The current GC requires that the gc data be aligned to at least a 4 byte boundary, because it uses the lower two bits of the address for flags (see LOOP and PRECISE in runtime/mgc0.c). As the gc data is stored as a [...]uintptr, that is normally always true. However, on m68k, that only guarantees 2 byte alignment. Fix it by forcing the alignment. The parfor code used by the current GC requires that the parfor data be aligned to at least an 8 byte boundary. The code in parfor.c verifies this. This is normally true, as the data uses uint64_t values, but, again, this must be enforced explicitly on m68k. Fixes GCC PR 79037. Reviewed-on: https://go-review.googlesource.com/35478 Modified: trunk/gcc/go/gofrontend/MERGE trunk/gcc/go/gofrontend/types.cc trunk/libgo/runtime/go-unsafe-pointer.c trunk/libgo/runtime/parfor.c trunk/libgo/runtime/runtime.h
[Bug go/79037] gccgo: Binaries crash with parforsetup: pos is not aligned on m68k
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79037
--- Comment #11 from John Paul Adrian Glaubitz ---
Testing the latest patchset from Gerrit [1], I can confirm the bug is now
fixed:
(sid-m68k-sbuild)root@jessie64:~# cat hello-world.go
package main
import "fmt"
func main() {
fmt.Println("Hello World!")
}
(sid-m68k-sbuild)root@jessie64:~# gccgo-6 hello-world.go -o hello-world
(sid-m68k-sbuild)root@jessie64:~# ./hello-world
Hello World!
(sid-m68k-sbuild)root@jessie64:~#
> [1] https://go-review.googlesource.com/#/c/35478/
[Bug go/79037] gccgo: Binaries crash with parforsetup: pos is not aligned on m68k
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79037 --- Comment #10 from Michael Karcher --- OK, I got it. I retract my last comment.
[Bug go/79037] gccgo: Binaries crash with parforsetup: pos is not aligned on m68k
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79037 --- Comment #9 from Ian Lance Taylor --- The backend.h interface says that if you set the alignment, that is the alignment. I could change it to the GCC version--you can only increase the alignment--but I'd rather keep the backend.h interface simple.
[Bug go/79037] gccgo: Binaries crash with parforsetup: pos is not aligned on m68k
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79037 --- Comment #8 from Michael Karcher --- The patch looks like it should work fine, I guess John Paul Adrian Glaubitz is going to test it soon. But I wonder whether the determination of alignment is in types.cc really needed, as user-specified alignment directives can only make alignment stricter. So always passing 4 to implicit variable should do no harm and keep the code simpler.
[Bug go/79037] gccgo: Binaries crash with parforsetup: pos is not aligned on m68k
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79037 --- Comment #7 from Ian Lance Taylor --- Can someone with m68k hardware please test the patch at https://golang.org/cl/35478? Thanks. (To download just the patch as a zip file: https://go-review.googlesource.com/changes/35478/revisions/1/patch?zip).
[Bug go/79037] gccgo: Binaries crash with parforsetup: pos is not aligned on m68k
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79037 --- Comment #6 from Ian Lance Taylor --- I don't think it's the type descriptors that need to be aligned, I think it's just the GC symbol pointers. Those are the ones whose names end in "$gc" in the list above.
[Bug go/79037] gccgo: Binaries crash with parforsetup: pos is not aligned on m68k
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79037 --- Comment #5 from Michael Karcher --- The root issue now is that the ABI gcc implements on m68k is incompatible with the Go runtime shipped with gcc. The Go runtime uses the lowest two bits in the type information pointer as flags (called PRECISE and LOOP) and relies on the fact that the actual type information structure is aligned to a 32-bit boundary. The type descriptors are Go literals created by the Go frontend as standard Go structures. In the gcc backend for Go, the layout and alignment rules of Go structures match the rules for C structures (which is most likely necessary for interoperability). On m68k this means we get 16-bit alignment for historical reasons. The current interface between the go frontend and its backend has no interface to request stricter alignment, so the Go frontend is unable to ensure that type descriptors are aligned to 32-bit boundaries. A possible way of solving this would be to extend Backend::struct_type(vector<...> fields) by an (optional?) second parameter to communicate alignment requests, such that Gcc_backend::struct_type can use DECL_ALIGN before calling fill_in_struct. This enables the Go frontend to request the required alignment for its type descriptors. Requesting increased alignment for Go type descriptors is impossible to break legacy ABIs on m68k as there is no ABI involving Go type descriptors yet. This change should possibly go align with adding "__attribute__((aligned(4)))" to several types in libgo/runtime/go-type.h if type descriptors are ever created from the C side of things. Proof for the issue: (sid-m68)root:~# nm --dynamic /usr/lib/m68k-linux-gnu/libgo.so.9 | grep __go_td | head -n 6 00bcc410 V __go_td_AAAN5_uint8ee1e 00bcc440 V __go_td_AAAN5_uint8ee1e$gc 00bd0058 V __go_td_AAAN5_uint8eee 00bd0080 V __go_td_AAAN5_uint8eee$gc 00bfc4e6 V __go_td_AAAN6_uint329e3e16e 00bfc5d2 V __go_td_AAAN6_uint329e3e16e$gc shows unaligned type descriptors.
[Bug go/79037] gccgo: Binaries crash with parforsetup: pos is not aligned on m68k
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79037 --- Comment #4 from John Paul Adrian Glaubitz --- Disabling the Go garbage collector fixes this particular crash: (sid-m68k-sbuild)root@jessie64:~# GOGC=off ./hello-world Hello World! (sid-m68k-sbuild)root@jessie64:~# So it seems GOGC is not working properly on m68k.
[Bug go/79037] gccgo: Binaries crash with parforsetup: pos is not aligned on m68k
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79037 --- Comment #3 from John Paul Adrian Glaubitz --- (In reply to John Paul Adrian Glaubitz from comment #2) > (In reply to John Paul Adrian Glaubitz from comment #1) > > I'll report back tomorrow. > > Problem persists: Correction: The patch fixes the alignment problem, but it exposes another bug: root@mama:~# gccgo-6 hello-world.go -o hello-world2 root@mama:~# ./hello-world2 fatal error: unexpected signal during runtime execution [signal 0xb code=0x2 addr=0xa5eec0cf] runtime stack: runtime_dopanic ../../../src/libgo/runtime/panic.c:135 runtime_throw ../../../src/libgo/runtime/panic.c:193 sig_panic_leadin ../../../src/libgo/runtime/go-signal.c:249 sig_panic_info_handler ../../../src/libgo/runtime/go-signal.c:283 :0 scanblock ../../../src/libgo/runtime/mgc0.c:1117 markroot ../../../src/libgo/runtime/mgc0.c:1368 runtime_parfordo ../../../src/libgo/runtime/parfor.c:93 gc ../../../src/libgo/runtime/mgc0.c:2270 mgc ../../../src/libgo/runtime/mgc0.c:2215 runtime_mstart ../../../src/libgo/runtime/proc.c:1076 goroutine 16 [garbage collection]: runtime_mcall ../../../src/libgo/runtime/proc.c:295 runtime_gc ../../../src/libgo/runtime/mgc0.c:2191 runtime_mallocgc ../../../src/libgo/runtime/malloc.goc:259 __go_new ../../../src/libgo/runtime/go-new.c:16 main.main /root/hello-world.go:6 runtime_main ../../../src/libgo/runtime/proc.c:626 goroutine 17 [runnable]: kickoff ../../../src/libgo/runtime/proc.c:232 created by runtime_main ../../../src/libgo/runtime/proc.c:598 goroutine 18 [runnable]: kickoff ../../../src/libgo/runtime/proc.c:232 created by runtime_createfing ../../../src/libgo/runtime/mgc0.c:2577 root@mama:~# Looks like we're missing a signal handler on m68k.
[Bug go/79037] gccgo: Binaries crash with parforsetup: pos is not aligned on m68k
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79037 --- Comment #2 from John Paul Adrian Glaubitz --- (In reply to John Paul Adrian Glaubitz from comment #1) > I'll report back tomorrow. Problem persists: root@mama:~# ./hello-world fatal error: parforsetup: pos is not aligned runtime stack: runtime_dopanic ../../../src/libgo/runtime/panic.c:135 runtime_throw ../../../src/libgo/runtime/panic.c:193 runtime_parforsetup ../../../src/libgo/runtime/parfor.c:63 gc ../../../src/libgo/runtime/mgc0.c:2259 mgc ../../../src/libgo/runtime/mgc0.c:2215 runtime_mstart ../../../src/libgo/runtime/proc.c:1076 goroutine 16 [garbage collection]: runtime_mcall ../../../src/libgo/runtime/proc.c:295 runtime_gc ../../../src/libgo/runtime/mgc0.c:2191 runtime_mallocgc ../../../src/libgo/runtime/malloc.goc:259 __go_new ../../../src/libgo/runtime/go-new.c:16 main.main /root/hello-world.go:6 runtime_main ../../../src/libgo/runtime/proc.c:626 goroutine 17 [runnable]: kickoff ../../../src/libgo/runtime/proc.c:232 created by runtime_main ../../../src/libgo/runtime/proc.c:598 goroutine 18 [runnable]: kickoff ../../../src/libgo/runtime/proc.c:232 created by runtime_createfing ../../../src/libgo/runtime/mgc0.c:2577 root@mama:~#
[Bug go/79037] gccgo: Binaries crash with parforsetup: pos is not aligned on m68k
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79037 --- Comment #1 from John Paul Adrian Glaubitz --- Rebuilding gcc with the following change now: --- a/src/libgo/runtime/runtime.h.old 2016-02-12 23:10:09.0 +0100 +++ b/src/libgo/runtime/runtime.h 2017-01-10 00:12:08.404802087 +0100 @@ -429,7 +429,7 @@ uint32 cnt; // iteration space [0, cnt) bool wait; // if true, wait while all threads finish processing, // otherwise parfor may return while other threads are still working - ParForThread *thr; // array of thread descriptors + ParForThread *thr __attribute__((aligned(4))); // array of thread descriptors // stats uint64 nsteal; uint64 nstealcnt; I'll report back tomorrow.
