]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: use 1-byte load for address checking in racecallatomic
authorCherry Mui <cherryyz@google.com>
Thu, 15 Jun 2023 22:16:51 +0000 (18:16 -0400)
committerCherry Mui <cherryyz@google.com>
Fri, 16 Jun 2023 14:09:02 +0000 (14:09 +0000)
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 <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: David Chase <drchase@google.com>
src/runtime/race/race_linux_test.go
src/runtime/race_amd64.s
src/runtime/race_arm64.s
src/runtime/race_ppc64le.s

index e8a2d0fd8c91c44e0c9b3aa100b97280e21a02e8..947ed7ca39457612559466d7ac800ea90c8c3f7b 100644 (file)
@@ -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)
+       }
+}
index 0697be7180bb8339916ca1b4abf2d9eb5729689c..4fa130e8613703275e2959937f5983c524a9904a 100644 (file)
@@ -333,7 +333,7 @@ TEXT        sync∕atomic·CompareAndSwapUintptr(SB), NOSPLIT, $0-25
 TEXT   racecallatomic<>(SB), NOSPLIT|NOFRAME, $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
index edbb3b12c73423d4de21761783cba11e2c9e3aa2..c818345852f6b07bd5d9f4d3465ca30297495d1d 100644 (file)
@@ -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
index 5fd4f785c8c8ce9f9d63250260cb1a985e52d390..39cfffc39bbeb7fd89321a60d5f73ec5245f6d41 100644 (file)
@@ -363,7 +363,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