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

runtime: unexpected return pc from label followed by fallthrough #70173

Closed
leabit opened this issue Nov 2, 2024 · 14 comments
Closed

runtime: unexpected return pc from label followed by fallthrough #70173

leabit opened this issue Nov 2, 2024 · 14 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.

Comments

@leabit
Copy link

leabit commented Nov 2, 2024

Go version

Go 1.23

Output of go env in your module/workspace:

/

What did you do?

Playground: https://go.dev/play/p/Cb94cJlyvNM

package main

func main() {
	switch {
	case true:
		goto a
	a:
		fallthrough
	default:
	}
}

What did you see happen?

SIGTRAP: trace trap
PC=0x466522 m=0 sigcode=128

goroutine 1 gp=0xc0000061c0 m=0 mp=0x4ef280 [running]:
main.main()
?:0 +0x2 fp=0xc00004874f sp=0xc000048748 pc=0x466522
runtime: g 1: unexpected return pc for main.main called from 0x43116b00
stack: frame={sp:0xc000048748, fp:0xc00004874f} stack=[0xc000048000,0xc000048800)
0x000000c000048648: 0x000000c000048660 0x00000000004122f6 <runtime.gcTrigger.test+0x00000000000000b6>
0x000000c000048658: 0x000000000050e920 0x000000c000048700
0x000000c000048668: 0x000000000045b6a5 <runtime.mallocgc+0x0000000000000925> 0x00007ec01177d5b8
0x000000c000048678: 0x000000c000048613 0x0000000000405888 <runtime.chanrecv+0x00000000000004e8>
0x000000c000048688: 0x000000c00009e000 0x00007ebfcac815e8
0x000000c000048698: 0x0000010100000010 0x00007ebfcac81368
0x000000c0000486a8: 0x0000000000000070 0x0100000000000010
0x000000c0000486b8: 0x00007ec01177d5b8 0x000000c0000486e8
0x000000c0000486c8: 0x0000000000409db0 <runtime.unlock2+0x0000000000000050> 0x000000c000098060
0x000000c0000486d8: 0x00000000004ef280 0x000000c0000061c0
0x000000c0000486e8: 0x000000c000048740 0x00000000004051eb <runtime.closechan+0x000000000000034b>
0x000000c0000486f8: 0x0000000000405392 <runtime.chanrecv1+0x0000000000000012> 0x000000c000048740
0x000000c000048708: 0x0000000000000000 0x0000000000000068
0x000000c000048718: 0x0000000000000000 0x000000c0000a2060
0x000000c000048728: 0x00000000004ea378 0x000000000046c5c0
0x000000c000048738: 0x000000c0000061c0 0x000000c0000487d0
0x000000c000048748: <0x000000000043116b <runtime.main+0x000000000000028b> 0x000000c0000a2000
0x000000c000048758: 0x0000000000000000 0x0000000000000000
0x000000c000048768: 0x0000000000000000 0x0100000000000000
0x000000c000048778: 0x0000000000000000 0x0000000000000002
0x000000c000048788: 0x00000000004ea860 0x0000000000000000
0x000000c000048798: 0x0000000000000001 0x00000000004ea100
0x000000c0000487a8: 0x00000000004ea1c0 0x00000000004ef280
0x000000c0000487b8: 0x0000000000431300 <runtime.main.func2+0x0000000000000000> 0x000000c000048776
0x000000c0000487c8: 0x000000c0000487b8 0x0000000000000000
0x000000c0000487d8: 0x00000000004634c1 <runtime.goexit+0x0000000000000001> 0x0000000000000000
0x000000c0000487e8: 0x0000000000000000 0x0000000000000000
0x000000c0000487f8: 0x0000000000000000

goroutine 2 gp=0xc000006700 m=nil [force gc (idle)]:
runtime.gopark(0x0?, 0x0?, 0x0?, 0x0?, 0x0?)
/usr/local/go-faketime/src/runtime/proc.go:424 +0xce fp=0xc000048fa8 sp=0xc000048f88 pc=0x45d2ee
runtime.goparkunlock(...)
/usr/local/go-faketime/src/runtime/proc.go:430
runtime.forcegchelper()
/usr/local/go-faketime/src/runtime/proc.go:337 +0xa5 fp=0xc000048fe0 sp=0xc000048fa8 pc=0x4314a5
runtime.goexit({})
/usr/local/go-faketime/src/runtime/asm_amd64.s:1700 +0x1 fp=0xc000048fe8 sp=0xc000048fe0 pc=0x4634c1
created by runtime.init.7 in goroutine 1
/usr/local/go-faketime/src/runtime/proc.go:325 +0x1a

