Skip to content

[Frontend] Re-enable ASAN builds support in load-pass-plugin test (NFC) #77870

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

Conversation

antoniofrighetto
Copy link
Contributor

libTestPlugin.dylib dynamic library was previously linking against libLLVMSupport.a, whose library is already linked in the Swift compiler binary, hence leading to multiple conflicting definitions of libLLVMSupport symbols. This issue has been addressed by telling the linker to resolve undefined symbols from the Swift frontend at runtime.

Fixes: #77771.

@antoniofrighetto
Copy link
Contributor Author

cc/ @al45tair

@al45tair
Copy link
Contributor

@swift-ci Please test

@antoniofrighetto
Copy link
Contributor Author

Ugh, I couldn't reproduce the failure locally. I'm taking a better look.

@antoniofrighetto antoniofrighetto force-pushed the feature/reenable-asan-plugin-test branch from fe8475b to f6a62fb Compare December 2, 2024 10:25
@antoniofrighetto
Copy link
Contributor Author

Okay, the plugin is being built via clang so dynamic_lookup is not an option. I think using -bundle_loader is the correct option for the purpose of the test. May we test this again please?

@al45tair
Copy link
Contributor

al45tair commented Dec 2, 2024

@swift-ci Please test

…NFC)

`libTestPlugin.dylib` dynamic library was previously linking against
`libLLVMSupport.a`, which is already linked into the Swift compiler
binary. This caused multiple conflicting definitions of `LLVMSupport`
lib symbols, leading to ODR violations. This issue has been addressed
by linking against `libLLVMSupport` via `-hidden-lLLVMSupport` flag,
ensuring `libLLVMSupport` symbols remain hidden within the plugin,
preventing conflicts with those in the Swift compiler.

Fixes: swiftlang#77771.
@antoniofrighetto antoniofrighetto force-pushed the feature/reenable-asan-plugin-test branch from f6a62fb to d30af4c Compare December 5, 2024 09:09
@antoniofrighetto
Copy link
Contributor Author

@al45tair Sorry, was not reproducing this correctly. Even though the Swift compiler already links against libLLVMSupport, the symbol __ZN4llvm23EnableABIBreakingChecksE doesn't appear to be there, so -bundle_loader doesn't work either. For the purpose of the test, I suggest keep linking against libLLVMSupport, yet ensuring symbols remain private within the plugin, thus avoiding possible conflicting definitions. Tested this both with --presets=buildbot_incremental and --presets=buildbot_incremental_asan, think it should be on track now.

@al45tair
Copy link
Contributor

al45tair commented Dec 5, 2024

@swift-ci Please test

@antoniofrighetto
Copy link
Contributor Author

Should be on track?

@al45tair al45tair enabled auto-merge December 30, 2024 14:07
@al45tair
Copy link
Contributor

LGTM. Can't merge right now because the branches are locked, but we should take this. Thank-you :-)

@antoniofrighetto
Copy link
Contributor Author

Looks like auto-merging did not trigger?

@al45tair al45tair disabled auto-merge January 9, 2025 14:22
@al45tair al45tair merged commit c68df8f into swiftlang:main Jan 9, 2025
5 checks passed
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.

🟠 OSS Swift CI: oss-swift-incremental-ASAN-RA-macos failed: test: Frontend/load-pass-plugin.swift (exit code 1)
2 participants