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/link, runtime: Invalid symbol table for cgo function #10747

Closed
DanielMorsing opened this issue May 7, 2015 · 6 comments

Comments

Projects
None yet
3 participants
@DanielMorsing
Copy link
Contributor

commented May 7, 2015

While running cgo tests for https://go-review.googlesource.com/#/c/9506/, I encountered a problem where the symbol table will return an invalid entry if queried for a cgo function. Even though we don't seem to lookup cgo functions in the symbol table right now, I'd expect it to return nil instead of an invalid entry. Reproducing is a case of applying the testing patch on top of 9506 and running GOTRACEBACK=3 go test -ldflags '-linkmode=internal' in misc/cgo/test. Fails about 50% of the time.

The Error

runtime: invalid pc-encoded table f=encoding/binary.(*bigEndian).GoString pc=0x5814b0 targetpc=0x582ef6 tab=[0/0]0x0
    value=0 until pc=0x58140a
    value=48 until pc=0x581499
    value=0 until pc=0x58149a
    value=48 until pc=0x5814b0
fatal error: invalid runtime symbol table

While the function for the PC 0x582ef6 is:

0000000000582ed0 <_cgo_4f27153e4087_Cfunc_issue7978c>:
  582ed0:       53                      push   %rbx
  582ed1:       48 8b 1f                mov    (%rdi),%rbx
  582ed4:       0f 1f 40 00             nopl   0x0(%rax)
  582ed8:       31 c0                   xor    %eax,%eax
  582eda:       f0 0f c1 03             lock xadd %eax,(%rbx)
  582ede:       85 c0                   test   %eax,%eax
  582ee0:       75 f6                   jne    582ed8 <_cgo_4f27153e4087_Cfunc_issue7978c+0x8>
  582ee2:       f0 83 03 01             lock addl $0x1,(%rbx)
  582ee6:       66 2e 0f 1f 84 00 00    nopw   %cs:0x0(%rax,%rax,1)
  582eed:       00 00 00 
  582ef0:       31 c0                   xor    %eax,%eax
  582ef2:       f0 0f c1 03             lock xadd %eax,(%rbx)
  582ef6:       83 f8 02                cmp    $0x2,%eax
  582ef9:       75 f5                   jne    582ef0 <_cgo_4f27153e4087_Cfunc_issue7978c+0x20>
  582efb:       e8 d0 ed ff ff          callq  581cd0 <issue7978cb>
  582f00:       f0 83 03 01             lock addl $0x1,(%rbx)
  582f04:       0f 1f 40 00             nopl   0x0(%rax)
  582f08:       31 c0                   xor    %eax,%eax
  582f0a:       f0 0f c1 03             lock xadd %eax,(%rbx)
  582f0e:       83 f8 06                cmp    $0x6,%eax
  582f11:       75 f5                   jne    582f08 <_cgo_4f27153e4087_Cfunc_issue7978c+0x38>
  582f13:       5b                      pop    %rbx
  582f14:       c3                      retq   
  582f15:       66 2e 0f 1f 84 00 00    nopw   %cs:0x0(%rax,%rax,1)
  582f1c:       00 00 00 
  582f1f:       90                      nop

This is the testing patch applied on top of CL9506:

