This is an archive of the discontinued LLVM Phabricator instance.

Enable Asan on Windows
Needs ReviewPublic

Authored by akhaldi on Sep 16 2014, 6:28 AM.

Details

Reviewers
compnerd
timurrrr

Diff Detail

Event Timeline

akhaldi updated this revision to Diff 13747.Sep 16 2014, 6:28 AM
akhaldi retitled this revision from to Enable Asan on Windows.
akhaldi updated this object.
akhaldi edited the test plan for this revision. (Show Details)
akhaldi added a reviewer: timurrrr.
akhaldi added a subscriber: samsonov.
timurrrr edited edge metadata.Sep 16 2014, 6:37 AM
timurrrr added a subscriber: Unknown Object (MLST).

[Sorry I forgot to mention - please add llvm-commits@ to subscribers for all your compiler-rt patches]

The patch looks mostly fine for me, I'll refine it slightly and commit soon (when I have some extra time).

Can you please confirm that this patch doesn't regress "ninja check-all" in your build environment?
(i.e. it's fine if most things that were broken remain broken; it's not OK if anything else starts to fail)

timurrrr added inline comments.Sep 16 2014, 6:43 AM
lib/asan/CMakeLists.txt
200

Is there an equivalent of /Zl (Omit Default Library Name) in non-MSVC compilers?

lib/asan/asan_win_dll_thunk.cc
27

maybe DEBUG_BREAK is more appropriate, what do you think?

akhaldi added inline comments.Sep 16 2014, 7:01 AM
lib/asan/CMakeLists.txt
200

Hmm I don't think so, it looks pretty MSVC centric.

lib/asan/asan_win_dll_thunk.cc
27

It's more descriptive, okay.

rnk added a subscriber: rnk.Sep 16 2014, 9:27 AM
rnk added inline comments.
lib/asan/asan_win_dll_thunk.cc
29

I think you need volatile or some other constraint to prevent gcc from removing this.

akhaldi updated this revision to Diff 13757.Sep 16 2014, 10:26 AM
akhaldi edited edge metadata.
emaste added a subscriber: emaste.Sep 17 2014, 6:42 AM
samsonov added inline comments.Sep 17 2014, 4:08 PM
lib/asan/CMakeLists.txt
198

Instead, add a check for this flag existence in cmake/config-ix.cmake, and append it if it's known:

set(ASAN_THUNK_CFLAGS ${ASAN_CFLAGS} -DASAN_DYNAMIC_RUNTIME_THUNK)
append_if(COMPILER_RT_HAS_Zl_FLAG "-Zl" ASAN_THUNK_CFLAGS)

or if it's always available in MSVC and should be added only there, use

append_if(MSVC -Zl ASAN_THUNK_CFLAGS)

if it's always available in MSVC and should be added only there, use

append_if(MSVC -Zl ASAN_THUNK_CFLAGS)

It is available since VS2003 at least, so we can consider it as always.
Can you please use ASAN_DYNAMIC_RUNTIME_THUNK _CFLAGS?