[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-03-09 Thread Mitch Phillips via Phabricator via cfe-commits
hctim added a comment. In D96120#2615085 , @russell.gallop wrote: > In D96120#2597877 , @kcc wrote: > >> I am afraid we will have to delete the older (non-standalone) variant >> entirely. >> (And the sooner the b

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-03-09 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop abandoned this revision. russell.gallop added a comment. In D96120#2597877 , @kcc wrote: > I am afraid we will have to delete the older (non-standalone) variant > entirely. > (And the sooner the better) Okay, think we found a few things o

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-03-02 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added inline comments. Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:489 + if (TC.getSanitizerArgs().needsScudoRt()) { +for (const auto &Lib : {"scudo", "scudo_cxx"}) { We need this for both legacy and standalone scudo? C

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-03-02 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added reviewers: kcc, pcc, vitalybuka. kcc added a comment. We can't possibly maintain two variants of scudo. All effort is currently spent on the newer (standalone) version. I am afraid we will have to delete the older (non-standalone) variant entirely. (And the sooner the better) CHANGE

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-03-01 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D96120#2591368 , @mstorsjo wrote: > Yes, it would need something similar - I tried whipping something together, > which after some tweaks seems to work: > Feel free to squash that into your patch (which saves me a bit of

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-03-01 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop updated this revision to Diff 327123. russell.gallop added a comment. Update with suggested changes to MinGW.cpp CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96120/new/ https://reviews.llvm.org/D96120 Files: clang/lib/Driver/ToolChains/MSVC.cpp clang/lib/Driver/Tool

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-02-27 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D96120#2591418 , @vitalybuka wrote: >>> This is intended as a step to porting scudo standalone. > > Why this is needed for scudo stadalone? It’s not strictly needed. It seemed like it was easier, as some of the work had

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-02-26 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added a comment. >> This is intended as a step to porting scudo standalone. Why this is needed for scudo stadalone? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96120/new/ https://reviews.llvm.org/D96120 __

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-02-26 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. In D96120#2590587 , @russell.gallop wrote: > Hi @mstorsjo. Thanks for the suggestions. I tried running up an mingw > environment with msys but had trouble getting it working (running into cmake > issues). Would you be able to h

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-02-26 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop marked 2 inline comments as done. russell.gallop added a comment. In D96120#2550941 , @mstorsjo wrote: > In D96120#2550876 , @russell.gallop > wrote: > >> In D96120#2546077

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-02-26 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop updated this revision to Diff 326716. russell.gallop added a comment. Added comment on AllocatorSize. Applied Mingw changes suggested by @mstorsjo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96120/new/ https://reviews.llvm.org/D9

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-02-09 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. In D96120#2550876 , @russell.gallop wrote: > In D96120#2546077 , @mstorsjo wrote: > >> As is, this breaks compilation for mingw. With the three modifications I >> suggest here, it no long

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-02-09 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D96120#2545151 , @aganea wrote: > A few questions: Does this work on x86 targets? I haven't tried. I'll test this out. > Are the scudo tests below being built with /MD or with /MT? The lit tests? They don't specify eit

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-02-09 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D96120#2546077 , @mstorsjo wrote: > As is, this breaks compilation for mingw. With the three modifications I > suggest here, it no longer breaks compilation for me - I have no idea if it > actually works in mingw config

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-02-05 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. As is, this breaks compilation for mingw. With the three modifications I suggest here, it no longer breaks compilation for me - I have no idea if it actually works in mingw configurations though, but not breaking compilation is at least the first step. =

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-02-05 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. A few questions: Does this work on x86 targets? Are the scudo tests below being built with /MD or with /MT? Would this change compile/work on MinGW as well? Comment at: compiler-rt/lib/scudo/scudo_platform.h:72 +#elif SANITIZER_WINDOWS +const uptr Alloc

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-02-05 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop created this revision. russell.gallop added reviewers: cryptoad, aganea. Herald added subscribers: phosek, mgorny. russell.gallop requested review of this revision. Herald added projects: clang, Sanitizers. Herald added subscribers: Sanitizers, cfe-commits. Based on https://reviews.