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.

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

reflect: tests fail with -linkshared #19003

Closed
crawshaw opened this issue Feb 9, 2017 · 10 comments
Closed

reflect: tests fail with -linkshared #19003

crawshaw opened this issue Feb 9, 2017 · 10 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.8Maybe

Comments

@crawshaw
Copy link
Member

crawshaw commented Feb 9, 2017

On tip:

$ go install -buildmode=shared runtime sync/atomic
$ go test -short -linkshared reflect
runtime: typeOff 0x289e0 base 0xc4203bbdc0 not in ranges:
	types 0x7ff1e846ad00 etypes 0x7ff1e84b4b10
	types 0xaaa6a0 etypes 0xd841a0
fatal error: runtime: type offset base pointer out of range

goroutine 75 [running]:
runtime.throw(0x7ff1e82656dc, 0x2e)
	/usr/local/google/home/crawshaw/go/src/runtime/panic.go:596 +0x97 fp=0xc42009dd18 sp=0xc42009dcf8
runtime.resolveTypeOff(0xc4203bbdc0, 0x289e0, 0xc420128750)
	/usr/local/google/home/crawshaw/go/src/runtime/type.go:223 +0x356 fp=0xc42009dd78 sp=0xc42009dd18
runtime.(*_type).typeOff(0xc4203bbdc0, 0x289e0, 0xc42009dde0)
	/usr/local/google/home/crawshaw/go/src/runtime/type.go:239 +0x35 fp=0xc42009dda0 sp=0xc42009dd78
reflect.resolveTypeOff(0xc4203bbdc0, 0x289e0, 0x0)
	/usr/local/google/home/crawshaw/go/src/runtime/runtime1.go:515 +0x35 fp=0xc42009ddc8 sp=0xc42009dda0
reflect.(*rtype).typeOff(0xc4203bbdc0, 0xc4000289e0, 0xc42009de88)
	/usr/local/google/home/crawshaw/go/src/reflect/type.go:679 +0x35 fp=0xc42009ddf0 sp=0xc42009ddc8
reflect.(*rtype).ptrTo(0xc4203bbdc0, 0xc4203bbdc0)
	/usr/local/google/home/crawshaw/go/src/reflect/type.go:1430 +0x57b fp=0xc42009dec0 sp=0xc42009ddf0
reflect.PtrTo(0xe23120, 0xc4203bbdc0, 0xe23120, 0xc4203bbdc0)
	/usr/local/google/home/crawshaw/go/src/reflect/type.go:1425 +0x47 fp=0xc42009dee8 sp=0xc42009dec0
reflect_test.TestPtrTo(0xc42012de10)
	/usr/local/google/home/crawshaw/go/src/reflect/all_test.go:2485 +0xa1 fp=0xc42009dfa8 sp=0xc42009dee8
testing.tRunner(0xc42012de10, 0xd808f8)
	/usr/local/google/home/crawshaw/go/src/testing/testing.go:657 +0xa0 fp=0xc42009dfd0 sp=0xc42009dfa8
runtime.goexit()
	/usr/local/google/home/crawshaw/go/src/runtime/asm_amd64.s:2197 +0x1 fp=0xc42009dfd8 sp=0xc42009dfd0
created by testing.(*T).Run
	/usr/local/google/home/crawshaw/go/src/testing/testing.go:697 +0x2e4

goroutine 1 [chan receive]:
testing.(*T).Run(0xc4200d3110, 0x63a653, 0x9, 0xd808f8, 0xc4200add01)
	/usr/local/google/home/crawshaw/go/src/testing/testing.go:698 +0x30e
testing.runTests.func1(0xc4200d3110)
	/usr/local/google/home/crawshaw/go/src/testing/testing.go:882 +0x69
testing.tRunner(0xc4200d3110, 0xc4200adde0)
	/usr/local/google/home/crawshaw/go/src/testing/testing.go:657 +0xa0
testing.runTests(0xc42000d540, 0xe1c940, 0x85, 0x85, 0xc420011970)
	/usr/local/google/home/crawshaw/go/src/testing/testing.go:888 +0x2ee
testing.(*M).Run(0xc4200adf20, 0xc4200adf20)
	/usr/local/google/home/crawshaw/go/src/testing/testing.go:822 +0x109
main.main()
	reflect/_test/_testmain.go:334 +0x118

