Skip to content

Commit

Permalink
reflect: remove short-circuits for zero-sized types in ABI algorithm
Browse files Browse the repository at this point in the history
This change removes two short-circuits for zero-sized types (zero-sized
structs and zero-sized struct fields) in the recursive cases of the ABI
algorithm, because this does not match the spec's algorithm, nor the
compiler's algorithm.

The failing case here is a struct with a field that is an array of
non-zero length but whose element type is zero-sized. This struct must
be stack-assigned because of the array, according to the algorithm.
The reflect package was register-assigning it.

Because there were two short-circuits, this can also appear if a struct
has a field that is a zero-sized struct but contains such an array,
also.

This change adds regression tests for both of these cases.

For #44816.
For #40724.

Change-Id: I956804170962448197a1c9853826e3436fc8b1ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/306929
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
  • Loading branch information
mknyszek committed Apr 2, 2021
1 parent 254948a commit 34b87b4
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 10 deletions.
10 changes: 0 additions & 10 deletions src/reflect/abi.go
Expand Up @@ -233,19 +233,9 @@ func (a *abiSeq) regAssign(t *rtype, offset uintptr) bool {
return false
}
case Struct:
if t.size == 0 {
// There's nothing to assign, so don't modify
// a.steps but succeed so the caller doesn't
// try to stack-assign this value.
return true
}
st := (*structType)(unsafe.Pointer(t))
for i := range st.fields {
f := &st.fields[i]
if f.typ.Size() == 0 {
// Ignore zero-sized fields.
continue
}
if !a.regAssign(f.typ, offset+f.offset()) {
return false
}
Expand Down
47 changes: 47 additions & 0 deletions src/reflect/abi_test.go
Expand Up @@ -300,6 +300,8 @@ var abiCallTestCases = []interface{}{
passStruct11,
passStruct12,
passStruct13,
passStruct14,
passStruct15,
pass2Struct1,
passEmptyStruct,
passStruct10AndSmall,
Expand Down Expand Up @@ -521,6 +523,18 @@ func passStruct13(a Struct13) Struct13 {
return a
}

//go:registerparams
//go:noinline
func passStruct14(a Struct14) Struct14 {
return a
}

//go:registerparams
//go:noinline
func passStruct15(a Struct15) Struct15 {
return a
}

//go:registerparams
//go:noinline
func pass2Struct1(a, b Struct1) (x, y Struct1) {
Expand Down Expand Up @@ -581,6 +595,8 @@ var abiMakeFuncTestCases = []interface{}{
callArgsStruct11,
callArgsStruct12,
callArgsStruct13,
callArgsStruct14,
callArgsStruct15,
callArgs2Struct1,
callArgsEmptyStruct,
}
Expand Down Expand Up @@ -801,6 +817,18 @@ func callArgsStruct13(f func(Struct13, MagicLastTypeNameForTestingRegisterABI) S
return f(a0, MagicLastTypeNameForTestingRegisterABI{})
}

//go:registerparams
//go:noinline
func callArgsStruct14(f func(Struct14, MagicLastTypeNameForTestingRegisterABI) Struct14, a0 Struct14) Struct14 {
return f(a0, MagicLastTypeNameForTestingRegisterABI{})
}

//go:registerparams
//go:noinline
func callArgsStruct15(f func(Struct15, MagicLastTypeNameForTestingRegisterABI) Struct15, a0 Struct15) Struct15 {
return f(a0, MagicLastTypeNameForTestingRegisterABI{})
}

//go:registerparams
//go:noinline
func callArgs2Struct1(f func(Struct1, Struct1, MagicLastTypeNameForTestingRegisterABI) (Struct1, Struct1), a0, a1 Struct1) (r0, r1 Struct1) {
Expand Down Expand Up @@ -904,6 +932,25 @@ type Struct13 struct {
B int
}

// Struct14 tests a non-zero-sized (and otherwise register-assignable)
// struct with a field that is a non-zero length array with zero-sized members.
type Struct14 struct {
A uintptr
X [3]struct{}
B float64
}

// Struct15 tests a non-zero-sized (and otherwise register-assignable)
// struct with a struct field that is zero-sized but contains a
// non-zero length array with zero-sized members.
type Struct15 struct {
A uintptr
X struct {
Y [3]struct{}
}
B float64
}

const genValueRandSeed = 0

// genValue generates a pseudorandom reflect.Value with type t.
Expand Down

0 comments on commit 34b87b4

Please sign in to comment.