Skip to content

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

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

Closed
al45tair opened this issue Nov 21, 2024 · 5 comments · Fixed by #77870
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. found by asan Flag: An issue found by the Address Sanitizer test failure

Comments

@al45tair
Copy link
Contributor

al45tair commented Nov 21, 2024

Description

#68985 introduced a way to load an LLVM pass from a dynamic library into the compiler. Unfortunately the test case for this is subtly broken as it links against the (static) library libLLVMSupport.a, which is already linked into the compiler binary, and so we end up with ODR violations, which are picked up by ASAN; in particular, the ASAN build is complaining about llvm::circular_raw_ostream.

Reproduction

https://ci.swift.org/job/oss-swift-incremental-ASAN-RA-macos/7303/consoleFull#1161724776d6fdb6cb-f376-4f2e-8bce-d31c7304698b

Expected behavior

The test case should pass.

Environment

Swift OSS CI (ASAN build)

Additional information

rdar://140319292

@al45tair al45tair added bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. found by asan Flag: An issue found by the Address Sanitizer test failure triage needed This issue needs more specific labels labels Nov 21, 2024
@al45tair
Copy link
Contributor Author

I think the fix for this is probably to remove -lLLVMSupport from the command line for the libTestPlugin.dylib build and add instead -Wl,-bundle_loader -Wl,%swift_frontend_plain. Something like that, at any rate.

@al45tair
Copy link
Contributor Author

@antoniofrighetto Would you be able to take a look at this? // @aschwaighofer

al45tair added a commit to al45tair/swift that referenced this issue Nov 21, 2024
This test is buggy and fails under ASAN (which detects the bug).
See swiftlang#77771.
@al45tair
Copy link
Contributor Author

(I've disabled the test when doing the ASAN build for now, but we should fix this and re-enable it.)

@antoniofrighetto
Copy link
Contributor

@al45tair Thank you for the temporary fix! I agree we should have not been linking against libLLVMSupport.a in the plugin in the first place, as the Swift compiler already links against it. I think resolving the undefined symbols at runtime using the symbols already available in the Swift frontend binary looks definitely better. Let me know if you'd like me to test it and open a PR with the fix.

@al45tair
Copy link
Contributor Author

Let me know if you'd like me to test it and open a PR with the fix.

Yes please. There's no rush — this is new functionality and it's the test case itself that's slightly wrong (while I wouldn't want people to go building passes and making the same mistake because they copied the commands from the test case, I doubt that's really a problem in practice as I imagine most passes added this way are things people are experimenting with).

tshortli pushed a commit to tshortli/swift that referenced this issue Nov 21, 2024
This test is buggy and fails under ASAN (which detects the bug).
See swiftlang#77771.
antoniofrighetto added a commit to antoniofrighetto/swift that referenced this issue Nov 29, 2024
`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: swiftlang#77771.
antoniofrighetto added a commit to antoniofrighetto/swift that referenced this issue Nov 29, 2024
…NFC)

`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: swiftlang#77771.
antoniofrighetto added a commit to antoniofrighetto/swift that referenced this issue Dec 2, 2024
…NFC)

`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: swiftlang#77771.
@AnthonyLatsis AnthonyLatsis removed the triage needed This issue needs more specific labels label Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. found by asan Flag: An issue found by the Address Sanitizer test failure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants