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/internal/test/integration/misc: unrecognized failures #65862

Open
gopherbot opened this issue Feb 21, 2024 · 7 comments
Open

x/tools/gopls/internal/test/integration/misc: unrecognized failures #65862

gopherbot opened this issue Feb 21, 2024 · 7 comments
Labels
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.16.0

Comments

@gopherbot
Copy link 

#!watchflakes
default <- pkg == "golang.org/x/tools/gopls/internal/test/integration/misc" && test == ""

Issue created automatically to collect these failures.

Example (log):

serve.go:441: debug server listening at http://localhost:40237
serve.go:441: debug server listening at http://localhost:33159
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x567830]

goroutine 45869 [running]:
golang.org/x/tools/internal/gcimporter.iexportCommon.func1()
	/workdir/gopath/src/golang.org/x/tools/internal/gcimporter/iexport.go:106 +0xb0
panic({0xb61920?, 0x15b7d50?})
	/workdir/go/src/runtime/panic.go:759 +0x154
go/token.(*File).Offset(...)
	/workdir/go/src/go/token/position.go:315
golang.org/x/tools/internal/gcimporter.(*iexporter).fileIndexAndOffset(0xc002685300, 0x0, 0x100000000000006f)
	/workdir/gopath/src/golang.org/x/tools/internal/gcimporter/iexport.go:409 +0x229
golang.org/x/tools/internal/gcimporter.(*exportWriter).posV2(0xc002ba79a0, 0x100000000000006f)
	/workdir/gopath/src/golang.org/x/tools/internal/gcimporter/iexport.go:610 +0x94
golang.org/x/tools/internal/gcimporter.(*exportWriter).pos(0xc002ba79a8?, 0x430000c0013de806?)
	/workdir/gopath/src/golang.org/x/tools/internal/gcimporter/iexport.go:592 +0x60
golang.org/x/tools/internal/gcimporter.(*iexporter).doDecl(0xc002685300, {0xf74ea8, 0xc00125f380})
	/workdir/gopath/src/golang.org/x/tools/internal/gcimporter/iexport.go:503 +0x2a4
golang.org/x/tools/internal/gcimporter.iexportCommon({0xf63558, 0xc008979440}, 0xc002aad840, 0x0, 0x1, 0x2, {0xc00226aec8, 0x1, 0x0?})
	/workdir/gopath/src/golang.org/x/tools/internal/gcimporter/iexport.go:152 +0x35c
golang.org/x/tools/internal/gcimporter.IExportShallow(0xc002aad840, 0xc00125ecc0, 0xf6c830?)
	/workdir/gopath/src/golang.org/x/tools/internal/gcimporter/iexport.go:48 +0x88
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).checkPackageForImport.func3()
	/workdir/gopath/src/golang.org/x/tools/gopls/internal/cache/check.go:726 +0x4c
created by golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).checkPackageForImport in goroutine 45861
	/workdir/gopath/src/golang.org/x/tools/gopls/internal/cache/check.go:725 +0x590

— watchflakes
@gopherbot gopherbot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 21, 2024
@gopherbot
Copy link
Author

Found new dashboard test flakes for:

#!watchflakes
default <- pkg == "golang.org/x/tools/gopls/internal/test/integration/misc" && test == ""
2024-02-21 22:46 linux-ppc64-sid-buildlet tools@c111c4df go@507d1b22 x/tools/gopls/internal/test/integration/misc (log) 
serve.go:441: debug server listening at http://localhost:40237
serve.go:441: debug server listening at http://localhost:33159
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x567830]

goroutine 45869 [running]:
golang.org/x/tools/internal/gcimporter.iexportCommon.func1()
	/workdir/gopath/src/golang.org/x/tools/internal/gcimporter/iexport.go:106 +0xb0
panic({0xb61920?, 0x15b7d50?})
	/workdir/go/src/runtime/panic.go:759 +0x154
go/token.(*File).Offset(...)
	/workdir/go/src/go/token/position.go:315
golang.org/x/tools/internal/gcimporter.(*iexporter).fileIndexAndOffset(0xc002685300, 0x0, 0x100000000000006f)
	/workdir/gopath/src/golang.org/x/tools/internal/gcimporter/iexport.go:409 +0x229
golang.org/x/tools/internal/gcimporter.(*exportWriter).posV2(0xc002ba79a0, 0x100000000000006f)
	/workdir/gopath/src/golang.org/x/tools/internal/gcimporter/iexport.go:610 +0x94
golang.org/x/tools/internal/gcimporter.(*exportWriter).pos(0xc002ba79a8?, 0x430000c0013de806?)
	/workdir/gopath/src/golang.org/x/tools/internal/gcimporter/iexport.go:592 +0x60
golang.org/x/tools/internal/gcimporter.(*iexporter).doDecl(0xc002685300, {0xf74ea8, 0xc00125f380})
	/workdir/gopath/src/golang.org/x/tools/internal/gcimporter/iexport.go:503 +0x2a4
golang.org/x/tools/internal/gcimporter.iexportCommon({0xf63558, 0xc008979440}, 0xc002aad840, 0x0, 0x1, 0x2, {0xc00226aec8, 0x1, 0x0?})
	/workdir/gopath/src/golang.org/x/tools/internal/gcimporter/iexport.go:152 +0x35c
golang.org/x/tools/internal/gcimporter.IExportShallow(0xc002aad840, 0xc00125ecc0, 0xf6c830?)
	/workdir/gopath/src/golang.org/x/tools/internal/gcimporter/iexport.go:48 +0x88
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).checkPackageForImport.func3()
	/workdir/gopath/src/golang.org/x/tools/gopls/internal/cache/check.go:726 +0x4c
created by golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).checkPackageForImport in goroutine 45861
	/workdir/gopath/src/golang.org/x/tools/gopls/internal/cache/check.go:725 +0x590

— watchflakes

@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Feb 21, 2024
@gopherbot gopherbot added this to the Unreleased milestone Feb 21, 2024
@hyangah
Copy link
Contributor

hyangah commented Feb 22, 2024

This is a new crash pattern.
@adonovan @findleyr Adding to v0.15.0 milestone for quick investigation before the release.

@hyangah hyangah modified the milestones: Unreleased, gopls/v0.15.0 Feb 22, 2024
@findleyr
Copy link
Contributor

Hmm, I don't think this logic changed recently, though I can imagine that there could be bugs related to adjusted positions overflowing the file.

However, I tried to reproduce by e.g. looking for low position offsets, which could occur for when a position overflows a file. The lowest offset I found was 16, corresponding to other.com/b/b.go in TestImplementationsInVendor. I.e. I didn't observe any positions that overflowed, and found the wrong file.

But as an aside, TestImplementationsInVendor was only recently unskipped, in the CL preceding this failure: https://go.dev/cl/565458. The timing suggests that it may be related, though I don't see how.

@findleyr
Copy link
Contributor

Interestingly, this looks like a dupe of https://go.dev/issue/59080, which also occurred on ppc64.

@findleyr
Copy link
Contributor

I'll note that our debugging check for #59080 is still in place, but wouldn't have fired because we stopped using the parse cache when type checking for import in https://go.dev/cl/511337. Hmm.

@findleyr
Copy link
Contributor

Well, this is really puzzling since (1) I think if this were an overflowing amended position, we'd hit this flake more frequently, and (2) I believe checkPackageForImport is itself not using the tricky stuff we do with the token.FileSet (though we could call tokeninternal.AddExistingFiles from another operation).

It's almost as though the locking in AddExistingFiles isn't working as expected, and the export is observing the fileset in some broken state. I really can't make heads or tails of it.

Since this does appear to be a dupe of a flake from March, both on ppc64, I'm inclined to say that it probably spurious, and not worth blocking the v0.15.0 release. Let's see if the flake recurs in the next few days.

@findleyr findleyr modified the milestones: gopls/v0.15.0, gopls/v0.15.1 Feb 22, 2024
@findleyr
Copy link
Contributor

Moving to gopls@v0.15.1, at least to yet again add debugging instrumentation. @adonovan @hyangah let me know if you disagree with this decision, in light of the investigation above.

@findleyr findleyr modified the milestones: gopls/v0.15.1, gopls/v0.16.0 Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
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
Test Flakes
Status: No status
Milestone
gopls/v0.16.0
Development

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