Skip to content

add prctl constants on android #4531

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dead10ck
Copy link

@dead10ck dead10ck commented Jul 6, 2025

Description

Add all constants from bionic's prctl.h header

Sources

Checklist

  • Relevant tests in libc-test/semver have been updated
  • No placeholder or unstable values like *LAST or *MAX are
    included (see #3131)
  • Tested locally (cd libc-test && cargo test --target mytarget);
    especially relevant for platforms that may not be checked in CI

@rustbot label +stable-nominated

@rustbot
Copy link
Collaborator

rustbot commented Jul 6, 2025

Some changes occurred in the Android module

cc @maurer

@rustbot rustbot added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Jul 6, 2025
@dead10ck
Copy link
Author

dead10ck commented Jul 6, 2025

I am not sure if those FreeBSD failures have anything to do with my change. The errors appear to be about a missing symbol that only appears in the FreeBSD path.

Comment on lines 3609 to 3594
#[cfg_attr(termux, link(name = "android-posix-semaphore"))]
extern "C" {}
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this added for?

Copy link
Author

Choose a reason for hiding this comment

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

On Android's bionic, there are some libc functions which are not supported, like sem_open, sem_close, and sem_unlink, which causes link failures when compiling anything with libc as a dependency.

On the Android terminal app Termux, there is a substitute port available as a separate library, which must be linked in explicitly.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is outside of the scope of libc. We are trying to reduce the number of libraries that get implicitly linked, and a third party library that isn't libc doesn't seem to make the cutoff.

Are the sem_* functions you mentioned not available via bionic in some cases?

Copy link
Author

Choose a reason for hiding this comment

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

Fair enough. At the end of the day, it's easy enough to work around in downstream projects by linking in this third party lib via rustflags, etc. I've removed this.

The sem_* functions are not available at all. Or rather, they are "there", but are simply stubbed out, so anything that tries to use them just fails.

https://cs.android.com/android/platform/superproject/main/+/main:bionic/libc/bionic/semaphore.cpp;drc=2e65afecba6877eaffa6bda825ae156e6c682fe5;l=124

In the case of termux, for example, this would render python script that used multiprocessing broken.

termux/termux-packages#8990

bionic simply doesn't implement all of libc, so there are a few shims that are needed to supplement what's missing.

Comment on lines 2294 to 2297
| "toupper" | "getpwuid_r" | "tmpfile" | "getpwnam" | "getpwuid" | "tcdrain"
| "cfgetispeed" | "cfgetospeed" | "cfsetispeed" | "cfsetospeed" | "tcgetattr"
| "tcsetattr" | "tcflow" | "tcflush" | "tcgetsid" | "tcsendbreak" | "cfmakeraw"
| "cfsetspeed" => true,
Copy link
Contributor

Choose a reason for hiding this comment

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

These don't seem relevant to the change here

Copy link
Author

@dead10ck dead10ck Jul 8, 2025

Choose a reason for hiding this comment

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

It's not strictly related, but tests for all these functions fail currently.

I split it out into another commit to make that clearer

Copy link
Contributor

Choose a reason for hiding this comment

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

What circumstances make these fail? We have Android tests in CI and as far as I know this hasn't been a problem

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure, to be honest; I didn't spend the time to dig into the failures. It could be something specific to termux.

I went ahead and removed these changes.

@dead10ck dead10ck force-pushed the fix/android/prctl branch 2 times, most recently from f5e4401 to 5905f0a Compare July 9, 2025 01:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-android O-unix S-waiting-on-review stable-nominated This PR should be considered for cherry-pick to libc's stable release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants