From: Ian Lance Taylor <ian@gcc.gnu.org>
Date: Mon, 20 Oct 2014 18:04:55 +0000 (+0000)
Subject: reflect: allocate correct type in assignTo and cvtT2I
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=7b28fa2c6b956d08b2ef909026f42b5df60ae990;p=gcc.git

reflect: allocate correct type in assignTo and cvtT2I

Backport https://codereview.appspot.com/155450044 from the
master Go library.  Original description:

I came across this while debugging a GC problem in gccgo.
There is code in assignTo and cvtT2I that handles assignment
to all interface values.  It allocates an empty interface even
if the real type is a non-empty interface.  The fields are
then set for a non-empty interface, but the memory is recorded
as holding an empty interface.  This means that the GC has
incorrect information.

This is extremely unlikely to fail, because the code in the GC
that handles empty interfaces looks like this:

obj = nil;
typ = eface->type;
if(typ != nil) {
        if(!(typ->kind&KindDirectIface) || !(typ->kind&KindNoPointers))
                obj = eface->data;

In the current runtime the condition is always true--if
KindDirectIface is set, then KindNoPointers is clear--and we
always want to set obj = eface->data.  So the question is what
happens when we incorrectly store a non-empty interface value
in memory marked as an empty interface.  In that case
eface->type will not be a *rtype as we expect, but will
instead be a pointer to an Itab.  We are going to use this
pointer to look at a *rtype kind field.  The *rtype struct
starts out like this:

type rtype struct {
        size          uintptr
        hash          uint32            // hash of type; avoids computation in hash tables
        _             uint8             // unused/padding
        align         uint8             // alignment of variable with this type
        fieldAlign    uint8             // alignment of struct field with this type
        kind          uint8             // enumeration for C

An Itab always has at least two pointers, so on a
little-endian 64-bit system the kind field will be the high
byte of the second pointer.  This will normally be zero, so
the test of typ->kind will succeed, which is what we want.

On a 32-bit system it might be possible to construct a failing
case by somehow getting the Itab for an interface with one
method to be immediately followed by a word that is all ones.
The effect would be that the test would sometimes fail and the
GC would not mark obj, leading to an invalid dangling
pointer.  I have not tried to construct this test.

I noticed this in gccgo, where this error is much more likely
to cause trouble for a rather random reason: gccgo uses a
different layout of rtype, and in gccgo the kind field happens
to be the low byte of a pointer, not the high byte.

From-SVN: r216489
---

diff --git a/libgo/go/reflect/value.go b/libgo/go/reflect/value.go
index c390b8e2d6c..c36e9954427 100644
--- a/libgo/go/reflect/value.go
+++ b/libgo/go/reflect/value.go
@@ -1405,9 +1405,9 @@ func (v Value) send(x Value, nb bool) (selected bool) {
 func (v Value) Set(x Value) {
 	v.mustBeAssignable()
 	x.mustBeExported() // do not let unexported x leak
-	var target *interface{}
+	var target unsafe.Pointer
 	if v.kind() == Interface {
-		target = (*interface{})(v.ptr)
+		target = v.ptr
 	}
 	x = x.assignTo("reflect.Set", v.typ, target)
 	if x.flag&flagIndir != 0 {
@@ -2230,7 +2230,7 @@ func NewAt(typ Type, p unsafe.Pointer) Value {
 // assignTo returns a value v that can be assigned directly to typ.
 // It panics if v is not assignable to typ.
 // For a conversion to an interface type, target is a suggested scratch space to use.
-func (v Value) assignTo(context string, dst *rtype, target *interface{}) Value {
+func (v Value) assignTo(context string, dst *rtype, target unsafe.Pointer) Value {
 	if v.flag&flagMethod != 0 {
 		v = makeMethodValue(context, v)
 	}
@@ -2246,15 +2246,15 @@ func (v Value) assignTo(context string, dst *rtype, target *interface{}) Value {
 
 	case implements(dst, v.typ):
 		if target == nil {
-			target = new(interface{})
+			target = unsafe_New(dst)
 		}
 		x := valueInterface(v, false)
 		if dst.NumMethod() == 0 {
-			*target = x
+			*(*interface{})(target) = x
 		} else {
-			ifaceE2I(dst, x, unsafe.Pointer(target))
+			ifaceE2I(dst, x, target)
 		}
-		return Value{dst, unsafe.Pointer(target) /* 0, */, flagIndir | flag(Interface)<<flagKindShift}
+		return Value{dst, target /* 0, */, flagIndir | flag(Interface)<<flagKindShift}
 	}
 
 	// Failed.
@@ -2537,14 +2537,14 @@ func cvtDirect(v Value, typ Type) Value {
 
 // convertOp: concrete -> interface
 func cvtT2I(v Value, typ Type) Value {
-	target := new(interface{})
+	target := unsafe_New(typ.common())
 	x := valueInterface(v, false)
 	if typ.NumMethod() == 0 {
-		*target = x
+		*(*interface{})(target) = x
 	} else {
-		ifaceE2I(typ.(*rtype), x, unsafe.Pointer(target))
+		ifaceE2I(typ.(*rtype), x, target)
 	}
-	return Value{typ.common(), unsafe.Pointer(target) /* 0, */, v.flag&flagRO | flagIndir | flag(Interface)<<flagKindShift}
+	return Value{typ.common(), target /* 0, */, v.flag&flagRO | flagIndir | flag(Interface)<<flagKindShift}
 }
 
 // convertOp: interface -> interface