russell.gallop marked 2 inline comments as done. russell.gallop added a comment.
In D96120#2550941 <https://reviews.llvm.org/D96120#2550941>, @mstorsjo wrote: > In D96120#2550876 <https://reviews.llvm.org/D96120#2550876>, @russell.gallop > wrote: > >> In D96120#2546077 <https://reviews.llvm.org/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 configurations though, but not breaking compilation >>> is at least the first step. >> >> Thanks for the information I'll try to run up a mingw environment and check >> it works. > > In case that turns out to be tricky, I might be able to help with that, at > least for building a simple test program with it and running it, if you say > how it's supposed to be used (building/linking with `-fsanitize=scudo`?) and > how to inspect whether it actually works. 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 help? Yes, building and linking a program with -fsanitize=scudo is the simple way to build a simple program. The support for this is in clang/lib/Driver/ToolChains/MSVC.cpp. Would this require support in clang/lib/Driver/ToolChains/MinGW.cpp? I think the best way to test is to run the scudo LIT tests with (e.g.) "ninja check-scudo". These build and run some simple test programs and check that problems are detected. Beyond this I built LLVM with scudo and ran check-all. I don't know whether you want to go this far. Note that -fsanitize=scudo doesn't work for the MSVC LLVM build which uses link/lld-link for linking rather than going via the compiler driver so I hooked it into LLVM_INTEGRATED_CRT_ALLOC (see https://reviews.llvm.org/D96133). If mingw uses the compiler driver to link (like Linux) then you may be able to use -DLLVM_USE_SANITIZER=Scudo on a stage2 build. This would require some changes to llvm/cmake/modules/HandleLLVMOptions.cmake to permit it (I had similar changes in https://reviews.llvm.org/D86694). It's harder to tell whether this has worked correctly. With MSVC faster ThinLTO links on multi-core machines are a good indicator but I guess mingw wouldn't be using the MSVC allocator. Thanks Russ ================ Comment at: compiler-rt/lib/scudo/scudo_new_delete.cpp:30 +#ifdef _WIN64 +COMMENT_EXPORT("??2@YAPEAX_K@Z") // operator new +COMMENT_EXPORT("??2@YAPEAX_KAEBUnothrow_t@std@@@Z") // operator new nothrow ---------------- mstorsjo wrote: > These don't work in this form for mingw targets (which use the itanium c++ > abi). > > By changing `#if SANTIZER_WINDOWS` into `#if SANITIZER_WINDOWS && > !defined(__MINGW32__)`, I fixed the linker errors at least. I've applied this suggestion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96120/new/ https://reviews.llvm.org/D96120 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
