From 969ab34e4629bdda410c8468d4f45a08e4fec9f8 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Tue, 18 Apr 2023 15:10:46 -0700 Subject: [PATCH] go/types, types2: don't panic for invalid assignments of comma-ok expressions The relevant code was broken with CL 478218. Before that CL, Checker.assignVar used to return the assigned type, or nil, in case of failure. Checker.recordCommaOkTypes used to take two types (not two operands), and if one of those types was nil, it would simply not record. CL 478218, lost that (nil) signal. This change consistently reports an assignment check failure by setting x.mode to invalid for initVar and assignVar and then tests if x.mode != invalid before recording a comma-ok expression. Fixes #59371. Change-Id: I193815ff3e4b43e3e510fe25bd0e72e0a6a816c6 Reviewed-on: https://go-review.googlesource.com/c/go/+/486135 Reviewed-by: Robert Griesemer Run-TryBot: Robert Griesemer Auto-Submit: Robert Griesemer Reviewed-by: Robert Findley TryBot-Result: Gopher Robot --- .../compile/internal/types2/assignments.go | 21 +++++++++++++----- src/go/types/assignments.go | 22 ++++++++++++++----- .../types/testdata/fixedbugs/issue59371.go | 20 +++++++++++++++++ 3 files changed, 51 insertions(+), 12 deletions(-) create mode 100644 src/internal/types/testdata/fixedbugs/issue59371.go diff --git a/src/cmd/compile/internal/types2/assignments.go b/src/cmd/compile/internal/types2/assignments.go index 30ed9ae701..59f29b8f62 100644 --- a/src/cmd/compile/internal/types2/assignments.go +++ b/src/cmd/compile/internal/types2/assignments.go @@ -17,7 +17,7 @@ import ( // if necessary by attempting to convert untyped values to the appropriate // type. context describes the context in which the assignment takes place. // Use T == nil to indicate assignment to an untyped blank identifier. -// x.mode is set to invalid if the assignment failed. +// If the assignment check fails, x.mode is set to invalid. func (check *Checker) assignment(x *operand, T Type, context string) { check.singleValue(x) @@ -135,11 +135,14 @@ func (check *Checker) initConst(lhs *Const, x *operand) { // initVar checks the initialization lhs = x in a variable declaration. // If lhs doesn't have a type yet, it is given the type of x, // or Typ[Invalid] in case of an error. +// If the initialization check fails, x.mode is set to invalid. func (check *Checker) initVar(lhs *Var, x *operand, context string) { if x.mode == invalid || x.typ == Typ[Invalid] || lhs.typ == Typ[Invalid] { if lhs.typ == nil { lhs.typ = Typ[Invalid] } + x.mode = invalid + return } // If lhs doesn't have a type yet, use the type of x. @@ -150,6 +153,7 @@ func (check *Checker) initVar(lhs *Var, x *operand, context string) { if typ == Typ[UntypedNil] { check.errorf(x, UntypedNilUse, "use of untyped nil in %s", context) lhs.typ = Typ[Invalid] + x.mode = invalid return } typ = Default(typ) @@ -227,10 +231,14 @@ func (check *Checker) lhsVar(lhs syntax.Expr) Type { // assignVar checks the assignment lhs = rhs (if x == nil), or lhs = x (if x != nil). // If x != nil, it must be the evaluation of rhs (and rhs will be ignored). +// If the assignment check fails and x != nil, x.mode is set to invalid. func (check *Checker) assignVar(lhs, rhs syntax.Expr, x *operand) { T := check.lhsVar(lhs) // nil if lhs is _ if T == Typ[Invalid] { check.use(rhs) + if x != nil { + x.mode = invalid + } return } @@ -238,9 +246,6 @@ func (check *Checker) assignVar(lhs, rhs syntax.Expr, x *operand) { x = new(operand) check.expr(T, x, rhs) } - if x.mode == invalid { - return - } context := "assignment" if T == nil { @@ -396,7 +401,9 @@ func (check *Checker) initVars(lhs []*Var, orig_rhs []syntax.Expr, returnStmt sy for i, lhs := range lhs { check.initVar(lhs, rhs[i], context) } - if commaOk { + // Only record comma-ok expression if both initializations succeeded + // (go.dev/issue/59371). + if commaOk && rhs[0].mode != invalid && rhs[1].mode != invalid { check.recordCommaOkTypes(orig_rhs[0], rhs) } return @@ -458,7 +465,9 @@ func (check *Checker) assignVars(lhs, orig_rhs []syntax.Expr) { for i, lhs := range lhs { check.assignVar(lhs, nil, rhs[i]) } - if commaOk { + // Only record comma-ok expression if both assignments succeeded + // (go.dev/issue/59371). + if commaOk && rhs[0].mode != invalid && rhs[1].mode != invalid { check.recordCommaOkTypes(orig_rhs[0], rhs) } return diff --git a/src/go/types/assignments.go b/src/go/types/assignments.go index 71fbbea46f..f036142caa 100644 --- a/src/go/types/assignments.go +++ b/src/go/types/assignments.go @@ -17,7 +17,7 @@ import ( // if necessary by attempting to convert untyped values to the appropriate // type. context describes the context in which the assignment takes place. // Use T == nil to indicate assignment to an untyped blank identifier. -// x.mode is set to invalid if the assignment failed. +// If the assignment check fails, x.mode is set to invalid. func (check *Checker) assignment(x *operand, T Type, context string) { check.singleValue(x) @@ -73,6 +73,7 @@ func (check *Checker) assignment(x *operand, T Type, context string) { check.updateExprType(x.expr, newType, false) } } + // x.typ is typed // A generic (non-instantiated) function value cannot be assigned to a variable. if sig, _ := under(x.typ).(*Signature); sig != nil && sig.TypeParams().Len() > 0 { @@ -133,11 +134,14 @@ func (check *Checker) initConst(lhs *Const, x *operand) { // initVar checks the initialization lhs = x in a variable declaration. // If lhs doesn't have a type yet, it is given the type of x, // or Typ[Invalid] in case of an error. +// If the initialization check fails, x.mode is set to invalid. func (check *Checker) initVar(lhs *Var, x *operand, context string) { if x.mode == invalid || x.typ == Typ[Invalid] || lhs.typ == Typ[Invalid] { if lhs.typ == nil { lhs.typ = Typ[Invalid] } + x.mode = invalid + return } // If lhs doesn't have a type yet, use the type of x. @@ -148,6 +152,7 @@ func (check *Checker) initVar(lhs *Var, x *operand, context string) { if typ == Typ[UntypedNil] { check.errorf(x, UntypedNilUse, "use of untyped nil in %s", context) lhs.typ = Typ[Invalid] + x.mode = invalid return } typ = Default(typ) @@ -225,10 +230,14 @@ func (check *Checker) lhsVar(lhs ast.Expr) Type { // assignVar checks the assignment lhs = rhs (if x == nil), or lhs = x (if x != nil). // If x != nil, it must be the evaluation of rhs (and rhs will be ignored). +// If the assignment check fails and x != nil, x.mode is set to invalid. func (check *Checker) assignVar(lhs, rhs ast.Expr, x *operand) { T := check.lhsVar(lhs) // nil if lhs is _ if T == Typ[Invalid] { check.use(rhs) + if x != nil { + x.mode = invalid + } return } @@ -236,9 +245,6 @@ func (check *Checker) assignVar(lhs, rhs ast.Expr, x *operand) { x = new(operand) check.expr(T, x, rhs) } - if x.mode == invalid { - return - } context := "assignment" if T == nil { @@ -394,7 +400,9 @@ func (check *Checker) initVars(lhs []*Var, orig_rhs []ast.Expr, returnStmt ast.S for i, lhs := range lhs { check.initVar(lhs, rhs[i], context) } - if commaOk { + // Only record comma-ok expression if both initializations succeeded + // (go.dev/issue/59371). + if commaOk && rhs[0].mode != invalid && rhs[1].mode != invalid { check.recordCommaOkTypes(orig_rhs[0], rhs) } return @@ -456,7 +464,9 @@ func (check *Checker) assignVars(lhs, orig_rhs []ast.Expr) { for i, lhs := range lhs { check.assignVar(lhs, nil, rhs[i]) } - if commaOk { + // Only record comma-ok expression if both assignments succeeded + // (go.dev/issue/59371). + if commaOk && rhs[0].mode != invalid && rhs[1].mode != invalid { check.recordCommaOkTypes(orig_rhs[0], rhs) } return diff --git a/src/internal/types/testdata/fixedbugs/issue59371.go b/src/internal/types/testdata/fixedbugs/issue59371.go new file mode 100644 index 0000000000..d60810a6f0 --- /dev/null +++ b/src/internal/types/testdata/fixedbugs/issue59371.go @@ -0,0 +1,20 @@ +// Copyright 2023 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package p + +var m map[int]int + +func _() { + _, ok /* ERROR "undefined: ok" */ = m[0] // must not crash +} + +func _() { + var ok = undef /* ERROR "undefined: undef" */ + x, ok := m[0] // must not crash + _ = x + // The next line is only needed for go/types, not types2. + // TODO(gri) find cause and fix + _ = ok +} -- 2.48.1