Skip to content

Conversation

artemcm
Copy link
Contributor

@artemcm artemcm commented Apr 6, 2023

Unless we are building a static library, we need to propagate -fsanitizer= flag to the clang linker driver, otherwise sanitizers libraries will not be found at link-time.

Resolves rdar://107733898

…for dynamic libraries

Unless we are building a static library, we need to propagate `-fsanitizer=` flag to the clang linker driver, otherwise sanitizers libraries will not be found at link-time.

Resolves rdar://107733898
@artemcm artemcm force-pushed the SanitizersForAll_dylibs branch from ceef248 to e769d62 Compare April 6, 2023 23:54
// other rpaths are involved, so we should make sure we add the rpaths after
// all user-specified rpaths.
if linkerOutputType == .executable && !sanitizers.isEmpty {
if linkerOutputType != .staticLibrary && !sanitizers.isEmpty {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@keith do you, by any chance, remember why we had this restriction for .executable?

!= .staticLibrary seems like the right thing to do here.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

besides previously there was a check for excluding the fuzzer sanitizer for executables, so you might want that back a557d59#diff-d44ec7a906797a39be2e42ecb334167ae217537ad09a726d1f905c4ba9130200L365

@artemcm
Copy link
Contributor Author

artemcm commented Apr 6, 2023

@swift-ci test

Copy link
Member

@etcwilde etcwilde left a comment

Choose a reason for hiding this comment

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

LGTM

@artemcm artemcm merged commit 2810937 into swiftlang:main Apr 7, 2023
@artemcm artemcm deleted the SanitizersForAll_dylibs branch April 7, 2023 16:47
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.

3 participants