From 4f1b69827d34d350f0148869c49bf118cd8613ee Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Fri, 23 Aug 2024 16:08:43 +0200 Subject: [PATCH] reflect: support big-endian systems The reflect package needs to know the endianness of the system in a few places. Before this patch, it assumed little-endian systems. But with GOARCH=mips we now have a big-endian system which also needs to be supported. So this patch fixes the reflect package to work on big-endian systems. Also, I've updated the tests for MIPS: instead of running the little-endian tests, I've changed it to run the big-endian tests instead. The two are very similar except for endianness so this should be fine. To be sure we won't accidentally break little-endian support, I've kept a single MIPS little-endian test (the CGo test, which doesn't yet work on big-endian systems anyway). --- main_test.go | 26 +++++++++++++++----------- src/reflect/endian-big.go | 36 ++++++++++++++++++++++++++++++++++++ src/reflect/endian-little.go | 35 +++++++++++++++++++++++++++++++++++ src/reflect/value.go | 31 ++++--------------------------- 4 files changed, 90 insertions(+), 38 deletions(-) create mode 100644 src/reflect/endian-big.go create mode 100644 src/reflect/endian-little.go diff --git a/main_test.go b/main_test.go index 80c8a46cd1..62eb5c51b9 100644 --- a/main_test.go +++ b/main_test.go @@ -36,7 +36,7 @@ var supportedLinuxArches = map[string]string{ "X86Linux": "linux/386", "ARMLinux": "linux/arm/6", "ARM64Linux": "linux/arm64", - "MIPSLinux": "linux/mipsle/hardfloat", + "MIPSLinux": "linux/mips/hardfloat", "WASIp1": "wasip1/wasm", } @@ -177,17 +177,14 @@ func TestBuild(t *testing.T) { }) } } - t.Run("MIPS big-endian", func(t *testing.T) { - // Run a single test for GOARCH=mips to see whether it works at all. - // Big-endian MIPS isn't fully supported yet, but simple examples - // should work. - // Once big-endian is fully supported, we can probably flip this - // around and do full testing of MIPS big-endian support and only do - // limited testing of MIPS little-endian (because the two are some - // similar). + t.Run("MIPS little-endian", func(t *testing.T) { + // Run a single test for GOARCH=mipsle to see whether it works at + // all. It is already mostly tested because GOARCH=mips and + // GOARCH=mipsle are so similar, but it's good to have an extra test + // to be sure. t.Parallel() - options := optionsFromOSARCH("linux/mips/softfloat", sema) - runTest("map.go", options, t, nil, nil) + options := optionsFromOSARCH("linux/mipsle/softfloat", sema) + runTest("cgo/", options, t, nil, nil) }) t.Run("WebAssembly", func(t *testing.T) { t.Parallel() @@ -232,6 +229,13 @@ func runPlatTests(options compileopts.Options, tests []string, t *testing.T) { continue } } + if options.GOOS == "linux" && options.GOARCH == "mips" { + if name == "cgo/" { + // CGo isn't supported yet on big-endian systems (needs updates + // to bitfield access methods). + continue + } + } if options.Target == "simavr" { // Not all tests are currently supported on AVR. // Skip the ones that aren't. diff --git a/src/reflect/endian-big.go b/src/reflect/endian-big.go new file mode 100644 index 0000000000..94951e200d --- /dev/null +++ b/src/reflect/endian-big.go @@ -0,0 +1,36 @@ +//go:build mips + +package reflect + +import "unsafe" + +// loadValue loads a value that may or may not be word-aligned. The number of +// bytes given in size are loaded. The biggest possible size it can load is that +// of an uintptr. +func loadValue(ptr unsafe.Pointer, size uintptr) uintptr { + loadedValue := uintptr(0) + for i := uintptr(0); i < size; i++ { + loadedValue <<= 8 + loadedValue |= uintptr(*(*byte)(ptr)) + ptr = unsafe.Add(ptr, 1) + } + return loadedValue +} + +// storeValue is the inverse of loadValue. It stores a value to a pointer that +// doesn't need to be aligned. +func storeValue(ptr unsafe.Pointer, size, value uintptr) { + // This could perhaps be optimized using bits.ReverseBytes32 if needed. + value <<= (unsafe.Sizeof(uintptr(0)) - size) * 8 + for i := uintptr(0); i < size; i++ { + *(*byte)(ptr) = byte(value >> ((unsafe.Sizeof(uintptr(0)) - 1) * 8)) + ptr = unsafe.Add(ptr, 1) + value <<= 8 + } +} + +// maskAndShift cuts out a part of a uintptr. Note that the offset may not be 0. +func maskAndShift(value, offset, size uintptr) uintptr { + mask := ^uintptr(0) >> ((unsafe.Sizeof(uintptr(0)) - size) * 8) + return (uintptr(value) >> ((unsafe.Sizeof(uintptr(0)) - offset - size) * 8)) & mask +} diff --git a/src/reflect/endian-little.go b/src/reflect/endian-little.go new file mode 100644 index 0000000000..7d7e30059d --- /dev/null +++ b/src/reflect/endian-little.go @@ -0,0 +1,35 @@ +//go:build !mips + +package reflect + +import "unsafe" + +// loadValue loads a value that may or may not be word-aligned. The number of +// bytes given in size are loaded. The biggest possible size it can load is that +// of an uintptr. +func loadValue(ptr unsafe.Pointer, size uintptr) uintptr { + loadedValue := uintptr(0) + shift := uintptr(0) + for i := uintptr(0); i < size; i++ { + loadedValue |= uintptr(*(*byte)(ptr)) << shift + shift += 8 + ptr = unsafe.Add(ptr, 1) + } + return loadedValue +} + +// storeValue is the inverse of loadValue. It stores a value to a pointer that +// doesn't need to be aligned. +func storeValue(ptr unsafe.Pointer, size, value uintptr) { + for i := uintptr(0); i < size; i++ { + *(*byte)(ptr) = byte(value) + ptr = unsafe.Add(ptr, 1) + value >>= 8 + } +} + +// maskAndShift cuts out a part of a uintptr. Note that the offset may not be 0. +func maskAndShift(value, offset, size uintptr) uintptr { + mask := ^uintptr(0) >> ((unsafe.Sizeof(uintptr(0)) - size) * 8) + return (uintptr(value) >> (offset * 8)) & mask +} diff --git a/src/reflect/value.go b/src/reflect/value.go index 9e602f69de..15a900f9e1 100644 --- a/src/reflect/value.go +++ b/src/reflect/value.go @@ -887,26 +887,6 @@ func (v Value) Index(i int) Value { } } -// loadValue loads a value that may or may not be word-aligned. The number of -// bytes given in size are loaded. The biggest possible size it can load is that -// of an uintptr. -func loadValue(ptr unsafe.Pointer, size uintptr) uintptr { - loadedValue := uintptr(0) - shift := uintptr(0) - for i := uintptr(0); i < size; i++ { - loadedValue |= uintptr(*(*byte)(ptr)) << shift - shift += 8 - ptr = unsafe.Add(ptr, 1) - } - return loadedValue -} - -// maskAndShift cuts out a part of a uintptr. Note that the offset may not be 0. -func maskAndShift(value, offset, size uintptr) uintptr { - mask := ^uintptr(0) >> ((unsafe.Sizeof(uintptr(0)) - size) * 8) - return (uintptr(value) >> (offset * 8)) & mask -} - func (v Value) NumMethod() int { return v.typecode.NumMethod() } @@ -1088,9 +1068,7 @@ func (v Value) Set(x Value) { if v.typecode.Kind() == Interface && x.typecode.Kind() != Interface { // move the value of x back into the interface, if possible if x.isIndirect() && x.typecode.Size() <= unsafe.Sizeof(uintptr(0)) { - var value uintptr - memcpy(unsafe.Pointer(&value), x.value, x.typecode.Size()) - x.value = unsafe.Pointer(value) + x.value = unsafe.Pointer(loadValue(x.value, x.typecode.Size())) } intf := composeInterface(unsafe.Pointer(x.typecode), x.value) @@ -1101,12 +1079,11 @@ func (v Value) Set(x Value) { } size := v.typecode.Size() - xptr := x.value if size <= unsafe.Sizeof(uintptr(0)) && !x.isIndirect() { - value := x.value - xptr = unsafe.Pointer(&value) + storeValue(v.value, size, uintptr(x.value)) + } else { + memcpy(v.value, x.value, size) } - memcpy(v.value, xptr, size) } func (v Value) SetZero() {