Skip to content

[stdlib] Buildfix getting stack bounds on OpenBSD. #39995

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

Merged
merged 1 commit into from
Nov 3, 2021

Conversation

3405691582
Copy link
Member

OpenBSD doesn't have pthread_attr_get_np and expects something like
pthread_attr_getstackaddr to be used to get the initial stack size.
We need to use pthread_stackseg_np on this platform to get the
stack size and location of pthread_self.

@3405691582
Copy link
Member Author

cc @grynspan

@3405691582
Copy link
Member Author

FTR, the buildbreak was in Stubs.cpp in #37666.

OpenBSD doesn't have `pthread_attr_get_np` and expects something like
`pthread_attr_getstackaddr` to be used to get the initial stack size.
We need to use `pthread_stackseg_np` on this platform to get the
stack size and location of `pthread_self`.
@grynspan
Copy link
Contributor

grynspan commented Nov 2, 2021

@swift-ci please test

@grynspan
Copy link
Contributor

grynspan commented Nov 2, 2021

Please note swift-ci does not run tests on OpenBSD and it's not a platform actively supported by the Swift team at this time. Can you confirm:

  1. You were able to build for OpenBSD with this change, and
  2. The "temporary_allocation" and "TemporaryAllocation" tests ran successfully on OpenBSD with this change?

@3405691582
Copy link
Member Author

You were able to build for OpenBSD with this change, and

Correct.

(I made an earlier comment about a little confused about which pointer should be which in this stub, but I deleted it since think I've sorted it out now.)

The "temporary_allocation" and "TemporaryAllocation" tests ran successfully on OpenBSD with this change?

There are a number of other unrelated issues that are currently preventing some of the automated tests from running that I need to debug and fix. However, I can extract the actual test executable build commands from test/stdlib/TemporaryAllocation.swift and run the result manually (e.g., running the result ./build/Ninja-DebugAssert/swift-openbsd-amd64/test-openbsd-amd64/stdlib/Output/TemporaryAllocation.swift.tmp/a.out). This ends in TemporaryAllocation: All tests passed.

I can't find a temporary_allocation test, but swift/test/SILGen/memberwise_init_temporary_allocations.swift passes.

@grynspan
Copy link
Contributor

grynspan commented Nov 2, 2021

I can't find a temporary_allocation test, but swift/test/SILGen/memberwise_init_temporary_allocations.swift passes.

Those tests are located in swift/test/IRGen/temporary_allocation/. There are multiple test files. I do not expect them to fail if they were passing previously since the change you made should not affect SIL or IR. At least on macOS, you can run just the temporary allocation IRGen tests with this command:

utils/run-test --lit ../llvm-project/llvm/utils/lit/lit.py \
  ../build/Ninja-RelWithDebInfoAssert/swift-macosx-$(uname -m)/test-macosx-$(uname -m) \
  --filter="temporary_allocation"

I'm not certain how to modify that command to work on OpenBSD (I imagine just replacing macosx with openbsd?)

@3405691582
Copy link
Member Author

Got it. Both swift/test/IRGen/temporary_allocation and swift/test/stdlib/TemporaryAllocation.swift are passing.

@swift-ci
Copy link
Contributor

swift-ci commented Nov 2, 2021

Build failed
Swift Test Linux Platform
Git Sha - b5135fa

@3405691582
Copy link
Member Author

Test failures appear unrelated.

@swift-ci
Copy link
Contributor

swift-ci commented Nov 2, 2021

Build failed
Swift Test OS X Platform
Git Sha - b5135fa

@grynspan
Copy link
Contributor

grynspan commented Nov 2, 2021

@swift-ci please test

Copy link
Member

@lorentey lorentey left a comment

Choose a reason for hiding this comment

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

Thank you! 👍

@lorentey lorentey merged commit 613116f into swiftlang:main Nov 3, 2021
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.

4 participants