Skip to content

cmd/link: cgo symbols can fall outside race detector mapped memory on darwin #17065

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.

Sign up for GitHub

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

Jump to bottom
Closed
josharian opened this issue Sep 11, 2016 · 10 comments
Closed

cmd/link: cgo symbols can fall outside race detector mapped memory on darwin #17065

josharian opened this issue Sep 11, 2016 · 10 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Darwin RaceDetector
Milestone
Go1.8

Comments

@josharian
Copy link
Contributor

josharian commented Sep 11, 2016 •
edited
Loading 

package main

// int ii[65537];
import "C"

var sink C.int

func main() {
    for i := range C.ii {
        sink = C.ii[i]
    }
}

panics on darwin when run under race detector. The problem is that ii is not grouped with other bss symbols.

See https://groups.google.com/forum/#!topic/golang-dev/UVBznzrj5S4 for much more discussion.

Possible fixes include (1) somehow getting the linked symbols organized correctly and (2) teaching the race detector to find all C symbols.

I'll send a CL with a disabled failing test shortly. I'd love help fixing this. I'm not even sure where to start.

cc @crawshaw @ianlancetaylor @mwhudson @dvyukov
@josharian josharian added this to the Go1.8 milestone Sep 11, 2016
@gopherbot
Copy link
Contributor

CL https://golang.org/cl/28964 mentions this issue.

gopherbot pushed a commit that referenced this issue Sep 11, 2016
@josharian
misc/cgo/test: add skipped test for issue 17065
aada57f 
Updates #17065

Change-Id: I113caced6de666a9b032ab2684ece79482aa7357
Reviewed-on: https://go-review.googlesource.com/28964
Run-TryBot: Josh Bleecher Snyder <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
@aclements
Copy link
Member

On ELF platforms, a program has runtime access to its own section table. If the same is true of Mach-O, we could consult that for the bounds of the C symbols.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/29077 mentions this issue.

josharian added a commit to josharian/go that referenced this issue Sep 13, 2016
@josharian
runtime: temporarily bump up tsan map shadow size
81d0ec1 
Submitting CL 26668 broke the darwin 10.8 and
10.10 builders. The underlying issue is unrelated.
This CL should make the builders green again until
we fix the real issue, at which point we can
revert it.

Updates golang#17065

Change-Id: I0ad0ce2ff1af6d515b8ce6184ddeabc49806950f
@josharian
Copy link
Contributor Author

When this is fixed, we should revert CL 29077.

gopherbot pushed a commit that referenced this issue Sep 14, 2016
@josharian
cmd/dist: skip broken cgo race tests on darwin
059ada5 
CL 26668 exposed #17065.
Skip the cgo race tests on darwin for now.

Updates #17065

Change-Id: I0ad0ce2ff1af6d515b8ce6184ddeabc49806950f
Reviewed-on: https://go-review.googlesource.com/29077
Run-TryBot: Josh Bleecher Snyder <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
@dvyukov
Copy link
Member

dvyukov commented Sep 20, 2016

@josharian can you provide a GOTRACEBACK=system and/or gdb/lldb stacks?

We filter out addresses without shadow memory:

func isvalidaddr(addr unsafe.Pointer) bool {
    return racearenastart <= uintptr(addr) && uintptr(addr) < racearenaend ||
        racedatastart <= uintptr(addr) && uintptr(addr) < racedataend
}
TEXT    racecalladdr<>(SB), NOSPLIT, $0-0
    get_tls(R12)
    MOVQ    g(R12), R14
    MOVQ    g_racectx(R14), RARG0   // goroutine context
    // Check that addr is within [arenastart, arenaend) or within [racedatastart, racedataend).
    CMPQ    RARG1, runtime·racearenastart(SB)
    JB  data
    CMPQ    RARG1, runtime·racearenaend(SB)
    JB  call
data:
    CMPQ    RARG1, runtime·racedatastart(SB)
    JB  ret
    CMPQ    RARG1, runtime·racedataend(SB)
    JAE ret

So I don't understand what tries to access these addresses.

@quentinmit quentinmit added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 7, 2016
@josharian
Copy link
Contributor Author

I can no longer reproduce using the standalone snippet at the top of this issue. I'm not sure why. However, in misc/cgo/test, after removing the t.Skip in issue17065.go:

$ GOTRACEBACK=system go test -run=17065 -race
unexpected fault address 0x2000133cd180
fatal error: fault
[signal SIGSEGV: segmentation violation code=0x1 addr=0x2000133cd180 pc=0x41774bc]

goroutine 18 [running]:
runtime.throw(0x7fff5fbff440, 0x0)
    /Users/josh/go/tip/src/runtime/panic.go:596 +0x95 fp=0x7fff5fbff460 sp=0x7fff5fbff440
