cmd/link: do not reject addresses >= 2³¹

[davecheney]

What steps will reproduce the problem?

package main

import (
        "fmt"
        "time"
)

const (
        array_size = 1<<27
)

var a, b, c [array_size]int64

func main() {
        for i:=0; i<array_size; i+=1 {
                a[i&array_size]=2;
                b[i&array_size]=3;
                c[i&array_size]=1;
        }
        t1 :=time.Now()
        for i:=0; i<array_size; i+=1 {
                c[i&array_size]=a[i&array_size]+b[i&array_size]*3;
        }
        t2 := time.Since(t1)
        fmt.Printf("%v Triad Speed: %12.1f MB/s\n",t2,  array_size*8.0*3/1024.0/1024.0/t2.Seconds())
}

What is the expected output? What do you see instead?

odessa(~/src) % go run aa.go
unexpected fault address 0xffffffff801603e0
fatal error: fault
[signal 0xb code=0x1 addr=0xffffffff801603e0 pc=0x20a7]

goroutine 16 [running]:
runtime.throw(0x1400f3)
    /Users/dfc/go/src/pkg/runtime/panic.c:520 +0x69 fp=0x2c819be80
runtime.sigpanic()
    /Users/dfc/go/src/pkg/runtime/os_darwin.c:456 +0x13f fp=0x2c819be98
main.main()
    /Users/dfc/src/aa.go:18 +0xa7 fp=0x2c819bf50
runtime.main()
    /Users/dfc/go/src/pkg/runtime/proc.c:247 +0x11a fp=0x2c819bfa8
runtime.goexit()
    /Users/dfc/go/src/pkg/runtime/proc.c:1445 fp=0x2c819bfb0
created by _rt0_go
    /Users/dfc/go/src/pkg/runtime/asm_amd64.s:97 +0x120

goroutine 17 [runnable]:
runtime.MHeap_Scavenger()
    /Users/dfc/go/src/pkg/runtime/mheap.c:507
runtime.goexit()
    /Users/dfc/go/src/pkg/runtime/proc.c:1445

goroutine 18 [runnable]:
bgsweep()
    /Users/dfc/go/src/pkg/runtime/mgc0.c:1962
runtime.goexit()
    /Users/dfc/go/src/pkg/runtime/proc.c:1445

goroutine 19 [runnable]:
runfinq()
    /Users/dfc/go/src/pkg/runtime/mgc0.c:2588
runtime.goexit()
    /Users/dfc/go/src/pkg/runtime/proc.c:1445
exit status 2

Please use labels and text to provide additional information.

Reducing array_size to 1<<26 stops crashing.

odessa(~/src) % go version
go version devel +0f7c69d6c367 Mon May 12 17:19:02 2014 -0400 + darwin/amd64
odessa(~/src) % uname -a
Darwin odessa.fritz.box 13.1.0 Darwin Kernel Version 13.1.0: Wed Apr  2 23:52:02 PDT
2014; root:xnu-2422.92.1~2/RELEASE_X86_64 x86_64

[rsc]

Comment 1:

Status changed to Accepted.

[davecheney]

Comment 2:

Smaller repro
package main
const (
        array_size = 1<<27
)
var a, b, c [array_size]int64
func main() {
        for i:=0; i<array_size; i+=1 {
                c[i]=a[i]+b[i]*3;
        }
}

[josharian]

Comment 3:

All that's needed is to prevent the linker from stripping any of the arrays, so this
suffices:
func main() {
    _, _, _ = a[0], b[0], c[0]
}
Looks like a 32 bit overflow in the linker somewhere.
Disassembly of main, array size 1 << 26:
   0x0000000000002000 <+0>:   mov    0x20075cc0,%rdx
   0x0000000000002008 <+8>:   mov    0x40075cc0,%rcx
   0x0000000000002010 <+16>:  mov    0x75cc0,%rbx
   0x0000000000002018 <+24>:  retq   
Disassembly of main, array size 1 << 27:
   0x0000000000002000 <+0>:   mov    0x40075cc0,%rdx
   0x0000000000002008 <+8>:   mov    0xffffffff80075cc0,%rcx
   0x0000000000002010 <+16>:  mov    0x75cc0,%rbx
   0x0000000000002018 <+24>:  retq

[josharian]

Comment 4:

Scratch the overflow comment. It appears that the 32 bit operand to MOV gets
sign-extended by the processor. Note that the machine code does not contain 0xffffffff:
   0x0000000000002000 <+0>:   48 8b 14 25 c0 5c 07 40 mov    0x40075cc0,%rdx
   0x0000000000002008 <+8>:   48 8b 0c 25 c0 5c 07 80 mov    0xffffffff80075cc0,%rcx
   0x0000000000002010 <+16>:  48 8b 1c 25 c0 5c 07 00 mov    0x75cc0,%rbx
gdb disassembler's sign extension matches observed behavior--the panic is due to an
attempt to dereference 0xffffffff80075cc0. It also matches
http://onlinedisassembler.com/odaweb/iH4HYC/0.
The two disassemblers I tried that do symbol resolution (Hopper.app and rsc's new x86
disassembler) incorrectly resolve this address to the main.c symbol.
I think that we need to do relative addressing for addresses above 2**31. This might be
a biggish change. It's also worth noting that this bug is not new -- it reproduces with
Go 1.2.
I'm going to stop working on this for now; my head hurts from wading through Intel's
docs.

[minux]

Comment 5:

for amd64, offsets are always signed, and also only 32-bit offset is offered.
so to access address larger than 2^31-1, we need to first do a 64-bit literal load into
a register and then indirectly load from that register.
this might not be the only issue when the linker cannot handle 2GB+ data, so i think
that we can defer it to 1.4.
(although it's easy to fix this particular problem in liblink.)

[gopherbot]

Comment 6:

CL https://golang.org/cl/91500046 mentions this issue.

[bradfitz]

Comment 7:

Owner changed to @minux.

Status changed to Started.

[minux]

Comment 8:

This issue was updated by revision 6612983.

This CL make the linker abort for the example program. For Go 1.4,
we need to find a general way to handle large memory model programs.
LGTM=dave, josharian, iant
R=iant, dave, josharian
CC=golang-codereviews
https://golang.org/cl/91500046
Committer: Russ Cox 

[rsc]

Comment 9:

Labels changed: added release-none, removed release-go1.3.

Owner changed to ---.

Status changed to Accepted.

[mdempsky]

Comment 10:

Tangentially, rejecting 32-bit addresses >=2**31 means 6l can't link programs using
the x86-64 kernel code model.  Under that model, the program is loaded into the top 2GB
of memory, so sign extending addresses is okay/desirable.
I prepared a CL (https://golang.org/cl/117800044/) to change the "(int32)o <
0" check with "o != (int32)o", and also to change INITTEXT and INITDAT from int64 to
uint64 (otherwise strtoll() caps the flags to LONG_MAX, and you can't specify an
appropriate text address).  With that CL, I'm able to successfully link a Go hello-world
program using -ldflags='-T 0xffffffff80000c00', and it looks right if I inspect it with
readelf.
Of course, the Go runtime doesn't support running as a kernel, and no supported Go OS
allows running userspace applications in this code model either, so it's perhaps moot.

[rsc]

I think we basically decided this was okay.