Omit -pie from linker flags under certain conditions

[siddharthab]

Fixes #3691.

[sfc-gh-ptabor]

We also stumped on this problem. Either we need to linkmode=pie our go_binaries to make the cross-compilation from mac to linux work or apply this patch.

[sfc-gh-ptabor]

@fmeum @tyler-french - could you please take a look at this patch.

Without this (or linkmode=pie), the linker on mac os is producing corrupted binaries. Confirmed with 0.48.1 on golang 1.22.5

Minimal failing code:

 package main

import (
       "fmt"
       "os/user"
)

func main() {
      fmt.Println("Hello")
      _ = user.Lookup; // Force CGO dependency to exist.
}

go_binary(
   name = "fips_go_binary",
   srcs = ["main.go"],
   cgo = True,
   pure = "off",
   static = "off",
  // linkmode = "pie",
   deps = [
       "//golang/proto:example_go_proto",
       "@org_golang_x_sync//errgroup",
   ],
)

Repro from OSX:

bazel build //golang:fips_go_binary --platforms=@sfc_container_images_gcbi//platforms:linux_arm64

docker run -v $(realpath ../bazel-bin/golang/fips_go_binary_/fips_go_binary):/fips_go_binary cgr.dev/chainguard/glibc-dynamic /fips_go_binary

runtime: pcHeader: magic= 0xfffffff1 pad1= 0 pad2= 0 minLC= 4 ptrSize= 8 pcHeader.textStart= 0xab4f0 text= 0xaaaac05db4f0 pluginpath=
fatal error: invalid function symbol table
runtime: panic before malloc heap initialized

runtime stack:
runtime.throw({0xaaaac0556208?, 0x0?})
	GOROOT/src/runtime/panic.go:1047 +0x40 fp=0xfffff31b05d0 sp=0xfffff31b05a0 pc=0xaaaac060a0e0
runtime.moduledataverify1(0xfffff31b06e4?)
	GOROOT/src/runtime/symtab.go:627 +0x650 fp=0xfffff31b06b0 sp=0xfffff31b05d0 pc=0xaaaac06250b0
runtime.moduledataverify(...)
	GOROOT/src/runtime/symtab.go:613
runtime.schedinit()
	GOROOT/src/runtime/proc.go:712 +0x58 fp=0xfffff31b0710 sp=0xfffff31b06b0 pc=0xaaaac060d838
runtime.rt0_go()
	src/runtime/asm_arm64.s:86 +0xa4 fp=0xfffff31b0740 sp=0xfffff31b0710 pc=0xaaaac0633ec4

Looking at the binary:

 readelf -a /fips_go_binary
ELF Header:
  Magic:   7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00
  Class:                             ELF64
  Data:                              2's complement, little endian
  Version:                           1 (current)
  OS/ABI:                            UNIX - System V
  ABI Version:                       0
  Type:                              DYN (Position-Independent Executable file)
  Machine:                           AArch64
  Version:                           0x1
  Entry point address:               0xab3a0
  Start of program headers:          64 (bytes into file)
  Start of section headers:          1258544 (bytes into file)
  Flags:                             0x0
  Size of this header:               64 (bytes)
  Size of program headers:           56 (bytes)
  Number of program headers:         12
  Size of section headers:           64 (bytes)
  Number of section headers:         37
  Section header string table index: 36

The linker execution:

external/go_sdk_host_exec/pkg/tool/darwin_arm64/link -importcfg bazel-out/darwin_arm64-fastbuild/bin/golang/fips_go_binary_/importcfg3070347299 -o bazel-out/darwin_arm64-fastbuild/bin/golang/fips_go_binary_/fips_go_binary -extar external/llvm_toolchain_with_sysroot_host/bin/llvm-ar -extld external/llvm_toolchain_with_sysroot_host/bin/cc_wrapper.sh -buildid=redacted -s -w -extldflags "-L/lib/aarch64-linux-gnu -L/usr/lib/aarch64-linux-gnu -fuse-ld=lld --target=aarch64-linux-gnu --sysroot=external/llvm_linux_arm64/sysroot" /private/var/tmp/_bazel_ptabor/496389416e9b24a913b8867f23ea227f/sandbox/darwin-sandbox/1723/execroot/sfc_container_images_examples/bazel-out/darwin_arm64-fastbuild/bin/golang/fips_go_binary.a

[fmeum]

I agree that we need something like this patch, but I would prefer one that disables the pic feature instead of manually stripping certain hardcodes flags. This may require inlining cgo_context into the core Go rules logic so that it is configured per target.

@siddharthab Would you be willing to do this?

[siddharthab]

@fmeum I can do the work but I am not sure what the request is. Are you asking me to ignore --force_pic completely in rules_go?

This patch is trying to introduce consistency between the tool invocations here vs the standard Go toolchain. The Go toolchain also strips out some user supplied flags like this if they don't make sense in the context.

[fmeum]

I think I'm a bit confused by the todo. If I understand the logic you linked correctly, it applies to cases where the linker by default (that is, without any flags) produces position independent executables and also runs with rules_go. That seems orthogonal to stripping any -pie flags added by some intermediary (in this case Bazel).

If that's the case, shouldn't we always remove any -pie flags coming from Bazel so that Go can fully control PIE-ness?

Just marking pie an unsupported toolchain feature in cgo_context may get us this without any further changes to the structure. We could also remove the todo.

[siddharthab]

Just run buildifier and we are good to go!

Done! Thanks for the quick review.