diff --git a/cl/_testgo/blankfield/expect.txt b/cl/_testgo/blankfield/expect.txt new file mode 100644 index 0000000000..9766475a41 --- /dev/null +++ b/cl/_testgo/blankfield/expect.txt @@ -0,0 +1 @@ +ok diff --git a/cl/_testgo/blankfield/in.go b/cl/_testgo/blankfield/in.go new file mode 100644 index 0000000000..ca346f6dfb --- /dev/null +++ b/cl/_testgo/blankfield/in.go @@ -0,0 +1,63 @@ +// LITTEST +package main + +import "unsafe" + +var calls int + +func sideEffect() int { + calls++ + return 42 +} + +type Nested struct { + Left int + _ int + Right int +} + +// CHECK-LABEL: define void @"{{.*}}/cl/_testgo/blankfield.main"() +// CHECK: call i64 @"{{.*}}/cl/_testgo/blankfield.sideEffect"() +// CHECK: call i64 @"{{.*}}/cl/_testgo/blankfield.sideEffect"() +// CHECK: call i64 @"{{.*}}/cl/_testgo/blankfield.sideEffect"() + +func main() { + value := struct { + _ int + Keep int + }{sideEffect(), 7} + nestedValue := struct { + _ Nested + Keep int + }{Nested{sideEffect(), 6, 7}, 8} + arrayValue := struct { + _ [2]int + Keep int + }{[2]int{sideEffect(), 9}, 10} + + if calls != 3 { + panic("blank field initializer side effect was not evaluated") + } + if value.Keep != 7 { + panic("non-blank field initializer was lost") + } + nestedWords := (*[4]int)(unsafe.Pointer(&nestedValue)) + for i := 0; i < 3; i++ { + if nestedWords[i] != 0 { + panic("nested blank field was not zeroed") + } + } + if nestedWords[3] != 8 { + panic("nested non-blank field initializer was lost") + } + arrayWords := (*[3]int)(unsafe.Pointer(&arrayValue)) + for i := 0; i < 2; i++ { + if arrayWords[i] != 0 { + panic("blank array field was not zeroed") + } + } + if arrayWords[2] != 10 { + panic("array non-blank field initializer was lost") + } + println("ok") +} diff --git a/cl/compile.go b/cl/compile.go index 8f99b9a0f5..1c462826c2 100644 --- a/cl/compile.go +++ b/cl/compile.go @@ -1843,13 +1843,38 @@ func (p *context) compileValue(b llssa.Builder, v ssa.Value) llssa.Expr { panic(fmt.Sprintf("compileValue: unknown value - %T\n", v)) } +// isBlankFieldStore also recognizes stores into descendants of an aggregate +// blank field. IndexAddr is followed only through an array's in-place storage, +// never through a slice header into its separately allocated backing array. +// The caller still evaluates the stored value for side effects. func isBlankFieldStore(addr ssa.Value) bool { - field, ok := addr.(*ssa.FieldAddr) - if !ok { - return false + for { + switch current := addr.(type) { + case *ssa.FieldAddr: + _, st, ok := fieldAddrStruct(current) + if !ok { + return false + } + if st.Field(current.Field).Name() == "_" { + return true + } + addr = current.X + case *ssa.IndexAddr: + if current.X == nil || current.X.Type() == nil { + return false + } + ptr, ok := current.X.Type().Underlying().(*types.Pointer) + if !ok { + return false + } + if _, ok := ptr.Elem().Underlying().(*types.Array); !ok { + return false + } + addr = current.X + default: + return false + } } - _, st, ok := fieldAddrStruct(field) - return ok && st.Field(field.Field).Name() == "_" } const rangeOverFuncYieldSynthetic = "range-over-func yield" diff --git a/cl/rewrite_internal_test.go b/cl/rewrite_internal_test.go index ce5d370d33..900c082cef 100644 --- a/cl/rewrite_internal_test.go +++ b/cl/rewrite_internal_test.go @@ -126,6 +126,174 @@ func Use() string { assertNoStoreToGlobal(t, ir, "@staticinit.MethodNames") } +func TestStaticGlobalBlankFieldInit(t *testing.T) { + const src = `package staticinit + +type Nested struct { + Left int + _ int + Right int +} + +type Outer struct { + Left int + _ Nested + Right int +} + +var Flat = Nested{1, 2, 3} +var Deep = Outer{4, Nested{5, 6, 7}, 8} + +func Use() int { + return Flat.Left + Flat.Right + Deep.Left + Deep.Right +} +` + ir := compileWithRewrites(t, src, nil) + for _, want := range []string{ + "@staticinit.Flat = global %staticinit.Nested { i64 1, i64 0, i64 3 }", + "@staticinit.Deep = global %staticinit.Outer { i64 4, %staticinit.Nested zeroinitializer, i64 8 }", + } { + if !strings.Contains(ir, want) { + t.Fatalf("blank field was not zeroed in static initializer %q:\n%s", want, ir) + } + } +} + +func TestBlankFieldStoreRejectsInvalidAddresses(t *testing.T) { + for name, addr := range map[string]ssa.Value{ + "nil": nil, + "invalid field": &ssa.FieldAddr{}, + "invalid index": &ssa.IndexAddr{}, + } { + t.Run(name, func(t *testing.T) { + if isBlankFieldStore(addr) { + t.Fatal("invalid address was classified as a blank field store") + } + }) + } +} + +func TestBlankFieldStoreClassification(t *testing.T) { + const src = `package blankstore + +type Leaf struct { + Value int +} + +type Outer struct { + _ Leaf + Keep Leaf + _ [2]Leaf + _ []Leaf +} + +func next() int { return 1 } + +var Value = Outer{ + Leaf{next()}, + Leaf{next()}, + [2]Leaf{{next()}, {next()}}, + []Leaf{{next()}}, +} +` + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "blankstore.go", src, 0) + if err != nil { + t.Fatal(err) + } + importer := gpackages.NewImporter(fset) + pkg, _, err := ssautil.BuildPackage( + &types.Config{Importer: importer}, + fset, + types.NewPackage("blankstore", "blankstore"), + []*ast.File{file}, + ssa.SanityCheckFunctions, + ) + if err != nil { + t.Fatal(err) + } + + addressPath := func(addr ssa.Value) (fields []string, indexed bool) { + for { + switch current := addr.(type) { + case *ssa.FieldAddr: + _, st, ok := fieldAddrStruct(current) + if !ok { + return fields, indexed + } + fields = append(fields, st.Field(current.Field).Name()) + addr = current.X + case *ssa.IndexAddr: + indexed = true + addr = current.X + default: + return fields, indexed + } + } + } + + var ( + blankSliceField *ssa.FieldAddr + sawDirectBlank, sawNestedBlank bool + sawBlankArray, sawNonBlankSibling bool + ) + initFn := pkg.Func("init") + for _, block := range initFn.Blocks { + for _, instr := range block.Instrs { + if field, ok := instr.(*ssa.FieldAddr); ok { + _, st, valid := fieldAddrStruct(field) + if valid && st.Field(field.Field).Name() == "_" { + if _, ok := st.Field(field.Field).Type().Underlying().(*types.Slice); ok { + blankSliceField = field + } + } + } + store, ok := instr.(*ssa.Store) + if !ok { + continue + } + fields, indexed := addressPath(store.Addr) + if len(fields) == 0 { + continue + } + want := false + for _, field := range fields { + want = want || field == "_" + } + if got := isBlankFieldStore(store.Addr); got != want { + t.Fatalf("isBlankFieldStore(%s) = %v, want %v", store.Addr, got, want) + } + switch { + case len(fields) == 1 && fields[0] == "_": + sawDirectBlank = true + case len(fields) > 1 && want && !indexed: + sawNestedBlank = true + case want && indexed: + sawBlankArray = true + case !want: + sawNonBlankSibling = true + } + } + } + if !sawDirectBlank || !sawNestedBlank || !sawBlankArray || !sawNonBlankSibling { + t.Fatalf( + "missing SSA classification coverage: direct=%v nested=%v array=%v sibling=%v", + sawDirectBlank, + sawNestedBlank, + sawBlankArray, + sawNonBlankSibling, + ) + } + if blankSliceField == nil { + t.Fatal("missing blank slice field address") + } + // This shape is not currently emitted by go/ssa: it defensively verifies + // that a future IndexAddr form cannot cross a blank slice header. + if isBlankFieldStore(&ssa.IndexAddr{X: blankSliceField}) { + t.Fatal("slice backing-array address crossed the blank slice header") + } +} + func TestStaticGlobalSliceLiteralInit(t *testing.T) { const src = `package staticinit diff --git a/cl/static_init.go b/cl/static_init.go index fd0f238151..5dc71b08f1 100644 --- a/cl/static_init.go +++ b/cl/static_init.go @@ -393,6 +393,9 @@ func (p *context) buildStaticInitExpr(typ types.Type, node *staticInitNode) (lls values := make([]llssa.Expr, u.NumFields()) for i := range values { child := node.children[i] + if u.Field(i).Name() == "_" { + child = nil + } value, ok := p.buildStaticInitExpr(u.Field(i).Type(), child) if !ok { return llssa.Expr{}, false diff --git a/test/goroot/xfail.yaml b/test/goroot/xfail.yaml index 8183574996..e3090017de 100644 --- a/test/goroot/xfail.yaml +++ b/test/goroot/xfail.yaml @@ -2408,10 +2408,6 @@ xfails: directive: run case: range4.go reason: range-over-function build exits successfully without producing a binary on darwin/arm64 - - platform: darwin/arm64 - directive: run - case: fixedbugs/issue31546.go - reason: reflection exposes the initializer value of a blank struct field instead of zero on darwin/arm64 - platform: darwin/arm64 directive: run case: fixedbugs/issue72063.go