Skip to content

go/types: deferred function executed multiple times #73267

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

Open
adonovan opened this issue Apr 8, 2025 · 6 comments
Open

go/types: deferred function executed multiple times #73267

adonovan opened this issue Apr 8, 2025 · 6 comments
Assignees
Labels
BugReport Issues describing a possible bug in the Go implementation. compiler/runtime Issues related to the Go compiler and/or runtime. gopls/memory-corruption "can't happen" gopls crashes (races, unsafe, miscompile, runtime bugs, faulty HW) gopls/telemetry-wins gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@adonovan
Copy link
Member

adonovan commented Apr 8, 2025

#!stacks
"runtime.gopanic" &&
"types.(*Checker).handleBailout:+7" &&
 /* unfortunately there is no way to express bailout-within-bailout
     so we must subtract all other patterns that use bailout */
 !("go/types.setDefType:+7") // #71029

Issue created by stacks.

Well this is a weird one. Checker.Files. defers a function to recover from certain expected panics; that function re-panics for all other cases. However, the re-panic causes the deferred function to execute a second time (!). Presumably this means the defer bookkeeping (e.g. *_panic.deferBitsPtr) has become corrupted.

func (check *Checker) handleBailout(err *error) {
	switch p := recover().(type) {
	case nil, bailout:
		// normal return or early exit
		*err = check.firstErr
	default:
		// re-panic
		panic(p) <-- causes deferred call to check.handleBailout to execute again!
	}
}

// Files checks the provided files as part of the checker's package.
func (check *Checker) Files(files []*ast.File) (err error) {
...
	defer check.handleBailout(&err) <-- this closure is called repeatedly
	check.checkFiles(files)
	return
}

This stack k7voyQ was reported by telemetry:

golang.org/x/tools/gopls@v0.18.1 go1.23.7 darwin/arm64 other (2)
@adonovan adonovan added gopls Issues related to the Go language server, gopls. gopls/telemetry-wins NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository. labels Apr 8, 2025
@adonovan
Copy link
Member Author

adonovan commented Apr 8, 2025

cc: @golang/runtime

@adonovan adonovan added the compiler/runtime Issues related to the Go compiler and/or runtime. label Apr 8, 2025
@gabyhelp gabyhelp added the BugReport Issues describing a possible bug in the Go implementation. label Apr 8, 2025
@adonovan adonovan added the gopls/memory-corruption "can't happen" gopls crashes (races, unsafe, miscompile, runtime bugs, faulty HW) label Apr 10, 2025
@mknyszek
Copy link
Contributor

In triage, @randall77 notes that it looks like the defer isn't properly taken off of the defer list, so it gets run twice. But what's weird is the first time defer is called is as an open-coded defer, and we only create the defer record the second time.

@mknyszek mknyszek added this to the Backlog milestone Apr 16, 2025
@randall77
Copy link
Contributor

I looked into this for a while, nothing obvious. All the compiler-generated metadata looks good.
There are various defer wrappers that complicate things, but nothing obviously wrong.

My only guess is that a stack copy happens at exactly the wrong time, causing defer selection to fail. But that would, I think, only cause defers to be missed, not duplicated. But who knows.
We're probably going to need a reproducer to make more progress.

@adonovan
Copy link
Member Author

We're probably going to need a reproducer to make more progress.

I suspect this just another the of many varied symptoms of a single bug causing memory corruption; see label. Unfortunately we haven't been able to (knowingly) reproduce any of them even once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BugReport Issues describing a possible bug in the Go implementation. compiler/runtime Issues related to the Go compiler and/or runtime. gopls/memory-corruption "can't happen" gopls crashes (races, unsafe, miscompile, runtime bugs, faulty HW) gopls/telemetry-wins gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
Development

No branches or pull requests

4 participants