From c9d5dd5466b017eb91d6d10e5fb71434acd94f36 Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Thu, 15 Jun 2023 18:16:51 -0400 Subject: [release-branch.go1.19] runtime: use 1-byte load for address checking in racecallatomic In racecallatomic, we do a load before calling into TSAN, so if the address is invalid we fault on the Go stack. We currently use a 8-byte load instruction, regardless of the data size that the atomic operation is performed on. So if, say, we are doing a LoadUint32 at an address that is the last 4 bytes of a memory mapping, we may fault unexpectedly. Do a 1-byte load instead. (Ideally we should do a load with the right size, so we fault correctly if we're given an unaligned address for a wide load across a page boundary. Leave that for another CL.) Fix AMD64, ARM64, and PPC64. The code already uses 1-byte load on S390X. Fixes #60844. Updates #60825. Change-Id: I3dee93eb08ba180c85e86a9d2e71b5b520e8dcf0 Reviewed-on: https://go-review.googlesource.com/c/go/+/503937 Run-TryBot: Cherry Mui TryBot-Result: Gopher Robot Reviewed-by: Than McIntosh Reviewed-by: David Chase (cherry picked from commit 1a7709d6af76c06d465c5e969b502fc206f8e687) Reviewed-on: https://go-review.googlesource.com/c/go/+/503977 Auto-Submit: Dmitri Shuralyov TryBot-Bypass: Dmitri Shuralyov Reviewed-by: Austin Clements --- src/runtime/race/race_linux_test.go | 28 ++++++++++++++++++++++++++++ src/runtime/race_amd64.s | 2 +- src/runtime/race_arm64.s | 2 +- src/runtime/race_ppc64le.s | 2 +- 4 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/runtime/race/race_linux_test.go b/src/runtime/race/race_linux_test.go index e8a2d0fd8c..947ed7ca39 100644 --- a/src/runtime/race/race_linux_test.go +++ b/src/runtime/race/race_linux_test.go @@ -35,3 +35,31 @@ func TestAtomicMmap(t *testing.T) { t.Fatalf("bad atomic value: %v, want 2", *a) } } + +func TestAtomicPageBoundary(t *testing.T) { + // Test that atomic access near (but not cross) a page boundary + // doesn't fault. See issue 60825. + + // Mmap two pages of memory, and make the second page inaccessible, + // so we have an address at the end of a page. + pagesize := syscall.Getpagesize() + b, err := syscall.Mmap(0, 0, 2*pagesize, syscall.PROT_READ|syscall.PROT_WRITE, syscall.MAP_ANON|syscall.MAP_PRIVATE) + if err != nil { + t.Fatalf("mmap failed %s", err) + } + defer syscall.Munmap(b) + err = syscall.Mprotect(b[pagesize:], syscall.PROT_NONE) + if err != nil { + t.Fatalf("mprotect high failed %s\n", err) + } + + // This should not fault. + a := (*uint32)(unsafe.Pointer(&b[pagesize-4])) + atomic.StoreUint32(a, 1) + if x := atomic.LoadUint32(a); x != 1 { + t.Fatalf("bad atomic value: %v, want 1", x) + } + if x := atomic.AddUint32(a, 1); x != 2 { + t.Fatalf("bad atomic value: %v, want 2", x) + } +} diff --git a/src/runtime/race_amd64.s b/src/runtime/race_amd64.s index c679a876b8..34ec200e5b 100644 --- a/src/runtime/race_amd64.s +++ b/src/runtime/race_amd64.s @@ -333,7 +333,7 @@ TEXT sync∕atomic·CompareAndSwapUintptr(SB), NOSPLIT, $0-25 TEXT racecallatomic<>(SB), NOSPLIT, $0-0 // Trigger SIGSEGV early. MOVQ 16(SP), R12 - MOVL (R12), R13 + MOVBLZX (R12), R13 // Check that addr is within [arenastart, arenaend) or within [racedatastart, racedataend). CMPQ R12, runtime·racearenastart(SB) JB racecallatomic_data diff --git a/src/runtime/race_arm64.s b/src/runtime/race_arm64.s index edbb3b12c7..c818345852 100644 --- a/src/runtime/race_arm64.s +++ b/src/runtime/race_arm64.s @@ -348,7 +348,7 @@ TEXT racecallatomic<>(SB), NOSPLIT, $0 // Trigger SIGSEGV early. MOVD 40(RSP), R3 // 1st arg is addr. after two times BL, get it at 40(RSP) - MOVD (R3), R13 // segv here if addr is bad + MOVB (R3), R13 // segv here if addr is bad // Check that addr is within [arenastart, arenaend) or within [racedatastart, racedataend). MOVD runtime·racearenastart(SB), R10 CMP R10, R3 diff --git a/src/runtime/race_ppc64le.s b/src/runtime/race_ppc64le.s index ac335b1819..2826501462 100644 --- a/src/runtime/race_ppc64le.s +++ b/src/runtime/race_ppc64le.s @@ -362,7 +362,7 @@ TEXT sync∕atomic·CompareAndSwapUintptr(SB), NOSPLIT, $0-25 TEXT racecallatomic<>(SB), NOSPLIT, $0-0 // Trigger SIGSEGV early if address passed to atomic function is bad. MOVD (R6), R7 // 1st arg is addr - MOVD (R7), R9 // segv here if addr is bad + MOVB (R7), R9 // segv here if addr is bad // Check that addr is within [arenastart, arenaend) or within [racedatastart, racedataend). MOVD runtime·racearenastart(SB), R9 CMP R7, R9 -- cgit v1.2.3-54-g00ecf