goroutine 17 [syscall, locked to thread]:
runtime.goexit()
	/usr/local/google/home/crawshaw/go/src/runtime/asm_amd64.s:2197 +0x1

goroutine 38 [sleep]:
time.Sleep(0x3b9aca00)
	/usr/local/google/home/crawshaw/go/src/runtime/time.go:59 +0x105
reflect_test.selectWatcher()
	/usr/local/google/home/crawshaw/go/src/reflect/all_test.go:1416 +0x3e
created by reflect_test.TestSelect.func1
	/usr/local/google/home/crawshaw/go/src/reflect/all_test.go:1146 +0x37
FAIL	reflect	0.089s

May just be a bogus test.

First seen on #16602.
@bradfitz bradfitz added this to the Go1.8Maybe milestone Feb 9, 2017
@bradfitz bradfitz added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 9, 2017
@bradfitz
Copy link
Contributor

bradfitz commented Feb 9, 2017

/cc @mwhudson @ianlancetaylor

@mwhudson
Copy link
Contributor

mwhudson commented Feb 9, 2017

So we have a type that lives on the heap ("base 0xc4203bbdc0") but has a non-zero ptrToThis? How does that happen?

@mwhudson
Copy link
Contributor

mwhudson commented Feb 9, 2017

Oh because when PtrTo needs to create a new type it starts by copying *unsafe.Pointer, which has a non-zero ptrToThis. One line fix incoming.

@mwhudson
Copy link
Contributor

mwhudson commented Feb 9, 2017

Well no, because the obvious fix, to whit this:

(master *)mwhudson@aeglos:/opt/opensource/go/src$ git diff
diff --git a/src/reflect/type.go b/src/reflect/type.go
index fbfda3a..4085909 100644
--- a/src/reflect/type.go
+++ b/src/reflect/type.go
@@ -1463,6 +1463,7 @@ func (t *rtype) ptrTo() *rtype {
        pp := *prototype
 
        pp.str = resolveReflectName(newName(s, "", "", false))
+       pp.ptrToThis = 0
 
        // For the type structures linked into the binary, the
        // compiler provides a good hash of the string.

causes this failure:

(master *)mwhudson@aeglos:/opt/opensource/go/src$ go test -short -linkshared reflect 
--- FAIL: TestTypelinksSorted (0.00s)
	all_test.go:4807: typelinks not sorted: "utf8.acceptRange" [2667] > "***[179999]*runtime.bucket" [2668]
FAIL
FAIL	reflect	0.092s

for reasons I totally don't understand at this point.

@crawshaw
Copy link
Member Author

crawshaw commented Feb 9, 2017

I believe TestTypelinksSorted was not designed with multiple modules in mind, and needs some rethinking. I'm happy to do this if you like.

@mwhudson
Copy link
Contributor

mwhudson commented Feb 9, 2017

Oh right yeah, this test is just invalid in this case isn't it. So I think the fix is valid.

@mwhudson
Copy link
Contributor

mwhudson commented Feb 9, 2017

FWIW, this bug isn't directly related to dynamic linking, for example this fails in the same way:

package main

import (
	"reflect"
	"unsafe"
)

var x unsafe.Pointer
var y = &x
var z = &y

func main() {
	var v unsafe.Pointer
	println(z)
	t := reflect.TypeOf(v)
	for i := 0; i < 10; i++ {
		t = reflect.PtrTo(t)
	}
}

@gopherbot
Copy link

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

@rsc rsc reopened this Feb 10, 2017
@rsc
Copy link
Contributor

rsc commented Feb 10, 2017

Cherry-pick for Go 1.8 https://go-review.googlesource.com/36718

@gopherbot
Copy link

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

@rsc rsc closed this as completed Feb 10, 2017
gopherbot pushed a commit that referenced this issue Feb 10, 2017
@mwhudson @rsc
[release-branch.go1.8] reflect: clear ptrToThis in Ptr when allocatin…
7cec9a5 
…g result on heap

Otherwise, calling PtrTo on the result will fail.

Fixes #19003

Change-Id: I8d7d1981a5d0417d5aee52740469d71e90734963
Reviewed-on: https://go-review.googlesource.com/36731
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-on: https://go-review.googlesource.com/36718
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators Feb 10, 2018
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.
Projects
None yet
Milestone
Go1.8Maybe
Development

No branches or pull requests
5 participants
@bradfitz @rsc @crawshaw @mwhudson @gopherbot