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: compile aarch64-linux-android binary on linux when enable lto. #83

Merged
merged 3 commits into from
May 5, 2024

Conversation

fengys1996
Copy link

This PR changes pthread_atfork from a strong symbol to a weak symbol.

Related issue

#81

@fengys1996 fengys1996 marked this pull request as ready for review April 30, 2024 08:06
@@ -338,6 +338,11 @@ fn main() {
println!("cargo:rustc-link-lib=atomic");
}
println!("cargo:rerun-if-changed=jemalloc");

cc::Build::new()
Copy link
Member

Choose a reason for hiding this comment

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

Should only be built for android.

// we're building on doesn't have them defined, so just make sure the symbols
// are available.
__attribute__((weak)) int
pthread_atfork(uint8_t *prefork __attribute__((unused)),
Copy link
Member

Choose a reason for hiding this comment

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

Should use the definition from https://man7.org/linux/man-pages/man3/pthread_atfork.3.html. The original definition is just for convenient.

@fengys1996
Copy link
Author

PTAL @BusyJay

@BusyJay
Copy link
Member

BusyJay commented Apr 30, 2024

You need to sign off your commits.

@fengys1996
Copy link
Author

You need to sign off your commits.

Done.

Copy link
Member

@BusyJay BusyJay left a comment

Choose a reason for hiding this comment

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

Rest LGTM

@@ -0,0 +1,13 @@
#include <pthread.h>
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you need to include this header.

Copy link
Author

Choose a reason for hiding this comment

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

Removed.

1. Compile pthread_atfork.c when target contains "android".
2. Use original definition of "pthread_atfork".

Signed-off-by: fys <fengys1996@gmail.com>
Signed-off-by: fys <fengys1996@gmail.com>
@BusyJay BusyJay merged commit 805804a into tikv:main May 5, 2024
8 of 9 checks passed
@BusyJay
Copy link
Member

BusyJay commented May 5, 2024

Thank you!

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.

2 participants