[libc] add snmalloc as an alternative allocator to libc #122284
Conversation
SchrodingerZhu requested review from RossComputerGuy, lntue, michaelrj-google and nickdesaulniers 
| cc @mjp41 |
SchrodingerZhu force-pushed the snmalloc-2 branch 2 times, most recently from 5551d5a to ddedf1e Compare 
| ✅ With the latest revision this PR passed the C/C++ code formatter. |
SchrodingerZhu force-pushed the snmalloc-2 branch 2 times, most recently from f9ace97 to 1382389 Compare 
| @llvm/pr-subscribers-libc Author: Schrodinger ZHU Yifan (SchrodingerZhu) ChangesDoes not work yet. I need to work out a lot more details. This is for collecting ideas.
llvm-project/llvm/lib/Support/CMakeLists.txt Line 101 in aa0191e Adding snmalloc will push forward our support on windows/macos easily. It also provides linux target with a modern allocator alternative. I have made snmalloc buildable with clean C headers in microsoft/snmalloc#722, hence, it can now be used in a way largely similar to scudo standalone allocator.
Full diff: https://github.com/llvm/llvm-project/pull/122284.diff 10 Files Affected:
diff --git a/libc/src/stdlib/CMakeLists.txt b/libc/src/stdlib/CMakeLists.txt index 40ba9ead9a7ae6..7d0dfd1aa9da4b 100644 --- a/libc/src/stdlib/CMakeLists.txt +++ b/libc/src/stdlib/CMakeLists.txt @@ -324,7 +324,46 @@ add_entrypoint_object( ) if(NOT LIBC_TARGET_OS_IS_BAREMETAL AND NOT LIBC_TARGET_OS_IS_GPU) - if(LLVM_LIBC_INCLUDE_SCUDO) + if (NOT "${LLVM_LIBC_INCLUDE_SNMALLOC}" STREQUAL "") + message(STATUS "Including snmalloc as the allocator, source directory: ${LLVM_LIBC_INCLUDE_SNMALLOC}") + add_subdirectory(snmalloc) + add_entrypoint_object( + malloc + ALIAS + DEPENDS + .snmalloc.malloc + ) + add_entrypoint_object( + calloc + ALIAS + DEPENDS + .snmalloc.calloc + ) + add_entrypoint_object( + realloc + ALIAS + DEPENDS + .snmalloc.realloc + ) + add_entrypoint_object( + aligned_alloc + ALIAS + DEPENDS + .snmalloc.aligned_alloc + ) + add_entrypoint_object( + free + ALIAS + DEPENDS + .snmalloc.free + ) + add_entrypoint_object( + mallopt + ALIAS + DEPENDS + .snmalloc.mallopt + ) + elseif(LLVM_LIBC_INCLUDE_SCUDO) set(SCUDO_DEPS "") include(${LIBC_SOURCE_DIR}/../compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake) diff --git a/libc/src/stdlib/snmalloc/CMakeLists.txt b/libc/src/stdlib/snmalloc/CMakeLists.txt new file mode 100644 index 00000000000000..e360b36f5fb3ad --- /dev/null +++ b/libc/src/stdlib/snmalloc/CMakeLists.txt @@ -0,0 +1,119 @@ +set(SNMALLOC_USE_SELF_VENDORED_STL ON CACHE BOOL "use freestanding snmalloc setup" FORCE) +set(SNMALLOC_BUILD_TESTING OFF CACHE BOOL "disable snmalloc tests" FORCE) +set(SNMALLOC_HEADER_ONLY_LIBRARY ON CACHE BOOL "use snmalloc as header only library" FORCE) + +# Disable installation +macro (install) +endmacro () + +add_subdirectory(${LLVM_LIBC_INCLUDE_SNMALLOC} ${CMAKE_CURRENT_BINARY_DIR}/snmalloc EXCLUDE_FROM_ALL) + +target_compile_options( + snmalloc + INTERFACE + -ffreestanding + -nostdinc + -Wno-newline-eof + -Wno-extra-semi + -Wno-unused-command-line-argument + -Wno-ctad-maybe-unsupported + # TODO: define this + -DSTDERR_FILENO=2 + -DSNMALLOC_USE_PTHREAD_DESTRUCTORS + # include_directories does not propagate, use options instead + -include ${CMAKE_CURRENT_SOURCE_DIR}/override.h + -isystem ${COMPILER_RESOURCE_DIR}/include + -isystem ${LIBC_INCLUDE_DIR} +) + +add_dependencies(snmalloc libc-headers) + +set(SNMALLOC_DEPS + # includes + libc.include.errno + libc.include.fcntl + libc.include.limits + libc.include.pthread + libc.include.stdio + libc.include.stdint + libc.include.stdlib + libc.include.string + libc.include.strings + libc.include.sys_mman + libc.include.sys_prctl + libc.include.sys_random + libc.include.sys_syscall + libc.include.sys_uio + libc.include.unistd + # symbols + libc.src.errno.errno + libc.src.fcntl.open + libc.src.pthread.pthread_key_create + libc.src.pthread.pthread_setspecific + libc.src.stdlib.abort + libc.src.stdlib.atexit + libc.src.string.memset + libc.src.string.strlen + libc.src.sys.mman.madvise + libc.src.sys.mman.mmap + libc.src.sys.uio.writev + libc.src.time.clock_gettime + libc.src.unistd.__llvm_libc_syscall + libc.src.unistd.close + libc.src.unistd.fsync + libc.src.unistd.getentropy + libc.src.unistd.read +) + +add_entrypoint_object( + malloc + SRCS + malloc.cpp + DEPENDS + snmalloc +) + +add_entrypoint_object( + calloc + SRCS + calloc.cpp + DEPENDS + snmalloc + ${SNMALLOC_DEPS} +) + +add_entrypoint_object( + realloc + SRCS + realloc.cpp + DEPENDS + snmalloc + ${SNMALLOC_DEPS} +) + +add_entrypoint_object( + aligned_alloc + SRCS + aligned_alloc.cpp + DEPENDS + snmalloc + ${SNMALLOC_DEPS} +) + +add_entrypoint_object( + free + SRCS + free.cpp + DEPENDS + snmalloc + ${SNMALLOC_DEPS} +) + +add_entrypoint_object( + mallopt + SRCS + mallopt.cpp + DEPENDS + snmalloc + ${SNMALLOC_DEPS} +) diff --git a/libc/src/stdlib/snmalloc/aligned_alloc.cpp b/libc/src/stdlib/snmalloc/aligned_alloc.cpp new file mode 100644 index 00000000000000..ee1d74ca9c778c --- /dev/null +++ b/libc/src/stdlib/snmalloc/aligned_alloc.cpp @@ -0,0 +1,17 @@ +//===-- Implementation of aligned_alloc -----------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "src/stdlib/aligned_alloc.h" +#include "snmalloc/snmalloc.h" +#include "src/__support/common.h" + +namespace LIBC_NAMESPACE_DECL { +LLVM_LIBC_FUNCTION(void *, aligned_alloc, (size_t alignment, size_t size)) { + return snmalloc::libc::aligned_alloc(alignment, size); +} +} // namespace LIBC_NAMESPACE_DECL diff --git a/libc/src/stdlib/snmalloc/calloc.cpp b/libc/src/stdlib/snmalloc/calloc.cpp new file mode 100644 index 00000000000000..39af67607bf709 --- /dev/null +++ b/libc/src/stdlib/snmalloc/calloc.cpp @@ -0,0 +1,17 @@ +//===-- Implementation of calloc ------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "src/stdlib/calloc.h" +#include "snmalloc/snmalloc.h" +#include "src/__support/common.h" + +namespace LIBC_NAMESPACE_DECL { +LLVM_LIBC_FUNCTION(void *, calloc, (size_t num, size_t size)) { + return snmalloc::libc::calloc(num, size); +} +} // namespace LIBC_NAMESPACE_DECL diff --git a/libc/src/stdlib/snmalloc/free.cpp b/libc/src/stdlib/snmalloc/free.cpp new file mode 100644 index 00000000000000..eb403710fffada --- /dev/null +++ b/libc/src/stdlib/snmalloc/free.cpp @@ -0,0 +1,17 @@ +//===-- Implementation of free --------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "src/stdlib/free.h" +#include "snmalloc/snmalloc.h" +#include "src/__support/common.h" + +namespace LIBC_NAMESPACE_DECL { +LLVM_LIBC_FUNCTION(void, free, (void *ptr)) { + return snmalloc::libc::free(ptr); +} +} // namespace LIBC_NAMESPACE_DECL diff --git a/libc/src/stdlib/snmalloc/malloc.cpp b/libc/src/stdlib/snmalloc/malloc.cpp new file mode 100644 index 00000000000000..ab58e63743016a --- /dev/null +++ b/libc/src/stdlib/snmalloc/malloc.cpp @@ -0,0 +1,17 @@ +//===-- Implementation of malloc ------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "src/stdlib/malloc.h" +#include "snmalloc/snmalloc.h" +#include "src/__support/common.h" + +namespace LIBC_NAMESPACE_DECL { +LLVM_LIBC_FUNCTION(void *, malloc, (size_t size)) { + return snmalloc::libc::malloc(size); +} +} // namespace LIBC_NAMESPACE_DECL diff --git a/libc/src/stdlib/snmalloc/mallopt.cpp b/libc/src/stdlib/snmalloc/mallopt.cpp new file mode 100644 index 00000000000000..c52d68e3d52230 --- /dev/null +++ b/libc/src/stdlib/snmalloc/mallopt.cpp @@ -0,0 +1,13 @@ +//===-- Implementation of mallopt stub ------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "src/__support/common.h" + +namespace LIBC_NAMESPACE_DECL { +LLVM_LIBC_FUNCTION(int, mallopt, (int, int)) { return 0; } +} // namespace LIBC_NAMESPACE_DECL diff --git a/libc/src/stdlib/snmalloc/override.h b/libc/src/stdlib/snmalloc/override.h new file mode 100644 index 00000000000000..a36a6316790fbd --- /dev/null +++ b/libc/src/stdlib/snmalloc/override.h @@ -0,0 +1,32 @@ +//===-- Macro Override for SnMalloc ---------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// skip special headers +#define LLVM_LIBC_COMMON_H +#define LLVM_LIBC_ERRNO_H + +// define common macros +#define __BEGIN_C_DECLS namespace LIBC_NAMESPACE { +#define __END_C_DECLS \ + } \ + using namespace LIBC_NAMESPACE; + +#define _Noreturn [[noreturn]] +#define _Alignas alignas +#define _Static_assert static_assert +#define _Alignof alignof +#define _Thread_local thread_local + +// Use empty definition to avoid spec mismatching +// We are building internally anyway, hence noexcept does not matter here +#define __NOEXCEPT + +// Enforce internal errno implementation +#include "hdr/errno_macros.h" +#include "src/errno/libc_errno.h" +#define errno libc_errno diff --git a/libc/src/stdlib/snmalloc/realloc.cpp b/libc/src/stdlib/snmalloc/realloc.cpp new file mode 100644 index 00000000000000..36972e0a2232ec --- /dev/null +++ b/libc/src/stdlib/snmalloc/realloc.cpp @@ -0,0 +1,17 @@ +//===-- Implementation of realloc -----------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "src/stdlib/realloc.h" +#include "snmalloc/snmalloc.h" +#include "src/__support/common.h" + +namespace LIBC_NAMESPACE_DECL { +LLVM_LIBC_FUNCTION(void *, realloc, (void *ptr, size_t size)) { + return snmalloc::libc::realloc(ptr, size); +} +} // namespace LIBC_NAMESPACE_DECL diff --git a/libc/test/src/stdlib/CMakeLists.txt b/libc/test/src/stdlib/CMakeLists.txt index 8cc0428632ba39..2346a8bae08f87 100644 --- a/libc/test/src/stdlib/CMakeLists.txt +++ b/libc/test/src/stdlib/CMakeLists.txt @@ -420,7 +420,9 @@ if(LLVM_LIBC_FULL_BUILD) ) # Only baremetal and GPU has an in-tree 'malloc' implementation. - if(LIBC_TARGET_OS_IS_BAREMETAL OR LIBC_TARGET_OS_IS_GPU) + if(LIBC_TARGET_OS_IS_BAREMETAL + OR LIBC_TARGET_OS_IS_GPU + OR NOT "${LLVM_LIBC_INCLUDE_SNMALLOC}" STREQUAL "") add_libc_test( malloc_test HERMETIC_TEST_ONLY |
SchrodingerZhu changed the title SchrodingerZhu changed the title 
There was a problem hiding this comment.
Are there circular dependency among these targets back to malloc?
There was a problem hiding this comment.
So atexit would only be called inside register_cleanup. This occurs when snmalloc detects this thread's allocator has not been initialised. If this is the case it initializes the allocator. Once initialised it calls register_cleanup, and then it performs the original operation on the allocator.
https://github.com/microsoft/snmalloc/blob/main/src%2Fsnmalloc%2Fmem%2Flocalalloc.h#L146-L148
I'm happy to expand the description of this code if helpful.
There was a problem hiding this comment.
Thanks for the explanation! I don't think it is a problem right now, but at some point we should break the circular dependency in our cmake targets, or at least among the subdirectories.
| LLVM-libc atexit implementation calls malloc to obtain chunks when public packaging mode is enabled, which looks suspicious to me. We should probably change the behavior. On the other hand, we can also call allocators cleanup hook (_malloc_thread_cleanup) internally since we do have full control of threads and main CRT startup/teardown in hermetic/fullbuild. |
So |
| @mjp41 Thanks for the suggestion. However, after investigating, I think libc's implementation is quite wrong currently. I am leaning to keep using pthread-style cleanup because it does not need me to inject So, right inside startup code, we do a call to //... // We want the fini array callbacks to be run after other atexit // callbacks are run. So, we register them before running the init // array callbacks as they can potentially register their own atexit // callbacks. atexit(&call_fini_array_callbacks); call_init_array_callbacks(static_cast<int>(app.args->argc), reinterpret_cast<char **>(app.args->argv), reinterpret_cast<char **>(env_ptr)); int retval = main(static_cast<int>(app.args->argc), reinterpret_cast<char **>(app.args->argv), reinterpret_cast<char **>(env_ptr)); exit(retval);When #if defined(LIBC_TARGET_ARCH_IS_GPU) using ExitCallbackList = FixedVector<AtExitUnit, 64>; #elif defined(LIBC_COPT_PUBLIC_PACKAGING) using ExitCallbackList = ReverseOrderBlockStore<AtExitUnit, 32>; #else using ExitCallbackList = FixedVector<AtExitUnit, CALLBACK_LIST_SIZE_FOR_TESTS>; #endif
Sorry, I think MUSL is also doing something similar: |
| I have updated according to my observations and the discussion above. Please take a look. |
Looks good from snmalloc's side. |
| A gentle ping~ @lntue |
There was a problem hiding this comment.
Nit: use LIBC_* for newer flag instead of LLVM_LIBC_*
nickdesaulniers left a comment
nickdesaulniers left a comment There was a problem hiding this comment.
Needs docs on how one configures llvm-libc to use snmalloc.
Can you file a bug about the atexit allocation you mention earlier?
SchrodingerZhu force-pushed the snmalloc-2 branch 2 times, most recently from b58cd1e to 655d5b1 Compare 
lntue left a comment
lntue left a comment There was a problem hiding this comment.
LGTM. Please wait for @nickdesaulniers. Thanks!
libc/src/stdlib/CMakeLists.txt Outdated
There was a problem hiding this comment.
What happens if LLVM_LIBC_INCLUDE_SCUDO and LLVM_LIBC_INCLUDE_SNMALLOC are used at the same time?
We now have four allocator options usable with the LLVM-libc:
- snmalloc and scudo, which are stubs that wrap external libraries
- baremetal malloc, which is a full malloc implementation
- gpu malloc, which is a set of wrappers using RPCs
I'm planning to lift baremetal malloc out of baremetal in the near future; it isn't all that baremetal specific, since it's based on dlmalloc, which is useable both on embedded and hosted OS systems. With that there will be three allocator options for Linux and friends, so this seems like a good time to introduce a switch-like option (LLVM_LIBC_ALLOCATOR?) to at least make it impossible to simultaneously specify two different allocators.
@nickdesaulniers: if we do this, would we need to keep LLVM_LIBC_INCLUDE_SCUDO for backcompat?
There was a problem hiding this comment.
so this seems like a good time to introduce a switch-like option
➕ 1️⃣
would we need to keep LLVM_LIBC_INCLUDE_SCUDO for backcompat?
While we are planning have some changes to cmake tied around upstream llvm releases, I'm not sure that we have enough downstream users (that use in tree cmake) at this point that care.
There was a problem hiding this comment.
What happens if LLVM_LIBC_INCLUDE_SCUDO and LLVM_LIBC_INCLUDE_SNMALLOC are used at the same time?
That is checked and will explicitly error out in another cmake place. However, I second that having a switch to better. Let me do it in this patch then.
There was a problem hiding this comment.
Catching some paper deadlines. Sorry for the delay -- finishing this patch is still of the first priority on my table once I get time.
SchrodingerZhu force-pushed the snmalloc-2 branch from 445cf04 to 92932fd Compare 
RossComputerGuy left a comment
RossComputerGuy left a comment There was a problem hiding this comment.
This looks great, looking forward to using this with Nix.
jhuber6 left a comment
jhuber6 left a comment There was a problem hiding this comment.
Honestly, if we're going this route, I think we should just make snmalloc the default allocator and copy it into the tree with a style port. Though I'm not enough of an expert to know the benefits of this versus scudo or why we can't copy SCUDO as well. I just think it's generally problematic that we need to do such weird build magic to get such a fundamental function.
| #ifdef LIBC_USE_MALLOC_THREAD_CLEANUP | ||
| // This symbol may not be defined if libc is not built with allocator | ||
| extern "C" [[gnu::weak]] void _malloc_thread_cleanup(); | ||
| #endif |
There was a problem hiding this comment.
Any way we can get it to work without this?
michaelrj-google left a comment
michaelrj-google left a comment There was a problem hiding this comment.
This needs some updates, and we should set up some sort of testing for this, but it looks overall fine.
| if(NOT LIBC_TARGET_OS_IS_BAREMETAL AND NOT LIBC_TARGET_OS_IS_GPU) | ||
| if(LLVM_LIBC_INCLUDE_SCUDO) | ||
| if (NOT "${LIBC_INCLUDE_SNMALLOC}" STREQUAL "") | ||
| message(STATUS "Including snmalloc as the allocator, source directory: ${LIBC_INCLUDE_SNMALLOC}") |
There was a problem hiding this comment.
this should be behind the verbose cmake flag
| # Only baremetal and GPU has an in-tree 'malloc' implementation. | ||
| if((LIBC_TARGET_OS_IS_BAREMETAL OR LIBC_TARGET_OS_IS_GPU) AND | ||
| NOT LIBC_TARGET_ARCHITECTURE_IS_NVPTX) | ||
| # Only baremetal, snmalloc and GPU has an in-tree 'malloc' |
There was a problem hiding this comment.
nit:
| # Only baremetal, snmalloc and GPU has an in-tree 'malloc' | |
| # Only baremetal, snmalloc and GPU have an in-tree 'malloc' |
libc/src/stdlib/snmalloc/override.h Outdated
| #define LLVM_LIBC_COMMON_H | ||
| #define LLVM_LIBC_ERRNO_H |
There was a problem hiding this comment.
This needs to be updated, the public header macro for _LLVM_LIBC_COMMON_H has changed.
| #define LLVM_LIBC_ERRNO_H | ||
| | ||
| // define common macros | ||
| #define __BEGIN_C_DECLS namespace LIBC_NAMESPACE { |
There was a problem hiding this comment.
this should be LIBC_NAMESPACE_DECL
| #define __BEGIN_C_DECLS namespace LIBC_NAMESPACE { | ||
| #define __END_C_DECLS \ | ||
| } \ | ||
| using namespace LIBC_NAMESPACE; |
There was a problem hiding this comment.
why is this necessary?
| #define __NOEXCEPT | ||
| | ||
| // TODO: define this in stdio.h | ||
| #define STDERR_FILENO 2 |
There was a problem hiding this comment.
this is already defined in unistd-macros.h, is it needed here?
SchrodingerZhu force-pushed the snmalloc-2 branch from e4ff8d8 to 2ff6528 Compare | @@ -0,0 +1,36 @@ | |||
| //===-- Macro Override for SnMalloc ---------------------------------------===// | |||
There was a problem hiding this comment.
This stuff really just makes me thing that we should just copy the code in-tree and port it as the allocator for linux.
| So after microsoft/snmalloc#762, we will be able to use pthread_setspecific to handle clean up. There will be no need to use a separate hook. |
SchrodingerZhu force-pushed the snmalloc-2 branch from 57e3b4a to 61a654a Compare 9 participants
Does not work yet. I need to work out a lot more details. This is for collecting ideas.snmallocis already supported in LLVM for windows CRT replacement, we can also use it as an implementation of our allocator:llvm-project/llvm/lib/Support/CMakeLists.txt
Line 101 in aa0191e
Adding snmalloc will push forward our support on windows/macos easily. It also provides linux target with a modern allocator alternative.
I have made snmalloc buildable with clean C headers in microsoft/snmalloc#722, hence, it can now be used in a way largely similar to scudo standalone allocator.
malloc.cpp.ois already buildable with some ad-hoc fixes, so I think there is no technical obstacle.Know to work: snmalloc
229c6d04fea5b6b0757be99a2b6394e3aea5d562