runtime.throw(0x1f, 0x0)
    /Users/josh/go/tip/src/runtime/panic.go:596 +0x95 fp=0x7fff5fbff480 sp=0x7fff5fbff460
created by testing.(*T).Run
    /Users/josh/go/tip/src/testing/testing.go:674 +0x535

goroutine 1 [chan receive]:
runtime.gopark(0x42254d8, 0xc4200a2298, 0x421ac2f, 0xc, 0xc42001f317, 0x3)
    /Users/josh/go/tip/src/runtime/proc.go:261 +0xfd fp=0xc420051a60 sp=0xc420051a30
runtime.goparkunlock(0xc4200a2298, 0x421ac2f, 0xc, 0x17, 0x3)
    /Users/josh/go/tip/src/runtime/proc.go:267 +0x5e fp=0xc420051aa0 sp=0xc420051a60
runtime.chanrecv(0x41ce7c0, 0xc4200a2240, 0x0, 0xc420051b01, 0x41248b5)
    /Users/josh/go/tip/src/runtime/chan.go:496 +0x1ed fp=0xc420051b20 sp=0xc420051aa0
runtime.chanrecv1(0x41ce7c0, 0xc4200a2240, 0x0)
    /Users/josh/go/tip/src/runtime/chan.go:378 +0x35 fp=0xc420051b58 sp=0xc420051b20
testing.(*T).Run(0xc4200c0000, 0x421a283, 0x9, 0x4222540, 0x4314301)
    /Users/josh/go/tip/src/testing/testing.go:675 +0x573 fp=0xc420051c48 sp=0xc420051b58
testing.runTests.func1(0xc4200c0000)
    /Users/josh/go/tip/src/testing/testing.go:830 +0xab fp=0xc420051ca0 sp=0xc420051c48
testing.tRunner(0xc4200c0000, 0xc420051d88)
    /Users/josh/go/tip/src/testing/testing.go:637 +0xca fp=0xc420051ce0 sp=0xc420051ca0
testing.runTests(0x42250d0, 0x4319c20, 0x43, 0x43, 0x40355ac)
    /Users/josh/go/tip/src/testing/testing.go:836 +0x4a5 fp=0xc420051db8 sp=0xc420051ce0
testing.(*M).Run(0xc420051ef0, 0x403b2ba)
    /Users/josh/go/tip/src/testing/testing.go:771 +0x130 fp=0xc420051e98 sp=0xc420051db8
main.main()
    _/Users/josh/go/tip/misc/cgo/test/_test/_testmain.go:188 +0x1b9 fp=0xc420051f50 sp=0xc420051e98
runtime.main()
    /Users/josh/go/tip/src/runtime/proc.go:185 +0x1a3 fp=0xc420051fa0 sp=0xc420051f50
runtime.goexit()
    /Users/josh/go/tip/src/runtime/asm_amd64.s:2160 +0x1 fp=0xc420051fa8 sp=0xc420051fa0

goroutine 17 [syscall, locked to thread]:
runtime.goexit()
    /Users/josh/go/tip/src/runtime/asm_amd64.s:2160 +0x1 fp=0xc420040fb8 sp=0xc420040fb0

goroutine 2 [force gc (idle)]:
runtime.gopark(0x42254d8, 0x431f7d0, 0x421b4a2, 0xf, 0x402ec14, 0x1)
    /Users/josh/go/tip/src/runtime/proc.go:261 +0xfd fp=0xc42002c748 sp=0xc42002c718
runtime.goparkunlock(0x431f7d0, 0x421b4a2, 0xf, 0x14, 0x1)
    /Users/josh/go/tip/src/runtime/proc.go:267 +0x5e fp=0xc42002c788 sp=0xc42002c748
runtime.forcegchelper()
    /Users/josh/go/tip/src/runtime/proc.go:226 +0x9e fp=0xc42002c7c0 sp=0xc42002c788
runtime.goexit()
    /Users/josh/go/tip/src/runtime/asm_amd64.s:2160 +0x1 fp=0xc42002c7c8 sp=0xc42002c7c0
created by runtime.init.4
    /Users/josh/go/tip/src/runtime/proc.go:215 +0x35

goroutine 3 [GC sweep wait]:
runtime.gopark(0x42254d8, 0x431f9a0, 0x421ae7a, 0xd, 0x4021614, 0x1)
    /Users/josh/go/tip/src/runtime/proc.go:261 +0xfd fp=0xc42002cf38 sp=0xc42002cf08
runtime.goparkunlock(0x431f9a0, 0x421ae7a, 0xd, 0x14, 0x1)
    /Users/josh/go/tip/src/runtime/proc.go:267 +0x5e fp=0xc42002cf78 sp=0xc42002cf38
runtime.bgsweep(0xc42005c000)
    /Users/josh/go/tip/src/runtime/mgcsweep.go:63 +0xb6 fp=0xc42002cfb8 sp=0xc42002cf78
