Skip to content

Commit 1a7709d

Browse files
committed
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. Should fix #60825. Change-Id: I3dee93eb08ba180c85e86a9d2e71b5b520e8dcf0 Reviewed-on: https://go-review.googlesource.com/c/go/+/503937 Run-TryBot: Cherry Mui <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Than McIntosh <[email protected]> Reviewed-by: David Chase <[email protected]>
1 parent 3e7ec13 commit 1a7709d

File tree

4 files changed

+31
-3
lines changed

4 files changed

+31
-3
lines changed

src/runtime/race/race_linux_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,31 @@ func TestAtomicMmap(t *testing.T) {
3535
t.Fatalf("bad atomic value: %v, want 2", *a)
3636
}
3737
}
38+
39+
func TestAtomicPageBoundary(t *testing.T) {
40+
// Test that atomic access near (but not cross) a page boundary
41+
// doesn't fault. See issue 60825.
42+
43+
// Mmap two pages of memory, and make the second page inaccessible,
44+
// so we have an address at the end of a page.
45+
pagesize := syscall.Getpagesize()
46+
b, err := syscall.Mmap(0, 0, 2*pagesize, syscall.PROT_READ|syscall.PROT_WRITE, syscall.MAP_ANON|syscall.MAP_PRIVATE)
47+
if err != nil {
48+
t.Fatalf("mmap failed %s", err)
49+
}
50+
defer syscall.Munmap(b)
51+
err = syscall.Mprotect(b[pagesize:], syscall.PROT_NONE)
52+
if err != nil {
53+
t.Fatalf("mprotect high failed %s\n", err)
54+
}
55+
56+
// This should not fault.
57+
a := (*uint32)(unsafe.Pointer(&b[pagesize-4]))
58+
atomic.StoreUint32(a, 1)
59+
if x := atomic.LoadUint32(a); x != 1 {
60+
t.Fatalf("bad atomic value: %v, want 1", x)
61+
}
62+
if x := atomic.AddUint32(a, 1); x != 2 {
63+
t.Fatalf("bad atomic value: %v, want 2", x)
64+
}
65+
}

src/runtime/race_amd64.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ TEXT sync∕atomic·CompareAndSwapUintptr(SB), NOSPLIT, $0-25
333333
TEXT racecallatomic<>(SB), NOSPLIT|NOFRAME, $0-0
334334
// Trigger SIGSEGV early.
335335
MOVQ 16(SP), R12
336-
MOVL (R12), R13
336+
MOVBLZX (R12), R13
337337
// Check that addr is within [arenastart, arenaend) or within [racedatastart, racedataend).
338338
CMPQ R12, runtime·racearenastart(SB)
339339
JB racecallatomic_data

src/runtime/race_arm64.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ TEXT racecallatomic<>(SB), NOSPLIT, $0
348348

349349
// Trigger SIGSEGV early.
350350
MOVD 40(RSP), R3 // 1st arg is addr. after two times BL, get it at 40(RSP)
351-
MOVD (R3), R13 // segv here if addr is bad
351+
MOVB (R3), R13 // segv here if addr is bad
352352
// Check that addr is within [arenastart, arenaend) or within [racedatastart, racedataend).
353353
MOVD runtime·racearenastart(SB), R10
354354
CMP R10, R3

src/runtime/race_ppc64le.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ TEXT sync∕atomic·CompareAndSwapUintptr(SB), NOSPLIT, $0-25
363363
TEXT racecallatomic<>(SB), NOSPLIT, $0-0
364364
// Trigger SIGSEGV early if address passed to atomic function is bad.
365365
MOVD (R6), R7 // 1st arg is addr
366-
MOVD (R7), R9 // segv here if addr is bad
366+
MOVB (R7), R9 // segv here if addr is bad
367367
// Check that addr is within [arenastart, arenaend) or within [racedatastart, racedataend).
368368
MOVD runtime·racearenastart(SB), R9
369369
CMP R7, R9

0 commit comments

Comments
 (0)