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

Always omit -pie from cc_toolchain linker flags #3693

Merged
merged 3 commits into from
Sep 5, 2024

Conversation

siddharthab
Copy link
Contributor

Fixes #3691.

@sfc-gh-ptabor
Copy link

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
Copy link

@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
Copy link
Collaborator

fmeum commented Jul 5, 2024

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
Copy link
Contributor Author

@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
Copy link
Collaborator

fmeum commented Jul 7, 2024

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 siddharthab changed the title Omit -pie from linker flags under certain conditions Always omit -pie from cc_toolchain linker flags Sep 5, 2024
go/private/context.bzl Outdated Show resolved Hide resolved
Copy link
Collaborator

@fmeum fmeum left a comment

Choose a reason for hiding this comment

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

Just run buildifier and we are good to go!

@siddharthab
Copy link
Contributor Author

Just run buildifier and we are good to go!

Done! Thanks for the quick review.

@fmeum fmeum enabled auto-merge (squash) September 5, 2024 15:16
@fmeum fmeum merged commit fb5f091 into bazelbuild:master Sep 5, 2024
2 checks passed
@siddharthab siddharthab deleted the nopie branch September 5, 2024 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove -pie from LDFLAGS if buildmode is not pie
3 participants