[PATCH] D119711: Add asan support for MSVC debug runtimes
rnk added subscribers: thieta, mstorsjo. rnk added a comment. + @mstorsjo @thieta Since we were talking about the complexity involved in choosing the correct ASan runtime, there is actually even more involved. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119711/new/ https://reviews.llvm.org/D119711 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D119711: Add asan support for MSVC debug runtimes
DanWillans added a comment. Herald added a subscriber: MaskRay. Herald added a project: All. Hi all, Has there been any progress on this? I want to run clang-tidy on some code being compiled in debug with MSVC but I hit the clang-diagnostic-error note 'D.Diag(clang::diag::note_drv_address_sanitizer_debug_runtime)'. Later versions of MSVC support ASan with debug runtime now Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119711/new/ https://reviews.llvm.org/D119711 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D119711: Add asan support for MSVC debug runtimes
lo1ol added a comment.
Ok, I get current situation.
Sorry for my late answer. I made some investigation yesterday and compared msvc
and clang versions of asan. In my tests msvc version seems more stable.
All my tests is written via catch2 framework (v2.13.8). So, first example is:
#define CATCH_CONFIG_MAIN
#include
#include
TEST_CASE( "kek" ) {
for (int i=0; i < 10; ++i) {
printf("lol ke\n");
free(malloc(4472));
}
}
If compiles it via msvc version of asan, everything is ok:
# copy MSVC\14.29.30133\lib\x64 to Llvm\x64\lib\clang\12.0.0\lib\windows
before
$ clang-cl kek.cpp -I ..\Catch2\single_include /MT -fsanitize=address
$ kek.exe
lol ke
lol ke
lol ke
lol ke
lol ke
lol ke
lol ke
lol ke
lol ke
lol ke
===
test cases: 1 | 1 passed
assertions: - none -
But version for clang's toolchain has strange behavior:
# With original content of Llvm\x64\lib\clang\12.0.0\lib\windows
$ clang-cl kek.cpp -I ..\Catch2\single_include /MT -fsanitize=address
$ kek.exe
lol ke
lol ke
lol ke
lol ke
lol ke
~~~
~~~
~~~
~~~
~~~
...
Nevertheless, I found some bug which presents inside both versions
#define CATCH_CONFIG_MAIN
#include
#include
#include
TEST_CASE( "kek" ) {
auto result = std::async(std::launch::async, []() {
std::cerr << "kek " << std::endl;
});
result.get();
}
The output is same for both versions:
$ clang-cl kek.cpp -I ..\Catch2\single_include /MT -fsanitize=address
$ kek.exe
~~~
~~~
~~~
~~~
...
I don't have a time to discover a root of problem. But msvc's version of asan
seems to me more stable and compatible with clang's asan. So, may be there is
point to distribute same version of asan for both toolchains.
If you want, I may create an issue for founded bugs.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119711/new/
https://reviews.llvm.org/D119711
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D119711: Add asan support for MSVC debug runtimes
cbezault added a comment. Imo I agree that this shouldn’t be merged until the debug variants of the asan runtime are getting built publicly. @stevewishnousky Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119711/new/ https://reviews.llvm.org/D119711 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D119711: Add asan support for MSVC debug runtimes
rnk added subscribers: cbezault, vitalybuka. rnk added a comment. I'm hesitant to do this. The main reason we disabled the use of the debug runtimes was that the debug runtimes interfered with malloc interception, which leads to crashes and a generally poor user experience. I'd like some confirmation beyond just your word that this is addressed. After that, this is kind of scary because we don't build and ship these "dbg" variants of the ASan runtime (so far as I know). They are only provided as part of the MSVC CRT. Is LLVM ASan actually compatible with the ASan runtimes provided by MSVC? I'm guessing the answer that things appear to work, but so far as I know, there is no usptream support for this. If things break, there's nobody who will fix them. I have reached out to Microsoft to ask them to help contribute upstream, but so far I haven't seen major contributions. This change is one small step towards encouraging users to combine clang-cl with the Microsoft provided ASan runtimes, and I'm not sure we want to guide users in that direction. We only promise interop with compiler runtime libraries that we vend. Other stakeholders: +@cbezault @vitalybuka Comment at: clang/lib/Driver/SanitizerArgs.cpp:829 - << lastArgumentForMask(D, Args, SanitizerKind::Address); - D.Diag(clang::diag::note_drv_address_sanitizer_debug_runtime); -} This is the only usage of this note, and if you remove it, the note should be removed as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119711/new/ https://reviews.llvm.org/D119711 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D119711: Add asan support for MSVC debug runtimes
lo1ol created this revision.
lo1ol added a reviewer: rnk.
lo1ol requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Use special debug asan versions (prefixed with asan_dbg) for MSVC debug
runtimes (/MTd, /MDd, /LDd).
Resolves:
https://github.com/llvm/llvm-project/compare/main...lo1ol:asan_dbg?expand=1
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D119711
Files:
clang/lib/Driver/SanitizerArgs.cpp
clang/lib/Driver/ToolChains/MSVC.cpp
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -553,13 +553,20 @@
CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
}
+ bool DebugRuntime = Args.hasArg(options::OPT__SLASH_MTd) ||
+ Args.hasArg(options::OPT__SLASH_MDd) ||
+ Args.hasArg(options::OPT__SLASH_LDd);
+
+ std::string AsanPrefix = DebugRuntime ? "asan_dbg" : "asan";
+ std::string AsanCXX = DebugRuntime ? "asan_cxx_dbg" : "asan_cxx";
+
if (TC.getSanitizerArgs(Args).needsAsanRt()) {
CmdArgs.push_back(Args.MakeArgString("-debug"));
CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
if (TC.getSanitizerArgs(Args).needsSharedRt() ||
Args.hasArg(options::OPT__SLASH_MD, options::OPT__SLASH_MDd)) {
- for (const auto : {"asan_dynamic", "asan_dynamic_runtime_thunk"})
-CmdArgs.push_back(TC.getCompilerRTArgString(Args, Lib));
+ for (const auto : {"_dynamic", "_dynamic_runtime_thunk"})
+CmdArgs.push_back(TC.getCompilerRTArgString(Args, AsanPrefix + Lib));
// Make sure the dynamic runtime thunk is not optimized out at link time
// to ensure proper SEH handling.
CmdArgs.push_back(Args.MakeArgString(
@@ -568,12 +575,14 @@
: "-include:__asan_seh_interceptor"));
// Make sure the linker consider all object files from the dynamic
runtime
// thunk.
- CmdArgs.push_back(Args.MakeArgString(std::string("-wholearchive:") +
- TC.getCompilerRT(Args, "asan_dynamic_runtime_thunk")));
+ CmdArgs.push_back(Args.MakeArgString(
+ std::string("-wholearchive:") +
+ TC.getCompilerRT(Args, AsanPrefix + "_dynamic_runtime_thunk")));
} else if (DLL) {
- CmdArgs.push_back(TC.getCompilerRTArgString(Args, "asan_dll_thunk"));
+ CmdArgs.push_back(
+ TC.getCompilerRTArgString(Args, AsanPrefix + "_dll_thunk"));
} else {
- for (const auto : {"asan", "asan_cxx"}) {
+ for (const auto : {AsanPrefix, AsanCXX}) {
CmdArgs.push_back(TC.getCompilerRTArgString(Args, Lib));
// Make sure the linker consider all object files from the static lib.
// This is necessary because instrumented dlls need access to all the
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -814,23 +814,6 @@
}
}
-if (Arg *WindowsDebugRTArg =
-Args.getLastArg(options::OPT__SLASH_MTd, options::OPT__SLASH_MT,
-options::OPT__SLASH_MDd, options::OPT__SLASH_MD,
-options::OPT__SLASH_LDd, options::OPT__SLASH_LD)) {
- switch (WindowsDebugRTArg->getOption().getID()) {
- case options::OPT__SLASH_MTd:
- case options::OPT__SLASH_MDd:
- case options::OPT__SLASH_LDd:
-if (DiagnoseErrors) {
- D.Diag(clang::diag::err_drv_argument_not_allowed_with)
- << WindowsDebugRTArg->getAsString(Args)
- << lastArgumentForMask(D, Args, SanitizerKind::Address);
- D.Diag(clang::diag::note_drv_address_sanitizer_debug_runtime);
-}
- }
-}
-
AsanUseAfterScope = Args.hasFlag(
options::OPT_fsanitize_address_use_after_scope,
options::OPT_fno_sanitize_address_use_after_scope, AsanUseAfterScope);
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -553,13 +553,20 @@
CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
}
+ bool DebugRuntime = Args.hasArg(options::OPT__SLASH_MTd) ||
+ Args.hasArg(options::OPT__SLASH_MDd) ||
+ Args.hasArg(options::OPT__SLASH_LDd);
+
+ std::string AsanPrefix = DebugRuntime ? "asan_dbg" : "asan";
+ std::string AsanCXX = DebugRuntime ? "asan_cxx_dbg" : "asan_cxx";
+
if (TC.getSanitizerArgs(Args).needsAsanRt()) {
CmdArgs.push_back(Args.MakeArgString("-debug"));
CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
if (TC.getSanitizerArgs(Args).needsSharedRt() ||
Args.hasArg(options::OPT__SLASH_MD, options::OPT__SLASH_MDd)) {
- for (const auto
