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

Avoid runtime crashes by removing undefined behavior. #1560

Merged
merged 4 commits into from
Jun 13, 2018

Conversation

mario-vera
Copy link
Contributor

From pthread docs:
"The effect of calling pthread_getspecific() or pthread_setspecific() with a key value not obtained from pthread_key_create() or after key has been deleted with pthread_key_delete() is undefined."

On some Linux flavors this results in getspecific returning uninitialized pointer with results in runtime crashes after dereferencing invalid memory.

Also NULL format for fprintf is invalid and results on crashes when calling CFShow.

@@ -628,6 +628,7 @@ static void *__CFTSDGetSpecific() {
#if DEPLOYMENT_TARGET_MACOSX || DEPLOYMENT_TARGET_EMBEDDED || DEPLOYMENT_TARGET_EMBEDDED_MINI
return pthread_getspecific(__CFTSDIndexKey);
#elif DEPLOYMENT_TARGET_LINUX
__CFTSDInitialize();
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 actually belongs here:

https://github.com/apple/swift-corelibs-foundation/blob/ac7f8030ad381f0f3d97359f574a6c97a6cf66e1/CoreFoundation/Base.subproj/CFRuntime.c#L1080

where we can probably just remove the requirement for DEPLOYMENT_RUNTIME_SWIFT and make it DEPLOYMENT_TARGET_LINUX.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It took me bit to validate, but this works and I agree its best suited. I just updated the pull request, please take a look.

@millenomi
Copy link
Contributor

I think you have the commits for your other PR merged into this one. Can you separate them?

@millenomi
Copy link
Contributor

@swift-ci please test

1 similar comment
@millenomi
Copy link
Contributor

@swift-ci please test

@millenomi
Copy link
Contributor

@swift-ci please test and merge

@swift-ci swift-ci merged commit e49dcf0 into swiftlang:master Jun 13, 2018
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