go/reflect: Incorrect behavior on arm64 when using MakeFunc / Call #52788

znkr · 2022-05-09T14:58:48Z

What version of Go are you using (`go version`)?

go version devel go1.19-c4311a4 Mon May 9 12:32:48 2022 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes (using gotip)

What operating system and processor architecture are you using (`go env`)?

QEMU AARCH64 emulation (this bug does not reproduce on x86)

What did you do?

% GOARCH=arm64 gotip build main.go
% qemu-aarch64 main

With main.go

package main

import (
	"fmt"
	"reflect"
)

func f(next func() bool) {
	for b := next(); b; b = next() {
		fmt.Printf("next() returned %v\n", b)
	}
}

func main() {
	next := reflect.MakeFunc(reflect.TypeOf((func() bool)(nil)), func(_ []reflect.Value) []reflect.Value {
		return []reflect.Value{reflect.ValueOf(false)}
	})
	reflect.ValueOf(f).Call([]reflect.Value{next})
}

What did you expect to see?

Empty output

What did you see instead?

unexpected fault address 0x2000051c760
fatal error: fault
[signal SIGSEGV: segmentation violation code=0x1 addr=0x2000051c760 pc=0x91518]

goroutine 1 [running]:
runtime.throw({0xb675b?, 0x56f54?})
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/panic.go:1028 +0x40 fp=0x400005f500 sp=0x400005f4d0 pc=0x40c40
runtime.sigpanic()
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/signal_unix.go:830 +0x1a0 fp=0x400005f530 sp=0x400005f500 pc=0x55a10
fmt.(*pp).printArg(0x400010a680, {0xa65a0?, 0x2000051c760}, 0x76)
        /usr/local/google/home/floriank/sdk/gotip/src/fmt/print.go:661 +0x508 fp=0x400005f5e0 sp=0x400005f540 pc=0x91518
fmt.(*pp).doPrintf(0x400010a680, {0xb8d75, 0x13}, {0x400005f788?, 0x1, 0x1})
        /usr/local/google/home/floriank/sdk/gotip/src/fmt/print.go:1026 +0x210 fp=0x400005f6e0 sp=0x400005f5e0 pc=0x93f60
fmt.Fprintf({0xd77d8, 0x400000e018}, {0xb8d75, 0x13}, {0x400005f788, 0x1, 0x1})
        /usr/local/google/home/floriank/sdk/gotip/src/fmt/print.go:204 +0x54 fp=0x400005f740 sp=0x400005f6e0 pc=0x8e7a4
fmt.Printf(...)
        /usr/local/google/home/floriank/sdk/gotip/src/fmt/print.go:213
main.f(0x400007a240)
        /usr/local/google/home/floriank/bug/main.go:10 +0x80 fp=0x400005f7a0 sp=0x400005f740 pc=0x94e00
runtime.call16(0x400007a270, 0xbf058, 0x0, 0x0, 0x0, 0x8, 0x400005fce0)
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/asm_arm64.s:461 +0x78 fp=0x400005f7c0 sp=0x400005f7a0 pc=0x6a488
runtime.reflectcall(0xa6860?, 0x400007a240?, 0x1?, 0xb8a18?, 0x0?, 0x12?, 0xa6860?)
        :1 +0x34 fp=0x400005f800 sp=0x400005f7c0 pc=0x6de14
reflect.Value.call({0xa6b20?, 0xbf058?, 0x1?}, {0xb6601, 0x4}, {0x4000104f50, 0x1, 0x1?})
        /usr/local/google/home/floriank/sdk/gotip/src/reflect/value.go:585 +0x61c fp=0x400005fe70 sp=0x400005f800 pc=0x819cc
reflect.Value.Call({0xa6b20?, 0xbf058?, 0xbf060?}, {0x4000104f50?, 0x0?, 0x0?})
        /usr/local/google/home/floriank/sdk/gotip/src/reflect/value.go:368 +0x90 fp=0x400005fef0 sp=0x400005fe70 pc=0x81300
main.main()
        /usr/local/google/home/floriank/bug/main.go:18 +0x148 fp=0x400005ff70 sp=0x400005fef0 pc=0x950d8
runtime.main()
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/proc.go:250 +0x254 fp=0x400005ffd0 sp=0x400005ff70 pc=0x43234
runtime.goexit()
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/asm_arm64.s:1155 +0x4 fp=0x400005ffd0 sp=0x400005ffd0 pc=0x6c4a4

goroutine 2 [force gc (idle)]:
runtime.gopark(0x0?, 0x0?, 0x0?, 0x0?, 0x0?)
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/proc.go:363 +0xe4 fp=0x400004efa0 sp=0x400004ef80 pc=0x43604
runtime.goparkunlock(...)
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/proc.go:369
runtime.forcegchelper()
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/proc.go:302 +0xb4 fp=0x400004efd0 sp=0x400004efa0 pc=0x43494
runtime.goexit()
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/asm_arm64.s:1155 +0x4 fp=0x400004efd0 sp=0x400004efd0 pc=0x6c4a4
created by runtime.init.6
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/proc.go:290 +0x24

goroutine 3 [GC sweep wait]:
runtime.gopark(0x0?, 0x0?, 0x0?, 0x0?, 0x0?)
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/proc.go:363 +0xe4 fp=0x400004f770 sp=0x400004f750 pc=0x43604
runtime.goparkunlock(...)
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/proc.go:369
runtime.bgsweep(0x0?)
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/mgcsweep.go:278 +0xa4 fp=0x400004f7b0 sp=0x400004f770 pc=0x309e4
runtime.gcenable.func1()
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/mgc.go:178 +0x28 fp=0x400004f7d0 sp=0x400004f7b0 pc=0x25408
runtime.goexit()
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/asm_arm64.s:1155 +0x4 fp=0x400004f7d0 sp=0x400004f7d0 pc=0x6c4a4
created by runtime.gcenable
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/mgc.go:178 +0x70

goroutine 4 [GC scavenge wait]:
runtime.gopark(0x4000074000?, 0xd71a8?, 0x1?, 0x0?, 0x0?)
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/proc.go:363 +0xe4 fp=0x400004ff50 sp=0x400004ff30 pc=0x43604
runtime.goparkunlock(...)
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/proc.go:369
runtime.(*scavengerState).park(0x158600)
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/mgcscavenge.go:389 +0x5c fp=0x400004ff80 sp=0x400004ff50 pc=0x2ea0c
runtime.bgscavenge(0x0?)
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/mgcscavenge.go:617 +0x44 fp=0x400004ffb0 sp=0x400004ff80 pc=0x2ef74
runtime.gcenable.func2()
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/mgc.go:179 +0x28 fp=0x400004ffd0 sp=0x400004ffb0 pc=0x253a8
runtime.goexit()
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/asm_arm64.s:1155 +0x4 fp=0x400004ffd0 sp=0x400004ffd0 pc=0x6c4a4
created by runtime.gcenable
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/mgc.go:179 +0xb4

goroutine 5 [finalizer wait]:
runtime.gopark(0x60000000000000?, 0x0?, 0x8?, 0x21?, 0x2000000000?)
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/proc.go:363 +0xe4 fp=0x400004e580 sp=0x400004e560 pc=0x43604
runtime.goparkunlock(...)
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/proc.go:369
runtime.runfinq()
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/mfinal.go:180 +0x128 fp=0x400004e7d0 sp=0x400004e580 pc=0x24628
runtime.goexit()
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/asm_arm64.s:1155 +0x4 fp=0x400004e7d0 sp=0x400004e7d0 pc=0x6c4a4
created by runtime.createfing
        /usr/local/google/home/floriank/sdk/gotip/src/runtime/mfinal.go:157 +0x94

The text was updated successfully, but these errors were encountered:

gopherbot · 2022-05-09T15:38:22Z

Change https://go.dev/cl/405114 mentions this issue: cmd/compile: fix If lowering on ARM64

gopherbot · 2022-05-09T17:03:05Z

Change https://go.dev/cl/405115 mentions this issue: cmd/compile: more fix on boolean ops on ARM64

Following CL 405114, the extension rule is also wrong. It is safe to drop the extension if the value is from a boolean-generating instruction, but not a boolean-typed Value in general (e.g. a Phi or a in-register parameter). Fix it. Updates #52788. Change-Id: Icf3028fe8e90806f9f57fbe2b38d47da27a97e2a Reviewed-on: https://go-review.googlesource.com/c/go/+/405115 Reviewed-by: David Chase <[email protected]> Run-TryBot: Cherry Mui <[email protected]> TryBot-Result: Gopher Robot <[email protected]>

gopherbot · 2022-05-09T19:04:55Z

Change https://go.dev/cl/405116 mentions this issue: cmd/compile: fix boolean comparison on PPC64 and RISCV64

Following CL 405114, for PPC64. Should fix PPC64 builds. Updates #52788. Change-Id: I193ac31cfba18b4f907dd2523b51368251fd6fad Reviewed-on: https://go-review.googlesource.com/c/go/+/405116 TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Cherry Mui <[email protected]> Reviewed-by: David Chase <[email protected]>

gopherbot · 2022-05-11T22:58:18Z

Change https://go.dev/cl/405553 mentions this issue: cmd/compile: fix boolean comparison on RISCV64

Following CL 405114, for RISCV64. May fix RISCV64 builds. Updates #52788. Change-Id: Ifc34658703d1e8b97665e7b862060152e3005d71 Reviewed-on: https://go-review.googlesource.com/c/go/+/405553 Reviewed-by: David Chase <[email protected]> Run-TryBot: Cherry Mui <[email protected]>

cherrymui · 2022-06-15T15:47:12Z

@gopherbot please open backport issues for previous releases. This is a miscompilation that could cause valid programs to behave incorrectly. Thanks.

gopherbot · 2022-06-15T15:47:28Z

Backport issue(s) opened: #53396 (for 1.17), #53397 (for 1.18).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

erifan · 2022-06-16T07:32:46Z

It seems that reflect.intToReg does not clear the high bits of the return value register. 32-bit or 8-bit arm64 instruction operations usually clear the unused high bits, which is why normal function calls do not have this problem. Do we need to make this change?

cherrymui · 2022-06-16T14:00:01Z

No. In general the compiler and the ABI only guarantee that the low bits of a Value is meaningful.

gopherbot · 2022-08-05T01:18:45Z

Change https://go.dev/cl/421457 mentions this issue: [release-branch.go1.18] cmd/compile: fix If lowering on ARM64

gopherbot · 2022-08-05T01:19:26Z

Change https://go.dev/cl/421459 mentions this issue: [release-branch.go1.18] cmd/compile: fix boolean comparison on PPC64

gopherbot · 2022-08-05T01:19:27Z

Change https://go.dev/cl/421460 mentions this issue: [release-branch.go1.18] cmd/compile: fix boolean comparison on RISCV64

gopherbot · 2022-08-05T01:19:28Z

Change https://go.dev/cl/421458 mentions this issue: [release-branch.go1.18] cmd/compile: more fix on boolean ops on ARM64

On ARM64, an If block is lowered to (NZ cond yes no). This is incorrect because cond is a boolean value and therefore only the last byte is meaningful (same as AMD64, see ARM64Ops.go). But here we are comparing a full register width with 0. Correct it by comparing only the last bit. For #52788. Fixes #53397. Change-Id: I2cacf9f3d2f45e149c361a290f511b2d4ed845c4 Reviewed-on: https://go-review.googlesource.com/c/go/+/405114 TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: David Chase <[email protected]> Run-TryBot: Cherry Mui <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/go/+/421457 Reviewed-by: Dmitri Shuralyov <[email protected]> Reviewed-by: Cherry Mui <[email protected]> Run-TryBot: Dmitri Shuralyov <[email protected]>

Following CL 421457, the extension rule is also wrong. It is safe to drop the extension if the value is from a boolean-generating instruction, but not a boolean-typed Value in general (e.g. a Phi or a in-register parameter). Fix it. Updates #52788. Updates #53397. Change-Id: Icf3028fe8e90806f9f57fbe2b38d47da27a97e2a Reviewed-on: https://go-review.googlesource.com/c/go/+/405115 Reviewed-by: David Chase <[email protected]> Run-TryBot: Cherry Mui <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/go/+/421458 Reviewed-by: Cherry Mui <[email protected]> Run-TryBot: Dmitri Shuralyov <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]>

Following CL 421457, for PPC64. Should fix PPC64 builds. Updates #52788. Updates #53397. Change-Id: I193ac31cfba18b4f907dd2523b51368251fd6fad Reviewed-on: https://go-review.googlesource.com/c/go/+/405116 TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Cherry Mui <[email protected]> Reviewed-by: David Chase <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/go/+/421459 Reviewed-by: Cherry Mui <[email protected]> Run-TryBot: Dmitri Shuralyov <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]>

Following CL 421457, for RISCV64. May fix RISCV64 builds. Updates #52788. Updates #53397. Change-Id: Ifc34658703d1e8b97665e7b862060152e3005d71 Reviewed-on: https://go-review.googlesource.com/c/go/+/405553 Reviewed-by: David Chase <[email protected]> Run-TryBot: Cherry Mui <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/go/+/421460 TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> Run-TryBot: Dmitri Shuralyov <[email protected]> Reviewed-by: Cherry Mui <[email protected]>

gopherbot closed this as completed in 9ae7dc3 May 9, 2022

randall77 mentioned this issue May 9, 2022

reflect: MakeFunc crash on arm64 #52799

Closed

cherrymui mentioned this issue Jun 15, 2022

reflect: func created w/MakeFunc returns mangled value when used as a parameter to another func via Call on ARM #53395

Closed

This was referenced Jun 15, 2022

go/reflect: Incorrect behavior on arm64 when using MakeFunc / Call [1.17 backport] #53396

Closed

go/reflect: Incorrect behavior on arm64 when using MakeFunc / Call [1.18 backport] #53397

Closed

dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 3, 2022

dmitshur added this to the Go1.19 milestone Aug 3, 2022

dmitshur added the arch-arm64 label Aug 3, 2022

golang locked and limited conversation to collaborators Aug 5, 2023

gopherbot added the FrozenDueToAge label Aug 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

go/reflect: Incorrect behavior on arm64 when using MakeFunc / Call #52788

go/reflect: Incorrect behavior on arm64 when using MakeFunc / Call #52788

znkr commented May 9, 2022 •

edited

Loading

gopherbot commented May 9, 2022

gopherbot commented May 9, 2022

gopherbot commented May 9, 2022

gopherbot commented May 11, 2022

cherrymui commented Jun 15, 2022

gopherbot commented Jun 15, 2022

erifan commented Jun 16, 2022

cherrymui commented Jun 16, 2022

gopherbot commented Aug 5, 2022

gopherbot commented Aug 5, 2022

gopherbot commented Aug 5, 2022

gopherbot commented Aug 5, 2022

go/reflect: Incorrect behavior on arm64 when using MakeFunc / Call #52788

go/reflect: Incorrect behavior on arm64 when using MakeFunc / Call #52788

Comments

znkr commented May 9, 2022 • edited Loading

What version of Go are you using (go version)?

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

What did you do?

What did you expect to see?

What did you see instead?

gopherbot commented May 9, 2022

gopherbot commented May 9, 2022

gopherbot commented May 9, 2022

gopherbot commented May 11, 2022

cherrymui commented Jun 15, 2022

gopherbot commented Jun 15, 2022

erifan commented Jun 16, 2022

cherrymui commented Jun 16, 2022

gopherbot commented Aug 5, 2022

gopherbot commented Aug 5, 2022

gopherbot commented Aug 5, 2022

gopherbot commented Aug 5, 2022

znkr commented May 9, 2022 •

edited

Loading

What version of Go are you using (`go version`)?

What operating system and processor architecture are you using (`go env`)?