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

x/tools/gopls: Completion: SIGSEGV in scorePenalty #71355

Closed
adonovan opened this issue Jan 20, 2025 · 3 comments
Closed

x/tools/gopls: Completion: SIGSEGV in scorePenalty #71355

adonovan opened this issue Jan 20, 2025 · 3 comments
Labels
BugReport Issues describing a possible bug in the Go implementation. 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

#!stacks
"sigpanic"&& "completion.(*deepCompletionState).scorePenalty:+2"

Issue created by stacks.

// scorePenalty computes a deep candidate score penalty. A candidate is
// penalized based on depth to favor shallower candidates. We also give a
// slight bonus to unexported objects and a slight additional penalty to
// function objects.
func (s *deepCompletionState) scorePenalty(cand *candidate) float64 {
	var deepPenalty float64
	for _, dc := range cand.path { <-- sigpanic
		deepPenalty++

		if !dc.Exported() {
			deepPenalty -= 0.1
		}

		if _, isSig := dc.Type().Underlying().(*types.Signature); isSig {
			deepPenalty += 0.1
		}
	}

	// Normalize penalty to a max depth of 10.
	return deepPenalty / 10
}

This stack AZThiA was reported by telemetry:

golang.org/x/tools/[email protected] go1.23.4 linux/amd64 vscode (1)
@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 Jan 20, 2025
@gopherbot gopherbot added this to the Unreleased milestone Jan 20, 2025
@adonovan adonovan changed the title x/tools/gopls: Completion: nil deref in scorePenalty x/tools/gopls: Completion: SIGSEGV in scorePenalty Jan 20, 2025
@adonovan
Copy link
Member Author

On the surface this looks like a nil dereference, but there's no way that a nil candidate value can reach this point. Furthermore, the dominating statements not only dereference the pointer but load fields at both lower and higher offsets than cand.path (such as c.obj and c.mods), so this can't be explained by cand being a pointer to bad memory near the bottom or top of a mapping (even though that would itself be very weird).

Even if the stack reporting was off by one, there is no plausible explanation on an adjacent line. The only thing that comes to mind is either a runtime bug or a hardware fault. Most unsatisfactory.

cc: @prattmic

@gabyhelp gabyhelp added the BugReport Issues describing a possible bug in the Go implementation. label Jan 20, 2025
@adonovan adonovan modified the milestones: Unreleased, gopls/v0.18.0 Jan 21, 2025
@findleyr
Copy link
Member

This came from the same report as #71177, which is similarly inexplicable. In both cases, it was a one-off report.
That seems to be sufficient evidence of faulty hardware.
There may still be a data race, but that is tracked by #71425.
Therefore, closing this as unactionable (and likely a red herring).

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. 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
None yet
Development

No branches or pull requests

4 participants