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

fix: undefined reference to `__zig_probe_stack' #792

Merged
merged 2 commits into from Jun 21, 2023

Conversation

kassane
Copy link
Contributor

@kassane kassane commented May 18, 2023

Allows another C compiler link to this library.

If the library allowed build ReleaseFast then this command would be useless.

cc: @batiati @kprotty

Reference:

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. - Andrewrk

Pre-merge checklist

  • I am very sure this PR could not affect performance.

@eatonphil
Copy link
Contributor

If the library allowed build ReleaseFast then this command would be useless.

I'm pretty sure we have comptime checks that fail the build if you try to do ReleaseFast.

Copy link
Contributor

@batiati batiati left a comment

Choose a reason for hiding this comment

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

Thanks Matheus,

It's worth adding a comment:

// Bundle rt to allow linking this static library
// with the final artifact using something other than Zig.

Could you please add static_lib.pie = true as well? So we follow the same configuration used by Go client's static libraries:

https://github.com/tigerbeetledb/tigerbeetle/blob/1c4000cbd5c4fb8c892019dd29787d1da0ae1e09/build.zig#L597-L598

@kassane kassane requested a review from batiati June 21, 2023 15:21
@kassane
Copy link
Contributor Author

kassane commented Jun 21, 2023

Hi @batiati ,

I have added the suggestion and the build has worked with gcc 13 (archlinux).
A doubt, does this change overwrite other bindings (duplicate parameters)?

At first sight, I think that the settings between bindings are standalone. Right?

@batiati
Copy link
Contributor

batiati commented Jun 21, 2023

A doubt, does this change overwrite other bindings (duplicate parameters)?
At first sight, I think that the settings between bindings are standalone. Right?

I'm not sure if I'm following, but the other bindings (clients) compile their libs independently, so it's not duplicated but only following the same build configuration for similar use cases (i.e. both c_client and go_client produce static libraries).

Did I get your question answered?

(...)

Maybe we can extract this part in a helper function, just like link_tracer_backend is called by every artifact, we can then create a fn static_lib_config(...) for doing that repetitive work of setting up correctly a static lib.

We can also place the comment there:

// Bundle rt to allow linking this static library
// with the final artifact using something other than Zig.

Looking at the code, shared libs have even more repeated code, but it can be done by another PR!

@kassane
Copy link
Contributor Author

kassane commented Jun 21, 2023

Did I get your question answered?

Yeah!!

Looking at the code, shared libs have even more repeated code, but it can be done by another PR!

Ok.

@batiati batiati added this pull request to the merge queue Jun 21, 2023
Merged via the queue into tigerbeetle:main with commit 0cbd19b Jun 21, 2023
110 of 112 checks passed
@kassane kassane deleted the compilert_for_gcc branch June 21, 2023 22:16
@kassane kassane mentioned this pull request Aug 30, 2023
7 tasks
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.

None yet

3 participants