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

cmd/compile: CMOV optimization allows GC to see invalid pointer #56990

Closed
randall77 opened this issue Nov 29, 2022 · 3 comments
Closed

cmd/compile: CMOV optimization allows GC to see invalid pointer #56990

randall77 opened this issue Nov 29, 2022 · 3 comments
Assignees
@randall77
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.20

Comments

@randall77
Copy link
Contributor

This program when run on the playground, go1.18, go1.19, or tip, crashes when the Go runtime finds an invalid pointer on the stack.

@cherrymui determined that this has to do with a conditional move optimization. We have code like:

if q != nil {
    p = &q.f
}

Which gets rewritten to a conditional move:

tmp := &q.f
p = Select q!=nil, tmp, p

Unfortunately, we can't compute &q.f before we've checked if q is nil, because if it is nil, &q.f is an invalid pointer (if f's offset is nonzero but small).

Normally this is not a problem because the tmp variable above immediately dies, and is thus not live across any safepoint. However, if later there is another &q.f computation, those two computations are CSEd, causing tmp to be used at both use points. That will extend tmp's lifetime, possibly across a call.
@randall77 randall77 added this to the Go1.20 milestone Nov 29, 2022
@randall77 randall77 self-assigned this Nov 29, 2022
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Nov 29, 2022
@gopherbot
Copy link

Change https://go.dev/cl/454155 mentions this issue: cmd/compile: disallow CMOV optimization with ptr arithmetic as an arg

@cherrymui
Copy link
Member

Simpler reproducer

package main

type ConfigSpec int

// Config is the Schema for the configs API.
type Config struct {
	x, y int
	Spec ConfigSpec
}

//go:noinline
func f1(cs *ConfigSpec) {
	grow(100000)
}

func grow(n int) { if n > 0 { grow(n-1) } }


//go:noinline
func f2(cs *ConfigSpec) {}

//go:noinline
func Foo(config *Config) {
	var cs *ConfigSpec
	if config != nil {
		cs = &config.Spec
	}
	f1(cs)
	if config != nil {
		cs = &config.Spec
		f1(cs)
	}
	f2(cs)

}

func main() {
	Foo(nil)
}

This fails since at least Go 1.11 (possibly even earlier).

@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Nov 30, 2022
@toothrot
Copy link
Contributor

Removing the release-blocker label as this is not a new regression.

@golang golang locked and limited conversation to collaborators Nov 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@randall77 randall77

Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.20
Development

No branches or pull requests
5 participants
@toothrot @bcmills @randall77 @gopherbot @cherrymui