Cheri-research/snmalloc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add memcpy with bounds checks.

Browse Source 
The memcpy implementation is not completely stupid but is almost
certainly not as good as a carefully tuned and optimised one.

Building snmalloc with FreeBSD's libc memcpy + jemalloc and with this,
each 10 times, does not show a statistically significant performance
difference at 95% confidence.  The snmalloc version has very slightly
lower median and worst-case times.  This is in no way a sensible
benchmark, but it serves as a smoke test for significant performance
regressions.

The CI self-host job now uses the checked memcpy.

This also fixes an off-by-one error in the external bounds.  This is
triggered by ninja, so we will see breakage in CI if it is reintroduced.

In debug builds, we provide a verbose error containing the address of
the allocation, the base and bounds of the allocation, and a backtrace.

The backtrace was broken by the CI cleanup moving the BACKTRACE_HEADER
macro into the SNMALLOC_ namespace.  This is also fixed.

The test involves hijacking `abort`, which doesn't work everywhere.  It
also requires `backtrace` to work in configurations where stack traces
are enabled.  This is disabled in QEMU because `backtrace` appears to
crash reliably in QEMU user mode.

For now, in the -checks build configurations, we are hitting a slow path
in the pagemap on accesses so that the pages that are `PROT_NONE` don't
cause crashes.  These need to be made read-only, but this requires a PAL
change.
This commit is contained in:
David Chisnall
2021-08-10 11:36:53 +01:00
committed by David Chisnall
parent d524ef5cac
commit 51e75bca89
12 changed files with 482 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
.github/workflows/main.yml vendored
View File

@@ -18,6 +18,13 @@ jobs:
# Build each combination of OS and release/debug variants
os: [ "ubuntu-latest", "ubuntu-18.04", "macos-11", "macos-10.15", "freebsd-12.2", "freebsd-13.0" ]
build-type: [ Release, Debug ]
# Extra cmake flags. GitHub Actions matrix overloads `include` to mean
# 'add extra things to a job' and 'add jobs'. You can add extra things
# to a job by specifying things that exist in a job created from the
# matrix definition and adding things. You can specify extra jobs by
# specifying properties that don't match existing jobs. We use
# `cmake-flags` to add cmake flags to all jobs matching a pattern and
# `extra-cmake-flags` to specify a new job with custom CMake flags.
extra-cmake-flags: [ "" ]
# Modify the complete matrix
include:
@@ -36,16 +43,23 @@ jobs:
- os: "macos-10.15"
dependencies: "rm -f /usr/local/bin/2to3 ; brew update && brew install ninja"
# Skip the tests for the FreeBSD release builds
# Also build-test the checked memcpy implementation while doing these.
# It is run-tested on Linux and should be the same everywhere.
- os: "freebsd-13.0"
build-type: Release
build-only: yes
cmake-flags: "-DSNMALLOC_MEMCPY_BOUNDS=ON -DSNMALLOC_CHECK_LOADS=ON"
- os: "freebsd-12.2"
build-type: Debug
build-only: yes
# Add the self-host build
cmake-flags: "-DSNMALLOC_MEMCPY_BOUNDS=ON -DSNMALLOC_CHECK_LOADS=ON"
# Add the self-host build, using the bounds-checked memcpy in
# maximally paranoid mode (checking loads and stores)
- os: "ubuntu-latest"
build-type: Debug
self-host: true
extra-cmake-flags: "-DSNMALLOC_MEMCPY_BOUNDS=ON -DSNMALLOC_CHECK_LOADS=ON"
dependencies: "sudo apt install ninja-build"
# Extra build to check using pthread library for destructing local state.
- os: "ubuntu-latest"
variant: "Ubuntu (with pthread destructors)."

17
CMakeLists.txt
View File

@@ -19,6 +19,8 @@ option(SNMALLOC_USE_CXX17 "Build as C++17 for legacy support." OFF)
# Options that apply only if we're not building the header-only library
cmake_dependent_option(SNMALLOC_RUST_SUPPORT "Build static library for rust" OFF "NOT SNMALLOC_HEADER_ONLY_LIBRARY" OFF)
cmake_dependent_option(SNMALLOC_STATIC_LIBRARY "Build static libraries" ON "NOT SNMALLOC_HEADER_ONLY_LIBRARY" OFF)
cmake_dependent_option(SNMALLOC_MEMCPY_BOUNDS "Perform bounds checks on memcpy with heap objects" OFF "NOT SNMALLOC_HEADER_ONLY_LIBRARY" OFF)
cmake_dependent_option(SNMALLOC_CHECK_LOADS "Perform bounds checks on the source argument to memcpy with heap objects" OFF "SNMALLOC_MEMCPY_BOUNDS" OFF)
cmake_dependent_option(SNMALLOC_OPTIMISE_FOR_CURRENT_MACHINE "Compile for current machine architecture" Off "NOT SNMALLOC_HEADER_ONLY_LIBRARY" OFF)
if (NOT SNMALLOC_HEADER_ONLY_LIBRARY)
# Pick a sensible default for the thread cleanup mechanism
@@ -291,20 +293,27 @@ if(NOT SNMALLOC_HEADER_ONLY_LIBRARY)
target_link_options(${name} PRIVATE $<$<BOOL:${LLD_WORKS}>:-Wl,--icf=all -fuse-ld=lld>)
endif()
target_compile_definitions(${name} PRIVATE
SNMALLOC_CHECK_LOADS=$<IF:$<BOOL:${SNMALLOC_CHECK_LOADS}>,true,false>)
install(TARGETS ${name} EXPORT snmallocConfig)
endfunction()
set(SHIM_FILES src/override/new.cc)
if (SNMALLOC_MEMCPY_BOUNDS)
list(APPEND SHIM_FILES src/override/memcpy.cc)
endif ()
if (SNMALLOC_STATIC_LIBRARY)
add_shim(snmallocshim-static STATIC src/override/new.cc)
add_shim(snmallocshim-static STATIC ${SHIM_FILES})
target_compile_definitions(snmallocshim-static PRIVATE
SNMALLOC_STATIC_LIBRARY_PREFIX=${SNMALLOC_STATIC_LIBRARY_PREFIX})
endif ()
if(NOT WIN32)
set(SHARED_FILES src/override/new.cc)
add_shim(snmallocshim SHARED ${SHARED_FILES})
add_shim(snmallocshim-checks SHARED ${SHARED_FILES})
add_shim(snmallocshim SHARED ${SHIM_FILES})
add_shim(snmallocshim-checks SHARED ${SHIM_FILES})
target_compile_definitions(snmallocshim-checks PRIVATE SNMALLOC_CHECK_CLIENT)
endif()

22
src/backend/commonconfig.h
View File

@@ -115,4 +115,26 @@ namespace snmalloc
&unused_remote != fake_large_remote,
"Compilation should ensure these are different");
};
/**
* SFINAE helper. Matched only if `T` implements `is_initialised`. Calls
* it if it exists.
*/
template<typename T>
SNMALLOC_FAST_PATH auto call_is_initialised(T*, int)
-> decltype(T::is_initialised())
{
return T::is_initialised();
}
/**
* SFINAE helper. Matched only if `T` does not implement `is_initialised`.
* Unconditionally returns true if invoked.
*/
template<typename T>
SNMALLOC_FAST_PATH auto call_is_initialised(T*, long)
{
return true;
}
} // namespace snmalloc

9
src/backend/pagemap.h
View File

@@ -244,7 +244,14 @@ namespace snmalloc
}
// This means external pointer on Windows will be slow.
if constexpr (potentially_out_of_range && !pal_supports<LazyCommit, PAL>)
// TODO: With SNMALLOC_CHECK_CLIENT we should map that region read-only,
// not no-access, but this requires a PAL change.
if constexpr (
potentially_out_of_range
#ifndef SNMALLOC_CHECK_CLIENT
&& !pal_supports<LazyCommit, PAL>
#endif
)
{
register_range(p, 1);
}

4
src/ds/address.h
View File

@@ -231,11 +231,11 @@ namespace snmalloc
* expected to point to the base of some (sub)allocation into which cursor
* points; would-be negative answers trip an assertion in debug builds.
*/
inline size_t pointer_diff(void* base, void* cursor)
inline size_t pointer_diff(const void* base, const void* cursor)
{
SNMALLOC_ASSERT(cursor >= base);
return static_cast<size_t>(
static_cast<char*>(cursor) - static_cast<char*>(base));
static_cast<const char*>(cursor) - static_cast<const char*>(base));
}
template<

2
src/ds/defines.h
View File

@@ -15,6 +15,7 @@
# define SNMALLOC_PURE
# define SNMALLOC_COLD
# define SNMALLOC_REQUIRE_CONSTINIT
# define SNMALLOC_UNUSED_FUNCTION
#else
# define likely(x) __builtin_expect(!!(x), 1)
# define unlikely(x) __builtin_expect(!!(x), 0)
@@ -25,6 +26,7 @@
# define SNMALLOC_FAST_PATH_LAMBDA SNMALLOC_FAST_PATH
# define SNMALLOC_PURE __attribute__((const))
# define SNMALLOC_COLD __attribute__((cold))
# define SNMALLOC_UNUSED_FUNCTION __attribute((unused))
# ifdef __clang__
# define SNMALLOC_REQUIRE_CONSTINIT \
[[clang::require_constant_initialization]]

27
src/mem/localalloc.h
View File

@@ -287,27 +287,6 @@ namespace snmalloc
return local_cache.remote_allocator;
}
/**
* SFINAE helper. Matched only if `T` implements `is_initialised`. Calls
* it if it exists.
*/
template<typename T>
SNMALLOC_FAST_PATH auto call_is_initialised(T*, int)
-> decltype(T::is_initialised())
{
return T::is_initialised();
}
/**
* SFINAE helper. Matched only if `T` does not implement `is_initialised`.
* Unconditionally returns true if invoked.
*/
template<typename T>
SNMALLOC_FAST_PATH auto call_is_initialised(T*, long)
{
return true;
}
/**
* Call `SharedStateHandle::is_initialised()` if it is implemented,
* unconditionally returns true otherwise.
@@ -639,9 +618,9 @@ namespace snmalloc
if constexpr (location == Start)
return start;
else if constexpr (location == End)
return pointer_offset(start, rsize);
else
return pointer_offset(start, rsize - 1);
else
return pointer_offset(start, rsize);
}
#else
UNUSED(p_raw);
@@ -649,7 +628,7 @@ namespace snmalloc
if constexpr ((location == End) || (location == OnePastEnd))
// We don't know the End, so return MAX_PTR
return pointer_offset<void, void>(nullptr, UINTPTR_MAX);
return reinterpret_cast<void*>(UINTPTR_MAX);
else
// We don't know the Start, so return MIN_PTR
return nullptr;

27
src/override/malloc.cc
View File

@@ -1,35 +1,10 @@
// Core implementation of snmalloc independent of the configuration mode
#include "../snmalloc_core.h"
#ifndef SNMALLOC_PROVIDE_OWN_CONFIG
# include "../backend/globalconfig.h"
// The default configuration for snmalloc is used if alternative not defined
namespace snmalloc
{
using Alloc = snmalloc::LocalAllocator<snmalloc::Globals>;
} // namespace snmalloc
#endif
// User facing API surface, needs to know what `Alloc` is.
#include "../snmalloc_front.h"
#include "override.h"
#include <errno.h>
#include <string.h>
using namespace snmalloc;
#ifndef SNMALLOC_EXPORT
# define SNMALLOC_EXPORT
#endif
#ifdef SNMALLOC_STATIC_LIBRARY_PREFIX
# define __SN_CONCAT(a, b) a##b
# define __SN_EVALUATE(a, b) __SN_CONCAT(a, b)
# define SNMALLOC_NAME_MANGLE(a) \
__SN_EVALUATE(SNMALLOC_STATIC_LIBRARY_PREFIX, a)
#elif !defined(SNMALLOC_NAME_MANGLE)
# define SNMALLOC_NAME_MANGLE(a) a
#endif
#ifndef MALLOC_USABLE_SIZE_QUALIFIER
# define MALLOC_USABLE_SIZE_QUALIFIER
#endif

232
src/override/memcpy.cc Normal file
View File

@@ -0,0 +1,232 @@
#include "override.h"
#include <errno.h>
#include <stdio.h>
#include <string.h>
#if __has_include(<xlocale.h>)
# include <xlocale.h>
#endif
using namespace snmalloc;
// glibc lacks snprintf_l
#ifdef __linux__
# define snprintf_l(buf, size, loc, msg, ...) \
snprintf(buf, size, msg, __VA_ARGS__)
// Windows has it with an underscore prefix
#elif defined(_MSC_VER)
# define snprintf_l(buf, size, loc, msg, ...) \
_snprintf_l(buf, size, msg, loc, __VA_ARGS__)
#endif
namespace
{
/**
* Should we check loads? This defaults to on in debug builds, off in
* release (store-only checks)
*/
static constexpr bool CheckReads =
#ifdef SNMALLOC_CHECK_LOADS
SNMALLOC_CHECK_LOADS
#else
# ifdef NDEBUG
false
# else
true
# endif
#endif
;
/**
* Should we fail fast when we encounter an error? With this set to true, we
* just issue a trap instruction and crash the process once we detect an
* error. With it set to false we print a helpful error message and then crash
* the process. The process may be in an undefined state by the time the
* check fails, so there are potentially security implications to turning this
* off. It defaults to true for debug builds, false for release builds.
*/
static constexpr bool FailFast =
#ifdef SNMALLOC_FAIL_FAST
SNMALLOC_FAIL_FAST
#else
# ifdef NDEBUG
true
# else
false
# endif
#endif
;
/**
* The largest register size that we can use for loads and stores. These
* types are expected to work for overlapping copies: we can always load them
* into a register and store them. Note that this is at the C abstract
* machine level: the compiler may spill temporaries to the stack, just not
* to the source or destination object.
*/
static constexpr size_t LargestRegisterSize =
#ifdef __AVX__
32
#elif defined(__SSE__)
16
#else
sizeof(uint64_t)
#endif
;
/**
* Copy a single element of a specified size. Uses a compiler builtin that
* expands to a single load and store.
*/
template<size_t Size>
SNMALLOC_FAST_PATH inline void copy_one(void* dst, const void* src)
{
#if __has_builtin(__builtin_memcpy_inline)
__builtin_memcpy_inline(dst, src, Size);
#else
// Define a structure of size `Size` that has alignment 1 and a default
// copy-assignment operator. We can then copy the data as this type. The
// compiler knows the exact width and so will generate the correct wide
// instruction for us (clang 10 and gcc 12 both generate movups for the
// 16-byte version of this when targeting SSE.
struct Block
{
char data[Size];
};
auto* d = static_cast<Block*>(dst);
auto* s = static_cast<const Block*>(src);
*d = *s;
#endif
}
SNMALLOC_SLOW_PATH SNMALLOC_UNUSED_FUNCTION void crashWithMessage
[[noreturn]] (
void* p, size_t len, const char* msg, decltype(ThreadAlloc::get())& alloc)
{
// We're going to crash the program now, but try to avoid heap
// allocations if possible, since the heap may be in an undefined
// state.
std::array<char, 1024> buffer;
snprintf_l(
buffer.data(),
buffer.size(),
/* Force C locale */ nullptr,
"%s: %p is in allocation %p--%p, offset 0x%zx is past the end.\n",
msg,
p,
alloc.template external_pointer<Start>(p),
alloc.template external_pointer<OnePastEnd>(p),
len);
Pal::error(buffer.data());
}
/**
* Check whether a pointer + length is in the same object as the pointer.
* Fail with the error message from the third argument if not.
*
* The template parameter indicates whether this is a read. If so, this
* function is a no-op when `CheckReads` is false.
*/
template<bool IsRead = false>
SNMALLOC_FAST_PATH inline void
check_bounds(const void* ptr, size_t len, const char* msg = "")
{
if constexpr (!IsRead || CheckReads)
{
auto& alloc = ThreadAlloc::get();
void* p = const_cast<void*>(ptr);
if (unlikely(
pointer_diff(ptr, alloc.external_pointer<OnePastEnd>(p)) < len))
{
if constexpr (FailFast)
{
UNUSED(ptr);
UNUSED(len);
UNUSED(msg);
__builtin_trap();
}
else
{
crashWithMessage(p, len, msg, alloc);
}
}
}
else
{
UNUSED(ptr);
UNUSED(len);
UNUSED(msg);
}
}
/**
* Copy a block using the specified size. This copies as many complete
* chunks of size `Size` as are possible from `len`.
*/
template<size_t Size>
SNMALLOC_FAST_PATH inline void
block_copy(void* dst, const void* src, size_t len)
{
for (size_t i = 0; (i + Size) <= len; i += Size)
{
copy_one<Size>(pointer_offset(dst, i), pointer_offset(src, i));
}
}
/**
* Perform an overlapping copy of the end. This will copy one (potentially
* unaligned) `T` from the end of the source to the end of the destination.
* This may overlap other bits of the copy.
*/
template<size_t Size>
SNMALLOC_FAST_PATH inline void
copy_end(void* dst, const void* src, size_t len)
{
copy_one<Size>(
pointer_offset(dst, len - Size), pointer_offset(src, len - Size));
}
/**
* Predicate indicating whether the source and destination are sufficiently
* aligned to be copied as aligned chunks of `Size` bytes.
*/
template<size_t Size>
SNMALLOC_FAST_PATH bool is_aligned_memcpy(void* dst, const void* src)
{
return (pointer_align_down<Size>(const_cast<void*>(src)) == src) &&
(pointer_align_down<Size>(dst) == dst);
}
}
extern "C"
{
/**
* Snmalloc checked memcpy.
*/
SNMALLOC_EXPORT void*
SNMALLOC_NAME_MANGLE(memcpy)(void* dst, const void* src, size_t len)
{
// 0 is a very common size for memcpy and we don't need to do external
// pointer checks if we hit it. It's also the fastest case, to encourage
// the compiler to favour the other cases.
if (unlikely(len == 0))
{
return dst;
}
// Check the bounds of the arguments.
check_bounds(
dst, len, "memcpy with destination out of bounds of heap allocation");
check_bounds<true>(
src, len, "memcpy with source out of bounds of heap allocation");
// If this is a small size, do byte-by-byte copies.
if (len < LargestRegisterSize)
{
block_copy<1>(dst, src, len);
return dst;
}
block_copy<LargestRegisterSize>(dst, src, len);
copy_end<LargestRegisterSize>(dst, src, len);
return dst;
}
}

28
src/override/override.h Normal file
View File

@@ -0,0 +1,28 @@
#pragma once
// Core implementation of snmalloc independent of the configuration mode
#include "../snmalloc_core.h"
#ifndef SNMALLOC_PROVIDE_OWN_CONFIG
# include "../backend/globalconfig.h"
// The default configuration for snmalloc is used if alternative not defined
namespace snmalloc
{
using Alloc = snmalloc::LocalAllocator<snmalloc::Globals>;
} // namespace snmalloc
#endif
// User facing API surface, needs to know what `Alloc` is.
#include "../snmalloc_front.h"
#ifndef SNMALLOC_EXPORT
# define SNMALLOC_EXPORT
#endif
#ifdef SNMALLOC_STATIC_LIBRARY_PREFIX
# define __SN_CONCAT(a, b) a##b
# define __SN_EVALUATE(a, b) __SN_CONCAT(a, b)
# define SNMALLOC_NAME_MANGLE(a) \
__SN_EVALUATE(SNMALLOC_STATIC_LIBRARY_PREFIX, a)
#elif !defined(SNMALLOC_NAME_MANGLE)
# define SNMALLOC_NAME_MANGLE(a) a
#endif

2
src/pal/pal_posix.h
View File

@@ -134,7 +134,7 @@ namespace snmalloc
static void print_stack_trace()
{
#ifdef BACKTRACE_HEADER
#ifdef SNMALLOC_BACKTRACE_HEADER
constexpr int SIZE = 1024;
void* buffer[SIZE];
auto nptrs = backtrace(buffer, SIZE);

155
src/test/func/memcpy/func-memcpy.cc Normal file
View File

@@ -0,0 +1,155 @@
// Windows doesn't like changing the linkage spec of abort.
#if defined(_MSC_VER)
int main()
{
return 0;
}
#else
// QEMU user mode does not support the code that generates backtraces and so we
// also need to skip this test if we are doing a debug build and targeting
// QEMU.
# if defined(SNMALLOC_QEMU_WORKAROUND) && defined(SNMALLOC_BACKTRACE_HEADER)
# undef SNMALLOC_BACKTRACE_HEADER
# endif
# ifdef SNMALLOC_STATIC_LIBRARY_PREFIX
# undef SNMALLOC_STATIC_LIBRARY_PREFIX
# endif
# ifdef SNMALLOC_FAIL_FAST
# undef SNMALLOC_FAIL_FAST
# endif
# define SNMALLOC_FAIL_FAST false
# define SNMALLOC_STATIC_LIBRARY_PREFIX my_
# include "ds/defines.h"
# ifndef SNMALLOC_PASS_THROUGH
# include "override/malloc.cc"
# else
# define my_malloc(x) malloc(x)
# define my_free(x) free(x)
# endif
# include "override/memcpy.cc"
# include <assert.h>
# include <csetjmp>
# include <stdio.h>
# include <stdlib.h>
# include <string.h>
/**
* Jump buffer used to jump out of `abort()` for recoverable errors.
*/
static std::jmp_buf jmp;
/**
* Flag indicating whether `jmp` is valid. If this is set then calls to
* `abort` will jump to the jump buffer, rather than exiting.
*/
static bool can_longjmp;
/**
* Replacement for the C standard `abort` that returns to the `setjmp` call for
* recoverable errors.
*/
extern "C" void abort()
{
if (can_longjmp)
{
longjmp(jmp, 1);
}
exit(-1);
}
/**
* Check that memcpy works in correct use. This allocates a pair of buffers,
* fills one with a well-known pattern, and then copies subsets of this at
* one-byte increments to a target. This gives us unaligned starts.
*/
void check_size(size_t size)
{
auto* s = reinterpret_cast<unsigned char*>(my_malloc(size + 1));
auto* d = reinterpret_cast<unsigned char*>(my_malloc(size + 1));
d[size] = 0;
s[size] = 255;
for (size_t start = 0; start < size; start++)
{
unsigned char* src = s + start;
unsigned char* dst = d + start;
size_t sz = (size - start);
for (size_t i = 0; i < sz; ++i)
{
src[i] = static_cast<unsigned char>(i);
}
for (size_t i = 0; i < sz; ++i)
{
dst[i] = 0;
}
my_memcpy(dst, src, sz);
for (size_t i = 0; i < sz; ++i)
{
if (dst[i] != static_cast<unsigned char>(i))
{
fprintf(
stderr,
"Testing size %zd %hhx == %hhx\n",
sz,
static_cast<unsigned char>(i),
dst[i]);
}
SNMALLOC_CHECK(dst[i] == (unsigned char)i);
}
SNMALLOC_CHECK(d[size] == 0);
}
my_free(s);
my_free(d);
}
void check_bounds(size_t size, size_t out_of_bounds)
{
auto* s = reinterpret_cast<unsigned char*>(my_malloc(size));
auto* d = reinterpret_cast<unsigned char*>(my_malloc(size));
for (size_t i = 0; i < size; ++i)
{
s[i] = static_cast<unsigned char>(i);
}
for (size_t i = 0; i < size; ++i)
{
d[i] = 0;
}
bool bounds_error = false;
can_longjmp = true;
if (setjmp(jmp) == 0)
{
my_memcpy(d, s, size + out_of_bounds);
}
else
{
bounds_error = true;
}
can_longjmp = false;
SNMALLOC_CHECK(bounds_error == (out_of_bounds > 0));
my_free(s);
my_free(d);
}
int main()
{
// Skip the checks that expect bounds checks to fail when we are not the
// malloc implementation.
# if !defined(SNMALLOC_PASS_THROUGH)
// Some sizes to check for out-of-bounds access
std::initializer_list<size_t> sizes = {16, 1024, 2 * 1024 * 1024};
for (auto sz : sizes)
{
// Check in bounds
check_bounds(sz, 0);
// Check one byte out
check_bounds(sz, 1);
// Check one object out of bounds
check_bounds(sz, sz);
}
# endif
for (size_t x = 0; x < 2048; x++)
{
check_size(x);
}
}
#endif