Skip to content

[ld.lld] do not add --gc-sections by default #11577

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

Closed
wants to merge 1 commit into from

Conversation

motiejus
Copy link
Contributor

@motiejus motiejus commented May 4, 2022

When building object files, zig cc will instruct lld to remove unused
sections via --gc-sections. This is problematic for cgo builds that don't
explicitly use C code. Briefly, go builds cgo executables as follows*:

  1. build _cgo_.o, which links (on linux_amd64 systems) to
    /usr/local/go/src/runtime/race/race_linux_amd64.syso.
  2. That .syso contains references to symbols from libc.

If the user program uses at least one libc symbol, it will link
correctly. However, if Go is building a cgo executable, but without C
code, the sections from .syso file will be garbage-collected, leaving
a _cgo_.o without any references to libc, causing the final linking
step to not link libc.

Until now, this could be worked around by -linkmode external flag to
go build. This causes Go to link the final executable using the
external linker (which implicitly links libc). However, that flag brings
in a whole different world of worms.

I assume the gc_sections is an optimization; I tried to re-add
--gc-sections to the final executable, but that didn't go well. I know
removing such an optimization may be contentious, so let's start the
discussion here. Quoting @andrewrk in 1 (it was about --as-needed,
but the point remains the same):

The C source code for the temporary executable needs to have dummy
calls to getuid, pthread_self, sleep, and every other libc function
that cgo/race code wants to call.

I agree this is how it should work. However, while we could fix it for
go, I don't know how many other systems rely on that, and we'll never
know we've fixed the last one. The point is, GCC/Clang does not optimize
sections by default, and downstream tools rely on that. If we want to
consider zig cc a drop-in clang replacement (except for
-fsanitize=undefined, which I tend to agree with), then it should not
be optimizing the intermediate object files. Or at least have a very
prominent fine-print that this is happening, with ways to work around
it.

Fixes #11398
Fixes golang/go#44695
Fixes golang/go#52690

[*]: Empirically observed with CGO_ENABLED=1 go test -race -x -v

@motiejus
Copy link
Contributor Author

motiejus commented May 4, 2022

CI failure:

ld.lld: error: undefined symbol: main

So treat this as a conversation starter w.r.t. --gc-sections, not a real pull request. :)

@motiejus motiejus changed the title [ld.lld] --gc-sections only when output is executable [ld.lld] do not add --gc-sections by default May 4, 2022
@motiejus
Copy link
Contributor Author

motiejus commented May 4, 2022

To remove confusion: the first diff removed --gc-sections when the output was .Exe, but that broke a bunch of other stuff. So I am being conservative here (or adventurous, depends how you look at it) and am not adding the flag by default altogether.

@motiejus
Copy link
Contributor Author

motiejus commented May 4, 2022

For the record, https://builds.hut.lavatech.top/~andrewrk/job/23490 seems to be failing on master too.

@andrewrk andrewrk self-requested a review May 4, 2022 20:35
When building object files, `zig cc` will instruct lld to remove unused
sections via `--gc-sections`. This is problematic cgo builds that don't
explicitly use C code. Briefly, go builds cgo executables as follows*:

1. build `_cgo_.o`, which links (on linux_amd64 systems) to
   `/usr/local/go/src/runtime/race/race_linux_amd64.syso`.
2. That `.syso` contains references to symbols from libc.

If the user program uses at least one libc symbol, it will link
correctly. However, if Go is building a cgo executable, but without C
code, the sections from `.syso` file will be garbage-collected, leaving
a `_cgo_.o` without any references to libc, causing the final linking
step to not link libc.

Until now, this could be worked around by `-linkmode external` flag to
`go build`. This causes Go to link the final executable using the
external linker (which implicitly links libc). However, that flag brings
in a whole different world of worms.

I assume the `gc_sections` is an optimization; I tried to re-add
`--gc-sections` to the final executable, but that didn't go well. I know
removing such an optimization may be contentious, so let's start the
discussion here. Quoting @andrewrk in [1] (it was about `--as-needed`,
but the point remains the same):

> The C source code for the temporary executable needs to have dummy
> calls to getuid, pthread_self, sleep, and every other libc function
> that cgo/race code wants to call.

I agree this is how it *should* work. However, while we could fix it for
go, I don't know how many other systems rely on that, and we'll never
know we've fixed the last one. The point is, GCC/Clang does not optimize
sections by default, and downstream tools rely on that. If we want to
consider `zig cc` a drop-in clang replacement (except for
`-fsanitize=undefined`, which I tend to agree with), then it should not
be optimizing the intermediate object files. Or at least have a very
prominent fine-print that this is happening, with ways to work around
it.

Fixes ziglang#11398
Fixes golang/go#44695
Fixes golang/go#52690

[*]: Empirically observed with `CGO_ENABLED=1 go test -race -x -v`

[1]: golang/go#52690 (comment)
@motiejus
Copy link
Contributor Author

motiejus commented May 5, 2022

For the record, I added -Wl,--no-gc-sections to bazel-zig-cc cflags1. Now bazel-zig-cc can build more cgo stuff!

@andrewrk
Copy link
Member

I agree this is how it should work. However, while we could fix it for
go, I don't know how many other systems rely on that, and we'll never
know we've fixed the last one. The point is, GCC/Clang does not optimize
sections by default, and downstream tools rely on that. If we want to
consider zig cc a drop-in clang replacement (except for
-fsanitize=undefined, which I tend to agree with), then it should not
be optimizing the intermediate object files. Or at least have a very
prominent fine-print that this is happening, with ways to work around
it.

The Zig project is about rebuilding systems programming from the ground up, re-examining assumptions, and making different decisions where they are warranted. Fully imitating GCC including its limitations is not a goal we have. In fact that already exists, it's Clang! zig cc is interesting precisely because, while it is a drop-in replacement, it also does some things differently.

Ultimately, the downstream tools need improvements in this case, not Zig.

@andrewrk andrewrk closed this May 10, 2022
@motiejus motiejus deleted the gc-sections branch June 27, 2022 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants