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

__zig_probe_stack undefined in static libraries #6817

Closed
jedisct1 opened this issue Oct 26, 2020 · 3 comments
Closed

__zig_probe_stack undefined in static libraries #6817

jedisct1 opened this issue Oct 26, 2020 · 3 comments
Labels
bug Observed behavior contradicts documented or intended behavior frontend Tokenization, parsing, AstGen, Sema, and Liveness.
Milestone

Comments

@jedisct1
Copy link
Contributor

When building static libraries, the __zig_probe_stack symbol is undefined:

Undefined symbols for architecture x86_64:
  "___zig_probe_stack", referenced from:
      _std.debug.printLineInfo in x.o)
      _std.sort.sort in x.o)
ld: symbol(s) not found for architecture x86_64

Passing -fno-stack-check fixes this (thanks @FireFox317 !), but maybe stack checking should be disabled automatically when building static libraries. Or maybe we want to link compile_rt?

What would be the best way to improve this?

@LemonBoy
Copy link
Contributor

maybe stack checking should be disabled automatically when building static libraries

That's not a good idea, isn't it? Silently turning off safety measures is not nice.
Two possible solutions are to always link in compiler-rt or build the stack probe function as a separate object that's unconditionally linked in.
The former is easier to implement but it will increase the size of the generated libraries for no good reason: when a Zig static library is linked against some other Zig code we still use our compile_rt implementation, whenever you link the library with some C object we still have libgcc or LLVM's compiler_rt to fall-back on. On the other hand relying on a third-party library provided by the C compiler means we may end up with missing symbols or with a copy afected by bugs we can't fix (it hasn't happened so far as our implementations were the buggy ones but it's definitely something to take into account).

@andrewrk
Copy link
Member

andrewrk commented Oct 26, 2020

we may end up with missing symbols or with a copy afected by bugs we can't fix (it hasn't happened so far as our implementations were the buggy ones but it's definitely something to take into account).

It has happened - here are a couple examples: e3ea0b6 9153681

It looks like I regressed --bundle-compiler-rt in #6250. There is even still a build.zig API that sets the flag, only to get a CLI error. Fixing that should address the issue here. I think the only thing left would be whether to re-evaluate the logic for the default value of this flag.

@andrewrk andrewrk added bug Observed behavior contradicts documented or intended behavior frontend Tokenization, parsing, AstGen, Sema, and Liveness. labels Oct 26, 2020
@andrewrk andrewrk added this to the 0.7.0 milestone Oct 26, 2020
@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0, 0.7.1 Nov 8, 2020
andrewrk added a commit that referenced this issue Nov 30, 2020
@andrewrk
Copy link
Member

andrewrk commented Dec 1, 2020

Now you can pass -fcompiler-rt to include compiler-rt in static libraries. Feel free to open a proposal to change the default, which is currently only dynamic libraries and executables get compiler-rt included.

After #7264 you would be able to also do this to get a compiler_rt object (or library): zig build-obj -fcompiler-rt / zig build-lib -fcompiler-rt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior frontend Tokenization, parsing, AstGen, Sema, and Liveness.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants