Details

Reviewers

Brecht Van Lommel (brecht)
Sergey Sharybin (sergey)

Commits

rBf9eaf93d3795: MSVC: ASAN support for VS 16.9

Summary

Requirements

VS 2019 16.7 preview 6 or newer
Asan libraries are off by default in the installer, you need to manually enable them in the C++ workload
Toggle with the cmake option WITH_COMPILER_ASAN as usual (by default enabled in the developer profile)

This diff enables building with ASAN, there is a small list of issues it brings up in testing mostly coming out of glog and eigen, unsure if those are real issues or not however that should not stop other developers from making use of asan on windows, and solving these issues should not be part of this diff.

Diff Detail

Repository

rB Blender

Branch

tmp_asan_2019pre4 (branched from master)

Build Status

Buildable 13304
Build 13304: arc lint + arc unit

Event Timeline

Ray Molenkamp (LazyDodo) requested review of this revision.May 20 2020, 2:25 AM

Ray Molenkamp (LazyDodo) created this revision.

Harbormaster completed remote builds in B8129: Diff 24947.May 20 2020, 2:25 AM

not quite ready for review yet, but too useful not to share.

Fix building tests
Fix building the various build configurations/

Harbormaster completed remote builds in B8139: Diff 24968.May 20 2020, 7:21 PM

Ray Molenkamp (LazyDodo) edited the summary of this revision. (Show Details)May 20 2020, 7:24 PM

move the asan cmake option out of the compiler check.
Merge remote-tracking branch 'origin/master' into arcpatch-D7794

Harbormaster completed remote builds in B8708: Diff 26259.Jun 25 2020, 1:41 AM

Ray Molenkamp (LazyDodo) edited the summary of this revision. (Show Details)Jun 25 2020, 1:41 AM

Ray Molenkamp (LazyDodo) added reviewers: Brecht Van Lommel (brecht), Sergey Sharybin (sergey).

bad testing on my part, its still broken with preview 3

Ray Molenkamp (LazyDodo) abandoned this revision.Jul 3 2020, 10:26 PM

Update to latest master and final testing

Harbormaster completed remote builds in B9408: Diff 27526.Aug 7 2020, 7:18 PM

Ready for review now.

From reading seems fine.
Can give it a test on Windows later on if you think it worth it.

This revision is now accepted and ready to land.Aug 10 2020, 9:30 AM

There's some small changes needed since we moved some of the gtest stuff around, however during final testing it keeps spewing stack buffer overflows that i know for a fact are wrong.

to make matters worse, i'm seemingly unable to extract a repro case to submit a bug

Not sure how we want to proceed here, there's one of these bugs in the startup codepath where i can work around it by turning a stack allocation into a heap allocation however generally when eigen gets involved later on in the code msvc's asan just craps out

i'm torn here, i want to enable this for other developers to use and unless you encounter the bug "it works great" but if you do you'll be distracted by an army of red herrings

nothing quite as bad as a tool you cannot trust

nothing quite as bad as a tool you cannot trust

Maybe it means we should wait until it stabilizes?

Merge remote-tracking branch 'origin/master' into tmp_asan_cleanup

Harbormaster completed remote builds in B10907: Diff 30348.Oct 25 2020, 7:06 PM

Quick update for broader testing, not ready for review yet.

Update to 16.8.

Sadly things are still broken in debug mode

This revision is now accepted and ready to land.Nov 11 2020, 9:57 PM

Harbormaster completed remote builds in B11234: Diff 30985.Nov 11 2020, 9:57 PM

still not OK for general use.

Known issue: blendthumb does not link in the relwithdebinfo config

Merge remote-tracking branch 'origin/master' into arcpatch-D7794_2

This revision is now accepted and ready to land.Dec 17 2020, 3:19 PM

Harbormaster completed remote builds in B11800: Diff 32106.Dec 17 2020, 3:19 PM

Remove unneeded asan libraries (16.9 pre-2 and newer will automatically link the required libs)
Allow Ninja+msvc+asan
Merge remote-tracking branch 'origin/master' into arcpatch-D7794_2

Harbormaster completed remote builds in B11839: Diff 32209.Dec 21 2020, 8:48 AM

Good news, bad news:

Good: Issue is confirmed fixed in 16.9 pre2
Bad: There's currently no easy way to distinguish 16.8 and 16.9 (both report 1928 currently) so landing this would cause issues with 16.8 even if accidentally enabled (ie though the developer profile)

So I'm going so sit on this for a little while longer, any devs requiring ASAN on windows can apply this patch manually but be mindful of the requirements, you *need* 16.9pre2+

after applying this patch you can use the pre-release compiler and enable asan using

make 2019pre asan any_other_options_you_normaly_pass

Were you actually able to start blender normally on your side? I'm having a bit of trouble unless I _really_ force things:

I did a make full nobuild 2019pre asan configure and tried building both a Debug and Release (it's the same with a lite configure too)
After building I attempt to run the executable and it just hangs. Waited 10+ minutes. No UI comes up at all.
Trying to debug in VS actually hits a similar issue I filed for my own projects some weeks ago -- a breakpoint, not really ignorable, inside RtlValidateHeap: Dev Community Link
Tonight I found that if I just hold F5 down and try to continue from the above the UI does comes up, but the RtlValidateHeap breaks will still be going on. Once you see the blender window with a proper icon in the taskbar you can go to the next step
At that point I can "Detach" the debugger and run normally

But yay! It kinda does sorta work so I'm hopeful that 16.9 will get things to a fully working state.

In D7794#247785, @Jesse Yurkovich (deadpin) wrote:

Were you actually able to start blender normally on your side? I'm having a bit of trouble unless I _really_ force things:

I only tested from the command line + unit tests, where it seems to work ok for me, there's some flaky asan crashes randomly popping up (ie from fsmenu_read_system) but i have not determined yet if that is 'us' or 'them'

I did a make full nobuild 2019pre asan configure and tried building both a Debug and Release (it's the same with a lite configure too)

more or less same, when cli building I prefer ninja but realistically that should not make a difference.

After building I attempt to run the executable and it just hangs. Waited 10+ minutes. No UI comes up at all.

Seems ok here, instant grey screen that sticks around a little longer than usually (2-3 sec at best) and then the UI pops up.

Trying to debug in VS actually hits a similar issue I filed for my own projects some weeks ago -- a breakpoint, not really ignorable, inside RtlValidateHeap: Dev Community Link

Tonight I found that if I just hold F5 down and try to continue from the above the UI does comes up, but the RtlValidateHeap breaks will still be going on. Once you see the blender window with a proper icon in the taskbar you can go to the next step

Had not tested the debugger, seems to crash the debugger on RegisterDragDrop holding F5 does let it go on but will get unhappy inside the messageloop which is harder to bypass.

At that point I can "Detach" the debugger and run normally

But yay! It kinda does sorta work so I'm hopeful that 16.9 will get things to a fully working state.

It's slow progress but we're getting there....eventually..

It has taken almost a year, but ASAN finally works as it should with VS2019 16.9 preview 4+

this is finally ready for review

This revision is now accepted and ready to land.Feb 19 2021, 2:58 AM

Harbormaster completed remote builds in B12991: Diff 34154.Feb 19 2021, 2:58 AM

Brecht Van Lommel (brecht) added inline comments.Feb 22 2021, 2:56 PM

build_files/cmake/platform/platform_win32.cmake

161–170

I think the logic should be:

if(WITH_COMPILER_ASAN)
  if(MSVC AND NOT MSVC_CLANG AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 19.28.29828)
    ..
  else()
    message("WITH_COMPILER_ASAN not supported on MSVC ${CMAKE_CXX_COMPILER_VERSION}")
  endif()
endif()

Germano Cavalcante (mano-wii) added a subscriber: Germano Cavalcante (mano-wii).Feb 22 2021, 3:49 PM

Merge remote-tracking branch 'origin/master' into tmp_asan_2019pre4
Fix warning about ASAN being displayed in wrong circumstances

Harbormaster completed remote builds in B13304: Diff 34850.Mar 7 2021, 6:08 PM

Ray Molenkamp (LazyDodo) added inline comments.Mar 7 2021, 6:13 PM

build_files/cmake/platform/platform_win32.cmake
161–170	Yes but also no.. the warning should only display if we're sure we are on MSVC (so not clang) ASAN is on and we are on a "bad" version rather than when any of the conditions is false. good job spotting it!

This patch has an approved status however it was not committed yet. So I'm assuming the other reviewers are blocking. Updating the reviewers list to reflect this. This way the patch status still show as Need Review.

This revision now requires review to proceed.Mar 26 2021, 5:57 PM

Brecht had one minor comment, i do not deem his review of the fix blocking , i will land this over the weekend

Evan Wilson (EAW) mentioned this in T86952: Heap Buffer Overflow when viewing dds thumbnails in the file browser..Mar 27 2021, 6:49 AM

This revision was not accepted when it landed; it landed in state Needs Review.Mar 30 2021, 3:11 AM

Closed by commit rBf9eaf93d3795: MSVC: ASAN support for VS 16.9 (authored by Ray Molenkamp (LazyDodo)). · Explain Why

This revision was automatically updated to reflect the committed changes.

Ray Molenkamp (LazyDodo) added a commit: rBf9eaf93d3795: MSVC: ASAN support for VS 16.9.

MSVC: Preliminary ASAN support.
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 34850

CMakeLists.txt

build_files/cmake/platform/platform_win32.cmake

build_files/windows/configure_msbuild.cmd

build_files/windows/configure_ninja.cmd

source/blender/blendthumb/CMakeLists.txt

source/blender/blenlib/BLI_asan.h

source/creator/CMakeLists.txt

MSVC: Preliminary ASAN support.ClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 34850

CMakeLists.txt

build_files/cmake/platform/platform_win32.cmake

build_files/windows/configure_msbuild.cmd

build_files/windows/configure_ninja.cmd

source/blender/blendthumb/CMakeLists.txt

source/blender/blenlib/BLI_asan.h

source/creator/CMakeLists.txt

MSVC: Preliminary ASAN support.
ClosedPublic
Actions

Revision Contents
Changeset List