VS_USE_SPECTRE_MITIGATION_RUNTIME should prefer the Spectre library path, not use it exclusively #73
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The problem here is that the fallback would happen for the Spectre runtime, too. So if someone asked for the Spectre runtime and VS didn't have it installed, then the non-Spectre runtime would be picked-up silently. Which is a problem. #40 tracks error'ing if the Spectre runtime is asked for but not installed. Including that fix here would probably help. |
MarkSchofield
force-pushed
the
mschofie/spectre-asan
branch
from
0b55718 to
a77ae6f
Compare
…ath, not use it exclusively
MarkSchofield
force-pushed
the
mschofie/spectre-asan
branch
from
a77ae6f to
f87dc29
Compare
|
This PR includes the work to FATAL_ERROR if the Spectre runtimes aren't installed. But in order to do that, there - realistically - needs to be one CMake option for each 'unit' of Spectre runtime. The VS installer separates the C/C++ runtime from the ATL/MFC runtime, so I'm using |
geraldcombs
pushed a commit
to wireshark/wireshark
that referenced
this pull request
May 18, 2024
Problem: - SPECTRE-mitigation (/Qspectre) is enabled by default but this does not allow ASAN (-fsanitize=address) to be enabled because of failing compiler checks for ASAN_FLAG. This problem can also be reproduced with a C++ console application sample using Visual Studio 2019/2022 where it will report linking error related to ASAN library if both the options are enabled. Cause: - This happens because the ASAN libraries are not present in the path that is used by the linker when SPECTRE-mitigation is enabled. - ASAN libraries are present in VS_TOOLSET_PATH/lib/<arch> but are not present in VS_TOOLSET_PATH/lib/spectre/<arch> which is the path used by SPECTRE-mitigation libraries. - Following links also talk about the same problem. MarkSchofield/WindowsToolchain#72 MarkSchofield/WindowsToolchain#73 Fixes: - First fix is to disable SPECTRE-mitigation when ASAN is requested and is considered a default fix by this change. - Second fix is to add path where ASAN libraries are present and is activated only when ENABLE_ASAN_WITH_SPECTRE is provided on the CMake command line along with ENABLE_ASAN. Note: - Both the fixes are applicable for Debug and RelWithDebInfo build types only because they provide the necessary debug information required for ASAN to be functional. If ASAN is enabled on Release and MinSizeRel build types too, then warning C5072 gets generated causing build failure (because this warning is treated as an error). Example: - error C2220: the following warning is treated as an error - warning C5072: ASAN enabled without debug information emission. Enable debug info for better ASAN error reporting. Important: - Checking if ASAN is supported requires SPECTRE-mitigation to be disabled globally impacting all build types. If ASAN is supported, then it is re-enabled selectively depending on the build types. - As per /libpath documentation, path provided to it is used before all the other paths. This causes SPECTRE-mitigated libraries to be skipped if non-mitigated versions are present in the ASAN library path. For example, comsuppw.lib is present in both the paths. If SPECTRE-mitigation is enabled (along with ASAN), and only ASAN library path is added, then comsuppw.lib from ASAN library path is used! Summary: - For First or Default Fix - Debug and RelWithDebInfo: SPECTRE-mitigation stays disabled ASAN is enabled - Release and MinSizeRel: SPECTRE-mitigation is re-enabled ASAN is skipped - For Second Fix - Debug and RelWithDebInfo: SPECTRE-mitigation is re-enabled ASAN is enabled - Release and MinSizeRel: SPECTRE-mitigation is re-enabled ASAN is skipped
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
VS_USE_SPECTRE_MITIGATION_RUNTIMEinstructs the toolchain to switch the library include path for the compiler runtime to aspectresub-directory. When compiling with MSVC's ASAN support (i.e./fsanitize=address) the compilation depends on.libfiles that aren't present in thespectresub-directory, but are present in the default library include path, breaking ASAN builds ifVS_USE_SPECTRE_MITIGATION_RUNTIMEis used. IfVS_USE_SPECTRE_MITIGATION_RUNTIMEwere to prefer thespectresubdirectory - rather than using it exclusively - then the ASAN libraries would still be found.