From 412ee174ef74e13441079fb4bf293dd4101dcdcf Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Sat, 10 Jul 2021 10:37:28 +0700 Subject: [PATCH] all: add SliceToArrayPointer instruction CL 332049 added support for slice to array pointer, which introduced in go1.17, by using Convert instruction. However, the conversion can fail dynamically, while all current conversion instructions can not. That leads to inconsistent with other parts of SSA IR. @timothy-king suggested to add new instruction for this conversion, this CL implements that. Fixes golang/go#46987 Change-Id: I1a00d51e257d2b3eabc2c37e3a09b78754193a78 Reviewed-on: https://go-review.googlesource.com/c/tools/+/333749 Trust: Cuong Manh Le Trust: Bryan C. Mills Run-TryBot: Cuong Manh Le gopls-CI: kokoro TryBot-Result: Go Bot Reviewed-by: Tim King --- go/ssa/builder.go | 2 + go/ssa/builder_go117_test.go | 31 +++++++++ go/ssa/doc.go | 89 ++++++++++++------------ go/ssa/emit.go | 2 +- go/ssa/interp/interp.go | 3 + go/ssa/interp/interp_go117_test.go | 12 ++++ go/ssa/interp/ops.go | 23 ++++++ go/ssa/interp/testdata/slice2arrayptr.go | 23 ++++++ go/ssa/print.go | 9 +-- go/ssa/sanity.go | 8 +-- go/ssa/ssa.go | 21 +++++- 11 files changed, 166 insertions(+), 57 deletions(-) create mode 100644 go/ssa/interp/interp_go117_test.go create mode 100644 go/ssa/interp/testdata/slice2arrayptr.go diff --git a/go/ssa/builder.go b/go/ssa/builder.go index 2d0fdaa4e6..e1540dbdc0 100644 --- a/go/ssa/builder.go +++ b/go/ssa/builder.go @@ -579,6 +579,8 @@ func (b *builder) expr0(fn *Function, e ast.Expr, tv types.TypeAndValue) Value { y.pos = e.Lparen case *MakeInterface: y.pos = e.Lparen + case *SliceToArrayPointer: + y.pos = e.Lparen } } return y diff --git a/go/ssa/builder_go117_test.go b/go/ssa/builder_go117_test.go index e7ba21422f..f6545e5e2c 100644 --- a/go/ssa/builder_go117_test.go +++ b/go/ssa/builder_go117_test.go @@ -49,3 +49,34 @@ func TestBuildPackageGo117(t *testing.T) { }) } } + +func TestBuildPackageFailuresGo117(t *testing.T) { + tests := []struct { + name string + src string + importer types.Importer + }{ + {"slice to array pointer - source is not a slice", "package p; var s [4]byte; var _ = (*[4]byte)(s)", nil}, + {"slice to array pointer - dest is not a pointer", "package p; var s []byte; var _ = ([4]byte)(s)", nil}, + {"slice to array pointer - dest pointer elem is not an array", "package p; var s []byte; var _ = (*byte)(s)", nil}, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, "p.go", tc.src, parser.ParseComments) + if err != nil { + t.Error(err) + } + files := []*ast.File{f} + + pkg := types.NewPackage("p", "") + conf := &types.Config{Importer: tc.importer} + if _, _, err := ssautil.BuildPackage(conf, fset, pkg, files, ssa.SanityCheckFunctions); err == nil { + t.Error("want error, but got nil") + } + }) + } +} diff --git a/go/ssa/doc.go b/go/ssa/doc.go index 1a13640f9d..fe0099bb8d 100644 --- a/go/ssa/doc.go +++ b/go/ssa/doc.go @@ -50,50 +50,51 @@ // Instruction interfaces. The following table shows for each // concrete type which of these interfaces it implements. // -// Value? Instruction? Member? -// *Alloc ✔ ✔ -// *BinOp ✔ ✔ -// *Builtin ✔ -// *Call ✔ ✔ -// *ChangeInterface ✔ ✔ -// *ChangeType ✔ ✔ -// *Const ✔ -// *Convert ✔ ✔ -// *DebugRef ✔ -// *Defer ✔ -// *Extract ✔ ✔ -// *Field ✔ ✔ -// *FieldAddr ✔ ✔ -// *FreeVar ✔ -// *Function ✔ ✔ (func) -// *Global ✔ ✔ (var) -// *Go ✔ -// *If ✔ -// *Index ✔ ✔ -// *IndexAddr ✔ ✔ -// *Jump ✔ -// *Lookup ✔ ✔ -// *MakeChan ✔ ✔ -// *MakeClosure ✔ ✔ -// *MakeInterface ✔ ✔ -// *MakeMap ✔ ✔ -// *MakeSlice ✔ ✔ -// *MapUpdate ✔ -// *NamedConst ✔ (const) -// *Next ✔ ✔ -// *Panic ✔ -// *Parameter ✔ -// *Phi ✔ ✔ -// *Range ✔ ✔ -// *Return ✔ -// *RunDefers ✔ -// *Select ✔ ✔ -// *Send ✔ -// *Slice ✔ ✔ -// *Store ✔ -// *Type ✔ (type) -// *TypeAssert ✔ ✔ -// *UnOp ✔ ✔ +// Value? Instruction? Member? +// *Alloc ✔ ✔ +// *BinOp ✔ ✔ +// *Builtin ✔ +// *Call ✔ ✔ +// *ChangeInterface ✔ ✔ +// *ChangeType ✔ ✔ +// *Const ✔ +// *Convert ✔ ✔ +// *SliceToArrayPointer ✔ ✔ +// *DebugRef ✔ +// *Defer ✔ +// *Extract ✔ ✔ +// *Field ✔ ✔ +// *FieldAddr ✔ ✔ +// *FreeVar ✔ +// *Function ✔ ✔ (func) +// *Global ✔ ✔ (var) +// *Go ✔ +// *If ✔ +// *Index ✔ ✔ +// *IndexAddr ✔ ✔ +// *Jump ✔ +// *Lookup ✔ ✔ +// *MakeChan ✔ ✔ +// *MakeClosure ✔ ✔ +// *MakeInterface ✔ ✔ +// *MakeMap ✔ ✔ +// *MakeSlice ✔ ✔ +// *MapUpdate ✔ +// *NamedConst ✔ (const) +// *Next ✔ ✔ +// *Panic ✔ +// *Parameter ✔ +// *Phi ✔ ✔ +// *Range ✔ ✔ +// *Return ✔ +// *RunDefers ✔ +// *Select ✔ ✔ +// *Send ✔ +// *Slice ✔ ✔ +// *Store ✔ +// *Type ✔ (type) +// *TypeAssert ✔ ✔ +// *UnOp ✔ ✔ // // Other key types in this package include: Program, Package, Function // and BasicBlock. diff --git a/go/ssa/emit.go b/go/ssa/emit.go index df9ca4ff0f..02d0e4b447 100644 --- a/go/ssa/emit.go +++ b/go/ssa/emit.go @@ -232,7 +232,7 @@ func emitConv(f *Function, val Value, typ types.Type) Value { if slice, ok := ut_src.(*types.Slice); ok { if ptr, ok := ut_dst.(*types.Pointer); ok { if arr, ok := ptr.Elem().(*types.Array); ok && types.Identical(slice.Elem(), arr.Elem()) { - c := &Convert{X: val} + c := &SliceToArrayPointer{X: val} c.setType(ut_dst) return f.emit(c) } diff --git a/go/ssa/interp/interp.go b/go/ssa/interp/interp.go index d776594271..bf7862289f 100644 --- a/go/ssa/interp/interp.go +++ b/go/ssa/interp/interp.go @@ -210,6 +210,9 @@ func visitInstr(fr *frame, instr ssa.Instruction) continuation { case *ssa.Convert: fr.env[instr] = conv(instr.Type(), instr.X.Type(), fr.get(instr.X)) + case *ssa.SliceToArrayPointer: + fr.env[instr] = sliceToArrayPointer(instr.Type(), instr.X.Type(), fr.get(instr.X)) + case *ssa.MakeInterface: fr.env[instr] = iface{t: instr.X.Type(), v: fr.get(instr.X)} diff --git a/go/ssa/interp/interp_go117_test.go b/go/ssa/interp/interp_go117_test.go new file mode 100644 index 0000000000..58bbaa39c9 --- /dev/null +++ b/go/ssa/interp/interp_go117_test.go @@ -0,0 +1,12 @@ +// Copyright 2021 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. + +//go:build go1.17 +// +build go1.17 + +package interp_test + +func init() { + testdataTests = append(testdataTests, "slice2arrayptr.go") +} diff --git a/go/ssa/interp/ops.go b/go/ssa/interp/ops.go index 90d945291b..9c12d4a66c 100644 --- a/go/ssa/interp/ops.go +++ b/go/ssa/interp/ops.go @@ -1357,6 +1357,29 @@ func conv(t_dst, t_src types.Type, x value) value { panic(fmt.Sprintf("unsupported conversion: %s -> %s, dynamic type %T", t_src, t_dst, x)) } +// sliceToArrayPointer converts the value x of type slice to type t_dst +// a pointer to array and returns the result. +func sliceToArrayPointer(t_dst, t_src types.Type, x value) value { + utSrc := t_src.Underlying() + utDst := t_dst.Underlying() + + if _, ok := utSrc.(*types.Slice); ok { + if utSrc, ok := utDst.(*types.Pointer); ok { + if arr, ok := utSrc.Elem().(*types.Array); ok { + x := x.([]value) + a := make(array, arr.Len()) + for i := range a { + a[i] = x[i] + } + v := value(a) + return &v + } + } + } + + panic(fmt.Sprintf("unsupported conversion: %s -> %s, dynamic type %T", t_src, t_dst, x)) +} + // checkInterface checks that the method set of x implements the // interface itype. // On success it returns "", on failure, an error message. diff --git a/go/ssa/interp/testdata/slice2arrayptr.go b/go/ssa/interp/testdata/slice2arrayptr.go new file mode 100644 index 0000000000..21f990624b --- /dev/null +++ b/go/ssa/interp/testdata/slice2arrayptr.go @@ -0,0 +1,23 @@ +// Copyright 2021 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 main + +// Test for slice to array pointer conversion introduced in go1.17 + +import "fmt" + +var s = []byte{1, 2, 3, 4} +var a = (*[4]byte)(s) + +func main() { + for i := range s { + if a[i] != s[i] { + panic(fmt.Sprintf("value mismatched: %v - %v\n", a[i], s[i])) + } + if (*a)[i] != s[i] { + panic(fmt.Sprintf("value mismatched: %v - %v\n", (*a)[i], s[i])) + } + } +} diff --git a/go/ssa/print.go b/go/ssa/print.go index 3333ba41a0..c1b6d22b3e 100644 --- a/go/ssa/print.go +++ b/go/ssa/print.go @@ -159,10 +159,11 @@ func printConv(prefix string, v, x Value) string { relName(x, v.(Instruction))) } -func (v *ChangeType) String() string { return printConv("changetype", v, v.X) } -func (v *Convert) String() string { return printConv("convert", v, v.X) } -func (v *ChangeInterface) String() string { return printConv("change interface", v, v.X) } -func (v *MakeInterface) String() string { return printConv("make", v, v.X) } +func (v *ChangeType) String() string { return printConv("changetype", v, v.X) } +func (v *Convert) String() string { return printConv("convert", v, v.X) } +func (v *ChangeInterface) String() string { return printConv("change interface", v, v.X) } +func (v *SliceToArrayPointer) String() string { return printConv("slice to array pointer", v, v.X) } +func (v *MakeInterface) String() string { return printConv("make", v, v.X) } func (v *MakeClosure) String() string { var b bytes.Buffer diff --git a/go/ssa/sanity.go b/go/ssa/sanity.go index 16df7e4f0c..1d4e20f6a2 100644 --- a/go/ssa/sanity.go +++ b/go/ssa/sanity.go @@ -132,14 +132,8 @@ func (s *sanity) checkInstr(idx int, instr Instruction) { case *Call: case *ChangeInterface: case *ChangeType: + case *SliceToArrayPointer: case *Convert: - if _, ok := instr.X.Type().Underlying().(*types.Slice); ok { - if ptr, ok := instr.Type().Underlying().(*types.Pointer); ok { - if _, ok := ptr.Elem().(*types.Array); ok { - break - } - } - } if _, ok := instr.X.Type().Underlying().(*types.Basic); !ok { if _, ok := instr.Type().Underlying().(*types.Basic); !ok { s.errorf("convert %s -> %s: at least one type must be basic", instr.X.Type(), instr.Type()) diff --git a/go/ssa/ssa.go b/go/ssa/ssa.go index d3faf44388..8358681c7f 100644 --- a/go/ssa/ssa.go +++ b/go/ssa/ssa.go @@ -615,9 +615,10 @@ type ChangeType struct { // - between pointers and unsafe.Pointer. // - between unsafe.Pointer and uintptr. // - from (Unicode) integer to (UTF-8) string. -// - from slice to array pointer. // A conversion may imply a type name change also. // +// This operation cannot fail dynamically. +// // Conversions of untyped string/number/bool constants to a specific // representation are eliminated during SSA construction. // @@ -649,6 +650,20 @@ type ChangeInterface struct { X Value } +// The SliceToArrayPointer instruction yields the conversion of slice X to +// array pointer. +// +// Pos() returns the ast.CallExpr.Lparen, if the instruction arose +// from an explicit conversion in the source. +// +// Example printed form: +// t1 = slice to array pointer *[4]byte <- []byte (t0) +// +type SliceToArrayPointer struct { + register + X Value +} + // MakeInterface constructs an instance of an interface type from a // value of a concrete type. // @@ -1566,6 +1581,10 @@ func (v *Convert) Operands(rands []*Value) []*Value { return append(rands, &v.X) } +func (v *SliceToArrayPointer) Operands(rands []*Value) []*Value { + return append(rands, &v.X) +} + func (s *DebugRef) Operands(rands []*Value) []*Value { return append(rands, &s.X) }