goroutine 17 gp=0xc000084380 m=nil [GC sweep wait]:
runtime.gopark(0x0?, 0x0?, 0x0?, 0x0?, 0x0?)
/usr/local/go-faketime/src/runtime/proc.go:424 +0xce fp=0xc000044780 sp=0xc000044760 pc=0x45d2ee
runtime.goparkunlock(...)
/usr/local/go-faketime/src/runtime/proc.go:430
runtime.bgsweep(0xc000098000)
/usr/local/go-faketime/src/runtime/mgcsweep.go:277 +0x94 fp=0xc0000447c8 sp=0xc000044780 pc=0x41d6b4
runtime.gcenable.gowrap1()
/usr/local/go-faketime/src/runtime/mgc.go:203 +0x25 fp=0xc0000447e0 sp=0xc0000447c8 pc=0x412185
runtime.goexit({})
/usr/local/go-faketime/src/runtime/asm_amd64.s:1700 +0x1 fp=0xc0000447e8 sp=0xc0000447e0 pc=0x4634c1
created by runtime.gcenable in goroutine 1
/usr/local/go-faketime/src/runtime/mgc.go:203 +0x66

goroutine 18 gp=0xc000084540 m=nil [GC scavenge wait]:
runtime.gopark(0xc000098000?, 0x4948b8?, 0x1?, 0x0?, 0xc000084540?)
/usr/local/go-faketime/src/runtime/proc.go:424 +0xce fp=0xc000044f78 sp=0xc000044f58 pc=0x45d2ee
runtime.goparkunlock(...)
/usr/local/go-faketime/src/runtime/proc.go:430
runtime.(*scavengerState).park(0x4ee4e0)
/usr/local/go-faketime/src/runtime/mgcscavenge.go:425 +0x49 fp=0xc000044fa8 sp=0xc000044f78 pc=0x41b129
runtime.bgscavenge(0xc000098000)
/usr/local/go-faketime/src/runtime/mgcscavenge.go:653 +0x3c fp=0xc000044fc8 sp=0xc000044fa8 pc=0x41b65c
runtime.gcenable.gowrap2()
/usr/local/go-faketime/src/runtime/mgc.go:204 +0x25 fp=0xc000044fe0 sp=0xc000044fc8 pc=0x412125
runtime.goexit({})
/usr/local/go-faketime/src/runtime/asm_amd64.s:1700 +0x1 fp=0xc000044fe8 sp=0xc000044fe0 pc=0x4634c1
created by runtime.gcenable in goroutine 1
/usr/local/go-faketime/src/runtime/mgc.go:204 +0xa5

rax 0x466520
rbx 0xc0000a2060
rcx 0xc0000061c0
rdx 0x47ee78
rdi 0x1
rsi 0x3d0000
rbp 0xc0000487d0
rsp 0xc000048748
r8 0x0
r9 0x1
r10 0x7ebfcac81368
r11 0x0
r12 0xc0000a2000
r13 0x0
r14 0xc0000061c0
r15 0x0
rip 0x466522
rflags 0x246
cs 0x33
fs 0x0
gs 0x0

What did you expect to see?

To work without errors.

@seankhliao seankhliao changed the title Labeled fallthrough statement gives a runtime error runtime: unexpected return pc from goto label followed by fallthrough Nov 2, 2024
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Nov 2, 2024
@seankhliao
Copy link
Member

only if there's nothing between the label and fallthrough https://go.dev/play/p/amqZtpIop21

@leabit
Copy link
Author

leabit commented Nov 2, 2024

@seankhliao I think the "title" you changed to is incorrect. The problem is not related to the goto statement but the "labeled fallthrough statement" as I indicated. The goto statement only served me to reference a label somehow. The following code also gives the same error (https://go.dev/play/p/jBTjBBiIEzw):

package main

func main() {
	switch {
	case true:
	_:
		fallthrough
	default:
	}
}

Yes, the problem is only when the fallthrough statement is labeled. Otherwise, it works as expected.

only if there's nothing between the label and fallthrough https://go.dev/play/p/amqZtpIop21

@seankhliao seankhliao changed the title runtime: unexpected return pc from goto label followed by fallthrough runtime: unexpected return pc from label followed by fallthrough Nov 2, 2024
@zigo101
Copy link

zigo101 commented Nov 2, 2024

It happens since 1.22.

And it looks it only happens for a true-expression branch:

package main

func main() {
	switch {
	case true: // okay for false
	_:
		fallthrough
	case true:
	}
}

@leabit
Copy link
Author

leabit commented Nov 2, 2024

@zigo101 I'm not sure I understood. The error occurs when a branch is entered (the false branch in the given example will never be entered since the default for switch is true). In the following example it will enter false-expression branch and the error will occur:

package main

func main() {
	switch false {
	case false:
	_:
		fallthrough
	case true:
	}
}

Are you talking about toolchain or language version? The error occurs for each language version on toolchain 1.22.0 and above.

It happens since 1.22.

@leabit
Copy link
Author

leabit commented Nov 2, 2024

It looks like it only happens when:

  • the expression in select is constant, and
  • the expression in case with "labeled fallthrough" is constant, and
  • the expressions in all cases above the case with "labeled fallthrough" are constants, and
  • in runtime process enters the case with "labeled fallthrough"
  1. error
package main

func main() {
	const a = 20

	switch a {
	case 10:
	case 20:
	_:
		fallthrough
	case 30:
	}
}
  1. error
package main

func main() {
	const a = 20
	const b = 20

	switch a {
	case 10:
	case b:
	_:
		fallthrough
	case 30:
	}
}
  1. error
package main

func main() {
	const a = 2
	var b = [2]int{10, 20}

	switch a {
	case 10:
	case len(b):
	_:
		fallthrough
	case 30:
	}
}
  1. error
package main

func main() {
	const a = 20
	var b = 30

	switch a {
	case 10:
	case 20:
	_:
		fallthrough
	case b:
	}
}
  1. error
package main

func main() {
	const a = 20
	const b = 10
	const c = 20
	var d = 30

	switch a {
	case b:
	case c:
	_:
		fallthrough
	case d:
	}
}
  1. not error (b is not constant and it is above "labeled fallthrough" case)
package main

func main() {
	const a = 20
	var b = 30

	switch a {
	case b:
	case 20:
	_:
		fallthrough
	case 30:
	}
}
  1. not error (b is not constant and it is above "labeled fallthrough" case)
package main

func main() {
	const a = 20
	var b = 10
	const c = 20
	var d = 30

	switch a {
	case b:
	case c:
	_:
		fallthrough
	case d:
	}
}
  1. not error (a is not constant)
package main

func main() {
	var a = 20
	const b = 10
	const c = 20
	var d = 30

	switch a {
	case b:
	case c:
	_:
		fallthrough
	case d:
	}
}

@Jorropo
Copy link
Member

Jorropo commented Nov 2, 2024

Bisected to 59037ac cc @mdempsky

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/624577 mentions this issue: cmd/compile: support a always true case clause ends in a labeled fallthrough

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/624717 mentions this issue: cmd/compile: fix mis-compilation with labeled fallthrough

@cuonglm cuonglm added the NeedsFix The path to resolution is known, but the work has not been done. label Nov 3, 2024
@cuonglm cuonglm self-assigned this Nov 3, 2024
@cuonglm
Copy link
Member

cuonglm commented Nov 3, 2024

@zigo101 @leabit FYI, this happens because for case clause that the compiler knows it always branches to, it will rewrite to a single default case if there's no fallthrough statement.

However, a fallthrough statement may be labeled, but the writer is not considering this case, causing the mis-compilation.

@randall77
Copy link
Contributor

I'm leaning towards not backporting this. It seems very rare, and has an easy workaround.

@leabit
Copy link
Author

leabit commented Nov 3, 2024

@randall77 IMO this should be fixed, even though it's a very rare case. The Go language specification guarantees that labeled fallthrough statements are valid, so the compiler should handle them correctly in all cases.

@randall77
Copy link
Contributor

Our policy for backports is here: https://go.dev/wiki/MinorReleases

The operative phrase is "serious problems with no workaround". I agree it is serious, but the workaround is easy.

@ldemailly
Copy link

no real code would trigger this so yes, it's great to have it fixed it that doesn't have downsides but back port is not need imo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

9 participants