runtime.goexit()
    /Users/josh/go/tip/src/runtime/asm_amd64.s:2160 +0x1 fp=0xc42002cfc0 sp=0xc42002cfb8
created by runtime.gcenable
    /Users/josh/go/tip/src/runtime/mgc.go:213 +0x61

goroutine 4 [finalizer wait]:
runtime.gopark(0x42254d8, 0x4cecf68, 0x421b198, 0xe, 0x14, 0x1)
    /Users/josh/go/tip/src/runtime/proc.go:261 +0xfd fp=0xc42002d708 sp=0xc42002d6d8
runtime.goparkunlock(0x4cecf68, 0x421b198, 0xe, 0x14, 0x1)
    /Users/josh/go/tip/src/runtime/proc.go:267 +0x5e fp=0xc42002d748 sp=0xc42002d708
runtime.runfinq()
    /Users/josh/go/tip/src/runtime/mfinal.go:161 +0xaf fp=0xc42002d7c0 sp=0xc42002d748
runtime.goexit()
    /Users/josh/go/tip/src/runtime/asm_amd64.s:2160 +0x1 fp=0xc42002d7c8 sp=0xc42002d7c0
created by runtime.createfing
    /Users/josh/go/tip/src/runtime/mfinal.go:142 +0x62

goroutine 5 [syscall]:
runtime.notetsleepg(0x4ced1a0, 0xffffffffffffffff, 0x0)
    /Users/josh/go/tip/src/runtime/lock_sema.go:257 +0x4b fp=0xc42002df60 sp=0xc42002df20
os/signal.signal_recv(0x405ba71)
    /Users/josh/go/tip/src/runtime/sigqueue.go:116 +0x104 fp=0xc42002df88 sp=0xc42002df60
os/signal.loop()
    /Users/josh/go/tip/src/os/signal/signal_unix.go:22 +0x30 fp=0xc42002dfc0 sp=0xc42002df88
runtime.goexit()
    /Users/josh/go/tip/src/runtime/asm_amd64.s:2160 +0x1 fp=0xc42002dfc8 sp=0xc42002dfc0
created by os/signal.init.1
    /Users/josh/go/tip/src/os/signal/signal_unix.go:28 +0x4f

goroutine 6 [syscall, locked to thread]:
runtime.cgocall(0x41a3d60, 0xc42002e7b0, 0x0)
    /Users/josh/go/tip/src/runtime/cgocall.go:130 +0x8b fp=0xc42002e770 sp=0xc42002e738
_/Users/josh/go/tip/misc/cgo/test._Cfunc_usleep(0xc400002710, 0x0)
    _/Users/josh/go/tip/misc/cgo/test/_test/_obj_test/_cgo_gotypes.go:1746 +0x6b fp=0xc42002e7b0 sp=0xc42002e770
runtime.goexit()
    /Users/josh/go/tip/src/runtime/asm_amd64.s:2160 +0x1 fp=0xc42002e7b8 sp=0xc42002e7b0
created by _/Users/josh/go/tip/misc/cgo/test.lockOSThreadCallback
    /Users/josh/go/tip/misc/cgo/test/issue3775.go:37 +0x55
FAIL    _/Users/josh/go/tip/misc/cgo/test   0.099s

I remember from previous debugging that the crash is from within tsan_read, called as part of reading the contents of the C array.

@rsc
Copy link
Contributor

rsc commented Oct 25, 2016

golang.org/cl/31824 for the fix.

@dvyukov, I can't find the source code to __tsan_map_shadow. I checked out github.com/llvm-mirror/llvm and cannot find that string anywhere in the checked out files. For future reference, where is it? Thanks.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/31824 mentions this issue.

@ianlancetaylor
Copy link
Contributor

__tsan_map_shadow is in the https://github.com/llvm-mirror/compiler-rt repository in the file lib/tsan/go/tsan_go.cc. It's a trivial wrapper around MapShadow which is in lib/tsan/rtl/tsan_rtl.cc in the same repo.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/32160 mentions this issue.

llvm-beanz pushed a commit to llvm-beanz/llvm-submodules that referenced this issue Nov 1, 2016
@dvyukov
tsan: round Go shadow to page boundary
4e48c24 
There is a corner case reported in Go issue tracker:
golang/go#17065
On darwin data/bss segments may not be aligned to page bounary
and mmap seems to be behaving differently than on linux
(shrinks instead of enlarge unaligned regions).

Explicitly round shadow to page bounary before mapping
to avoid any such problems.

git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@285454 91177308-0d34-0410-b5e6-96231b3b80d8
@golang golang locked and limited conversation to collaborators Oct 30, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Darwin RaceDetector
Projects
None yet
Milestone
Go1.8
Development

No branches or pull requests
7 participants
@josharian @rsc @quentinmit @dvyukov @aclements @ianlancetaylor @gopherbot