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

runtime: go 1.8 + ARM + 64k pages not working #18408

Closed
ncw opened this issue Dec 21, 2016 · 23 comments
Closed

runtime: go 1.8 + ARM + 64k pages not working #18408

ncw opened this issue Dec 21, 2016 · 23 comments
Milestone

Comments

@ncw
Copy link
Contributor

ncw commented Dec 21, 2016

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version devel +0ef4815 Wed Dec 21 12:27:41 2016 +0000 linux/amd64

What operating system and processor architecture are you using (go env)?

Compile host

GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/ncw/go"
GORACE=""
GOROOT="/opt/go/go1.8"
GOTOOLDIR="/opt/go/go1.8/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build680021883=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

Target machine

# getconf PAGESIZE
65536
# uname -a
Linux WDMyCloud 3.2.26 #1 SMP Thu Jul 9 11:14:15 PDT 2015 wd-2.4-rel
armv7l GNU/Linux
# cat /proc/cpuinfo
Processor	: ARMv7 Processor rev 1 (v7l)
processor	: 0
BogoMIPS	: 1299.25

processor	: 1
BogoMIPS	: 1292.69

Features	: swp half thumb fastmult vfp edsp neon vfpv3 tls
CPU implementer	: 0x41
CPU architecture: 7
CPU variant	: 0x2
CPU part	: 0xc09
CPU revision	: 1

Hardware	: Comcerto 2000 EVM
Revision	: 0001
Serial		: 0000000000000000

What did you do?

Compile this program and run it: https://play.golang.org/p/sqjFommznr

What did you expect to see?

Hello, World!

What did you see instead?

# ./hello
unexpected fault address 0x88868
fatal error: fault
[signal SIGSEGV: segmentation violation code=0x2 addr=0x88868 pc=0x88868]

goroutine 1 [running, locked to thread]:
runtime.throw(0xa454f, 0x5)
	/opt/go/go1.8/src/runtime/panic.go:596 +0x70 fp=0x10433f94 sp=0x10433f88
runtime.sigpanic()
	/opt/go/go1.8/src/runtime/signal_unix.go:297 +0x20c fp=0x10433fb8
sp=0x10433f94
main.init()
	/home/ncw/go/src/github.com/ncw/rclone/rclone-v1.34-75-gcbfec0d-arm-go-tip/hello.go:8 fp=0x10433fbc sp=0x10433fbc
runtime.main()
	/opt/go/go1.8/src/runtime/proc.go:173 +0x198 fp=0x10433fe4 sp=0x10433fbc
runtime.goexit()
	/opt/go/go1.8/src/runtime/asm_arm.s:1017 +0x4 fp=0x10433fe4 sp=0x10433fe4

This issue was discovered as part of rclone/rclone#426. @DTrace001 is the one with the hardware, I'm just reporting the problem.

@bradfitz
Copy link
Contributor

/cc @cherrymui @minux @aclements

@bradfitz bradfitz added this to the Go1.8Maybe milestone Dec 21, 2016
@minux
Copy link
Member

minux commented Dec 22, 2016 via email

@aclements
Copy link
Member

@DTrace001, while you're in gdb stopped at the SIGSEGV, could you also run info proc to get the process ID and paste the output of !cat /proc/<process ID>/maps?

Can you also confirm that the "hello" binary you're running is the one from http://pub.rclone.org/rclone-v1.34-75-gcbfec0d-arm-go-tip.zip (sha1sum da609dc3998c7f080e380c469aca6d3c623472c1)?

@EnorMOZ
Copy link

EnorMOZ commented Dec 23, 2016

@minux

Could @DTrace001 please run the hello world program under gdb

I will first need to compile gdb, for the wdmycloud device using 64K page size, since it does not come native to it. From there I can continue to help debug the issue. They did not make it easy to run custom programs on this device. Sorry, I probably wont be able to get to do this until early next week.

@minux
Copy link
Member

minux commented Dec 23, 2016 via email

@EnorMOZ
Copy link

EnorMOZ commented Dec 28, 2016

@minux Sorry but I am not having luck compiling gdb on a debian wheezy host for armhf with 64k page size. The wdmycloud device is a hacked version of debian wheezy running armhf 64k page size.

@aclements
Copy link
Member

