Navigation Menu

Skip to content
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

cmd/cgo: malloc/free - deadlock on ARM #5227

Closed
gopherbot opened this issue Apr 6, 2013 · 16 comments
Closed

cmd/cgo: malloc/free - deadlock on ARM #5227

gopherbot opened this issue Apr 6, 2013 · 16 comments
Milestone

Comments

@gopherbot
Copy link

by capnm9:

uname -a
Linux rpi 3.6.11+ #371 PREEMPT Thu Feb 7 16:31:35 GMT 2013 armv6l GNU/Linux

A simple app with RPi specific OpenVG bindings: 

GOTRACEBACK=3 ./bin/clock 
fatal error: malloc/free - deadlock
[signal 0xb code=0x1 addr=0x2f0 pc=0x2d4dc]

goroutine 1 [syscall]:
[fp=0xb6899ee4] return()
    /data4/go-bs/src/pkg/runtime/asm_arm.s:275
[fp=0xb6899f0c] runtime.cgocall(0xb861c, 0xb6899f1c)
    /data4/go-bs/src/pkg/runtime/cgocall.c:162 +0xec
[fp=0xb6899f18] openvg._Cfunc_init(0x102001a8, 0x102001a0)
    openvg/_obj/_cgo_defun.c:328 +0x30
[fp=0xb6899f2c] openvg.Init(0x0, 0x17fac)
    openvg/_obj/_cgo_gotypes.go:296 +0x58
[fp=0xb6899fb8] main.main()
    /home/pi/s/w/github/capnm/go_rpi/src/clock/clock.go:15 +0x28
[fp=0xb6899fd0] runtime.main()
    /data4/go-bs/src/pkg/runtime/proc.c:182 +0x78
[fp=0xb6899fd4] runtime.goexit()
    /data4/go-bs/src/pkg/runtime/proc.c:1214

goroutine 2 [syscall]:
runtime.goexit()
    /data4/go-bs/src/pkg/runtime/proc.c:1214

goroutine 3 [syscall]:
runtime.entersyscallblock()
    /data4/go-bs/src/pkg/runtime/proc.c:1324 +0xc0
runtime.MHeap_Scavenger()
    /data4/go-bs/src/pkg/runtime/mheap.c:434 +0xfc
runtime.goexit()
    /data4/go-bs/src/pkg/runtime/proc.c:1214
created by runtime.main
    /data4/go-bs/src/pkg/runtime/proc.c:165


Unfortunately this commit additionally broke the arm build,
but with the fix applied it appears to be a the
first bad commit:

https://code.google.com/p/go/source/detail?name=95c3a7bdfb03&;r=97cbf15abc2c18b2616349d1f936cd6e7a584b76
cmd/ld: replace -hostobj with -linkmode
Still disabled. Need to fix TLS.

fix
https://code.google.com/p/go/source/detail?name=95c3a7bdfb03&;r=31ae6d73fdb0b39373980163598dbad85ba0afb7

strace, gdb, etc.:
https://github.com/capnm/golang/wiki/bisect-clock-app
@remyoudompheng
Copy link
Contributor

Comment 1:

Please add to the gdb output the back trace of the segfault (ideas and registers are
extra, backtrace is really the first thing to have).
also, its better if the code is made available somewhere.

@gopherbot
Copy link
Author

Comment 2 by capnm9:

> Please add to the gdb output the back trace of the segfault (ideas and registers are >
extra, backtrace is really the first thing to have).
> also, its better if the code is made available somewhere.
> Needs the Raspberry Pi hardware.
> https://github.com/capnm/go_rpi
$ git clone git://github.com/capnm/go_rpi.git && cd go_rpi
$ go install -v circle
$ bin/circle
The back trace was not really helpful:
Program received signal SIGSEGV, Segmentation fault.
fast_loop () at memcpy.S:144
144 memcpy.S: No such file or directory.
(gdb) bt
#0  fast_loop () at memcpy.S:144
#1  0x000b5ea8 in init ()
#2  0xbefff5e0 in ?? ()
Cannot access memory at address 0x39fffff8

@gopherbot
Copy link
Author

Comment 3 by capnm9:

back traces from all threads:
https://github.com/capnm/go_rpi/wiki/OpenVG-bindigs-or-cgo-bug

@gopherbot
Copy link
Author

Comment 4 by capnm9:

Extracted a minimal code, it seems to be ARM specific.
The first bad commit is
https://code.google.com/p/go/source/detail?name=95c3a7bdfb03&r=97cbf15abc2c18b2616349d1f936cd6e7a584b76
git clone git://github.com/capnm/go_rpi.git && cd go_rpi
go install issue5227
bin/issue5227
   > fatal error: malloc/free - deadlock ...
$cat main.go
package main
//#include "init.h"
import "C"
func main() {
    C.init()
}
func selectfont() C.Fontinfo {
    return C.SansTypeface
}
$cat init.h
typedef struct {
    int Count;
} Fontinfo;
Fontinfo SansTypeface;
extern void init();
cat init.c
#include "init.h"
Fontinfo loadfont() {
    Fontinfo f;
    return f;
}
void init() {
    SansTypeface = loadfont();
}

@minux
Copy link
Member

minux commented Apr 8, 2013

Comment 5:

single file reproducer:
http://play.golang.org/p/2s-lb2fA8Q

Labels changed: added priority-soon, go1.1, cgo, arch-arm, removed priority-triage.

Status changed to Accepted.

@gopherbot
Copy link
Author

Comment 6 by capnm9:

fix?:
diff --git a/src/cmd/ld/go.c b/src/cmd/ld/go.c
index fa2ec4e..38084ca 100644
--- a/src/cmd/ld/go.c
+++ b/src/cmd/ld/go.c
@@ -476,10 +476,12 @@ loadcgo(char *file, char *pkg, char *p, int n)
                if(strcmp(f[0], "cgo_import_static") == 0) {
                        if(nf != 2)
                                goto err;
-                       local = f[1];
-                       s = lookup(local, 0);
-                       s->type = SHOSTOBJ;
-                       s->size = 0;
+                       if(linkmode == LinkExternal) {
+                               local = f[1];
+                               s = lookup(local, 0);
+                               s->type = SHOSTOBJ;
+                               s->size = 0;
+                       }
                        continue;
                }

@remyoudompheng
Copy link
Contributor

Comment 7:

No this does not look like the correct fix, because it would break linkmode=auto.
iant: I can find two possible ways of fixing:
Either this:
--- a/src/cmd/ld/ldelf.c    Tue Apr 09 12:41:58 2013 +0900
+++ b/src/cmd/ld/ldelf.c    Tue Apr 09 07:40:40 2013 +0200
@@ -571,7 +571,7 @@
            s = sym.sym;
            if(s->size < sym.size)
                s->size = sym.size;
-           if(s->type == 0 || s->type == SXREF)
+           if(s->type == 0 || s->type == SXREF || s->type == SHOSTOBJ)
                s->type = SBSS;
            continue;
        }
Or this:
diff -r c40f15e9e5ca src/cmd/ld/lib.c
--- a/src/cmd/ld/lib.c  Tue Apr 09 12:41:58 2013 +0900
+++ b/src/cmd/ld/lib.c  Tue Apr 09 07:40:40 2013 +0200
@@ -311,13 +311,14 @@
    // Switch to internal.
    if(linkmode == LinkAuto) {
        linkmode = LinkInternal;
-       // Drop all the cgo_import_static declarations.
-       // Turns out we won't be needing them.
-       for(s = allsym; s != S; s = s->allsym)
-           if(s->type == SHOSTOBJ)
-               s->type = 0;
    }
-   
+
+   // Drop all the cgo_import_static declarations.
+   // Turns out we won't be needing them.
+   for(s = allsym; s != S; s = s->allsym)
+       if(s->type == SHOSTOBJ)
+           s->type = 0;
+
    // Now that we know the link mode, trim the dynexp list.
    x = CgoExportDynamic;
    if(linkmode == LinkExternal)
Which one do you think is better (if any is OK at all).

@remyoudompheng
Copy link
Contributor

Comment 8:

Also this is not related to ARM in particular: the single file example given by minux
will produce garbage on amd64 too (either SIGSEGV or executable format error) if you use
-ldflags "-linkmode internal".

Labels changed: removed arch-arm.

@mikioh
Copy link
Contributor

mikioh commented Apr 9, 2013

Comment 9:

issue #5223 is the same?

@mikioh
Copy link
Contributor

mikioh commented Apr 9, 2013

Comment 10:

(again) issue #5233 is the same?

@remyoudompheng
Copy link
Contributor

Comment 11:

issue #5233 is a memory issue, this one is a linker bug.

@ianlancetaylor
Copy link
Contributor

Comment 12:

I think the one-line change to ldelf.c may only be correct in internal mode.  In
external mode we want it to remain a common symbol.
I think that resetting the symbol types in lib.c is correct if linkmode == LinkInternal.
 I think that is probably the best fix if it works.

@remyoudompheng
Copy link
Contributor

Comment 13:

There is also a cosmetic issue: the following crashy program
package main
/*
int *foo = (int*)0;
void crash() {
      *foo = 0;
}
*/
import "C"
func main() {
      defer func() {
            x := recover()
            println(x)
      }()
      C.crash()
}
On amd64 prints:
SIGSEGV: segmentation violation
PC=0x41c9aa
signal arrived during cgo execution
main._Cfunc_crash(0x40fd02)
    command-line-arguments/_obj/_cgo_defun.c:43 +0x2f
main.main()
    command-line-arguments/_obj/main.cgo1.go:14 +0x2b
goroutine 2 [syscall]:
goroutine 3 [runnable]:
rax     0x0
[...]
On arm prints a confusing message:
fatal error: malloc/free - deadlock
[signal 0xb code=0x1 addr=0x2f0 pc=0x28fcc]
goroutine 1 [syscall]:
[fp=0xb6964f80] return()
    /storage/remy/go/src/pkg/runtime/asm_arm.s:275
[fp=0xb6964fa8] runtime.cgocall(0x2e1f4, 0xb6964fb8)
    /storage/remy/go/src/pkg/runtime/cgocall.c:162 +0xec
[fp=0xb6964fb4] main._Cfunc_crash(0x22b1c)
    command-line-arguments/_obj/_cgo_defun.c:43 +0x30
[fp=0xb6964fb8] main.main()
    command-line-arguments/_obj/main.cgo1.go:14 +0x48
[fp=0xb6964fd0] runtime.main()
    /storage/remy/go/src/pkg/runtime/proc.c:182 +0x78
[fp=0xb6964fd4] runtime.goexit()
    /storage/remy/go/src/pkg/runtime/proc.c:1223
goroutine 2 [syscall]:
exit status 2

@ajstarks
Copy link
Contributor

Comment 14:

https://golang.org/cl/8602044/ fixes issue #5114 which I believe is a duplicate
of this issue.

@remyoudompheng
Copy link
Contributor

Comment 15:

This issue was closed by revision 0e76a94.

Status changed to Fixed.

@kisielk
Copy link
Contributor

kisielk commented Apr 17, 2013

Comment 16:

I'm seeing this on my amd64 ubuntu system with 58f8a30f5b78 while running all.bash:
# _/home/kamil/go/misc/cgo/test
issue5227.go: In function 'loadfont':
issue5227.go:21:9: error: 'f.Count' is used uninitialized in this function
[-Werror=uninitialized]
cc1: all warnings being treated as errors
FAIL    _/home/kamil/go/misc/cgo/test [build failed]
# _/home/kamil/go/misc/cgo/test
issue5227.go: In function 'loadfont':
issue5227.go:21:9: error: 'f.Count' is used uninitialized in this function
[-Werror=uninitialized]
cc1: all warnings being treated as errors
FAIL    _/home/kamil/go/misc/cgo/test [build failed]
# _/home/kamil/go/misc/cgo/test
issue5227.go: In function 'loadfont':
issue5227.go:21:9: error: 'f.Count' is used uninitialized in this function
[-Werror=uninitialized]
cc1: all warnings being treated as errors
FAIL    _/home/kamil/go/misc/cgo/test [build failed]
is this expected?

@rsc rsc added this to the Go1.1 milestone Apr 14, 2015
@rsc rsc removed the go1.1 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants