This is an archive of the discontinued LLVM Phabricator instance.

Add asan support for MSVC debug runtimes
Needs ReviewPublic

Authored by lo1ol on Feb 14 2022, 6:18 AM.

Details

Reviewers
rnk
Summary

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

Diff Detail

Event Timeline

lo1ol requested review of this revision.Feb 14 2022, 6:18 AM
lo1ol created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 14 2022, 6:18 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

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

clang/lib/Driver/SanitizerArgs.cpp
829

This is the only usage of this note, and if you remove it, the note should be removed as well.

cbezault added a comment.EditedFeb 14 2022, 12:10 PM

Imo I agree that this shouldn’t be merged until the debug variants of the asan runtime are getting built publicly.

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 <catch2/catch.hpp>

#include <iostream>

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 <catch2/catch.hpp>

#include <iostream>
#include <future>

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.

DanWillans added a subscriber: DanWillans.EditedJun 4 2023, 1:02 AM

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. Maybe I'm doing something wrong?

Herald added a project: Restricted Project. · View Herald TranscriptJun 4 2023, 1:02 AM
Herald added a subscriber: MaskRay. · View Herald Transcript

+ @mstorsjo @thieta

Since we were talking about the complexity involved in choosing the correct ASan runtime, there is actually even more involved.