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

[illumos] add pthread stack functions #3788

Merged
merged 1 commit into from
Aug 12, 2024

Conversation

sunshowers
Copy link
Contributor

@sunshowers sunshowers commented Jul 31, 2024

The goal of this change is to make the following improvements possible:

Add some pthread stack functions:

I've verified that these functions exist and do the right thing.

Note that there doesn't appear to be a file like linux.txt for illumos specifically.

Another note: cd libc-test && cargo test currently fails on illumos with:

     Running test/main.rs (/home/rain/dev/libc/target/debug/deps/main-6329db179eb2b631)
RUNNING ALL TESTS
bad PTHREAD_MUTEX_DEFAULT value at byte 0: rust: 0 (0x0) != c 8 (0x8)
thread 'main' panicked at /home/rain/dev/libc/target/debug/build/libc-test-cefbf7853a50b4e4/out/main.rs:12:21:
some tests failed
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
error: test failed, to rerun pass `--test main`

But it also fails on main with the same error, so this isn't a regression.

@rustbot
Copy link
Collaborator

rustbot commented Jul 31, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @JohnTitor (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot
Copy link
Collaborator

rustbot commented Jul 31, 2024

Some changes occurred in solarish module

cc @jclulow, @pfmooney

@sunshowers
Copy link
Contributor Author

Oh I guess I should target the 0.2 branch. Just a sec

@sunshowers sunshowers changed the base branch from main to libc-0.2 July 31, 2024 22:49
@bors
Copy link
Contributor

bors commented Jul 31, 2024

☔ The latest upstream changes made this pull request unmergeable. Please resolve the merge conflicts.

Copy link
Contributor

@pfmooney pfmooney left a comment

Choose a reason for hiding this comment

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

The tests are passing on illumos with this change included.

sunshowers added a commit to sunshowers/stacker that referenced this pull request Jul 31, 2024
Uses the same logic as FreeBSD and DragonflyBSD.

This does require a version of libc with
rust-lang/libc#3788 included. Hopefully that goes out
soon. For now I'm putting up this PR as a thing to point to.

I've verified locally on an illumos machine that stacker is able to:

* find the right stack size, and
* create new stack segments.
@tgross35
Copy link
Contributor

Could you add an illumos.txt file in libc-test/semver and add these?

There are probably other functions that should be added once the file exists, but those can come separately.

@tgross35
Copy link
Contributor

Also, can you target main? We can cherry pick from there.

@tgross35 tgross35 added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Aug 12, 2024
@sunshowers
Copy link
Contributor Author

Thanks @tgross35! Can do both of those things. (I wasn't sure about main vs 0.2, is there documentation somewhere about that? Sorry if I missed it.)

@tgross35
Copy link
Contributor

tgross35 commented Aug 12, 2024

(I wasn't sure about main vs 0.2, is there documentation somewhere about that? Sorry if I missed it.)

The README actually does suggest targeting 0.2, so you correctly did what is documented. I just intend to change that so we always update main and cherry-pick things to stable where needed, rather than maybe forgetting to apply some things to main and then losing fixes/additions when 1.0 is released.

The stable-nominated label can be applied with rustbot for any specific requests.

@rustbot
Copy link
Collaborator

rustbot commented Aug 12, 2024

Some changes occurred in OpenBSD module

cc @semarie

Some changes occurred in OpenBSD module

cc @semarie

@sunshowers sunshowers changed the base branch from libc-0.2 to main August 12, 2024 19:03
@sunshowers
Copy link
Contributor Author

I just intend to change that so we always update main and cherry-pick things to stable where needed, rather than maybe forgetting to apply some things to main and then losing fixes/additions when 1.0 is released.

That makes sense! (Landing on main first and then cherry-picking is also the flow I personally recommend to folks :) )

Anyway, updated the PR. (Sorry about the rustbot ping, should have updated the base first.)

@tgross35
Copy link
Contributor

tgross35 commented Aug 12, 2024

Thanks for the changes! Feel free to submit a cherry-pick PR yourself once this merges (updated instructions coming here #3804), otherwise I will be doing a batch of them at some point.

@tgross35 tgross35 added this pull request to the merge queue Aug 12, 2024
Merged via the queue into rust-lang:main with commit ad1fccc Aug 12, 2024
39 checks passed
@sunshowers sunshowers deleted the illumos-pthread branch August 12, 2024 21:36
@sunshowers
Copy link
Contributor Author

Thanks for the changes! Feel free to submit a cherry-pick PR yourself once this merges (updated instructions coming here #3804), otherwise I will be doing a batch of them at some point.

This is #3807, thanks!

@tgross35 tgross35 added stable-applied This PR has been cherry-picked to libc's stable release branch and removed stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Aug 13, 2024
sunshowers added a commit to sunshowers/stacker that referenced this pull request Aug 16, 2024
Uses the same logic as FreeBSD and DragonflyBSD.

This does require a version of libc with
rust-lang/libc#3788 included. Hopefully that goes out
soon. For now I'm putting up this PR as a thing to point to.

I've verified locally on an illumos machine that stacker is able to:

* find the right stack size, and
* create new stack segments.
nagisa pushed a commit to rust-lang/stacker that referenced this pull request Aug 21, 2024
Uses the same logic as FreeBSD and DragonflyBSD.

This does require a version of libc with
rust-lang/libc#3788 included. Hopefully that goes out
soon. For now I'm putting up this PR as a thing to point to.

I've verified locally on an illumos machine that stacker is able to:

* find the right stack size, and
* create new stack segments.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review stable-applied This PR has been cherry-picked to libc's stable release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants