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

Go link tool needs to find gcc on Arm platform #1574

Merged
merged 1 commit into from
Jun 29, 2018

Conversation

lubinszARM
Copy link
Contributor

Signed-off-by: Bin Lu [email protected]

@lubinszARM
Copy link
Contributor Author

Hi all,
I added a workaround for go link on Arm platform.
On Arm platform, go link tool needs some features of gcc to complete the job, but on x86 platform, go link tool does not need gcc.
So, I added the path for 'gcc' in the patch. If without this patch, google/gvisor will be compiled failed on Arm platform.
Please see my test log as more reference:

On Arm platform:

root@bin:/home/bblu/test# exec env - /bin/bash -c '/usr/local/go/bin/go tool link -s -v -o test test.a'
HEADER = -H4 -T0x11000 -D0x0 -R0x10000
searching for runtime.a in /usr/local/go/pkg/linux_arm64/runtime.a
0.00 deadcode
0.02 pclntab=612447 bytes, funcdata total 118962 bytes
0.03 dodata
0.03 dwarf
0.05 symsize = 0
0.08 reloc
0.10 asmb
0.10 datblk
0.11 sym
0.11 elfsym
0.11 symsize = 121488
0.11 symsize = 122016
0.13 header
0.14 host link: "gcc" "-s" "-o" "test" "-rdynamic" "/tmp/go-link-335745272/go.o" "/tmp/go-link-335745272/000000.o" "/tmp/go-link-335745272/000001.o" "/tmp/go-link-335745272/000002.o" "/tmp/go-link-335745272/000003.o" "/tmp/go-link-335745272/000004.o" "/tmp/go-link-335745272/000005.o" "/tmp/go-link-335745272/000006.o" "/tmp/go-link-335745272/000007.o" "/tmp/go-link-335745272/000008.o" "/tmp/go-link-335745272/000009.o" "/tmp/go-link-335745272/000010.o" "/tmp/go-link-335745272/000011.o" "/tmp/go-link-335745272/000012.o" "/tmp/go-link-335745272/000013.o" "-g" "-O2" "-g" "-O2" "-lpthread"
/usr/local/go/pkg/tool/linux_arm64/link: running gcc failed: exec: "gcc": executable file not found in $PATH

=====================================================
On X86 platform:

root@os:~/bblu/test# exec env - /bin/bash -c '/usr/local/go/bin/go tool link -s -v -o test test.a'
HEADER = -H4 -T0x401000 -D0x0 -R0x1000
searching for runtime.a in /usr/local/go/pkg/linux_amd64/runtime.a
0.00 deadcode
0.03 pclntab=624716 bytes, funcdata total 116096 bytes
0.03 dodata
0.03 dynreloc
0.04 reloc
0.04 asmb
0.04 codeblk
0.05 rodatblk
0.05 datblk
0.05 headr
0.05 cpu time
47771 symbols
68020 liveness data

@@ -103,5 +103,7 @@ def _bootstrap_compile(go, sources, out_lib, gc_goopts):
inputs = sources + go.sdk_files + go.sdk_tools,
outputs = [out_lib],
mnemonic = "GoCompile",
command = "export GOROOT=$(pwd)/{} && export GOROOT_FINAL=GOROOT && {} {}".format(go.root, go.go.path, " ".join(args)),
# workaround: go link tool needs some features of gcc to complete the job on Arm platform.
# So, PATH for 'gcc' is required here on Arm platform.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is gcc needed for compiling Go code? My expectation is that it should only be required for linking.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed it form compile.bzl
Thanks.

@@ -159,6 +159,8 @@ def _bootstrap_link(go, archive, executable, gc_linkopts):
inputs = inputs,
outputs = [executable],
mnemonic = "GoLink",
# workaround: go link tool needs some features of gcc to complete the job on Arm platform.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PATH isn't actually set here.

Instead of setting PATH though, I think it would be better to set CC to go.cgo_tools.ld_executable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jayconrod
'CC=go.cgo_tools.ld_executable' is useless here.
I modified the code like this:
command = "export GOROOT=$(pwd)/{} && export GOROOT_FINAL=GOROOT && export CC={} && {} {}".format(go.root, go.cgo_tools.ld_executable, go.go.path, " ".join(args)),
And google/gvisor still reported that 'gcc' was not found in $PATH.
Please see full logs as reference:

root@bin:/go/src/github.com/google/gvisor# bazel build runsc --verbose_failures
INFO: SHA256 (https://github.com/lubinsz/rules_go/archive/801a52883ef36ae078f069bc250a325ca7121acd.tar.gz) = 5ba38ec2b0bee96a77793f7f95669fa4ea101599aa39eb8dce1b9daf149431fd
INFO: Build options have changed, discarding analysis cache.
INFO: Analysed target //runsc:runsc (183 packages loaded).
INFO: Found 1 target...
ERROR: /root/.cache/bazel/_bazel_root/954c2359eaedb37da1533cf5fa671511/external/io_bazel_rules_go/go/tools/builders/BUILD.bazel:54:1: error executing shell command: 'export GOROOT=$(pwd)/external/go_sdk && export GOROOT_FINAL=GOROOT && export CC=/usr/bin/ld && external/go_sdk/bin/go tool link -s -o bazel-out/host/bin/external/io_bazel_rules_go/go/tools/builders...' failed (Exit 1): bash failed: error executing command
(cd /root/.cache/bazel/_bazel_root/954c2359eaedb37da1533cf5fa671511/execroot/main &&
exec env -
/bin/bash -c 'export GOROOT=$(pwd)/external/go_sdk && export GOROOT_FINAL=GOROOT && export CC=/usr/bin/ld && external/go_sdk/bin/go tool link -s -o bazel-out/host/bin/external/io_bazel_rules_go/go/tools/builders/linux_arm64_stripped/embed bazel-out/host/bin/external/io_bazel_rules_go/go/tools/builders/linux_arm64_stripped/embed~/go/tools/builders/embed.a')

Use --sandbox_debug to see verbose messages from the sandbox
/root/.cache/bazel/_bazel_root/954c2359eaedb37da1533cf5fa671511/sandbox/linux-sandbox/4/execroot/main/external/go_sdk/pkg/tool/linux_arm64/link: running gcc failed: exec: "gcc": executable file not found in $PATH

Target //runsc:runsc failed to build
ERROR: /go/src/github.com/google/gvisor/runsc/BUILD:5:1 error executing shell command: 'export GOROOT=$(pwd)/external/go_sdk && export GOROOT_FINAL=GOROOT && export CC=/usr/bin/ld && external/go_sdk/bin/go tool link -s -o bazel-out/host/bin/external/io_bazel_rules_go/go/tools/builders...' failed (Exit 1): bash failed: error executing command
(cd /root/.cache/bazel/_bazel_root/954c2359eaedb37da1533cf5fa671511/execroot/main &&
exec env -
/bin/bash -c 'export GOROOT=$(pwd)/external/go_sdk && export GOROOT_FINAL=GOROOT && export CC=/usr/bin/ld && external/go_sdk/bin/go tool link -s -o bazel-out/host/bin/external/io_bazel_rules_go/go/tools/builders/linux_arm64_stripped/embed bazel-out/host/bin/external/io_bazel_rules_go/go/tools/builders/linux_arm64_stripped/embed~/go/tools/builders/embed.a')

Use --sandbox_debug to see verbose messages from the sandbox
INFO: Elapsed time: 20.737s, Critical Path: 8.50s
INFO: 7 processes, linux-sandbox.
FAILED: Build did NOT complete successfully
root@bin:/go/src/github.com/google/gvisor# uname -p
aarch64

@lubinszARM
Copy link
Contributor Author

Hi @jayconrod
Agree with you, I have removed the workaround from compile.bzl
But for link.bzl, it seems that "CC" does not work.
Please see my test log:

root@bin:/home/bblu/test# exec env - /bin/bash -c 'export CC=/usr/bin/gcc && /usr/local/go/bin/go tool link -s -o test test.a'
/usr/local/go/pkg/tool/linux_arm64/link: running gcc failed: exec: "gcc": executable file not found in $PATH

root@bin:/home/bblu/test# uname -p
aarch64

@vielmetti
Copy link

Reference #1506 "arm64 support"

@lubinszARM
Copy link
Contributor Author

@jayconrod
Also I checked the golang code.
In github.com/golang/go/src/pkg/os/exec/lp_unix.go
In func LookPath(file string) (string, error), it only check 'PATH' to find 'gcc'
Thanks.

@jayconrod
Copy link
Contributor

@lubinszARM The os/exec package is the general package for running subcommands from the standard library. It's not specific to the linker.

So as I understand this, external linking (i.e., linking the final compiled Go code with a C linker like gcc or clang) is required on arm64 because of limitations in the Go toolchain. golang/go#10373 is an open feature request for this. There is also some discussion. I don't know the details of this; if you know, it would be helpful to add a comment explaining it. Otherwise, just add a comment pointing to golang/go#10373.

The right way to provide an external linker is to pass the -extld=<linker> flag. I believe that should be set to go.cgo_tools.ld_executable, which will be the full path to the linker.

Hard coding PATH=/usr/bin is not safe. Not all users will have their C/C++ toolchain in /usr/bin (in particular, Windows users will not). Many users will have a toolchain as part of their workspace, and if rules_go is using the system toolchain instead of the one they've requested, that would be unexpected. This may also break in remote execution environments, and it may break in the future with stricter sandboxing.

Note that this PR currently only affects linking of host mode tools (i.e., our wrappers for the compiler, linker, and other tools). It won't affect linking of go_binary or go_test rules on the target platform. I don't know whether that works on arm64 or not, but the right place to look for that is above in emit_link.

@lubinszARM
Copy link
Contributor Author

lubinszARM commented Jun 29, 2018

Hi @jayconrod
Thank you for your suggestions ~ It's very helpful.
I have updated the code, please check it.
Thank you.

For issue #10373 in golang, my colleague has finished a prototype for it. But as he said, there are still a lot of work to do.

Copy link
Contributor

@jayconrod jayconrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@jayconrod jayconrod merged commit 3fcd473 into bazel-contrib:master Jun 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants