runtime: panic in conn.ok() #41694

zhangshishen · 2020-09-29T14:37:33Z

What version of Go are you using (`go version`)?

$ go version
go 1.13

Does this issue reproduce with the latest release?

Don't know, I can't reproduce

What operating system and processor architecture are you using (`go env`)?

go env Output

$ go env
GOARCH="amd64"

GOBIN=""

GOEXE=""

GOFLAGS=""

GOHOSTARCH="amd64"

GOHOSTOS="linux"

GOOS="linux"

GOPROXY=""

GORACE=""

GOROOT="/opt/go/go"

GOTMPDIR=""

GCCGO="gccgo"

CC="gcc"

CXX="g++"

CGO_ENABLED="1"

GOMOD=""

CGO_CFLAGS="-g -O2"

CGO_CPPFLAGS=""

CGO_CXXFLAGS="-g -O2"

CGO_FFLAGS="-g -O2"

CGO_LDFLAGS="-g -O2"

PKG_CONFIG="pkg-config"

GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build345399552=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Access *net.TCPConn parallel, panic in net.conn.ok()

I get the panic stack by debug.Stack()

Recovered panic stack is:

1752602 goroutine 313367 [running]:
1752603 runtime/debug.Stack(0xc00d43ddd8, 0xbdc5c0, 0x1609300)
1752604 /usr/local/go/src/runtime/debug/stack.go:24 +0x9d
1752605 /rtcgw/pkg/panicutil.CheckPanic()
1752606 /rtcgw/pkg/panicutil/panic_check.go:28 +0x46
1752607 panic(0xbdc5c0, 0x1609300)
1752608 /usr/local/go/src/runtime/panic.go:679 +0x1b2
1752609 net.(*conn).ok(...)
1752610 /usr/local/go/src/net/net.go:175
1752611 net.(*conn).RemoteAddr(...)
1752612 /usr/local/go/src/net/net.go:229
1752613 /rtcgw/pkg/ice.(*tcpPacketConn).ReadFrom(0xc01a639560, 0xc03eedc000, 0x2000, 0x2000, 0xc04f490720, 0x10, 0xc024056000, 0x6c, 0x640)
1752614 /rtcgw/pkg/ice/candidateTcp.go:280 +0x9d
1752615 /rtcgw/pkg/ice.(*candidateBase).recv(0xc009008bb0)
1752616 /rtcgw/pkg/ice/candidateBase.go:100 +0x141
1752617 created by /rtcgw/pkg/ice.(*candidateBase).start
1752618 /rtcgw/pkg/ice/candidateBase.go:91 +0x78

What did you expect to see?

conn.ok() in go 1.13 is:

...
135 func (c *conn) ok() bool { return c != nil && c.fd != nil }
...

I don't know why it got panic here.

Seems that nil pointer wouldn't cause panic, and I've never used unsafe ptr.

What did you see instead?

panic in conn.ok(),

I'm not sure if it's my code's bug or the output of debug.Stack() is not accurate, or sth else.

The text was updated successfully, but these errors were encountered:

randall77 · 2020-09-29T14:56:28Z

What is the panic value? (The thing returned by recover().)

zhangshishen · 2020-09-30T03:13:54Z

What is the panic value? (The thing returned by recover().)

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x9084dd]

ianlancetaylor · 2020-09-30T05:04:33Z

Is there any way that we can reproduce the problem ourselves?

Note that Go 1.13 is no longer supported; can you try Go 1.15?

randall77 · 2020-09-30T05:15:21Z

Can you disassemble your binary and show us the output for the function ice.(*tcpPacketConn).ReadFrom? I want to see the instruction that faulted (which should be in that function at 0x9084dd), and the surrounding instructions.

Something looks strange to me about the arguments to the function ice.(*tcpPacketConn).ReadFrom. They don't parse correctly in my head. If this is the standard ReadFrom, it should have a pointer (receiver, 1 word), an interface (an io.Reader, 2 words), an int64 (1 word), and an error (2 words). That should be 6 words total, but there are 9 of them. They look like they start with a pointer and a slice, which is not what I was expecting. Can you show us the prototype of that function?

zhangshishen · 2020-09-30T09:36:28Z

Can you disassemble your binary and show us the output for the function ice.(*tcpPacketConn).ReadFrom? I want to see the instruction that faulted (which should be in that function at 0x9084dd), and the surrounding instructions.

Something looks strange to me about the arguments to the function ice.(*tcpPacketConn).ReadFrom. They don't parse correctly in my head. If this is the standard ReadFrom, it should have a pointer (receiver, 1 word), an interface (an io.Reader, 2 words), an int64 (1 word), and an error (2 words). That should be 6 words total, but there are 9 of them. They look like they start with a pointer and a slice, which is not what I was expecting. Can you show us the prototype of that function?

func (t *tcpPacketConn) ReadFrom(p []byte) (int, net.Addr, error)

I'm trying to get the disassemble code.

zhangshishen · 2020-09-30T10:23:00Z

Can you disassemble your binary and show us the output for the function ice.(*tcpPacketConn).ReadFrom? I want to see the instruction that faulted (which should be in that function at 0x9084dd), and the surrounding instructions.

Something looks strange to me about the arguments to the function ice.(*tcpPacketConn).ReadFrom. They don't parse correctly in my head. If this is the standard ReadFrom, it should have a pointer (receiver, 1 word), an interface (an io.Reader, 2 words), an int64 (1 word), and an error (2 words). That should be 6 words total, but there are 9 of them. They look like they start with a pointer and a slice, which is not what I was expecting. Can you show us the prototype of that function?

TEXT /rtcgw/pkg/ice.(*tcpPacketConn).ReadFrom(SB) /rtcgw/pkg/ice/candidateTcp.go
candidateTcp.go:273 0x908440 64488b0c25f8ffffff MOVQ FS:0xfffffff8, CX
candidateTcp.go:273 0x908449 483b6110 CMPQ 0x10(CX), SP
candidateTcp.go:273 0x90844d 0f86e9010000 JBE 0x90863c
candidateTcp.go:273 0x908453 4883ec60 SUBQ $0x60, SP
candidateTcp.go:273 0x908457 48896c2458 MOVQ BP, 0x58(SP)
candidateTcp.go:273 0x90845c 488d6c2458 LEAQ 0x58(SP), BP
candidateTcp.go:275 0x908461 488b442468 MOVQ 0x68(SP), AX
candidateTcp.go:275 0x908466 488b4858 MOVQ 0x58(AX), CX
candidateTcp.go:275 0x90846a 48890c24 MOVQ CX, 0(SP)
candidateTcp.go:275 0x90846e 48c744240800000000 MOVQ $0x0, 0x8(SP)
candidateTcp.go:275 0x908477 e8f4f5afff CALL runtime.chanrecv1(SB)
candidateTcp.go:278 0x90847c 488b442468 MOVQ 0x68(SP), AX
candidateTcp.go:278 0x908481 488b4870 MOVQ 0x70(AX), CX
candidateTcp.go:278 0x908485 48c744244000000000 MOVQ $0x0, 0x40(SP)
candidateTcp.go:278 0x90848e 0f57c0 XORPS X0, X0
candidateTcp.go:278 0x908491 0f11442448 MOVUPS X0, 0x48(SP)
candidateTcp.go:278 0x908496 488d542440 LEAQ 0x40(SP), DX
candidateTcp.go:278 0x90849b 48891424 MOVQ DX, 0(SP)
candidateTcp.go:278 0x90849f 48894c2408 MOVQ CX, 0x8(SP)
candidateTcp.go:278 0x9084a4 e807ffafff CALL runtime.selectnbrecv(SB)
candidateTcp.go:278 0x9084a9 807c241000 CMPB $0x0, 0x10(SP)
candidateTcp.go:278 0x9084ae 0f848c000000 JE 0x908540
candidateTcp.go:278 0x9084b4 488b442448 MOVQ 0x48(SP), AX
candidateTcp.go:278 0x9084b9 488b4c2440 MOVQ 0x40(SP), CX
candidateTcp.go:279 0x9084be 488b542478 MOVQ 0x78(SP), DX
candidateTcp.go:279 0x9084c3 4839c2 CMPQ AX, DX
candidateTcp.go:279 0x9084c6 480f4fd0 CMOVG AX, DX
candidateTcp.go:279 0x9084ca 488b5c2470 MOVQ 0x70(SP), BX
candidateTcp.go:279 0x9084cf 4839cb CMPQ CX, BX
candidateTcp.go:279 0x9084d2 754d JNE 0x908521
candidateTcp.go:280 0x9084d4 488b4c2468 MOVQ 0x68(SP), CX
candidateTcp.go:280 0x9084d9 488b4910 MOVQ 0x10(CX), CX
net.go:175 0x9084dd 48833900 CMPQ $0x0, 0(CX)
net.go:229 0x9084e1 7438 JE 0x90851b
net.go:232 0x9084e3 488b09 MOVQ 0(CX), CX
net.go:232 0x9084e6 488b5178 MOVQ 0x78(CX), DX
net.go:232 0x9084ea 488b4970 MOVQ 0x70(CX), CX
candidateTcp.go:280 0x9084ee 4889842488000000 MOVQ AX, 0x88(SP)
candidateTcp.go:280 0x9084f6 48898c2490000000 MOVQ CX, 0x90(SP)
candidateTcp.go:280 0x9084fe 4889942498000000 MOVQ DX, 0x98(SP)
candidateTcp.go:280 0x908506 0f57c0 XORPS X0, X0
candidateTcp.go:280 0x908509 0f118424a0000000 MOVUPS X0, 0xa0(SP)
candidateTcp.go:280 0x908511 488b6c2458 MOVQ 0x58(SP), BP
candidateTcp.go:280 0x908516 4883c460 ADDQ $0x60, SP
candidateTcp.go:280 0x90851a c3 RET
candidateTcp.go:280 0x90851b 31d2 XORL DX, DX
candidateTcp.go:280 0x90851d 31c9 XORL CX, CX
candidateTcp.go:280 0x90851f ebcd JMP 0x9084ee
candidateTcp.go:278 0x908521 4889442438 MOVQ AX, 0x38(SP)
candidateTcp.go:279 0x908526 48891c24 MOVQ BX, 0(SP)
candidateTcp.go:279 0x90852a 48894c2408 MOVQ CX, 0x8(SP)
candidateTcp.go:279 0x90852f 4889542410 MOVQ DX, 0x10(SP)
candidateTcp.go:279 0x908534 e8579db5ff CALL runtime.memmove(SB)
candidateTcp.go:280 0x908539 488b442438 MOVQ 0x38(SP), AX
candidateTcp.go:279 0x90853e eb94 JMP 0x9084d4
candidateTcp.go:284 0x908540 488b442468 MOVQ 0x68(SP), AX
candidateTcp.go:284 0x908545 488b4810 MOVQ 0x10(AX), CX
candidateTcp.go:284 0x908549 4885c9 TESTQ CX, CX
candidateTcp.go:284 0x90854c 0f848b000000 JE 0x9085dd
candidateTcp.go:289 0x908552 48890c24 MOVQ CX, 0(SP)
candidateTcp.go:289 0x908556 488b442470 MOVQ 0x70(SP), AX
candidateTcp.go:289 0x90855b 4889442408 MOVQ AX, 0x8(SP)
candidateTcp.go:289 0x908560 488b442478 MOVQ 0x78(SP), AX
candidateTcp.go:289 0x908565 4889442410 MOVQ AX, 0x10(SP)
candidateTcp.go:289 0x90856a 488b842480000000 MOVQ 0x80(SP), AX
candidateTcp.go:289 0x908572 4889442418 MOVQ AX, 0x18(SP)
candidateTcp.go:289 0x908577 e884fcffff CALL /rtcgw/pkg/ice.ReadPacket(SB)
candidateTcp.go:290 0x90857c 488b442468 MOVQ 0x68(SP), AX
candidateTcp.go:290 0x908581 488b4010 MOVQ 0x10(AX), AX
candidateTcp.go:289 0x908585 488b4c2430 MOVQ 0x30(SP), CX
candidateTcp.go:289 0x90858a 488b542428 MOVQ 0x28(SP), DX
candidateTcp.go:289 0x90858f 488b5c2420 MOVQ 0x20(SP), BX
net.go:175 0x908594 48833800 CMPQ $0x0, 0(AX)
net.go:229 0x908598 743d JE 0x9085d7
net.go:232 0x90859a 488b00 MOVQ 0(AX), AX
net.go:232 0x90859d 488b7078 MOVQ 0x78(AX), SI
net.go:232 0x9085a1 488b4070 MOVQ 0x70(AX), AX
candidateTcp.go:290 0x9085a5 48899c2488000000 MOVQ BX, 0x88(SP)
candidateTcp.go:290 0x9085ad 4889842490000000 MOVQ AX, 0x90(SP)
candidateTcp.go:290 0x9085b5 4889b42498000000 MOVQ SI, 0x98(SP)
candidateTcp.go:290 0x9085bd 48899424a0000000 MOVQ DX, 0xa0(SP)
candidateTcp.go:290 0x9085c5 48898c24a8000000 MOVQ CX, 0xa8(SP)
candidateTcp.go:290 0x9085cd 488b6c2458 MOVQ 0x58(SP), BP
candidateTcp.go:290 0x9085d2 4883c460 ADDQ $0x60, SP
candidateTcp.go:290 0x9085d6 c3 RET
candidateTcp.go:290 0x9085d7 31f6 XORL SI, SI
candidateTcp.go:290 0x9085d9 31c0 XORL AX, AX
candidateTcp.go:290 0x9085db ebc8 JMP 0x9085a5
errors.go:59 0x9085dd 488d053c2a2f00 LEAQ 0x2f2a3c(IP), AX
errors.go:59 0x9085e4 48890424 MOVQ AX, 0(SP)
errors.go:59 0x9085e8 e88366b0ff CALL runtime.newobject(SB)
errors.go:59 0x9085ed 488b442408 MOVQ 0x8(SP), AX
errors.go:59 0x9085f2 48c7400823000000 MOVQ $0x23, 0x8(AX)
errors.go:59 0x9085fa 488d0d946f4000 LEAQ 0x406f94(IP), CX
errors.go:59 0x908601 488908 MOVQ CX, 0(AX)
candidateTcp.go:286 0x908604 48c784248800000000000000 MOVQ $0x0, 0x88(SP)
candidateTcp.go:286 0x908610 0f57c0 XORPS X0, X0
candidateTcp.go:286 0x908613 0f11842490000000 MOVUPS X0, 0x90(SP)
candidateTcp.go:286 0x90861b 488d0d5e404f00 LEAQ go.itab.*errors.errorString,error(SB), CX
candidateTcp.go:286 0x908622 48898c24a0000000 MOVQ CX, 0xa0(SP)
candidateTcp.go:286 0x90862a 48898424a8000000 MOVQ AX, 0xa8(SP)
candidateTcp.go:286 0x908632 488b6c2458 MOVQ 0x58(SP), BP
candidateTcp.go:286 0x908637 4883c460 ADDQ $0x60, SP
candidateTcp.go:286 0x90863b c3 RET
candidateTcp.go:273 0x90863c e82f6bb5ff CALL runtime.morestack_noctxt(SB)
candidateTcp.go:273 0x908641 e9fafdffff JMP /rtcgw/pkg/ice.(*tcpPacketConn).ReadFrom(SB)

randall77 · 2020-09-30T14:50:26Z

Looks like you have something like:

c := *p // load c from somewhere
a := c.RemoteAddr() // which calls c.ok()

You're seeing an error for p being nil. As a result of inlining RemoteAddr, and then inlining ok, we end up combining the load c := *p and the comparison c != nil into a single instruction CMPQ $0x0, 0(AX). We ended up giving that instruction the line number of the comparison, not of the load, which is why the panic location is misleading.

We fixed that in https://go-review.googlesource.com/c/go/+/156937/ (which went out with 1.14, I think), so you'll get the correct line number at tip.

Closing, as this is fixed in all supported releases. (Unless I'm wrong about it going out in 1.14, in which case reopen.)

zhangshishen · 2020-09-30T17:00:13Z

Looks like you have something like:
c := *p // load c from somewhere
a := c.RemoteAddr() // which calls c.ok()
You're seeing an error for p being nil. As a result of inlining RemoteAddr, and then inlining ok, we end up combining the load c := *p and the comparison c != nil into a single instruction CMPQ $0x0, 0(AX). We ended up giving that instruction the line number of the comparison, not of the load, which is why the panic location is misleading.

We fixed that in https://go-review.googlesource.com/c/go/+/156937/ (which went out with 1.14, I think), so you'll get the correct line number at tip.

Closing, as this is fixed in all supported releases. (Unless I'm wrong about it going out in 1.14, in which case reopen.)

The function's implementation is:

func (t *tcpPacketConn) ReadFrom(p []byte) (int, net.Addr, error) {

	<-t.connExist

	select {
	case buf := <-t.buffered:
		copy(p, buf)
		return len(buf), t.realConn.RemoteAddr(), nil
	default:
	}

	if t.realConn == nil {
		//real conn not exist]
		return 0, nil, errors.New("tcp connect failed or listen failed")
	}

	n, err := ReadPacket(t.realConn, p)
	return n, t.realConn.RemoteAddr(), err
}

Did you mean that t is nil? But why it didn't panic on <-t.connExist

randall77 · 2020-09-30T17:45:57Z

Sorry, my analysis was wrong. Looking at your source code, that comparison instruction is not for c != nil, it is for c.fd != nil. The c != nil comparison looks like it was removed, probably incorrectly.

The arguments to ReadFrom look fine. I thought it would be an implementation of io.ReaderFrom but it isn't.

Just to be clear, it is this line in ReadFrom that is failing?

		return len(buf), t.realConn.RemoteAddr(), nil

Could you build with 1.14 or later and see if that fails? I know you said it would be hard to reproduce. Maybe even just posting the disassembly of ReadFrom built with 1.14+ would help, to see if the missing nil check is there or not.

I'm trying to reproduce on my end, having no luck.

zhangshishen · 2020-09-30T18:00:44Z

Sorry, my analysis was wrong. Looking at your source code, that comparison instruction is not for c != nil, it is for c.fd != nil. The c != nil comparison looks like it was removed, probably incorrectly.

The arguments to ReadFrom look fine. I thought it would be an implementation of io.ReaderFrom but it isn't.

Just to be clear, it is this line in ReadFrom that is failing?
		return len(buf), t.realConn.RemoteAddr(), nil

Yes, it is.
I’m trying to reproduce it with the newest go.

zhangshishen · 2020-09-30T18:19:22Z

Sorry, my analysis was wrong. Looking at your source code, that comparison instruction is not for c != nil, it is for c.fd != nil. The c != nil comparison looks like it was removed, probably incorrectly.

The arguments to ReadFrom look fine. I thought it would be an implementation of io.ReaderFrom but it isn't.

Just to be clear, it is this line in ReadFrom that is failing?
		return len(buf), t.realConn.RemoteAddr(), nil
Could you build with 1.14 or later and see if that fails? I know you said it would be hard to reproduce. Maybe even just posting the disassembly of ReadFrom built with 1.14+ would help, to see if the missing nil check is there or not.

I'm trying to reproduce on my end, having no luck.

Here is the disassembly with go 1.15.2

TEXT /rtcgw/pkg/ice.(*tcpPacketConn).ReadFrom(SB) /rtcgw/pkg/ice/candidateTcp.go
candidateTcp.go:273 0x9452c0 64488b0c25f8ffffff MOVQ FS:0xfffffff8, CX
candidateTcp.go:273 0x9452c9 483b6110 CMPQ 0x10(CX), SP
candidateTcp.go:273 0x9452cd 0f86ed010000 JBE 0x9454c0
candidateTcp.go:273 0x9452d3 4883ec60 SUBQ $0x60, SP
candidateTcp.go:273 0x9452d7 48896c2458 MOVQ BP, 0x58(SP)
candidateTcp.go:273 0x9452dc 488d6c2458 LEAQ 0x58(SP), BP
candidateTcp.go:275 0x9452e1 488b442468 MOVQ 0x68(SP), AX
candidateTcp.go:275 0x9452e6 488b4858 MOVQ 0x58(AX), CX
candidateTcp.go:275 0x9452ea 48890c24 MOVQ CX, 0(SP)
candidateTcp.go:275 0x9452ee 48c744240800000000 MOVQ $0x0, 0x8(SP)
candidateTcp.go:275 0x9452f7 e88431acff CALL runtime.chanrecv1(SB)
candidateTcp.go:278 0x9452fc 488b442468 MOVQ 0x68(SP), AX
candidateTcp.go:278 0x945301 488b4870 MOVQ 0x70(AX), CX
candidateTcp.go:278 0x945305 48c744244000000000 MOVQ $0x0, 0x40(SP)
candidateTcp.go:278 0x94530e 0f57c0 XORPS X0, X0
candidateTcp.go:278 0x945311 0f11442448 MOVUPS X0, 0x48(SP)
candidateTcp.go:278 0x945316 488d542440 LEAQ 0x40(SP), DX
candidateTcp.go:278 0x94531b 48891424 MOVQ DX, 0(SP)
candidateTcp.go:278 0x94531f 48894c2408 MOVQ CX, 0x8(SP)
candidateTcp.go:278 0x945324 e8773bacff CALL runtime.selectnbrecv(SB)
candidateTcp.go:278 0x945329 807c241000 CMPB $0x0, 0x10(SP)
candidateTcp.go:278 0x94532e 0f848e000000 JE 0x9453c2
candidateTcp.go:278 0x945334 488b442448 MOVQ 0x48(SP), AX
candidateTcp.go:278 0x945339 488b4c2440 MOVQ 0x40(SP), CX
candidateTcp.go:279 0x94533e 488b542478 MOVQ 0x78(SP), DX
candidateTcp.go:279 0x945343 4839d0 CMPQ DX, AX
candidateTcp.go:279 0x945346 480f4cd0 CMOVL AX, DX
candidateTcp.go:279 0x94534a 488b5c2470 MOVQ 0x70(SP), BX
candidateTcp.go:279 0x94534f 4839d9 CMPQ BX, CX
candidateTcp.go:279 0x945352 754e JNE 0x9453a2
candidateTcp.go:280 0x945354 488b4c2468 MOVQ 0x68(SP), CX
candidateTcp.go:280 0x945359 488b4910 MOVQ 0x10(CX), CX
candidateTcp.go:280 0x94535d 488b09 MOVQ 0(CX), CX
net.go:173 0x945360 4885c9 TESTQ CX, CX
net.go:227 0x945363 7435 JE 0x94539a
net.go:230 0x945365 488b5170 MOVQ 0x70(CX), DX
net.go:230 0x945369 488b4978 MOVQ 0x78(CX), CX
candidateTcp.go:280 0x94536d 4889842488000000 MOVQ AX, 0x88(SP)
candidateTcp.go:280 0x945375 4889942490000000 MOVQ DX, 0x90(SP)
candidateTcp.go:280 0x94537d 48898c2498000000 MOVQ CX, 0x98(SP)
candidateTcp.go:280 0x945385 0f57c0 XORPS X0, X0
candidateTcp.go:280 0x945388 0f118424a0000000 MOVUPS X0, 0xa0(SP)
candidateTcp.go:280 0x945390 488b6c2458 MOVQ 0x58(SP), BP
candidateTcp.go:280 0x945395 4883c460 ADDQ $0x60, SP
candidateTcp.go:280 0x945399 c3 RET
candidateTcp.go:280 0x94539a 31d2 XORL DX, DX
candidateTcp.go:280 0x94539c 31c9 XORL CX, CX
candidateTcp.go:280 0x94539e 6690 NOPW
candidateTcp.go:280 0x9453a0 ebcb JMP 0x94536d
candidateTcp.go:278 0x9453a2 4889442438 MOVQ AX, 0x38(SP)
candidateTcp.go:279 0x9453a7 48891c24 MOVQ BX, 0(SP)
candidateTcp.go:279 0x9453ab 48894c2408 MOVQ CX, 0x8(SP)
candidateTcp.go:279 0x9453b0 4889542410 MOVQ DX, 0x10(SP)
candidateTcp.go:279 0x9453b5 e806d2b2ff CALL runtime.memmove(SB)
candidateTcp.go:280 0x9453ba 488b442438 MOVQ 0x38(SP), AX
candidateTcp.go:280 0x9453bf 90 NOPL
candidateTcp.go:279 0x9453c0 eb92 JMP 0x945354
candidateTcp.go:284 0x9453c2 488b442468 MOVQ 0x68(SP), AX
candidateTcp.go:284 0x9453c7 488b4810 MOVQ 0x10(AX), CX
candidateTcp.go:284 0x9453cb 4885c9 TESTQ CX, CX
candidateTcp.go:284 0x9453ce 0f848a000000 JE 0x94545e
candidateTcp.go:289 0x9453d4 48890c24 MOVQ CX, 0(SP)
candidateTcp.go:289 0x9453d8 488b442470 MOVQ 0x70(SP), AX
candidateTcp.go:289 0x9453dd 4889442408 MOVQ AX, 0x8(SP)
candidateTcp.go:289 0x9453e2 488b442478 MOVQ 0x78(SP), AX
candidateTcp.go:289 0x9453e7 4889442410 MOVQ AX, 0x10(SP)
candidateTcp.go:289 0x9453ec 488b842480000000 MOVQ 0x80(SP), AX
candidateTcp.go:289 0x9453f4 4889442418 MOVQ AX, 0x18(SP)
candidateTcp.go:289 0x9453f9 e882fcffff CALL /rtcgw/pkg/ice.ReadPacket(SB)
candidateTcp.go:290 0x9453fe 488b442468 MOVQ 0x68(SP), AX
candidateTcp.go:290 0x945403 488b4010 MOVQ 0x10(AX), AX
candidateTcp.go:289 0x945407 488b4c2420 MOVQ 0x20(SP), CX
candidateTcp.go:289 0x94540c 488b542428 MOVQ 0x28(SP), DX
candidateTcp.go:289 0x945411 488b5c2430 MOVQ 0x30(SP), BX
candidateTcp.go:290 0x945416 488b00 MOVQ 0(AX), AX
net.go:173 0x945419 4885c0 TESTQ AX, AX
net.go:227 0x94541c 743a JE 0x945458
net.go:230 0x94541e 488b7070 MOVQ 0x70(AX), SI
net.go:230 0x945422 488b4078 MOVQ 0x78(AX), AX
candidateTcp.go:290 0x945426 48898c2488000000 MOVQ CX, 0x88(SP)
candidateTcp.go:290 0x94542e 4889b42490000000 MOVQ SI, 0x90(SP)
candidateTcp.go:290 0x945436 4889842498000000 MOVQ AX, 0x98(SP)
candidateTcp.go:290 0x94543e 48899424a0000000 MOVQ DX, 0xa0(SP)
candidateTcp.go:290 0x945446 48899c24a8000000 MOVQ BX, 0xa8(SP)
candidateTcp.go:290 0x94544e 488b6c2458 MOVQ 0x58(SP), BP
candidateTcp.go:290 0x945453 4883c460 ADDQ $0x60, SP
candidateTcp.go:290 0x945457 c3 RET
candidateTcp.go:290 0x945458 31f6 XORL SI, SI
candidateTcp.go:290 0x94545a 31c0 XORL AX, AX
candidateTcp.go:290 0x94545c ebc8 JMP 0x945426
errors.go:59 0x94545e 488d059ba12d00 LEAQ 0x2da19b(IP), AX
errors.go:59 0x945465 48890424 MOVQ AX, 0(SP)
errors.go:59 0x945469 e812afacff CALL runtime.newobject(SB)
errors.go:59 0x94546e 488b442408 MOVQ 0x8(SP), AX
errors.go:59 0x945473 48c7400823000000 MOVQ $0x23, 0x8(AX)
errors.go:59 0x94547b 488d0df5b23d00 LEAQ 0x3db2f5(IP), CX
errors.go:59 0x945482 488908 MOVQ CX, 0(AX)
candidateTcp.go:286 0x945485 48c784248800000000000000 MOVQ $0x0, 0x88(SP)
candidateTcp.go:286 0x945491 0f57c0 XORPS X0, X0
candidateTcp.go:286 0x945494 0f11842490000000 MOVUPS X0, 0x90(SP)
candidateTcp.go:286 0x94549c 488d0d5d1f4a00 LEAQ go.itab.*errors.errorString,error(SB), CX
candidateTcp.go:286 0x9454a3 48898c24a0000000 MOVQ CX, 0xa0(SP)
candidateTcp.go:286 0x9454ab 48898424a8000000 MOVQ AX, 0xa8(SP)
candidateTcp.go:286 0x9454b3 488b6c2458 MOVQ 0x58(SP), BP
candidateTcp.go:286 0x9454b8 4883c460 ADDQ $0x60, SP
candidateTcp.go:286 0x9454bc c3 RET
candidateTcp.go:273 0x9454bd 0f1f00 NOPL 0(AX)
candidateTcp.go:273 0x9454c0 e8bba6b2ff CALL runtime.morestack_noctxt(SB)
candidateTcp.go:273 0x9454c5 e9f6fdffff JMP /rtcgw/pkg/ice.(*tcpPacketConn).ReadFrom(SB)

randall77 · 2020-09-30T18:26:30Z

Ok, I think I see the problem. Or at least, my confusion.
Your bug is that t.realConn is nil, and you can't call RemoteAddr on that nil pointer.
Despite the fact that (*conn).RemoteAddr can be called on a nil pointer.

The reason is that the type of t.realConn is *net.IPConn, not *net.conn (am I right about that?). The definition of net.IPConn is (in net):

type IPConn struct {
	conn
}

The process of *net.IPConn inheriting the method set of *net.conn adds the additional constraint that the pointer must not be nil. The reason is that we might need to do pointer arithmetic to convert from a *net.IPConn to a *net.conn, if conn was not the first field of IPConn.

You can see the difference in this little program:

package main

type T struct {
	a, b, c int
}

func (t *T) f() {
}

type U struct {
	// pad [10000]byte
	T
}

func main() {
	var x *T = nil
	x.f()
	var y *U = nil
	y.f()
}

It does not panic on the call to x.f(), but does on the call to y.f(). Whether the pad field is there or not. Because the pad field might be there, and thus converting from a *U to a *T inside the call to y.f, we must check if y is nil (and panic if it is).

So the compiler is doing the right thing, I think. The nil check is being merged with the load of the fd field.

This behavior is identical for 1.13 - tip as far as I can tell.

zhangshishen · 2020-09-30T18:47:05Z

Ok, I think I see the problem. Or at least, my confusion.
Your bug is that t.realConn is nil, and you can't call RemoteAddr on that nil pointer.
Despite the fact that (*conn).RemoteAddr can be called on a nil pointer.

The reason is that the type of t.realConn is *net.IPConn, not *net.conn (am I right about that?). The definition of net.IPConn is (in net):
type IPConn struct {
	conn
}
The process of *net.IPConn inheriting the method set of *net.conn adds the additional constraint that the pointer must not be nil. The reason is that we might need to do pointer arithmetic to convert from a *net.IPConn to a *net.conn, if conn was not the first field of IPConn.

You can see the difference in this little program:
package main

type T struct {
	a, b, c int
}

func (t *T) f() {
}

type U struct {
	// pad [10000]byte
	T
}

func main() {
	var x *T = nil
	x.f()
	var y *U = nil
	y.f()
}
It does not panic on the call to x.f(), but does on the call to y.f(). Whether the pad field is there or not. Because the pad field might be there, and thus converting from a *U to a *T inside the call to y.f, we must check if y is nil (and panic if it is).

So the compiler is doing the right thing, I think. The nil check is being merged with the load of the fd field.

This behavior is identical for 1.13 - tip as far as I can tell.

Thank you so much! I think it is the problem.
The type of t.realConn is *net.TCPConn.

ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 30, 2020

ianlancetaylor added this to the Go1.16 milestone Sep 30, 2020

randall77 closed this as completed Sep 30, 2020

randall77 reopened this Sep 30, 2020

zhangshishen closed this as completed Sep 30, 2020

golang locked and limited conversation to collaborators Sep 30, 2021

gopherbot added the FrozenDueToAge label Sep 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

runtime: panic in conn.ok() #41694

runtime: panic in conn.ok() #41694

zhangshishen commented Sep 29, 2020 •

edited

randall77 commented Sep 29, 2020

zhangshishen commented Sep 30, 2020

ianlancetaylor commented Sep 30, 2020

randall77 commented Sep 30, 2020

zhangshishen commented Sep 30, 2020

zhangshishen commented Sep 30, 2020

randall77 commented Sep 30, 2020

zhangshishen commented Sep 30, 2020 •

edited

randall77 commented Sep 30, 2020

zhangshishen commented Sep 30, 2020

zhangshishen commented Sep 30, 2020

randall77 commented Sep 30, 2020

zhangshishen commented Sep 30, 2020

runtime: panic in conn.ok() #41694

runtime: panic in conn.ok() #41694

Comments

zhangshishen commented Sep 29, 2020 • edited

What version of Go are you using (go version)?

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

What did you do?

What did you expect to see?

What did you see instead?

randall77 commented Sep 29, 2020

zhangshishen commented Sep 30, 2020

ianlancetaylor commented Sep 30, 2020

randall77 commented Sep 30, 2020

zhangshishen commented Sep 30, 2020

zhangshishen commented Sep 30, 2020

randall77 commented Sep 30, 2020

zhangshishen commented Sep 30, 2020 • edited

randall77 commented Sep 30, 2020

zhangshishen commented Sep 30, 2020

zhangshishen commented Sep 30, 2020

randall77 commented Sep 30, 2020

zhangshishen commented Sep 30, 2020

zhangshishen commented Sep 29, 2020 •

edited

What version of Go are you using (`go version`)?

What operating system and processor architecture are you using (`go env`)?

zhangshishen commented Sep 30, 2020 •

edited