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

Cgo and --compiler=gcc #1357

Closed
jayconrod opened this issue Feb 28, 2018 · 46 comments · Fixed by #3748
Closed

Cgo and --compiler=gcc #1357

jayconrod opened this issue Feb 28, 2018 · 46 comments · Fixed by #3748

Comments

@jayconrod
Copy link
Contributor

Copied from bazel-go-discuss. cc @nathantchan


One of our verification builds uses gcc, and I currently cannot get past the installation portion of the stdlib rule. It's throwing errors like the following:

not-int-const: In function '__cgo_f_1_3':
not-int-const:1:60: error: invalid type argument of unary '*' (have 'int')
not-num-const: In function '__cgo_f_1_4':
not-num-const:1:73: error: expected expression before ';' token
not-str-lit: In function '__cgo_f_1_5':
not-str-lit:1:73: error: expected expression before ';' token
not-type: In function '__cgo_f_2_2':
not-type:2:39: error: '__cgo_undefined__2' undeclared (first use in this function)
not-type:2:39: note: each undeclared identifier is reported only once for each function it appears in
not-type:2:38: error: invalid operands to binary * (have 'int (*)(const struct sockaddr * __restrict__,  socklen_t,  char * __restrict__,  socklen_t,  char * __restrict__,  socklen_t,  int)' and 'const char *')
not-int-const: In function '__cgo_f_2_3':
not-int-const:2:67: error: invalid operands to binary * (have 'int (*)(const struct sockaddr * __restrict__,  socklen_t,  char * __restrict__,  socklen_t,  char * __restrict__,  socklen_t,  int)' and 'int')
not-num-const: In function '__cgo_f_2_4':
not-num-const:2:67: error: incompatible types when initializing type 'double' using type 'int (*)(const struct sockaddr * __restrict__,  socklen_t,  char * __restrict__,  socklen_t,  char * __restrict__,  socklen_t,  int)'
not-str-lit: In function '__cgo_f_2_5':
not-str-lit:2:1: error: invalid initializer
not-int-const: In function '__cgo_f_3_3':
not-int-const:3:65: error: invalid type argument of unary '*' (have 'int')
not-int-const:3:33: error: enumerator value for '__cgo_undefined__3' is not an integer constant
not-num-const: In function '__cgo_f_3_4':
not-num-const:3:78: error: expected expression before ';' token
not-num-const:3:78: error: initializer element is not constant
not-str-lit: In function '__cgo_f_3_5':
not-str-lit:3:78: error: expected expression before ';' token
not-str-lit:3:78: error: invalid initializer
completed: At top level:
completed:1:16: error: '__cgo__2' undeclared here (not in a function)
completed:1:1: error: initializer element is not constant

It doesn't seem like setting --compiler=gcc works right now in 0.9.0 of the go rules. Any ideas?

$ bazel test --compiler=gcc tests/...
...................
ERROR: No toolchain found for --cpu='k8' --compiler='gcc'. Valid toolchains are: [
  --cpu='armeabi-v7a' --compiler='compiler' --glibc='armeabi-v7a',
  --cpu='ios_x86_64' --compiler='compiler' --glibc='ios',
  --cpu='k8' --compiler='compiler' --glibc='local',
  --cpu='x64_windows' --compiler='cl' --glibc='msvcrt140',
]
@jayconrod
Copy link
Contributor Author

Cgo determines the types of symbols by attempting to compile a specially crafted C file, then checking whether errors occur in specific places. It's possible it's expecting the compiler to behave a certain way, but a different compiler is actually being used?

Could you tell me a bit more about your environment? OS, compiler version, Bazel version? What is the command that produces the top error message? Does anything different happen if the environment variable CC=/usr/bin/gcc is set?

@nathantchan
Copy link
Contributor

OS: debian jessie
compiler version:
Using built-in specs.
COLLECT_GCC=/usr/bin/x86_64-linux-gnu-gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.9/lto-wrapper
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian 4.9.2-10+deb8u1' --with-bugurl=file:///usr/share/doc/gcc-4.9/README.Bugs --enable-languages=c,c++,java,go,d,fortran,objc,obj-c++ --prefix=/usr --program-suffix=-4.9 --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.9 --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --enable-gnu-unique-object --disable-vtable-verify --enable-plugin --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk --enable-gtk-cairo --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64/jre --enable-java-home --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64 --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-4.9-amd64 --with-arch-directory=amd64 --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc --enable-multiarch --with-arch-32=i586 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 4.9.2 (Debian 4.9.2-10+deb8u1)
bazel version: 0.9.0

Here's the exact subcommand being run:
'export BAZEL_OUTPUT_ROOT="$(pwd)/" GOROOT="$(pwd)/bazel-out/k8-fastbuild/bin/external/go_stdlib_linux_amd64_cgo" GOROOT_FINAL="GOROOT" GOOS="linux" GOARCH="amd64" CGO_ENABLED="1" CC="$(pwd)/tools/cpp/debian_amd64_compilers/x86_64-linux-gnu-gcc" CXX="$(pwd)/tools/cpp/debian_amd64_compilers/x86_64-linux-gnu-gcc" COMPILER_PATH="tools/cpp/debian_amd64_compilers" CGO_CPPFLAGS="-nostdinc -isystem $BAZEL_OUTPUT_ROOT/external/amd64_compilers_repo/usr/lib/gcc/x86_64-linux-gnu/4.9/include -isystem $BAZEL_OUTPUT_ROOT/external/amd64_compilers_repo/usr/local/include -isystem $BAZEL_OUTPUT_ROOT/external/amd64_compilers_repo/usr/lib/gcc/x86_64-linux-gnu/4.9/include-fixed -isystem $BAZEL_OUTPUT_ROOT/external/amd64_compilers_repo/usr/include/x86_64-linux-gnu -isystem $BAZEL_OUTPUT_ROOT/external/amd64_compilers_repo/usr/include -L$BAZEL_OUTPUT_ROOT/external/amd64_compilers_repo/lib/x86_64-linux-gnu -fmax-errors=20 -pipe -ggdb3 -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 -fstack-protector -fPIE -fdiagnostics-color -fno-omit-frame-pointer -fno-strict-aliasing" CGO_LDFLAGS="-pipe -lstdc++ -B$BAZEL_OUTPUT_ROOT/external/amd64_compilers_repo/usr/bin -Wl,-z,relro,-z,now -no-canonical-prefixes -pass-exit-codes -Wl,--build-id=md5 -Wl,--hash-style=gnu -fuse-ld=gold -Wl,--warn-execstack -Wl,--detect-odr-violations" && export PATH=$PATH:$(cd "$COMPILER_PATH" && pwd) && mkdir -p bazel-out/k8-fastbuild/bin/external/go_stdlib_linux_amd64_cgo/src && mkdir -p bazel-out/k8-fastbuild/bin/external/go_stdlib_linux_amd64_cgo/pkg && cp -rf external/go_sdk/src/* bazel-out/k8-fastbuild/bin/external/go_stdlib_linux_amd64_cgo/src/ && cp -rf external/go_sdk/pkg/tool bazel-out/k8-fastbuild/bin/external/go_stdlib_linux_amd64_cgo/pkg/ && cp -rf external/go_sdk/pkg/include bazel-out/k8-fastbuild/bin/external/go_stdlib_linux_amd64_cgo/pkg/ && external/go_sdk/bin/go install -asmflags "-trimpath $(pwd)" std && external/go_sdk/bin/go install -asmflags "-trimpath $(pwd)" runtime/cgo'

The environment variable BAZEL_OUTPUT_ROOT is set to the execution root of the sandbox. CC is currently pointing to a shell script that converts the full paths to relative paths. Within that shell script if I change the compiler to point to /usr/bin/gcc instead, I get a similar set of errors.

If I don't try to set a full path for all of the -isystem options, then the install stops at an earlier stage where it cannot find the basic header files like stddef.h

@nathantchan
Copy link
Contributor

Actually if I remove the environment variables COMPILER_PATH and CGO_CPP_FLAGS, everything now succeeds. This is true is I set CC to either /usr/bin/gcc, or the sandboxed compiler we have in external/amd64_compilers. Maybe it's one of the flags that is throwing things off?

@jayconrod
Copy link
Contributor Author

Thanks for the info. I'll try and reproduce this, but I probably won't be able to get to it until tomorrow or Friday.

Something to try before then: what happens if you remove the -fdiagnostics-color flag from CGO_CPPFLAGS? We have blacklisted -fcolor-diagnostics because cgo doesn't understand color codes emitted by the compiler. Maybe this is a different name for the same thing.

Do you see the same error using a newer version of rules_go? We rewrote the stdlib builder where the error is happening, I think after 0.9.0. Might fix this.

Also, just to confirm: are you using the Bazel auto-configuration (detects compiler in PATH or CC) or do you have a custom CROSSTOOL file?

@nathantchan
Copy link
Contributor

Removing the -fdiagnostics-color flag does not solve the problem.

I just tried upgrading to a 0.10.1. It's now failing out at an earlier stage because it cannot find the compiler we placed in external/amd64_compilers_repo/usr/bin/x86_64-linux-gnu-gcc.

We are using a custom CROSSTOOL file.

@jayconrod
Copy link
Contributor Author

Ok, let's proceed with 0.10.1 if possible.

I'm only vaguely familiar with custom CROSSTOOL files. I'll help as much as I can, but I'm coming up to speed on this. :)

Could you paste the error you're getting about the compiler being missing? I'm wondering if this is a Bazel error (some dependency missing) or an execution error (bad path on the command line or something).

If you're able to get to the part where it builds the stdlib, could you paste the command line? I'm interested in both the command line for stdlib builder (Bazel will print this with the -s flag) and the command line the stdlib builder passes to go install. There's no simple way to get this. You'll need to check out a local copy of rules_go, apply the patch below, then build with --override_repository=io_bazel_rules_go=/path/to/local/copy.

diff --git a/go/tools/builders/stdlib.go b/go/tools/builders/stdlib.go
index 4f9264e..86126a1 100644
--- a/go/tools/builders/stdlib.go
+++ b/go/tools/builders/stdlib.go
@@ -21,6 +21,7 @@ import (
 	"log"
 	"os"
 	"os/exec"
+	"strings"
 )
 
 func install_stdlib(goenv *GoEnv, target string, args []string) error {
@@ -32,6 +33,7 @@ func install_stdlib(goenv *GoEnv, target string, args []string) error {
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
 	cmd.Env = goenv.Env()
+	fmt.Fprintf(os.Stderr, ">> stdlib:\n%s \\\n%s %s\n", strings.Join(cmd.Env, " \\\n"), cmd.Path, strings.Join(cmd.Args, " "))
 	if err := cmd.Run(); err != nil {
 		return fmt.Errorf("error running go install %s: %v", target, err)
 	}

@nathantchan
Copy link
Contributor

nathantchan commented Mar 1, 2018

I suspect it's a bad path. Here's the error I'm currently blocked on:

ERROR: /home/nathan/.cache/bazel/_bazel_nathan/8a7aba3b76b230775497a5ea4d4123bb/external/io_bazel_rules_go/BUILD.bazel:9:1: GoStdlib external/io_bazel_rules_go/linux_amd64/stdlib~/pkg failed (Exit 1)
>> stdlib:
BAZEL_OUTPUT_ROOT=$(pwd)/ \
TMPDIR=/dev/shm/bazel-sandbox.d71478fcc543f4fd657affe6afd3ad99/8504803830990673641/execroot/com_peloton_tech/tmp \
GOROOT=/dev/shm/bazel-sandbox.d71478fcc543f4fd657affe6afd3ad99/8504803830990673641/execroot/com_peloton_tech/bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64/stdlib~ \
GOROOT_FINAL=GOROOT \
GOOS=linux \
GOARCH=amd64 \
CGO_ENABLED=1 \
PATH=/dev/shm/bazel-sandbox.d71478fcc543f4fd657affe6afd3ad99/8504803830990673641/execroot/com_peloton_tech/tools/cpp/debian_amd64_compilers \
COMPILER_PATH=/dev/shm/bazel-sandbox.d71478fcc543f4fd657affe6afd3ad99/8504803830990673641/execroot/com_peloton_tech/tools/cpp/debian_amd64_compilers \
CC=/dev/shm/bazel-sandbox.d71478fcc543f4fd657affe6afd3ad99/8504803830990673641/execroot/com_peloton_tech/tools/cpp/debian_amd64_compilers/x86_64-linux-gnu-gcc \
CXX=/dev/shm/bazel-sandbox.d71478fcc543f4fd657affe6afd3ad99/8504803830990673641/execroot/com_peloton_tech/tools/cpp/debian_amd64_compilers/x86_64-linux-gnu-gcc \
CGO_CPPFLAGS=-nostdinc -isystem external/amd64_compilers_repo/usr/lib/gcc/x86_64-linux-gnu/4.9/include -isystem external/amd64_compilers_repo/usr/local/include -isystem external/amd64_compilers_repo/usr/lib/gcc/x86_64-linux-gnu/4.9/include-fixed -isystem external/amd64_compilers_repo/usr/include/x86_64-linux-gnu -isystem external/amd64_compilers_repo/usr/include -Lexternal/amd64_compilers_repo/lib/x86_64-linux-gnu -Wextra -fmax-errors=20 -pipe -ggdb3 -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 -fstack-protector -fPIE -fdiagnostics-color -fno-omit-frame-pointer -fno-strict-aliasing \
CGO_LDFLAGS=-pipe -lstdc++ -Bexternal/amd64_compilers_repo/usr/bin -pie -Wl,-z,relro,-z,now -no-canonical-prefixes -pass-exit-codes -Wl,--build-id=md5 -Wl,--hash-style=gnu -fuse-ld=gold -Wl,--warn-execstack -Wl,--detect-odr-violations \
external/go_sdk/bin/go external/go_sdk/bin/go install -asmflags -trimpath /dev/shm/bazel-sandbox.d71478fcc543f4fd657affe6afd3ad99/8504803830990673641/execroot/com_peloton_tech std
# runtime/cgo
/dev/shm/bazel-sandbox.d71478fcc543f4fd657affe6afd3ad99/8504803830990673641/execroot/com_peloton_tech/tools/cpp/debian_amd64_compilers/x86_64-linux-gnu-gcc: line 56: $(pwd)/external/amd64_compilers_repo/usr/bin/x86_64-linux-gnu-gcc: No such file or directory

Do you know where the commands are being executed?

@nathantchan
Copy link
Contributor

Forgot to paste in the subcommand:

SUBCOMMAND: # @io_bazel_rules_go//:stdlib [action 'GoStdlib external/io_bazel_rules_go/linux_amd64/stdlib~/pkg']
(cd /home/nathan/.cache/bazel/_bazel_nathan/8a7aba3b76b230775497a5ea4d4123bb/execroot/com_peloton_tech &&
exec env -
BAZEL_OUTPUT_ROOT='$(pwd)/'
bazel-out/host/bin/external/io_bazel_rules_go/go/tools/builders/linux_amd64_stripped/stdlib -go external/go_sdk/bin/go -root_file external/go_sdk/packages.txt -goos linux -goarch amd64 '-cgo=1' -compiler_path tools/cpp/debian_amd64_compilers/../../../ -cc tools/cpp/debian_amd64_compilers/x86_64-linux-gnu-gcc -cpp_flag -nostdinc -cpp_flag -isystem -cpp_flag external/amd64_compilers_repo/usr/lib/gcc/x86_64-linux-gnu/4.9/include -cpp_flag -isystem -cpp_flag external/amd64_compilers_repo/usr/local/include -cpp_flag -isystem -cpp_flag external/amd64_compilers_repo/usr/lib/gcc/x86_64-linux-gnu/4.9/include-fixed -cpp_flag -isystem -cpp_flag external/amd64_compilers_repo/usr/include/x86_64-linux-gnu -cpp_flag -isystem -cpp_flag external/amd64_compilers_repo/usr/include -cpp_flag -Lexternal/amd64_compilers_repo/lib/x86_64-linux-gnu -cpp_flag -Wextra -cpp_flag '-fmax-errors=20' -cpp_flag -pipe -cpp_flag -ggdb3 -cpp_flag -pthread -cpp_flag -U_FORTIFY_SOURCE -cpp_flag '-D_FORTIFY_SOURCE=1' -cpp_flag -fstack-protector -cpp_flag -fPIE -cpp_flag -fdiagnostics-color -cpp_flag -fno-omit-frame-pointer -cpp_flag -fno-strict-aliasing -ld_flag -pipe -ld_flag -lstdc++ -ld_flag -Bexternal/amd64_compilers_repo/usr/bin -ld_flag -pie -ld_flag -Wl,-z,relro,-z,now -ld_flag -no-canonical-prefixes -ld_flag -pass-exit-codes -ld_flag '-Wl,--build-id=md5' -ld_flag '-Wl,--hash-style=gnu' -ld_flag '-fuse-ld=gold' -ld_flag -Wl,--warn-execstack -ld_flag -Wl,--detect-odr-violations -out 'bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64/stdlib~')

@jayconrod
Copy link
Contributor Author

The commands will be executed in the execroot. Bazel prints it before each command. In this case,

/home/nathan/.cache/bazel/_bazel_nathan/8a7aba3b76b230775497a5ea4d4123bb/execroot/com_peloton_tech

I don't see anything odd, other than the relative paths for the includes. They might not be in the sandbox. Does this work if you build with --spawn_strategy=standalone (disabling the sandbox)?

The way I usually debug is to disable sandboxing, then execute commands manually in the execroot until I find something that works. Maybe try removing some of the flags and see what affects the build?

@AustinSchuh
Copy link
Contributor

I'm going to put together a reproduction repo to send you. I'm collecting up a fully hermetic compiler that I can share. I'm hoping doing that will make it easier to reproduce issues. I'm assuming if I were to get you a repo for Linux, that would be helpful.

@AustinSchuh
Copy link
Contributor

AustinSchuh commented Mar 14, 2018

@jayconrod , https://github.com/AustinSchuh/golang_demo is a working demo. It's setup for Debian Jessie (Debian 8) from our robotics repo (4 target architectures). If you edit tools/bazel.rc and remove the crosstool_top flags, everything works. If you add them back, you get cgo errors. I included a trivial C++ application as an example to show that the CROSSTOOL file works.

@nathantchan @bsilver8192 and I have some modifications we've had to make to get rules_go working with a similar crosstool setup. It would be nice to figure out how to get this setup to work out of the box to make future upgrades less painful. Once this works, we can figure out how to get a repo which has both GCC and Clang in it to show you that as well.

As a side note, that should be a fully reproducible and hermetic compiler. I did some basic md5sum tests on that repo.

@jayconrod
Copy link
Contributor Author

@AustinSchuh Thanks for posting that! I was able to reproduce the failure.

/usr/local/google/home/jayconrod/.cache/bazel/_bazel_jayconrod/abc4891756130f6096412cdbce30b66a/execroot/__main__/tools/cpp/clang_3p6/x86_64-linux-gnu-clang-3.6: line 13: bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/src/runtime/cgo/external/clang_3p6_repo/usr/bin/clang-3.6: No such file or directory

The script tools/cpp/clang_3p6/x86_64-linux-gnu-clang-3.6 sets LD_LIBRARY_PATH and execs the compiler. However, it expands ${BAZEL_OUTPUT_ROOT}, which is not normally set for these actions. Maybe it's set for C/C++ compile actions, but not for actions created by Skylark rules. So that script needs to be rewritten or generated in a way that would give it the complete path. I bypassed this by hardcoding the absolute path.

The next error I ran into was a missing <stddef.h>. It looks like --sysroot and most other include-related flags passed through CGO_CPPFLAGS (and other variables) use relative paths. go install (which we use to build the stdlib) may invoke the C compiler in different directories. It generally creates a temporary directory for each package it builds and invokes the C compiler there. Each directory passed to the C compiler should really be an absolute path. Is it possible to rewrite your CROSSTOOL so that all includes are absolute paths? If not, some wrapper might be needed to process the flags.

@AustinSchuh
Copy link
Contributor

@jayconrod Wohoo! Progress.

The challenge here is that if we convert anything to absolute paths in the clang/gcc invocation, they get baked into all our binaries. If we have a generated shell script with an absolute path to the compiler, that won't work with remote-ex. Bazel propper works really hard to not expose absolute paths to the action. The only way to be reproducible is to do relative paths. The toolchain lives at a relative path (or a path findable with $(location ...)). Locally, we've added "BAZEL_OUTPUT_ROOT": "$(pwd)/", to the environment. Is there a way to get a similar mechanism upstreamed? Or do you have another suggestion on how to detect this?

@jayconrod
Copy link
Contributor Author

@mhlopko Any tips for writing a CROSSTOOL for a compiler in the workspace that avoids relative paths while remaining reproducible? We're looking at this CROSSTOOL, specifically.

One idea (maybe not the best idea): if you have a wrapper script around the compiler, you could find the location of the wrapper script itself and then use that to transform paths relative to the workspace root.

@hlopko
Copy link
Member

hlopko commented Mar 15, 2018

I agree with Austin, we try hard not to produce absolute paths. Compilers are funny and when they see absolute paths they use them everywhere in output artifacts (be it binaries, libraries, or small things like .d files) and spoil the remote caches with host specific entries. There are things that some compilers stubbornly use absolute paths for (e.g. debug info), but we try.

Without knowing much about go use case, I'd expect that you will also want to get rid of all absolute paths. Is it an option?

@jayconrod
Copy link
Contributor Author

@mhlopko The Go compiler is a little more well-behaved w.r.t. absolute paths. It accepts some options like -trimpath that remove non-reproducible parts of the path from the final binary.

In this case though, go install (part of the Go toolchain that rules_go doesn't control) is invoking the C compiler in a temporary directory where the relative include paths are meaningless.

Does it make sense to wrap the C compiler with a script that transforms relative paths? Is there a better solution that has worked elsewhere?

@hlopko
Copy link
Member

hlopko commented Mar 15, 2018

Hmm tough. Then yes, you could use wrapper scripts, just note that they are quite costly on windows (TF build faster by ~30% when not using them). But ideally you would use the same crosstool in go that the rest of bazel world uses, and that can mean relative paths... Any cool idea how to fix the go install side?

@jayconrod
Copy link
Contributor Author

We have a wrapper Go program that we invoke go install with. That passes in the C compiler and flags through environment variables. We could potentially modify those in transit.

My concern is that we will either miss some relative paths that we should fix (different compilers have different ways of saying --sysroot, -iquote, etc.) or that we would "fix" some paths that we shouldn't (paths passed with -D or -Wl,-rpath should be as-is).

What causes wrapper scripts to be slow on Windows? Are regular executables much faster, or is it the extra exec that's slow?

@hlopko
Copy link
Member

hlopko commented Mar 15, 2018

@meteorcloudy knows the details of why wrapper scripts were slow on Windows, I always assumed it's the exec that's costly.

How do you get C compiler and flags currently? From CppConfiguration?

@meteorcloudy
Copy link
Member

IIRC, wrapper scripts are slow because creating new processes are expensive on Windows. @laszlocsomor can confirm that or correct me. :)

@laszlocsomor
Copy link
Contributor

because creating new processes are expensive on Windows

Yes, but not as slow as I used to tell everyone. Mea culpa.

I measured very slow process creation times (hundreds of milliseconds to several seconds for a single CreateProcess call) because -- I believe -- Bit9/Carbon Black was scanning, with each CreateProcess, the binaries I tried to run.

I measured process creation speed on our Windows VMs that we use in the Bazel CI, and process creation there is a lot faster, something like 5 to 20ms.

@laszlocsomor
Copy link
Contributor

...which is still orders of magnitudes slower than it is on Linux, but it is what it is.

@jayconrod
Copy link
Contributor Author

@mhlopko We currently get the C compiler and flags from the cpp fragment. This happens in an internal rule, go_context_data. That information gets propagated to other rules that need to invoke the C compiler indirectly.

@laszlocsomor 5-20ms sounds tolerable, though we should still seek to minimize process creation. I think anyone running Bazel (or any build tool for that matter) with a heavy antivirus in the background is going to have a long coffee break though.

@AustinSchuh
Copy link
Contributor

@jayconrod A wrapper script is going to be required. It's a matter of what it will look at this point from my experience.

Also, you will very likely find with this crosstool that there are flags hidden today that you can't get access to through the cpp fragment. We've had to patch bazel to expose them all :(. I'd love to somehow get this CROSSTOOL (or something with similar properties) in the automated tests for golang. If it needs to be dual licensed or something else to do that, please let me know and I'd be thrilled to help sort that out.

My rather ugly wrapper script which made this all work in an older version of rules_go (it's broken today...)

#!/bin/bash --norc

export LD_LIBRARY_PATH="${BAZEL_OUTPUT_ROOT}external/clang_3_7_repo/lib"

strip_absolute_path() {
  local file="$1"
  if [[ -e "$file" ]]; then
    sed -i 's/\/dev\/shm\/bazel-sandbox.*com_peloton_tech/./' "$file"
  fi
}

# Detect if we are running for go library compiles or as a host compiler.
if [[ -n "${BAZEL_OUTPUT_ROOT}" ]]; then
  # If we are doing go builds, we need to doctor up the args to get
  # reproducible builds.

  # Here's where we need to remove absolute paths.  golang doesn't worry about
  # them, so we get to do it ourselves.  Start by computing the relative path
  # from ./ to the bazel output folder.  This keeps changing so we need to
  # compute it dynamically.
  REL_BAZEL_OUTPUT_ROOT="$(realpath --relative-to="$(pwd)" \
                           "${BAZEL_OUTPUT_ROOT}")"

  # Now, replace strings out of the args array to remove absolute paths.
  ARGS_ARRAY=("$@")
  # We use \$BAZEL_OUTPUT_ROOT to signal to this phase of the compilation to
  # do a relative path replacement.  Do it.
  CONV_ARGS=("${ARGS_ARRAY[@]/\$BAZEL_OUTPUT_ROOT/${REL_BAZEL_OUTPUT_ROOT}}")
  # Find all refernces to the full path of ./ and replace them with ./
  CONV_ARGS=("${CONV_ARGS[@]/$(pwd)/.\/}")
  # Find any references to BAZEL_OUTPUT_ROOT and replace them too.
  CONV_ARGS=("${CONV_ARGS[@]/${BAZEL_OUTPUT_ROOT}/${REL_BAZEL_OUTPUT_ROOT}\/}")

  # Sigh, cgo is baking in absolute paths.  Rip them out.
  strip_absolute_path cgo_lookup_unix.cgo2.c
  strip_absolute_path getgrouplist_unix.cgo2.c
  strip_absolute_path plugin_dlopen.cgo2.c

  # PWD isn't getting set this far down, so add it back.
  export PWD="/proc/self/cwd"
  exec "${BAZEL_OUTPUT_ROOT}external/clang_3_7_repo/bin/clang" "${CONV_ARGS[@]}"
else
  exec "${BAZEL_OUTPUT_ROOT}external/clang_3_7_repo/bin/clang" "$@"
fi

ie, pass in an environment variable signaling where the root of the repo is, and do all the calculation to convert the flags to ones relative to the current directory from the repo root. It does a lot of path mangling to things like --sysroot to make that work.

Another thought would be to generate the wrapper script (instead of passing the args in that way) so the wrapper script can have all the flags in it. I'm pondering if the wrapper script could start with a cd such that it ends up back at the repo root, but that'll then end up with a path including the temporary golang directory needed for the go install in the final binary. I've got no stellar ideas here.

(The patch above is then paired with something like the following to mangle the input.)

--- a/go/private/rules/stdlib.bzl
+++ b/go/private/rules/stdlib.bzl
@@ -49,6 +49,39 @@ def _stdlib_impl(ctx):
       cpp.unfiltered_compiler_options(features) +
       cpp.link_options +
       cpp.mostly_static_link_options(features, False))
+
+  # compiler and linker flags need to be passed through to go install command
+  # as environment variables.
+  compiler_options = cpp.compiler_options(features)
+  prefixed_compiler_opts = [s.replace("external", "\\$BAZEL_OUTPUT_ROOT/external") for s in compiler_options]
+  prefixed_compiler_opts = [s.replace("-Lexternal", "-L\\$BAZEL_OUTPUT_ROOT/external") for s in prefixed_compiler_opts]
+  prefixed_compiler_opts = [s.replace("--sysroot=external", "--sysroot=\\$BAZEL_OUTPUT_ROOT/external") for s in prefixed_compiler_opts]
+  prefixed_compiler_opts = [s.replace("-Bexternal", "-B\\$BAZEL_OUTPUT_ROOT/external") for s in prefixed_compiler_opts]
+  cleaned_compiler_opts = []
+  for s in prefixed_compiler_opts:
+    # Remove flags that are not needed
+    if s == "-Wall" or s == "-Werror" or s == "-Wextra" or s == "-fcolor-diagnostics":
+       continue
+    else:
+      cleaned_compiler_opts.append(s)
+
+  # need double quotes so $(pwd) resolves
+  compiler_options_string = "\"" + " ".join(cleaned_compiler_opts) + "\""
+
+  linker_options = cpp.mostly_static_link_options(features, False)
+  linker_prefixed_opts= [s.replace("external", "\\$BAZEL_OUTPUT_ROOT/external") for s in linker_options]
+  linker_prefixed_opts= [s.replace("-Lexternal", "-L\\$BAZEL_OUTPUT_ROOT/external") for s in linker_prefixed_opts]
+  linker_prefixed_opts= [s.replace("-Bexternal", "-B\\$BAZEL_OUTPUT_ROOT/external") for s in linker_prefixed_opts]
+  linker_prefixed_opts= [s.replace("--sysroot=external", "--sysroot=\\$BAZEL_OUTPUT_ROOT/external") for s in linker_prefixed_opts]
+  linker_prefixed_opts= [s.replace("-Ltools", "-L\\$BAZEL_OUTPUT_ROOT/tools") for s in linker_prefixed_opts]
+  cleaned_linker_opts = []
+  for s in linker_prefixed_opts:
+    if s == "-pie" or s == "-Wl,--gc-sections":
+      continue
+    else:
+      cleaned_linker_opts.append(s)
+  linker_options_string= "\"" + " ".join(cleaned_linker_opts) + "\""
+
   linker_path, _ = cpp.ld_executable.rsplit("/", 1)
   ctx.actions.write(root_file, "")
   cc_path = cpp.compiler_executable

@jayconrod
Copy link
Contributor Author

Also, you will very likely find with this crosstool that there are flags hidden today that you can't get access to through the cpp fragment. We've had to patch bazel to expose them all :(

Hopefully that will be sorted out with the ongoing work to expose more C++ stuff in the Skylark API.

I'd love to somehow get this CROSSTOOL (or something with similar properties) in the automated tests for golang. If it needs to be dual licensed or something else to do that, please let me know and I'd be thrilled to help sort that out.

I'd be happy to have this be an integration test. It will be a little tricky to set up, but I think it would fit into our integration test framework. It would also be useful to point to this as an example. There are people who want custom CROSSTOOLs for cross-compilation, especially.

@laszlocsomor
Copy link
Contributor

@jayconrod : I ran the test again to get up-to-date numbers. See commit message of laszlocsomor/projects@bb6a2d9 .

@hlopko
Copy link
Member

hlopko commented Mar 16, 2018

Re hidden flags: Yes, that should be fixed by bazelbuild/bazel#4571.

@jayconrod
Copy link
Contributor Author

@laszlocsomor Wow. I know antivirus really slows things down, but I'm always surprised by how much. @ianthehat mentioned a while back that it took him ~5 minutes to build a "hello world" Go program on Windows (nearly all of that time is spent building the standard library). I guess that's why.

@jayconrod
Copy link
Contributor Author

@AustinSchuh @nathantchan

I've been reviewing #1365. As part of that PR, @steeve added some code to all the builders that absolutized include paths. He was able to build with a custom CROSSTOOL that used relative paths after that without needed wrapper scripts. That solved the problem you are seeing. See env.go:83.

However, this caused reproducibility problems, and it caused tests to fail on Linux when sandboxing was enabled. cgo embeds the contents of CGO_LDFLAGS in generated .go files as pragmas. Those flags are preserved by the compiler in .a files, and the linker uses them when invoking the external linker. This means CGO_LDFLAGS must not contain absolute paths to any Bazel artifact.

This makes it more difficult to build stdlib with a custom CROSSTOOL, but maybe not impossible. I think absolute paths in CGO_CFLAGS, CGO_CPPFLAGS, and CGO_CXXFLAGS may be okay; they don't seem to be saved. We may be able to build the stdlib without setting CGO_LDFLAGS.

@steeve
Copy link
Contributor

steeve commented Mar 16, 2018

The main issue is that go install cds in to a custom $WORK directory and issues its commands from there, instead of doing it from the bazel sandbox, which is ruining everything. Actually, it might be better to have go install work from the bazel sandbox?

@jayconrod
Copy link
Contributor Author

If go install could do its work without changing directories, that would solve everything. I don't think that's going to change though.

Ideally, we could build the standard library ourselves with normal compile / assemble / link actions. Converting that is a lot of work though, and I don't think it's feasible right now.

@AustinSchuh
Copy link
Contributor

@jayconrod @steeve , I think reproducibility is actually the easier part of the problem to solve. The trick is to use a compiler wrapper shell script to convert the paths. Pass in a "root of repo" magic string, and convert that to a relative path in the wrapper script. And/or convert any absolute paths to relative paths in that script. We did that in our main compiler wrapper script previously, but that could be a secondary wrapper script for just golang (which would be cleaner).

REL_BAZEL_OUTPUT_ROOT="$(realpath --relative-to="$(pwd)" "${BAZEL_OUTPUT_ROOT}")"

That lets you compute a relative path from the current directory to the root of the repo. Then, you can use bash to replace anything absolute (or your magic token) with that relative path. The following bash can do that. You'll need a couple of similar bash lines to convert the args as the following

CONV_ARGS=("${CONV_ARGS[@]/${BAZEL_OUTPUT_ROOT}/${REL_BAZEL_OUTPUT_ROOT}\/}")

@steeve
Copy link
Contributor

steeve commented Mar 18, 2018

I did some investigation real quick.

Doing two consecutive runs in a Docker container and comparing the hashes gave me a list of affected packages, I think those are CGo packages.

bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/crypto/sha512.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/net.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/vendor/golang_org/x/crypto/poly1305.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/crypto/sha512.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/vendor/golang_org/x/crypto/poly1305.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/os/user.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/sync/atomic.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/vendor/golang_org/x/crypto/chacha20poly130
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/plugin.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/runtime/internal/atomic.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/crypto/internal/cipherhw.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/os/signal.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/crypto/sha256.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/crypto/aes.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/crypto/internal/cipherhw.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/crypto/sha1.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/runtime/debug.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/runtime/cgo.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/crypto/aes.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/crypto/rc4.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/os/signal.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/os/user.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/math/big.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/vendor/golang_org/x/crypto/chacha20poly1305.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/math/big.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/strings.a
bazel-out/k8-fastbuild/bin/tests/core/c_linkmodes/linux_amd64_stripped_c-archive/adder_archive.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/math.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/crypto/sha256.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/reflect.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/internal/cpu.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/crypto/md5.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/crypto/elliptic.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/crypto/rc4.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/runtime/debug.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/net.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/syscall.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/crypto/sha1.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/syscall.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/runtime/internal/atomic.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/plugin.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/os/signal/internal/pty.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/runtime.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/runtime/cgo.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/sync/atomic.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/strings.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/hash/crc32.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/crypto/elliptic.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/vendor/golang_org/x/crypto/curve25519.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/hash/crc32.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/os/signal/internal/pty.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/runtime.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/math.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/vendor/golang_org/x/crypto/curve25519.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/internal/cpu.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/reflect.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/crypto/md5.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/math.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/vendor/golang_org/x/crypto/poly1305.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/net.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/crypto/sha1.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/crypto/elliptic.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/sync/atomic.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/crypto/internal/cipherhw.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/crypto/elliptic.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/runtime/cgo.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/os/user.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/syscall.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/vendor/golang_org/x/crypto/poly1305.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/math/big.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/os/signal/internal/pty.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/runtime/cgo.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/runtime.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/runtime/debug.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/reflect.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/crypto/md5.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/math.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/runtime/internal/atomic.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/vendor/golang_org/x/crypto/chacha20poly1305.a
bazel-out/k8-fastbuild/bin/tests/core/c_linkmodes/linux_amd64_stripped_c-archive/adder_archive.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/os/signal.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/internal/cpu.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/strings.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/crypto/sha512.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/vendor/golang_org/x/crypto/curve25519.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/sync/atomic.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/crypto/sha256.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/crypto/sha512.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/plugin.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/crypto/rc4.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/syscall.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/runtime/internal/atomic.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/hash/crc32.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/crypto/md5.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/net.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/runtime/debug.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/hash/crc32.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/crypto/aes.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/internal/cpu.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/vendor/golang_org/x/crypto/curve25519.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/crypto/sha256.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/crypto/sha1.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/math/big.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/crypto/internal/cipherhw.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/runtime.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/vendor/golang_org/x/crypto/chacha20poly130
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/os/user.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/crypto/rc4.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/reflect.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/strings.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/plugin.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped_c-archive/stdlib~/pkg/linux_amd64/crypto/aes.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/os/signal.a
bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib~/pkg/linux_amd64/os/signal/internal/pty.a

@steeve
Copy link
Contributor

steeve commented Mar 18, 2018

Looking at signal.a, here is the diff between two runs:

8,9c8,9
< 00000070  62 75 69 6c 64 20 69 64  20 22 49 69 78 62 6e 54  |build id "IixbnT|
< 00000080  46 35 61 53 30 4e 61 59  50 45 49 53 78 73 2f 73  |F5aS0NaYPEISxs/s|
---
> 00000070  62 75 69 6c 64 20 69 64  20 22 58 59 7a 63 77 6e  |build id "XYzcwn|
> 00000080  63 4e 4f 4b 4e 59 7a 35  57 53 44 70 43 44 2f 73  |cNOKNYz5WSDpCD/s|
12,13c12,13
< 000000b0  20 69 64 20 22 49 69 78  62 6e 54 46 35 61 53 30  | id "IixbnTF5aS0|
< 000000c0  4e 61 59 50 45 49 53 78  73 2f 73 71 38 38 4e 51  |NaYPEISxs/sq88NQ|
---
> 000000b0  20 69 64 20 22 58 59 7a  63 77 6e 63 4e 4f 4b 4e  | id "XYzcwncNOKN|
> 000000c0  59 7a 35 57 53 44 70 43  44 2f 73 71 38 38 4e 51  |Yz5WSDpCD/sq88NQ|
37,38c37,38
< 00000240  49 69 78 62 6e 54 46 35  61 53 30 4e 61 59 50 45  |IixbnTF5aS0NaYPE|
< 00000250  49 53 78 73 2f 73 71 38  38 4e 51 53 47 58 6a 6b  |ISxs/sq88NQSGXjk|
---
> 00000240  58 59 7a 63 77 6e 63 4e  4f 4b 4e 59 7a 35 57 53  |XYzcwncNOKNYz5WS|
> 00000250  44 70 43 44 2f 73 71 38  38 4e 51 53 47 58 6a 6b  |DpCD/sq88NQSGXjk|

It seems this is because of build id.

I think the key is in go tool compile's -buildid flag maybe ?

@jayconrod
Copy link
Contributor Author

I've filed #1391 to track -buildid. I don't know how it's determined yet; needs investigation.

Are you seeing any differences due to absolute paths? Any difference near a "sandbox" string would be interesting.

@jayconrod
Copy link
Contributor Author

@AustinSchuh Do you need any changes in the stdlib builder to make that work?

Ideally, rules_go would provide these wrappers, since they would be useful to anyone with a custom CROSSTOOL that contains relative paths. I don't think I'll have bandwidth to implement that soon though.

@AustinSchuh
Copy link
Contributor

AustinSchuh commented Mar 19, 2018

@jayconrod , Here are the changes we've made to support that.

diff --git a/go/private/rules/stdlib.bzl b/go/private/rules/stdlib.bzl
index 6881559..b3c7a46 100644
--- a/go/private/rules/stdlib.bzl
+++ b/go/private/rules/stdlib.bzl
@@ -49,6 +49,39 @@ def _stdlib_impl(ctx):
       cpp.unfiltered_compiler_options(features) +
       cpp.link_options +
       cpp.mostly_static_link_options(features, False))
+
+  # compiler and linker flags need to be passed through to go install command
+  # as environment variables.
+  compiler_options = cpp.compiler_options(features)
+  prefixed_compiler_opts = [s.replace("external", "\\$BAZEL_OUTPUT_ROOT/external") for s in compiler_options]
+  prefixed_compiler_opts = [s.replace("-Lexternal", "-L\\$BAZEL_OUTPUT_ROOT/external") for s in prefixed_compiler_opts]
+  prefixed_compiler_opts = [s.replace("--sysroot=external", "--sysroot=\\$BAZEL_OUTPUT_ROOT/external") for s in prefixed_compiler_opts]
+  prefixed_compiler_opts = [s.replace("-Bexternal", "-B\\$BAZEL_OUTPUT_ROOT/external") for s in prefixed_compiler_opts]
+  cleaned_compiler_opts = []
+  for s in prefixed_compiler_opts:
+    # Remove flags that are not needed
+    if s == "-Wall" or s == "-Werror" or s == "-Wextra" or s == "-fcolor-diagnostics":
+       continue
+    else:
+      cleaned_compiler_opts.append(s)
+
+  # need double quotes so $(pwd) resolves
+  compiler_options_string = "\"" + " ".join(cleaned_compiler_opts) + "\""
+
+  linker_options = cpp.mostly_static_link_options(features, False)
+  linker_prefixed_opts= [s.replace("external", "\\$BAZEL_OUTPUT_ROOT/external") for s in linker_options]
+  linker_prefixed_opts= [s.replace("-Lexternal", "-L\\$BAZEL_OUTPUT_ROOT/external") for s in linker_prefixed_opts]
+  linker_prefixed_opts= [s.replace("-Bexternal", "-B\\$BAZEL_OUTPUT_ROOT/external") for s in linker_prefixed_opts]
+  linker_prefixed_opts= [s.replace("--sysroot=external", "--sysroot=\\$BAZEL_OUTPUT_ROOT/external") for s in linker_prefixed_opts]
+  linker_prefixed_opts= [s.replace("-Ltools", "-L\\$BAZEL_OUTPUT_ROOT/tools") for s in linker_prefixed_opts]
+  cleaned_linker_opts = []
+  for s in linker_prefixed_opts:
+    if s == "-pie" or s == "-Wl,--gc-sections":
+      continue
+    else:
+      cleaned_linker_opts.append(s)
+  linker_options_string= "\"" + " ".join(cleaned_linker_opts) + "\""
+
   linker_path, _ = cpp.ld_executable.rsplit("/", 1)
   ctx.actions.write(root_file, "")
   cc_path = cpp.compiler_executable
@@ -56,12 +89,16 @@ def _stdlib_impl(ctx):
     cc_path = "$(pwd)/" + cc_path
   env = {
       "GOROOT": "$(pwd)/{}".format(goroot),
+      "GOROOT_FINAL": "GOROOT",
       "GOOS": ctx.attr.goos,
       "GOARCH": ctx.attr.goarch,
       "CGO_ENABLED": "1" if ctx.attr.cgo else "0",
       "CC": cc_path,
       "CXX": cc_path,
-      "COMPILER_PATH": linker_path
+      "COMPILER_PATH": "$(pwd)/{}".format(linker_path),
+      "CGO_CPPFLAGS": compiler_options_string,
+      "CGO_LDFLAGS": linker_options_string,
+      "BAZEL_OUTPUT_ROOT": "$(pwd)/",
   }
   inputs = ctx.files._host_sdk + [root_file]
   inputs.extend(ctx.files._host_tools)

You need a way to find the root of the repo, and need to convert relative paths in flags to absolute paths on the way into the rules. This is a big chunk of our solution, but it feels brittle.

(I think I've already gotten some of the GOROOT changes already upstreamed. We'll do another pass at that once the dust settles.)

@steeve
Copy link
Contributor

steeve commented Mar 19, 2018

@jayconrod I have not seen absolute paths in the generated objects, only the build id changes, and I think this is a bug we have already on regardless of this ticket

@AustinSchuh
Copy link
Contributor

@jayconrod , did you guys get anywhere?

@jayconrod
Copy link
Contributor Author

@AustinSchuh, We got the buildid out of the standard library (thanks @steeve !), but we still have the general problem with relative paths.

I think the proper way to solve this is a proper builder for the standard library instead of copying files and using go install. I expect that will be a lot of work though, and I haven't started on it yet.

Bazel folks are making progress on a cc_toolchain rule that will make it easier to configure custom toolchains and will integration C/C++ rules with the platform / toolchain system.

@steeve
Copy link
Contributor

steeve commented Apr 24, 2018

In the mean time, perhaps @AustinSchuh can use my absolute cgo commit ?
I know it breaks one test but I think this is linked to rpath and not paths being embedded inside the binaries (they are not, although rpath is, i believe).

@steeve
Copy link
Contributor

steeve commented Apr 24, 2018

Just so you know @jayconrod, we're using that commit for the Android support, it's working well for us, save for unforeseen side effects of course 😃

@steeve
Copy link
Contributor

steeve commented Apr 24, 2018

I'll open a PR for it so that we have a place to discuss it maybe?

@AustinSchuh
Copy link
Contributor

@jayconrod, this is the ticket I was talking about today at the Bazel Conference. #1357 (comment) in particular has the hermetic CROSSTOOLs. If you run into any issues with the CROSSTOOLs, I'm happy to update them to enable testing.

@mhlopko , FYI.

@jayconrod
Copy link
Contributor Author

(Re-triaging old issues)

Closing as obsolete. I think all of the stdlib buildid and relative path issues have been resolved. Bazel's C/C++ toolchain support and rules_go's integration with it have been rewritten since this was filed, so hopefully there's nothing more to do here.

@AustinSchuh
Copy link
Contributor

I can confirm this is fixed. I'm able to use custom CROSSTOOL files with golang now.

anrodrig added a commit to anrodrig/rules_go that referenced this issue Feb 12, 2021
When using a custom cpp toolchain that adds libc++.a and libc++abi.a
in link_libs, linking the stdlib fails because it can't find the archives
by their relative paths. Same as issue bazelbuild#1357
zecke added a commit to zecke/rules_go that referenced this issue Nov 9, 2023
Make the gopackagesdriver work when using a C/C++ toolchain with
sysroots. We need to turn the relative in flag like --sysroot into
an absolute.

Address the todo by using the code already available in the stdlib.go
as part of bazelbuild#1536.
fmeum pushed a commit that referenced this issue Nov 9, 2023
Make the gopackagesdriver work when using a C/C++ toolchain with
sysroots. We need to turn the relative in flag like --sysroot into
an absolute.

Address the todo by using the code already available in the stdlib.go
as part of #1536.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants