Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
davecheney opened this issue May 12, 2014 · 11 comments
Closed

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

davecheney opened this issue May 12, 2014 · 11 comments

Comments

@davecheney
Copy link
Contributor

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
Copy link
Contributor

rsc commented May 13, 2014

Comment 1:

Status changed to Accepted.

@davecheney
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member

minux commented May 14, 2014

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
Copy link

Comment 6:

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

@bradfitz
Copy link
Contributor

Comment 7:

Owner changed to @minux.

Status changed to Started.

@minux
Copy link
Member

minux commented May 20, 2014

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
Copy link
Contributor

rsc commented May 20, 2014

Comment 9:

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

Owner changed to ---.

Status changed to Accepted.

@mdempsky
Copy link
Member

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
Copy link
Contributor

rsc commented Jun 8, 2015

I think we basically decided this was okay.

@rsc rsc changed the title cmd/6l: do not reject addresses >= 2**31 cmd/link: do not reject addresses >= 2³¹ Jun 8, 2015
@rsc rsc closed this as completed Jun 8, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants