[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

russell.gallop added a comment. I believe that the MCJIT failures are due to bug https:/bugs.llvm.org/show_bug.cgi?id=24978 rather than a problem in the Scudo port so I have added details of how I hit it to that bugzilla and opened two reviews to get this landed:

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

russell.gallop added a comment. I've focussed on the test test-global-init-nonzero-sm-pic.ll which fails writing to an address which (I believe) should be in the .data section but isn't. With some breakpoints in SectionMemoryManager.cpp it appears that this fails when the top 32bits of the

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

russell.gallop added a comment. I managed to get this to fail in the debugger (for the cross-module-sm-pic-a.ll test): 01655e9d001e() Unknown 01655e9d0019() Unknown 017e5eb6b410() Unknown 017c5eb63810() Unknown

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

russell.gallop updated this revision to Diff 320806. russell.gallop added a comment. Remove -fsanitize=scudo support for Windows in LLVM cmake, using LLVM_INTEGRATED_CRT_ALLOC instead. Remove scudo_cxx from LLVM_INTEGRATED_CRT_ALLOC. CHANGES SINCE LAST ACTION

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

aganea added a comment. In D86694#2463429 , @russell.gallop wrote: > - With scudo all lit tests (generally) pass, but there are some intermittent > failures (Access Violation) on the following tests: > >>

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

russell.gallop updated this revision to Diff 312827. russell.gallop edited the summary of this revision. russell.gallop added a comment. Apologies for the delay, I've had other things taking my time. Latest version uploaded. This fixes stage1 lit tests (on Windows and Linux) and adds scudo_cxx

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

cryptoad added a comment. Thank you for all your work! IIRC everything was working except as you point out some of the very Unixy tests. Is your build creating a `clang_rt.scudo_cxx` library that also needs to be link as well? The C++ specific interceptors usually end up in a separate library

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

russell.gallop updated this revision to Diff 301331. russell.gallop added a comment. Herald added a subscriber: dexonsmith. Apologies for the delay, I've had a few other things on. I've worked through the tests. Several I've marked as unsupported on windows as they use things unixy things like

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

russell.gallop added a comment. In D86694#2288660 , @aganea wrote: > I'm also in favor, I think this is good direction ahead. It'd be nice if > following issues were fixed -- in subsequent patches if you wish: > > - Stage1 `ninja check-scudo` fails many

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

aganea added a comment. I'm also in favor, I think this is good direction ahead. It'd be nice if following issues were fixed -- in subsequent patches if you wish: - Stage1 `ninja check-scudo` fails many tests for me, see F13037612: errors.txt . - Stage2

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

cryptoad added a comment. In D86694#2287242 , @russell.gallop wrote: > I'm considering a slight change of plan. @cryptoad, in the name of > incremental development, would you be happy for me to put the scudo sanitizer > version up (with working tests),

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

russell.gallop added a comment. > I'm going to concentrate on the standalone port as I think that's the way > forward. If I get that working then can work through the other issues. I'm considering a slight change of plan. @cryptoad, in the name of incremental development, would you be happy

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

aganea added a comment. In D86694#2277456 , @cryptoad wrote: > I didn't try to make the Exclusive version work, mostly because I was using > the Windows TLS API and the Shared fit right in with those, but it would get > rid of a lot of the contention.

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

cryptoad added a comment. In D86694#2277371 , @aganea wrote: > @cryptoad What happens if the primary was much smaller? Or if pages were > //reserved// in much smaller ranges? The Primary can be made smaller, but this works better with the Standalone

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

aganea added a subscriber: maniccoder. aganea added a comment. In D86694#2274682 , @cryptoad wrote: > In D86694#2274548 , @russell.gallop > wrote: > >> I guess using scudo as a general purpose allocator that could

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

cryptoad added a comment. In D86694#2274548 , @russell.gallop wrote: > I guess using scudo as a general purpose allocator that could set a limit on > the number of cores that can be used at once as @aganea found. Would there be > any problem with

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

russell.gallop added a comment. In D86694#2274383 , @cryptoad wrote: > In D86694#2273815 , @aganea wrote: > >> If 4.4 TB of virtual pages are mapped in each process (this happens on >> startup), then we quickly

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

russell.gallop added a comment. In D86694#2273815 , @aganea wrote: > @russell.gallop I see a lots of failing tests when running `ninja check-all` > on a Scudo-enabled build (stage 2). Do you see the same thing on your end? Not a lot but a few, including

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

cryptoad added a comment. In D86694#2273815 , @aganea wrote: > If 4.4 TB of virtual pages are mapped in each process (this happens on > startup), then we quickly exaust the 48-bit (256 TB) addressable space with > 72+ programs running (on a 36-core).

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

aganea added a comment. @russell.gallop I see a lots of failing tests when running `ninja check-all` on a Scudo-enabled build (stage 2). Do you see the same thing on your end? ==89136==ERROR: Scudo failed to allocate 0x1 (65536) bytes of memory at address 0x4c0090e00 (error code: 1455)

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

russell.gallop added a comment. In D86694#2270433 , @aganea wrote: > Thanks for working on this @russell.gallop! > > I've reproduced your tests, please see below. The only difference is that > I've used a ThinLTO build for stage2: Thanks. It's good to

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

russell.gallop updated this revision to Diff 291511. russell.gallop added a comment. Re-upload patch with G LLVM Github Monorepo set. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86694/new/ https://reviews.llvm.org/D86694 Files:

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

aganea added a comment. Thanks for working on this @russell.gallop! I've reproduced your tests, please see below. The only difference is that I've used a ThinLTO build for stage2: -DCMAKE_CXX_FLAGS="/GS- -Xclang -O3 -fstrict-aliasing -march=skylake-avx512 -flto=thin -fwhole-program-vtables"

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

aganea added a comment. @russell.gallop Do you think you can upload the patch again by setting the Repository to "Monorepo"? F12921062: image.png Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86694/new/

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

cryptoad added a comment. In D86694#2268182 , @russell.gallop wrote: > > Thanks. I'm looking at porting the standalone variant, drawing on D42519 > and Windows support from sanitizer_common. > Does that sound like

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

russell.gallop added a comment. In D86694#2242559 , @cryptoad wrote: > In D86694#2242150 , @russell.gallop > wrote: > >> In D86694#2242140 , @cryptoad wrote: >> >>> That's

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

russell.gallop updated this revision to Diff 291235. russell.gallop edited the summary of this revision. russell.gallop added a comment. Herald added subscribers: phosek, hiraditya. Fixup scudo (sanitizer based) to work on Windows. This makes use of the CRT alloc hooks from D71786

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

cryptoad added a comment. In D86694#2242150 , @russell.gallop wrote: > In D86694#2242140 , @cryptoad wrote: > >> That's awesome! Is it meant to eventually be committed or only be used for >> comparison purposes?

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

russell.gallop added a comment. In D86694#2242150 , @russell.gallop wrote: > You marked D42519 as WIP, can you remember > what was still TBD? Maybe more tests could be enabled... Repository: rG LLVM Github Monorepo

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

russell.gallop added a comment. In D86694#2242140 , @cryptoad wrote: > That's awesome! Is it meant to eventually be committed or only be used for > comparison purposes? I'd like it to be committed, but can't claim I know the code from

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

cryptoad added a comment. That's awesome! Is it meant to eventually be committed or only be used for comparison purposes? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86694/new/ https://reviews.llvm.org/D86694

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

russell.gallop created this revision. russell.gallop added reviewers: cryptoad, aganea, hans. Herald added subscribers: llvm-commits, Sanitizers, cfe-commits, mgorny. Herald added projects: clang, Sanitizers, LLVM. russell.gallop requested review of this revision. This is basically