@DTrace001, in that case, can we get you to make a core dump? Set ulimit -c unlimited and run the hello binary with GOTRACEBACK=crash ./hello. You might have to adjust /proc/sys/kernel/core_pattern if you don't get an obvious core file (e.g., echo core | sudo tee /proc/sys/kernel/core_pattern).

@aclements
Copy link
Member

@minux, just looking at the binary, that address is suspiciously near the boundary between these two segments:

$ readelf --segments hello
...
  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
...
  LOAD           0x000000 0x00010000 0x00010000 0x788cc 0x788cc R E 0x1000
  LOAD           0x079000 0x00089000 0x00089000 0x619be 0x619be R   0x1000
...

The faulting address (0x88868) falls right near the end of the first of these two segments (0x888cc). The start of the second segment rounds down to 0x80000. I'm guessing the kernel mapped this wrong (even though in principle it could have done it right) and the page wound up marked no-execute. I think the core file should have enough information to confirm this.

@minux
Copy link
Member

minux commented Dec 29, 2016 via email

@davecheney
Copy link
Contributor

davecheney commented Dec 29, 2016 via email

@aclements
Copy link
Member

We need to increase executable segment alignment.

How hard would that be to do? Could we do it before the rc on Sunday?

I think we've already figured out the maximum page size for each GOARCH in runtime/internal/sys.DefaultPhysPageSize. We can base the alignments on those.

It looks like the default page size on Linux for all of the arches is 4K, which is probably why we're not seeing a more wide-spread problem with this.

/cc @randall77 @rsc

@aclements
Copy link
Member

I just read through Linux's binfmt_elf and confirmed that it doesn't try to do anything smart with overlapping segments. It just maps them in the order and with the permissions given, rounded out to page boundaries. So, if we want to be clever, we don't need to align all segments to the maximum page boundary. We only need to do this where we go from more to less permissive segments, such as the read+exec segment to the read-only segment. But it's probably better to just align all of the segments unless that causes too much binary bloat, which I doubt that it will.

@EnorMOZ
Copy link

EnorMOZ commented Dec 29, 2016

Hope this helps.

GOTRACEBACK=crash ./hello
unexpected fault address 0x88868
fatal error: fault
[signal SIGSEGV: segmentation violation code=0x2 addr=0x88868 pc=0x88868]

goroutine 1 [running, locked to thread]:
runtime.throw(0xa454f, 0x5)
	/opt/go/go1.8/src/runtime/panic.go:596 +0x70 fp=0x10439f94 sp=0x10439f88
runtime.sigpanic()
	/opt/go/go1.8/src/runtime/signal_unix.go:297 +0x20c fp=0x10439fb8 sp=0x10439f94
main.init()
	/home/ncw/go/src/github.com/ncw/rclone/rclone-v1.34-75-gcbfec0d-arm-go-tip/hello.go:8 fp=0x10439fbc sp=0x10439fbc
runtime.main()
	/opt/go/go1.8/src/runtime/proc.go:173 +0x198 fp=0x10439fe4 sp=0x10439fbc
runtime.goexit()
	/opt/go/go1.8/src/runtime/asm_arm.s:1017 +0x4 fp=0x10439fe4 sp=0x10439fe4

goroutine 2 [runnable]:
runtime.forcegchelper()
	/opt/go/go1.8/src/runtime/proc.go:218 fp=0x104247ec sp=0x104247ec
runtime.goexit()
	/opt/go/go1.8/src/runtime/asm_arm.s:1017 +0x4 fp=0x104247ec sp=0x104247ec
created by runtime.init.3
	/opt/go/go1.8/src/runtime/proc.go:215 +0x24

goroutine 3 [GC sweep wait]:
runtime.gopark(0xaac0c, 0xedda0, 0xa52b8, 0xd, 0x2ba14, 0x1)
	/opt/go/go1.8/src/runtime/proc.go:261 +0xfc fp=0x10424fac sp=0x10424f98
runtime.goparkunlock(0xedda0, 0xa52b8, 0xd, 0x14, 0x1)
	/opt/go/go1.8/src/runtime/proc.go:267 +0x44 fp=0x10424fc8 sp=0x10424fac
runtime.bgsweep(0x1041c040)
	/opt/go/go1.8/src/runtime/mgcsweep.go:56 +0x90 fp=0x10424fe4 sp=0x10424fc8
runtime.goexit()
	/opt/go/go1.8/src/runtime/asm_arm.s:1017 +0x4 fp=0x10424fe4 sp=0x10424fe4
created by runtime.gcenable
	/opt/go/go1.8/src/runtime/mgc.go:209 +0x4c
Aborted (core dumped)

core.zip

@aclements
Copy link
Member

@DTrace001, thanks, but it looks like you forgot to attach the actual "core" file that produced.

@EnorMOZ
Copy link

EnorMOZ commented Dec 29, 2016

@aclements Sorry updated comment with core.zip attached

@aclements
Copy link
Member

Thanks for the core file. This further confirms that the segments are getting rounded such that we're losing the executable bit on part of the text section. We can compare the LOAD segments from the original binary with the LOAD segments in the core file:

Original binary:

  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
...
  LOAD           0x000000 0x00010000 0x00010000 0x788cc 0x788cc R E 0x1000
  LOAD           0x079000 0x00089000 0x00089000 0x619be 0x619be R   0x1000

Core file:

  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
...
  LOAD           0x010000 0x00010000 0x00000000 0x00000 0x70000 R E 0x10000
  LOAD           0x010000 0x00080000 0x00000000 0x00000 0x60000 R   0x10000

The executable segment got truncated to a length of 0x70000 bytes from 0x788cc and the start of the read-only segment got rounded down to 0x80000, which means the faulting PC 0x88868 lands in the read-only segment instead of the executable segment.

@aclements
Copy link
Member

@DTrace001 or @ncw, can you build the test binary with the -R 0x10000 linker flag and try that? E.g., go build -ldflags '-R 0x10000' ....

@aclements
Copy link
Member

As a quick experiment, I rebuilt all of the usual Go binaries with 64K segment alignment (on amd64). On average, they got 3% larger:

name       old data     new data     delta
addr2line  3.43MB ± 0%  3.51MB ± 0%  +2.27%
api        4.85MB ± 0%  4.96MB ± 0%  +2.20%
asm        3.94MB ± 0%  4.01MB ± 0%  +1.98%
cgo        3.91MB ± 0%  3.98MB ± 0%  +1.68%
compile    14.4MB ± 0%  14.5MB ± 0%  +0.60%
cover      4.81MB ± 0%  4.96MB ± 0%  +3.06%
dist       3.14MB ± 0%  3.29MB ± 0%  +4.70%
doc        3.76MB ± 0%  3.85MB ± 0%  +2.40%
fix        2.85MB ± 0%  3.01MB ± 0%  +5.60%
link       4.74MB ± 0%  4.88MB ± 0%  +2.85%
nm         3.39MB ± 0%  3.51MB ± 0%  +3.26%
objdump    3.71MB ± 0%  3.85MB ± 0%  +3.64%
pack       1.96MB ± 0%  2.11MB ± 0%  +7.73%
pprof      9.97MB ± 0%  10.2MB ± 0%  +2.01%
trace      9.01MB ± 0%  9.21MB ± 0%  +2.18%
vet        6.25MB ± 0%  6.34MB ± 0%  +1.38%
[Geo mean] 4.59MB       4.73MB       +2.96%

@ncw
Copy link
Contributor Author

ncw commented Jan 4, 2017

@DTrace001 I've built a binary with this for you to try

GOARCH=arm GOOS=linux go build -ldflags '-R 0x10000' hello.go

Here it is (zipped): hello.zip

@rsc
Copy link
Contributor

rsc commented Jan 4, 2017

We already set the default FlagRound to 64k on a variety of GOOS/GOARCH combinations. It's easy to add linux/arm. I sent CL 34629.

@rsc rsc modified the milestones: Go1.8, Go1.8Maybe Jan 4, 2017
@EnorMOZ
Copy link

EnorMOZ commented Jan 4, 2017

@ncw @aclements Jackpot !!!

WDMyCloud:~# ulimit -c unlimited
WDMyCloud:~# GOTRACEBACK=crash ./hello
Hello, World!

@gopherbot
Copy link
Contributor

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

@ncw
Copy link
Contributor Author

ncw commented Jan 8, 2017

We've now confirmed that this fixes the original issue in rclone/rclone#426.

Very pleased we can get this into 1.8.

Thank you all for your help.

@golang golang locked and limited conversation to collaborators Jan 8, 2018
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

9 participants