Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reflect.Copy Incorrect When Source is Array of Len > 8 #4554

Open
ben-krieger opened this issue Oct 27, 2024 · 4 comments
Open

reflect.Copy Incorrect When Source is Array of Len > 8 #4554

ben-krieger opened this issue Oct 27, 2024 · 4 comments
Labels
next-release Will be part of next release

Comments

@ben-krieger
Copy link

Tested from dev. Minimal reproduction:

//go:build ignore

package main

import (
	"fmt"
	"reflect"
)

func main() {
	guid := [9]byte{0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09}
	b := make([]byte, 9)

	// Copy builtin works
	copy(b, guid[:])
	fmt.Printf("%x\n", b)

	// Reflect with slices works
	reflect.Copy(reflect.ValueOf(b), reflect.ValueOf(guid[:]))
	fmt.Printf("%x\n", b)

	// Reflect with array doesn't work
	reflect.Copy(reflect.ValueOf(b), reflect.ValueOf(guid))
	fmt.Printf("%x\n", b)
}

Result:

$ tinygo run ./test.go
010203040506070809
010203040506070809
303ac73ca07f000002

$ go run ./test.go
010203040506070809
010203040506070809
010203040506070809

Not shown in the reproduction: a [8]byte array works fine, so the issue seems to only be for lengths > 8.

@ben-krieger
Copy link
Author

It seems that both [8]byte and [9]byte do not have the indirect flag set. However, the [9]byte should be indirect and isn't, presumably because the data is greater than 64 bits.

(I'm not sure why the >64 bit size is relevant, though, since the field is either a pointer to data or a pointer to a pointer to data).

@ben-krieger
Copy link
Author

The ValueOf implementation never sets the indirect flag. Are there some primitives (other than arrays over a certain size) which should be marked indirect?

func ValueOf(i interface{}) Value {
	typecode, value := decomposeInterface(i)
	return Value{
		typecode: (*rawType)(typecode),
		value:    value,
		flags:    valueFlagExported,
	}
}

@ben-krieger
Copy link
Author

Perhaps a solution would be this?

diff --git a/src/reflect/value.go b/src/reflect/value.go
index d1f8cb2f..df0031b9 100644
--- a/src/reflect/value.go
+++ b/src/reflect/value.go
@@ -72,11 +72,15 @@ func decomposeInterface(i interface{}) (unsafe.Pointer, unsafe.Pointer)
 
 func ValueOf(i interface{}) Value {
 	typecode, value := decomposeInterface(i)
-	return Value{
+	v := Value{
 		typecode: (*rawType)(typecode),
 		value:    value,
 		flags:    valueFlagExported,
 	}
+	if v.typecode.Kind() == Array && v.typecode.Size() > unsafe.Sizeof(uintptr(0)) {
+		v.flags |= valueFlagIndirect
+	}
+	return v
 }
 
 func (v Value) Interface() interface{} {

@ben-krieger
Copy link
Author

ben-krieger commented Oct 27, 2024

Added fix and test in #4543 @ 05e2f5d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next-release Will be part of next release
Projects
None yet
Development

No branches or pull requests

2 participants