[PATCH] D96133: Allow building with scudo memory allocator on Windows

Fri Feb 5 09:09:57 PST 2021

aganea added inline comments.

================
Comment at: llvm/docs/CMake.rst:496
+
+    $ D:\llvm-project> cmake ... -DLLVM_INTEGRATED_CRT_ALLOC=<stage1>\lib\clang\<version>\lib\windows\clang_rt.scudo-x86_64.lib
+
----------------
How did you end up fixing the new/delete link issues in https://reviews.llvm.org/D86694#2463429 ?

================
Comment at: llvm/lib/Support/CMakeLists.txt:70
     add_definitions(-DENABLE_OVERRIDE -DENABLE_PRELOAD)
     set(ALLOCATOR_FILES "${LLVM_INTEGRATED_CRT_ALLOC}/rpmalloc/rpmalloc.c")
   elseif(LLVM_INTEGRATED_CRT_ALLOC MATCHES "snmalloc$")
----------------
Strangely we don't `#include <rpnew.h>`[1] anywhere when activating rpmalloc, and that still works (I don't see calls to Win32 Heap functions in the profile traces, only calls to rpmalloc functions). I think this is because the default behavior of the Windows CRT new/delete is to call malloc/free. So I'm not quite sure in which situation we need `clang_rt.scudo_cxx` / if it is needed here?

[1] https://github.com/mjansson/rpmalloc/blob/develop/rpmalloc/rpnew.h

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96133/new/

https://reviews.llvm.org/D96133