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

Allow use of AddressSanitizer on Windows by linking to existing libraries #89369

Closed
wants to merge 1 commit into from

Conversation

danielframpton
Copy link
Contributor

This change makes it possible to use address sanitizer on Windows by determining the right library name to use based on the way that the crt is being linked.

Currently, this change does not build the address sanitizer libraries (as we do for other platforms) so it is necessary to provide the libraries either from building those libraries from llvm or using other libraries from your C++ toolchain.

The current support for AddressSanitizer outside of windows involves renaming things compiled out of the llvm fork. I don't quite follow how this will work when sanitizing hybrid C/C++/Rust programs, and the approach causes additional problems on Windows because the renamed import library still references the dll with the original name.

I expect some of these questions will need to be resolved as part of stabilization (see #47174) but I wanted to make some progress on the Windows support as mentioned there and in other issues like #89339 so that we can see how this will all fit together.

@rust-highfive
Copy link
Collaborator

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 29, 2021
@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 14, 2021
@wesleywiser
Copy link
Member

wesleywiser commented Oct 28, 2021

Re-assigning to myself per compiler team triage meeting.

r? @wesleywiser

@wesleywiser wesleywiser assigned wesleywiser and unassigned estebank Oct 28, 2021
@bors
Copy link
Contributor

bors commented Nov 7, 2021

☔ The latest upstream changes (presumably #90668) made this pull request unmergeable. Please resolve the merge conflicts.

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 5, 2021
@apiraino
Copy link
Contributor

Reporting some sparse notes from the T-compiler discussion on Zulip of a while ago.

Here a Windows / ASAN knowledgeable person would help with the review, possibly opening an MCP or similar. The general consensus was that the Windows / Microsoft people have better context here, perhaps drafting a writeup of the tradeoffs in the tracking issue.

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 6, 2022
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 6, 2022
@wesleywiser wesleywiser added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 10, 2022
@Dylan-DPC
Copy link
Member

@danielframpton any updates on this?

@danielframpton
Copy link
Contributor Author

I don't have an update, but I may get some more cycles to look at this soon. If anyone gets to it first, then I would be happy to discuss any details of this change or what we need to have things working on Windows.

I think in addition to this change we may want to do the work to also build the runtimes for Windows (with correct dll names) and also make it possible to configure an override for the runtime libraries to use (to support hybrid Rust and C/C++ scenarios).

@JohnCSimon
Copy link
Member

@danielframpton
Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this.
Note: if you do please open the PR BEFORE you force push to it, else you won't be able to reopen.

@rustbot label: +S-inactive

@JohnCSimon JohnCSimon closed this May 7, 2022
@rustbot rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label May 7, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 3, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Enable address sanitizer for MSVC targets using INFERASANLIBS linker flag

This enables address sanitizer for x86_64-pc-windows-msvc and i686-pc-windows-msvc targets when linked with the MSVC linker (link.exe) by leveraging the `/INFERASANLIBS` option to automatically find and link in Microsoft's address sanitizer runtime: <https://learn.microsoft.com/en-us/cpp/sanitizers/asan-runtime?view=msvc-170>

Implements rust-lang/compiler-team#702
Fixes rust-lang#89339 (for MSVC targets using the MSVC linker only)
Supercedes rust-lang#89369

Successful x86_64-msvc build showing the sanitizer tests working: https://github.com/rust-lang/rust/actions/runs/7228346880/job/19697628258?pr=118521
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Jan 4, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Enable address sanitizer for MSVC targets using INFERASANLIBS linker flag

This enables address sanitizer for x86_64-pc-windows-msvc and i686-pc-windows-msvc targets when linked with the MSVC linker (link.exe) by leveraging the `/INFERASANLIBS` option to automatically find and link in Microsoft's address sanitizer runtime: <https://learn.microsoft.com/en-us/cpp/sanitizers/asan-runtime?view=msvc-170>

Implements rust-lang/compiler-team#702
Fixes rust-lang#89339 (for MSVC targets using the MSVC linker only)
Supercedes rust-lang#89369

Successful x86_64-msvc build showing the sanitizer tests working: https://github.com/rust-lang/rust/actions/runs/7228346880/job/19697628258?pr=118521
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 4, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Enable address sanitizer for MSVC targets using INFERASANLIBS linker flag

This enables address sanitizer for x86_64-pc-windows-msvc and i686-pc-windows-msvc targets when linked with the MSVC linker (link.exe) by leveraging the `/INFERASANLIBS` option to automatically find and link in Microsoft's address sanitizer runtime: <https://learn.microsoft.com/en-us/cpp/sanitizers/asan-runtime?view=msvc-170>

Implements rust-lang/compiler-team#702
Fixes rust-lang#89339 (for MSVC targets using the MSVC linker only)
Supercedes rust-lang#89369

Successful x86_64-msvc build showing the sanitizer tests working: https://github.com/rust-lang/rust/actions/runs/7228346880/job/19697628258?pr=118521
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 4, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Rollup merge of rust-lang#118521 - dpaoliello:asan, r=wesleywiser

Enable address sanitizer for MSVC targets using INFERASANLIBS linker flag

This enables address sanitizer for x86_64-pc-windows-msvc and i686-pc-windows-msvc targets when linked with the MSVC linker (link.exe) by leveraging the `/INFERASANLIBS` option to automatically find and link in Microsoft's address sanitizer runtime: <https://learn.microsoft.com/en-us/cpp/sanitizers/asan-runtime?view=msvc-170>

Implements rust-lang/compiler-team#702
Fixes rust-lang#89339 (for MSVC targets using the MSVC linker only)
Supercedes rust-lang#89369

Successful x86_64-msvc build showing the sanitizer tests working: https://github.com/rust-lang/rust/actions/runs/7228346880/job/19697628258?pr=118521
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants