Bug 79281 - gccgo: Binaries using goroutines crash on m68k
Summary: gccgo: Binaries using goroutines crash on m68k
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: go (show other bugs)
Version: 6.3.1
: P3 normal
Target Milestone: ---
Assignee: Ian Lance Taylor
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-30 11:03 UTC by John Paul Adrian Glaubitz
Modified: 2021-11-17 22:48 UTC (History)
4 users (show)

See Also:
Host:
Target: m68k-*-*
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
Proposed fix (512 bytes, patch)
2017-02-01 14:09 UTC, Jessica Clarke
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Paul Adrian Glaubitz 2017-01-30 11:03:26 UTC
After fixing #79037, I can compile and run Go programs on m68k now. However, there is still a problem when using Go routines:

(sid-m68k-sbuild)root@ikarus:/# cat routines.go
package main

import (
        "fmt"
        "time"
)

func say(s string) {
        for i := 0; i < 5; i++ {
                time.Sleep(100 * time.Millisecond)
                fmt.Println(s)
        }
}

func main() {
        go say("World")
        go say("Alice")
        go say("Bob")
        say("Hello")
}

(sid-m68k-sbuild)root@ikarus:/# gccgo-6 routines.go -o routines
(sid-m68k-sbuild)root@ikarus:/# ./routines
futexwakeup addr=0x90416c5e returned -1
fatal error: unexpected signal during runtime execution
[signal 0xb code=0x1 addr=0x1006]

goroutine 22 [syscall]:
runtime_dopanic
        ../../../src/libgo/runtime/panic.c:131
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
runtime_futexwakeup
        ../../../src/libgo/runtime/thread-linux.c:70
runtime_notewakeup
        ../../../src/libgo/runtime/lock_futex.c:121
handoffp
        ../../../src/libgo/runtime/proc.c:1518
runtime_entersyscallblock
        ../../../src/libgo/runtime/proc.c:2081
runtime_notetsleepg
        ../../../src/libgo/runtime/lock_futex.c:200
timerproc
        ../../../src/libgo/runtime/time.goc:263
kickoff
        ../../../src/libgo/runtime/proc.c:235
created by addtimer
        ../../../src/libgo/runtime/time.goc:147

goroutine 16 [running]:
        goroutine running on other thread; stack unavailable

goroutine 17 [syscall]:
        goroutine in C code; stack unavailable
created by runtime_main
        ../../../src/libgo/runtime/proc.c:598

goroutine 18 [finalizer wait]:
Hello
runtime_mcall
        ../../../src/libgo/runtime/proc.c:295
runtime_parkunlock
        ../../../src/libgo/runtime/proc.c:1887
runfinq
        ../../../src/libgo/runtime/mgc0.c:2512
kickoff
        ../../../src/libgo/runtime/proc.c:235
created by runtime_createfing
        ../../../src/libgo/runtime/mgc0.c:2577

goroutine 19 [sleep]:
runtime_mcall
        ../../../src/libgo/runtime/proc.c:295
runtime_parkunlock
        ../../../src/libgo/runtime/proc.c:1887
runtime_tsleep
        ../../../src/libgo/runtime/time.goc:97
time.Sleep
        ../../../src/libgo/runtime/time.goc:39
main.say
        //routines.go:10
kickoff
        ../../../src/libgo/runtime/proc.c:235
created by main.main
        //routines.go:16

goroutine 20 [sleep]:
runtime_mcall
        ../../../src/libgo/runtime/proc.c:295
runtime_parkunlock
        ../../../src/libgo/runtime/proc.c:1887
runtime_tsleep
        ../../../src/libgo/runtime/time.goc:97
time.Sleep
        ../../../src/libgo/runtime/time.goc:39
main.say
        //routines.go:10
kickoff
        ../../../src/libgo/runtime/proc.c:235
created by main.main
        //routines.go:17

goroutine 21 [sleep]:
runtime_mcall
        ../../../src/libgo/runtime/proc.c:295
runtime_parkunlock
        ../../../src/libgo/runtime/proc.c:1887
runtime_tsleep
        ../../../src/libgo/runtime/time.goc:97
time.Sleep
        ../../../src/libgo/runtime/time.goc:39
main.say
        //routines.go:10
kickoff
        ../../../src/libgo/runtime/proc.c:235
created by main.main
        //routines.go:18

goroutine 23 [GC sweep wait]:
runtime_mcall
        ../../../src/libgo/runtime/proc.c:295
runtime_parkunlock
        ../../../src/libgo/runtime/proc.c:1887
bgsweep
        ../../../src/libgo/runtime/mgc0.c:1844
kickoff
        ../../../src/libgo/runtime/proc.c:235
created by mgc
        ../../../src/libgo/runtime/mgc0.c:2215
(sid-m68k-sbuild)root@ikarus:/#

I have not looked into the problem in detail yet, but I assume it's either the 16-bit alignment striking again or maybe a missing syscall definition.

Let me know if there is something I can try to address the issue.
Comment 1 Andreas Schwab 2017-01-30 13:28:37 UTC
A futex word must be 4-byte aligned.
Comment 2 Ian Lance Taylor 2017-01-30 15:42:01 UTC
Perhaps this would be fixed by adding __attribute__((aligned(4))) to some field of struct Sched in proc.c.

This code is all different on trunk/GCC7.  I don't know whether it still has the same problem or not.  Probably it does but the fix would have to be different.
Comment 3 Jessica Clarke 2017-01-30 15:44:56 UTC
I believe the problem is in the "m" type in runtime2.go. There are 4 bools in a row, which is fine, as they will take up 4 bytes, but then "printlock" is an int8, which means "fastrand" will only be 2-byte aligned. This continues with "ncgocall" and "ncgo" being 2-byte aligned, and then the "park" field will also be 2-byte aligned only. Is there a way to tell gcngo that the "note" struct (and in theory the "mutex" struct too, since that will be a futex, and I don't know if anything embeds it incorrectly-aligned) should be 4-byte aligned, i.e. an equivalent of __attribute__((aligned(4)))?
Comment 4 Jessica Clarke 2017-01-30 15:52:32 UTC
Ah, sorry, there's a separate C implementation of all this. I imagine it's the bools in "struct M" in runtime.h messing it up, so "struct Note" and "struct Lock" need __attribute__((aligned(4))) on their key fields.
Comment 5 John Paul Adrian Glaubitz 2017-01-30 17:19:18 UTC
(In reply to James Clarke from comment #4)
> Ah, sorry, there's a separate C implementation of all this. I imagine it's
> the bools in "struct M" in runtime.h messing it up, so "struct Note" and
> "struct Lock" need __attribute__((aligned(4))) on their key fields.

I just did that on gcc-6.

The result is:

(sid-m68k-sbuild)root@ikarus:/# ./routines 
Alice
Bob
Hello
World
Alice
Bob
Hello
World
Alice
Bob
Hello
World
Alice
Bob
Hello
World
Alice
Bob
Hello
(sid-m68k-sbuild)root@ikarus:/#

:D

I haven't tested gcc-7 yet, but at least this fix for gcc-6 can be added.
Comment 6 John Paul Adrian Glaubitz 2017-02-01 08:28:31 UTC
On gcc-7, we actually can't build when enabling the Go frontend:

libtool: compile:  /<<PKGBUILDDIR>>/build/./gcc/xgcc -B/<<PKGBUILDDIR>>/build/./gcc/ -B/usr/m68k-linux-gnu/bin/ -B/usr/m68k-linux-gnu/lib/ -isystem /usr/m68k-linux-gnu/include -isystem /usr/m68k-linux-gnu/sys-include -isystem /<<PKGBUILDDIR>>/build/sys-include -DHAVE_CONFIG_H -I. -I../../../src/libgfortran -iquote../../../src/libgfortran/io -I../../../src/libgfortran/../gcc -I../../../src/libgfortran/../gcc/config -I../.././gcc -I../../../src/libgfortran/../libgcc -I../libgcc -I../../../src/libgfortran/../libbacktrace -I../libbacktrace -I../libbacktrace -std=gnu11 -Wall -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -Wextra -Wwrite-strings -Werror=implicit-function-declaration -Werror=vla -fcx-fortran-rules -ffunction-sections -fdata-sections -g -O2 -MT minloc1_16_r4.lo -MD -MP -MF .deps/minloc1_16_r4.Tpo -c ../../../src/libgfortran/generated/minloc1_16_r4.c -o minloc1_16_r4.o >/dev/null 2>&1
go1: internal compiler error: in write_specific_type_functions, at go/gofrontend/types.cc:2002
mmap: Permission denied
Please submit a full bug report,
with preprocessed source if appropriate.
See <file:///usr/share/doc/gcc-7/README.Bugs> for instructions.
Makefile:3338: recipe for target 'runtime/internal/sys.lo' failed

But first it would be great to get the gcc-6 issue resolved since it will be the default compiler until gcc-7 actually gets released.
Comment 7 Ian Lance Taylor 2017-02-01 13:54:52 UTC
It sounds like you have a patch for GCC 6.  If you send it in I can apply it.

The error you show must be from `make -j`, as compiling a file in libgfortran would not invoke go1.  What is the actual failure?
Comment 8 Jessica Clarke 2017-02-01 14:09:08 UTC
Created attachment 40645 [details]
Proposed fix

I believe this patch is what Adrian did; Adrian, can you please confirm?
Comment 9 John Paul Adrian Glaubitz 2017-02-01 14:10:07 UTC
Comment on attachment 40645 [details]
Proposed fix

Yes, this is the correct fix for gcc-6.
Comment 10 ian@gcc.gnu.org 2017-02-01 22:59:15 UTC
Author: ian
Date: Wed Feb  1 22:58:43 2017
New Revision: 245109

URL: https://gcc.gnu.org/viewcvs?rev=245109&root=gcc&view=rev
Log:
	PR go/79281

Force Lock and Note to be aligned to a 4 byte boundary.  This is
required by the kernel but is not enforced on m68k.

Patch by John Paul Adrian Glaubitz.

Modified:
    branches/gcc-6-branch/libgo/runtime/runtime.h
Comment 11 Ian Lance Taylor 2017-02-01 23:02:47 UTC
Thanks.  I committed the patch to the GCC 6 branch.

GCC 7 will require a different fix, as the code has moved from C to Go.  I'm not sure what the best approach is.
Comment 12 John Paul Adrian Glaubitz 2017-02-01 23:11:27 UTC
(In reply to Ian Lance Taylor from comment #11)
> Thanks.  I committed the patch to the GCC 6 branch.
> 
> GCC 7 will require a different fix, as the code has moved from C to Go.  I'm
> not sure what the best approach is.

Ok, thanks. We can look into this later.

Could you maybe also backport the fix for PR/79037? [1]

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79037
Comment 13 John Paul Adrian Glaubitz 2017-02-01 23:22:20 UTC
(In reply to Ian Lance Taylor from comment #11)
> GCC 7 will require a different fix, as the code has moved from C to Go.  I'm
> not sure what the best approach is.

Btw, even with the fixes from this PR/79281 and PR/79037, the "go" command is still crashing on m68k with gcc-6. It might be possible that this is the reason the build of gcc-7 fails for this reason, doesn't it?

The crash in "go" on gcc-6 seems to be related to Go's Mutex type as this code [1] crashes as well. I will file a separate bug report for that.

> [1] https://tour.golang.org/concurrency/9
Comment 14 Ian Lance Taylor 2017-02-01 23:37:33 UTC
> Could you maybe also backport the fix for PR/79037? [1]

Done.

> Btw, even with the fixes from this PR/79281 and PR/79037, the "go" command is still crashing on m68k with gcc-6. It might be possible that this is the reason the build of gcc-7 fails for this reason, doesn't it?

It could be related, but building GCC does not actually run the go command.
Comment 15 John Paul Adrian Glaubitz 2017-12-07 15:49:30 UTC
(In reply to Ian Lance Taylor from comment #11)
> Thanks.  I committed the patch to the GCC 6 branch.
> 
> GCC 7 will require a different fix, as the code has moved from C to Go.  I'm
> not sure what the best approach is.

Just as a heads-up: I just successfully built gcc-7 with the Go frontend enabled on m68k without any issues (gcc-7_7.2.0 - SVN r255408). It did not work when I tested a gcc-7 snapshot from 20170221.

I did not perform any extended tests yet though, so I don't know whether there are other issues left.
Comment 16 Jeffrey A. Law 2021-11-17 22:48:43 UTC
Fixed years ago.