[release-branch.go1.4] runtime: remove assumption that noptrdata data bss noptrbss...

««« CL 179980043 / d71cc7e8a0e0
runtime: remove assumption that noptrdata data bss noptrbss are ordered and contiguous

The assumption can be violated by external linkers reordering them or
inserting non-Go sections in between them. I looked briefly at trying
to write out the _go_.o in external linking mode in a way that forced
the ordering, but no matter what there's no way to force Go's data
and Go's bss to be next to each other. If there is any data or bss from
non-Go objects, it's very likely to get stuck in between them.

Instead, rewrite the two places we know about that make the assumption.
I grepped for noptrdata to look for more and didn't find any.

The added race test (os/exec in external linking mode) fails without
the changes in the runtime. It crashes with an invalid pointer dereference.

Fixes #9133.

LGTM=dneil
R=dneil
CC=dvyukov, golang-codereviews, iant
https://golang.org/cl/179980043
»»»

LGTM=dneil
R=dneil
CC=golang-codereviews
https://golang.org/cl/173510043

diff --git a/src/run.bash b/src/run.bash
index 3c9430c87e97ae018a120d7df8f28b7eca1a5460..9a0e1cb0f207c751b03d3d1367bfbf339cbe5be2 100755 (executable)
--- a/src/run.bash
+++ b/src/run.bash
@@ -70,9 +70,10 @@ case "$GOHOSTOS-$GOOS-$GOARCH-$CGO_ENABLED" in
 linux-linux-amd64-1 | freebsd-freebsd-amd64-1 | darwin-darwin-amd64-1)
        echo
        echo '# Testing race detector.'
-       go test -race -i runtime/race flag
+       go test -race -i runtime/race flag os/exec
        go test -race -run=Output runtime/race
-       go test -race -short flag
+       go test -race -short flag os/exec
+       go test -race -short -ldflags=-linkmode=external flag os/exec
 esac
 
 xcd() {

diff --git a/src/runtime/malloc.go b/src/runtime/malloc.go
index 8cf1c3d342664269703fe0fdc727851cddc308b0..117044944041547324f195e2c6e8a63316bdf73b 100644 (file)
--- a/src/runtime/malloc.go
+++ b/src/runtime/malloc.go
@@ -490,6 +490,8 @@ func GC() {
 
 // linker-provided
 var noptrdata struct{}
+var enoptrdata struct{}
+var noptrbss struct{}
 var enoptrbss struct{}
 
 // SetFinalizer sets the finalizer associated with x to f.
@@ -566,8 +568,13 @@ func SetFinalizer(obj interface{}, finalizer interface{}) {
                //      func main() {
                //              runtime.SetFinalizer(Foo, nil)
                //      }
-               // The segments are, in order: text, rodata, noptrdata, data, bss, noptrbss.
-               if uintptr(unsafe.Pointer(&noptrdata)) <= uintptr(e.data) && uintptr(e.data) < uintptr(unsafe.Pointer(&enoptrbss)) {
+               // The relevant segments are: noptrdata, data, bss, noptrbss.
+               // We cannot assume they are in any order or even contiguous,
+               // due to external linking.
+               if uintptr(unsafe.Pointer(&noptrdata)) <= uintptr(e.data) && uintptr(e.data) < uintptr(unsafe.Pointer(&enoptrdata)) ||
+                       uintptr(unsafe.Pointer(&data)) <= uintptr(e.data) && uintptr(e.data) < uintptr(unsafe.Pointer(&edata)) ||
+                       uintptr(unsafe.Pointer(&bss)) <= uintptr(e.data) && uintptr(e.data) < uintptr(unsafe.Pointer(&ebss)) ||
+                       uintptr(unsafe.Pointer(&noptrbss)) <= uintptr(e.data) && uintptr(e.data) < uintptr(unsafe.Pointer(&enoptrbss)) {
                        return
                }
                gothrow("runtime.SetFinalizer: pointer not in allocated block")

diff --git a/src/runtime/race.c b/src/runtime/race.c
index 9ac73fbccf11df09b72b7bf021045dd7fc5ae8c3..e400c8d102982b990e2bc2152b90bcbebe7c7621 100644 (file)
--- a/src/runtime/race.c
+++ b/src/runtime/race.c
@@ -63,8 +63,14 @@ void __tsan_go_ignore_sync_end(void);
 #pragma cgo_import_static __tsan_go_atomic64_compare_exchange
 
 extern byte runtime·noptrdata[];
+extern byte runtime·enoptrdata[];
+extern byte runtime·data[];
+extern byte runtime·edata[];
+extern byte runtime·bss[];
+extern byte runtime·ebss[];
+extern byte runtime·noptrbss[];
 extern byte runtime·enoptrbss[];
-  
+
 // start/end of heap for race_amd64.s
 uintptr runtime·racearenastart;
 uintptr runtime·racearenaend;
@@ -86,7 +92,13 @@ isvalidaddr(uintptr addr)
 {
        if(addr >= runtime·racearenastart && addr < runtime·racearenaend)
                return true;
-       if(addr >= (uintptr)runtime·noptrdata && addr < (uintptr)runtime·enoptrbss)
+       if(addr >= (uintptr)runtime·noptrdata && addr < (uintptr)runtime·enoptrdata)
+               return true;
+       if(addr >= (uintptr)runtime·data && addr < (uintptr)runtime·edata)
+               return true;
+       if(addr >= (uintptr)runtime·bss && addr < (uintptr)runtime·ebss)
+               return true;
+       if(addr >= (uintptr)runtime·noptrbss && addr < (uintptr)runtime·enoptrbss)
                return true;
        return false;
 }
@@ -95,15 +107,37 @@ isvalidaddr(uintptr addr)
 uintptr
 runtime·raceinit(void)
 {
-       uintptr racectx, start, size;
+       uintptr racectx, start, end, size;
 
        // cgo is required to initialize libc, which is used by race runtime
        if(!runtime·iscgo)
                runtime·throw("raceinit: race build must use cgo");
        runtime·racecall(__tsan_init, &racectx, runtime·racesymbolizethunk);
        // Round data segment to page boundaries, because it's used in mmap().
-       start = (uintptr)runtime·noptrdata & ~(PageSize-1);
-       size = ROUND((uintptr)runtime·enoptrbss - start, PageSize);
+       // The relevant sections are noptrdata, data, bss, noptrbss.
+       // In external linking mode, there may be other non-Go data mixed in,
+       // and the sections may even occur out of order.
+       // Work out a conservative range of addresses.
+       start = ~(uintptr)0;
+       end = 0;
+       if(start > (uintptr)runtime·noptrdata)
+               start = (uintptr)runtime·noptrdata;
+       if(start > (uintptr)runtime·data)
+               start = (uintptr)runtime·data;
+       if(start > (uintptr)runtime·noptrbss)
+               start = (uintptr)runtime·noptrbss;
+       if(start > (uintptr)runtime·bss)
+               start = (uintptr)runtime·bss;
+       if(end < (uintptr)runtime·enoptrdata)
+               end = (uintptr)runtime·enoptrdata;
+       if(end < (uintptr)runtime·edata)
+               end = (uintptr)runtime·edata;
+       if(end < (uintptr)runtime·enoptrbss)
+               end = (uintptr)runtime·enoptrbss;
+       if(end < (uintptr)runtime·ebss)
+               end = (uintptr)runtime·ebss;
+       start = start & ~(PageSize-1);
+       size = ROUND(end - start, PageSize);
        runtime·racecall(__tsan_map_shadow, start, size);
        return racectx;
 }