Skip to content

Commit

Permalink
gopls/internal/cache: reproduce and fix crash on if cond overflow
Browse files Browse the repository at this point in the history
Through reverse engineering, I was able to reproduce the overflow of
golang/go#72026, and verify the fix of CL 653596.

Along the way, I incidentally reproduced golang/go#66766, which I think
we can safely ignore now that we understand it.

Updates golang/go#72026
Fixes golang/go#66766

Change-Id: I2131d771c13688c1ad47f6bc6285e524fb4c04a1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/654336
Reviewed-by: Alan Donovan <[email protected]>
Auto-Submit: Robert Findley <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
  • Loading branch information
findleyr authored and gopherbot committed Mar 4, 2025
1 parent d81d6fc commit 8d38122
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 8 deletions.
15 changes: 7 additions & 8 deletions gopls/internal/cache/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -2005,15 +2005,14 @@ func typeErrorsToDiagnostics(pkg *syntaxPackage, inputs *typeCheckInputs, errs [
posn := safetoken.StartPosition(e.Fset, start)
if !posn.IsValid() {
// All valid positions produced by the type checker should described by
// its fileset.
// its fileset, yet since type checker errors are associated with
// positions in the AST, and AST nodes can overflow the file
// (golang/go#48300), we can't rely on this.
//
// Note: in golang/go#64488, we observed an error that was positioned
// over fixed syntax, which overflowed its file. So it's definitely
// possible that we get here (it's hard to reason about fixing up the
// AST). Nevertheless, it's a bug.
if pkg.hasFixedFiles() {
bug.Reportf("internal error: type checker error %q outside its Fset (fixed files)", e)
} else {
// We should fix the parser, but in the meantime type errors are not
// significant if there are parse errors, so we can safely ignore this
// case.
if len(pkg.parseErrors) == 0 {
bug.Reportf("internal error: type checker error %q outside its Fset", e)
}
continue
Expand Down
1 change: 1 addition & 0 deletions gopls/internal/cache/parsego/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,7 @@ func fixInitStmt(bad *ast.BadExpr, parent ast.Node, tok *token.File, src []byte)
if err != nil {
return false
}
assert(end <= len(src), "offset overflow") // golang/go#72026
stmtBytes := src[start:end]
stmt, err := parseStmt(tok, bad.Pos(), stmtBytes)
if err != nil {
Expand Down
17 changes: 17 additions & 0 deletions gopls/internal/test/integration/completion/fixedbugs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,20 @@ package
}
})
}

func TestFixInitStatementCrash_Issue72026(t *testing.T) {
// This test checks that we don't crash when the if condition overflows the
// file (as is possible with a malformed struct type).

const files = `
-- go.mod --
module example.com
go 1.18
`

Run(t, files, func(t *testing.T, env *Env) {
env.CreateBuffer("p.go", "package p\nfunc _() {\n\tfor i := struct")
env.AfterChange()
})
}

0 comments on commit 8d38122

Please sign in to comment.