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

x/tools/gopls: out-of-bounds index in "stub methods" code action #64087

Closed
adonovan opened this issue Nov 12, 2023 · 6 comments
Closed

x/tools/gopls: out-of-bounds index in "stub methods" code action #64087

adonovan opened this issue Nov 12, 2023 · 6 comments
Assignees
@adonovan
Labels
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
gopls/v0.15.0

Comments

@adonovan
Copy link
Member

adonovan commented Nov 12, 2023 •
edited

Another crash w2Y-6g, H0UbWA reported by telemetry:

golang.org/x/tools/gopls/internal/lsp/analysis/stubmethods fromReturnStmt:18
golang.org/x/tools/gopls/internal/lsp/analysis/stubmethods GetStubInfo:10
golang.org/x/tools/gopls/internal/lsp/analysis/stubmethods DiagnosticForError:6
golang.org/x/tools/gopls/internal/lsp (*Server).codeAction
golang.org/x/tools/gopls/internal/lsp (*Server).codeAction:210
golang.org/x/tools/gopls/internal/lsp (*Server).CodeAction:1
- golang.org/x/tools/gopls@v0.14.1 go1.19.1 darwin/amd64 (1)

func fromReturnStmt(fset *token.FileSet, ti *types.Info, pos token.Pos, path []ast.Node, rs *ast.ReturnStmt) (*StubInfo, error) {
	returnIdx := -1
	for i, r := range rs.Results {
		if pos >= r.Pos() && pos <= r.End() {
			returnIdx = i
		}
	}
	if returnIdx == -1 {
		return nil, fmt.Errorf("pos %d not within return statement bounds: [%d-%d]", pos, rs.Pos(), rs.End())
	}
	concObj, pointer := concreteType(rs.Results[returnIdx], ti)
	if concObj == nil || concObj.Obj().Pkg() == nil {
		return nil, nil
	}
	ef := enclosingFunction(path, ti)
	if ef == nil {
		return nil, fmt.Errorf("could not find the enclosing function of the return statement")
	}
	iface := ifaceType(ef.Results.List[returnIdx].Type, ti) /// <--- this line fails, presumably because the index is OOB
	if iface == nil {
		return nil, nil
	}
...

Duplicates:

This stack R0oUQw was reported by telemetry:

gopls/bug
golang.org/x/tools/gopls/internal/bug.report:35
golang.org/x/tools/gopls/internal/bug.Errorf:2
golang.org/x/tools/gopls/internal/lsp.(*Server).codeAction.func1.1:2
runtime.gopanic:88
runtime.panicmem:?261
runtime.sigpanic:9
golang.org/x/tools/gopls/internal/lsp/analysis/stubmethods.fromReturnStmt:18
golang.org/x/tools/gopls/internal/lsp/analysis/stubmethods.GetStubInfo:10
golang.org/x/tools/gopls/internal/lsp/analysis/stubmethods.DiagnosticForError:6
golang.org/x/tools/gopls/internal/lsp.(*Server).codeAction.func1:11
golang.org/x/tools/gopls/internal/lsp.(*Server).codeAction:210
golang.org/x/tools/gopls/internal/lsp.(*Server).CodeAction:1
golang.org/x/tools/gopls/internal/lsp/protocol.serverDispatch:142
golang.org/x/tools/gopls/internal/lsp/lsprpc.(*StreamServer).ServeStream.ServerHandler.func3:5
golang.org/x/tools/gopls/internal/lsp/lsprpc.(*StreamServer).ServeStream.handshaker.func4:52
golang.org/x/tools/gopls/internal/lsp/protocol.Handlers.MustReplyHandler.func1:2

golang.org/x/tools/gopls@v0.14.2 go1.21.4 windows/amd64 vscode (1)

Issue created by golang.org/x/tools/gopls/internal/telemetry/cmd/stacks.

Dups: NNo9gw 8eUSuQ Mw6nJQ
@gopherbot gopherbot added the gopls Issues related to the Go language server, gopls. label Nov 12, 2023
@adonovan adonovan added 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. and removed gopls Issues related to the Go language server, gopls. labels Nov 12, 2023
@adonovan
Copy link
Member Author

This program reproduces the panic. The int("") triggers a "cannot convert" error, which causes GetStubInfo to assume the problem is because of a mismatched return operand and result type, but in fact there is no corresponding result type.

package p

func f() error {
	return nil, myerror{int("")}
}

type myerror struct{}

@seankhliao seankhliao changed the title gopls: out-of-bounds index in "stub methods" code action x/tools/gopls: out-of-bounds index in "stub methods" code action Nov 12, 2023
@gopherbot gopherbot added this to the Unreleased milestone Nov 12, 2023
@findleyr findleyr modified the milestones: Unreleased, gopls/v0.15.0 Nov 13, 2023
@gopherbot
Copy link

Change https://go.dev/cl/543018 mentions this issue: gopls/internal/lsp/source: stubmethods: fix out-of-bounds index

@adonovan adonovan self-assigned this Nov 16, 2023
@adonovan adonovan changed the title x/tools/gopls: out-of-bounds index in "stub methods" code action x/tools/gopls: out-of-bounds index in "stub methods" code action [w2Y-6g] Nov 17, 2023
@adonovan adonovan changed the title x/tools/gopls: out-of-bounds index in "stub methods" code action [w2Y-6g] x/tools/gopls: out-of-bounds index in "stub methods" code action Nov 17, 2023
@adonovan adonovan added the Tools This label describes issues relating to any tools in the x/tools repository. label Nov 17, 2023
@adonovan adonovan mentioned this issue Dec 4, 2023
Closed
@gopherbot
Copy link

Change https://go.dev/cl/548737 mentions this issue: gopls/internal/analysis/stubmethods: fix OOB panic in fromValueSpec

gopherbot pushed a commit to golang/tools that referenced this issue Dec 11, 2023
@adonovan @gopherbot
gopls/internal/analysis/stubmethods: fix OOB panic in fromValueSpec
f40889d 
The loop over ValueSpec.Rhs assumed it was nonempty, but it's
possible to spuriously trigger a "cannot convert" error when
the problem is on the LHS. (I don't know if the attached test
case is what caused the panic in the field, but it seems
plausible.)

Added a test, similar to the one to fix golang/go#64087.

Also, audit for similar mistakes, and tidy the code up,
to use conventional variable names and simpler logic.

Fixes golang/go#64545

Change-Id: I49851435e7a363641a2844620633099b11f1e414
Reviewed-on: https://go-review.googlesource.com/c/tools/+/548737
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@adonovan
Copy link
Member Author

This stack Mw6nJQ was reported by telemetry:

gopls/bug
golang.org/x/tools/gopls/internal/bug.report:35
golang.org/x/tools/gopls/internal/bug.Errorf:2
golang.org/x/tools/gopls/internal/lsp.(*Server).codeAction.func1.1:2
runtime.gopanic:80
runtime.panicmem:?260
runtime.sigpanic:9
golang.org/x/tools/gopls/internal/lsp/analysis/stubmethods.fromReturnStmt:18
golang.org/x/tools/gopls/internal/lsp/analysis/stubmethods.GetStubInfo:10
golang.org/x/tools/gopls/internal/lsp/analysis/stubmethods.DiagnosticForError:6
golang.org/x/tools/gopls/internal/lsp.(*Server).codeAction.func1:11
golang.org/x/tools/gopls/internal/lsp.(*Server).codeAction:210
golang.org/x/tools/gopls/internal/lsp.(*Server).CodeAction:1
golang.org/x/tools/gopls/internal/lsp/protocol.serverDispatch:142
golang.org/x/tools/gopls/internal/lsp/protocol.ServerHandler.func1:5
golang.org/x/tools/gopls/internal/lsp/lsprpc.handshaker.func1:52
golang.org/x/tools/internal/jsonrpc2.MustReplyHandler.func1:2

golang.org/x/tools/gopls@v0.14.2 go1.19.6 windows/amd64 vscode (1)

Issue created by golang.org/x/tools/gopls/internal/telemetry/cmd/stacks.

@adonovan
Copy link
Member Author

This stack 8eUSuQ was reported by telemetry:

gopls/bug
golang.org/x/tools/gopls/internal/bug.report:35
golang.org/x/tools/gopls/internal/bug.Errorf:2
golang.org/x/tools/gopls/internal/lsp.(*Server).codeAction.func1.1:2
runtime.gopanic:80
runtime.goPanicIndex:2
golang.org/x/tools/gopls/internal/lsp/analysis/stubmethods.fromReturnStmt:18
golang.org/x/tools/gopls/internal/lsp/analysis/stubmethods.GetStubInfo:10
golang.org/x/tools/gopls/internal/lsp/analysis/stubmethods.DiagnosticForError:6
golang.org/x/tools/gopls/internal/lsp.(*Server).codeAction.func1:11
golang.org/x/tools/gopls/internal/lsp.(*Server).codeAction:210
golang.org/x/tools/gopls/internal/lsp.(*Server).CodeAction:1
golang.org/x/tools/gopls/internal/lsp/protocol.serverDispatch:142
golang.org/x/tools/gopls/internal/lsp/protocol.ServerHandler.func1:5
golang.org/x/tools/gopls/internal/lsp/lsprpc.handshaker.func1:52
golang.org/x/tools/internal/jsonrpc2.MustReplyHandler.func1:2
golang.org/x/tools/internal/jsonrpc2.AsyncHandler.func1.2:3

golang.org/x/tools/gopls@v0.14.2 go1.20.4 linux/amd64 vscode (1)

Issue created by golang.org/x/tools/gopls/internal/telemetry/cmd/stacks.

@adonovan
Copy link
Member Author

This stack NNo9gw was reported by telemetry:

gopls/bug
golang.org/x/tools/gopls/internal/bug.report:35
golang.org/x/tools/gopls/internal/bug.Errorf:2
golang.org/x/tools/gopls/internal/lsp.(*Server).codeAction.func1.1:2
runtime.gopanic:50
runtime.panicmem:?261
runtime.sigpanic:19
golang.org/x/tools/gopls/internal/lsp/analysis/stubmethods.fromReturnStmt:18
golang.org/x/tools/gopls/internal/lsp/analysis/stubmethods.GetStubInfo:10
golang.org/x/tools/gopls/internal/lsp/analysis/stubmethods.DiagnosticForError:6
golang.org/x/tools/gopls/internal/lsp.(*Server).codeAction.func1:11
golang.org/x/tools/gopls/internal/lsp.(*Server).codeAction:210
golang.org/x/tools/gopls/internal/lsp.(*Server).CodeAction:1
golang.org/x/tools/gopls/internal/lsp/protocol.serverDispatch:142
golang.org/x/tools/gopls/internal/lsp/lsprpc.(*StreamServer).ServeStream.ServerHandler.func3:5
golang.org/x/tools/gopls/internal/lsp/lsprpc.(*StreamServer).ServeStream.handshaker.func4:52
golang.org/x/tools/gopls/internal/lsp/protocol.Handlers.MustReplyHandler.func1:2

golang.org/x/tools/gopls@v0.14.2 go1.22.0 darwin/arm64 vscode (2)
golang.org/x/tools/gopls@v0.14.2 devel darwin/arm64 vscode (1)

Issue created by golang.org/x/tools/gopls/internal/telemetry/cmd/stacks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@adonovan adonovan

Labels
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
Milestone
gopls/v0.15.0
Development

No branches or pull requests
3 participants
@adonovan @gopherbot @findleyr