diff --git a/misc/cgo/test/issue7978.go b/misc/cgo/test/issue7978.go
index 613f28e..45d0cfd 100644
--- a/misc/cgo/test/issue7978.go
+++ b/misc/cgo/test/issue7978.go
@@ -103,7 +103,7 @@ func test7978(t *testing.T) {
    if C.HAS_SYNC_FETCH_AND_ADD == 0 {
        t.Skip("clang required for __sync_fetch_and_add support on darwin/arm")
    }
-   if os.Getenv("GOTRACEBACK") != "2" {
+   if os.Getenv("GOTRACEBACK") != "3" {
        t.Fatalf("GOTRACEBACK must be 2")
    }
    issue7978sync = 0
diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go
index 1ed0995..c4bf486 100644
--- a/src/cmd/dist/test.go
+++ b/src/cmd/dist/test.go
@@ -442,7 +442,7 @@ func (t *tester) supportedBuildmode(mode string) bool {
 }

 func (t *tester) cgoTest() error {
-   env := mergeEnvLists([]string{"GOTRACEBACK=2"}, os.Environ())
+   env := mergeEnvLists([]string{"GOTRACEBACK=3"}, os.Environ())

    if t.goos == "android" || t.iOS() {
        cmd := t.dirCmd("misc/cgo/test", "go", "test")
@@ -450,6 +450,7 @@ func (t *tester) cgoTest() error {
        return cmd.Run()
    }

+   fmt.Println("misc/cgo/test", "go", "test", "-ldflags", "-linkmode=auto")
    cmd := t.dirCmd("misc/cgo/test", "go", "test", "-ldflags", "-linkmode=auto")
    cmd.Env = env
    if err := cmd.Run(); err != nil {
@@ -458,6 +459,7 @@ func (t *tester) cgoTest() error {

    if t.gohostos != "dragonfly" {
        // linkmode=internal fails on dragonfly since errno is a TLS relocation.
+       fmt.Println("misc/cgo/test", "go", "test", "-ldflags", "-linkmode=internal")
        cmd := t.dirCmd("misc/cgo/test", "go", "test", "-ldflags", "-linkmode=internal")
        cmd.Env = env
        if err := cmd.Run(); err != nil {
@@ -489,16 +491,19 @@ func (t *tester) cgoTest() error {
        "linux-386", "linux-amd64", "linux-arm",
        "netbsd-386", "netbsd-amd64":

+       fmt.Println("misc/cgo/test", "go", "test", "-ldflags", "-linkmode=external")
        cmd := t.dirCmd("misc/cgo/test", "go", "test", "-ldflags", "-linkmode=external")
        cmd.Env = env
        if err := cmd.Run(); err != nil {
            return err
        }
+       fmt.Println("misc/cgo/testtls", "go", "test", "-ldflags", "-linkmode=auto")
        cmd = t.dirCmd("misc/cgo/testtls", "go", "test", "-ldflags", "-linkmode=auto")
        cmd.Env = env
        if err := cmd.Run(); err != nil {
            return err
        }
+       fmt.Println("misc/cgo/testtls", "go", "test", "-ldflags", "-linkmode=external")
        cmd = t.dirCmd("misc/cgo/testtls", "go", "test", "-ldflags", "-linkmode=external")
        cmd.Env = env
        if err := cmd.Run(); err != nil {
diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index f725fc8..242b1c7 100644
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -108,6 +108,7 @@ func main() {
        // has a main, but it is not executed.
        return
    }
+   SetCPUProfileRate(100)
    main_main()
    if raceenabled {
        racefini()
diff --git a/src/runtime/proc1.go b/src/runtime/proc1.go
index d15c3b9..bb3e747 100644
--- a/src/runtime/proc1.go
+++ b/src/runtime/proc1.go
@@ -2623,6 +2623,10 @@ func setsSP(pc uintptr) bool {
        // so assume the worst and stop traceback
        return true
    }
+   if s := gotraceback(nil); s == 3 {
+       println(hex(pc))
+       println("foo", funcname(f))
+   }
    switch f.entry {
    case gogoPC, systemstackPC, mcallPC, morestackPC, asmcgocallPC,
        cgocallback_gofuncPC:
@minux

This comment has been minimized.

Copy link
Member

commented May 8, 2015

@DanielMorsing

This comment has been minimized.

Copy link
Contributor Author

commented May 12, 2015

Smaller replication at http://play.golang.org/p/vbF8LEsMsm

Prints

4516720
0x4bd8e8
sync/atomic.StoreUintptr

in internal mode
and

4198832
0x0

in auto mode.

@DanielMorsing

This comment has been minimized.

Copy link
Contributor Author

commented May 19, 2015

This bug is why the race detector builds are intermittently failing. I'll have a shot at this.

@DanielMorsing DanielMorsing added this to the Go1.5 milestone May 19, 2015

@DanielMorsing DanielMorsing self-assigned this May 19, 2015

@DanielMorsing

This comment has been minimized.

Copy link
Contributor Author

commented May 19, 2015

The simple fix of just not putting out pcln info for external functions didn't seem to work. Leaving this for people who know the linker.

@DanielMorsing DanielMorsing removed their assignment May 19, 2015

@rsc rsc changed the title cmd/internal/ld, runtime: Invalid symbol table for cgo function cmd/link, runtime: Invalid symbol table for cgo function Jun 8, 2015

@gopherbot

This comment has been minimized.

Copy link

commented Jun 27, 2015

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

@gopherbot gopherbot closed this in 214c7a2 Jun 29, 2015

@gopherbot

This comment has been minimized.

Copy link

commented Jun 29, 2015

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

rsc added a commit that referenced this issue Jun 29, 2015

cmd/link: record missing pcdata tables correctly
The old code was recording the current table output offset,
so the table from the next function would be used instead of
the runtime realizing that there was no table at all.

Add debug constant in runtime to check this for every function
at startup. It's too expensive to do that by default, but we can
do the last five functions. The end of the table is usually where
the C symbols end up, so that's where the problems typically are.

Fixes #10747.
Fixes #11396.

Change-Id: I13592e78017969fc22979fa902e19e1b151d41b1
Reviewed-on: https://go-review.googlesource.com/11657
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>

@golang golang locked and limited conversation to collaborators Jun 28, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.