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: relative paths are not respected when checking supported linker flags #59952

Closed
JamesMBartlett opened this issue May 3, 2023 · 1 comment
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@JamesMBartlett
Copy link
Contributor

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

$ go version
go version go1.20 linux/amd64

Does this issue reproduce with the latest release?

Yes, it even reproduces when building go off of main.
From looking at the code, it seems like all versions of go should be affected.

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/james/.cache/go-build"
GOENV="/home/james/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/james/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/james"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/px_dev/tools/golang"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/px_dev/tools/golang/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.20"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2321536912=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I created the following script to reproduce the exact issue I'm seeing.

repro.sh
#!/bin/bash -e

go="go"
if [[ "$USE_DEV_GO" = "1" ]]; then
  go="$(pwd)/bin/go"
fi

tmpdir="$(mktemp -d)"
pushd "${tmpdir}" &>/dev/null

cleanup() {
  rm -rf "${tmpdir}"
}
trap cleanup ERR
trap cleanup EXIT

cat <<EOF > trivial.go
package main
func main() {}
EOF

cat <<EOF > mycompiler.sh
#!/bin/bash
clang "\$@"
EOF

chmod +x mycompiler.sh

export CGO_ENABLED=1
"${go}" build -ldflags '-extld ./mycompiler.sh -linkmode=external -buildmode=exe -v' trivial.go 2>&1 | tee -a build_out

host_link="$(grep "host link:" build_out)"

if (echo "${host_link}" | grep -- '-no-pie') || (echo "${host_link}" | grep -- '-nopie'); then
  echo "Host link has -no-pie or -nopie"
else
  echo "Host link missing expected -no-pie or -nopie"
  exit 10
fi

cleanup

Ran ./repro.sh. Also ran USE_DEV_GO=1 ./repro.sh inside of the go repository after running ./all.bash off of main, to test against main.

What did you expect to see?

./repro.sh to not return an error code.

What did you see instead?

./repro.sh returns an error code because -no-pie or -nopie are not added to the external link flags.

The way that the linkerFlagSupported function works, means that if -extld or -extldflags reference relative paths they can cause linkerFlagSupported to return false, leading linker flags to not be added when they should be. In my repro script, this manifests itself in -no-pie not getting added to the external link flags, which can cause downstream issues. For example, when using bazelbuild/rules_go with a relative compiler path and with a compiler that defaults to PIE, the built go binaries end up segfaulting because go was compiled expecting to be run as a static binary but the external linker links it as PIE.
The repro script I provided is just one example of issues with linkerFlagSupported being run outside of the current working directory, but you could for example also have a relative path in -extldflags say --sysroot=relative/path/to/sysroot.

The fix is pretty simple, linkerFlagSupported should be run in the current working directory instead of inside the created temporary directory. I will put out a PR shortly.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label May 3, 2023
@gopherbot
Copy link

Change https://go.dev/cl/491796 mentions this issue: cmd/link: fix checks for supported linker flags with relative paths.

@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 3, 2023
@mknyszek mknyszek changed the title cmd/link: relative paths are not respected when checking supported linker flags. cmd/link: relative paths are not respected when checking supported linker flags May 10, 2023
@mknyszek mknyszek added this to the Backlog milestone May 10, 2023
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 12, 2023
@dmitshur dmitshur modified the milestones: Backlog, Go1.21 May 12, 2023
siddharthab added a commit to siddharthab/rules_go that referenced this issue Sep 14, 2023
Go toolchain checks which linker flags are available by running the
linker on a temporary file. Before Go 1.21, this command was run in a
temporary directory, but this has now been fixed upstream.
golang/go#59952

This change will not be needed for Go versions 1.21 and above, but will
be beneficial for users on lower versions and using a hermetic
toolchain.

See bazel-contrib/toolchains_llvm#183 for some more
context.
siddharthab added a commit to siddharthab/rules_go that referenced this issue Sep 14, 2023
Go toolchain checks which linker flags are available by running the
linker on a temporary file. Before Go 1.21, this command was run in a
temporary directory, but this has now been fixed upstream.
golang/go#59952

This change will not be needed for Go versions 1.21 and above, but will
be beneficial for users on lower versions and using a hermetic
toolchain.

See bazel-contrib/toolchains_llvm#183 for some more
context.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants