Fix fallout from the merge.

- CI merge issues:
   - The malloc shim libraries are renamed.
   - CMake gets very unhappy if you don't enable the C language and
     tries to link with the C compiler instead of the C++ compiler if
     you do enable it.
   - The Ubuntu packages for QEMU install a `binfmt_misc` activator for
     PowerPC64 little-endian, but set the page size to 4 KiB.  We then
     tried to run the tests (which expect 64 KiB pages) and became very
     confused when `mmap` returned 4 KiB-aligned memory.
 - Test failures:
   - Fix all of the issues UBsan found.
     - Underflow in `pointer_offset` when used to add negative offsets.
     - `CoreAlloc`'s `LocalState` accessed on a null `CoreAlloc` pointer.
     - Out of bounds access in the sizeclass list on attempts to access
       more memory than fits in the VA space.
     -
   - There was an integer overflow in `AddressSpace` that could cause it
     to try to allocate a zero-sized object, get a null pointer, and
     then try to do something with 0 - {size of the real allocation}.
   - The malloc tests weren't setting `errno` to 0 before doing
     calling `malloc`, which should set `errno` on failure, and then
     checking that `errno` was 0.
   - Don't call `PAL::error` on PAL allocation failure, return `nullptr`.
     The PALs were inconsistent about that and the new code expects to be
     able to report address-space exhaustion.
   - The malloc checks can behave differently with 0-sized allocations
     on different platforms but were very fragile about their
     expectations.
   - The malloc test didn't report failure for all of the ways that it
     could fail and so was spuriously passing on some platforms.
   - The perf test for external pointer is currently very slow on
     Windows.  The number of loops have been reduced and a timeout added
     for the Windows CI runs.
   - The logic to capture `errno` across calls was using
     `decltype(errno)`, which on some platforms where `errno` is a macro
     evaluated to `int&` and so they captured a reference rather than
     the value and failed to reset `errno`.
   - The Apple PAL can set `errno` on `notify_using` if it's called with
     memory that was not previously passed to `notify_not_using` but was
     not adequately protected against this and so would sometimes cause
     `malloc` to set `errno` to `EINVAL`.
This commit is contained in:
David Chisnall
2021-08-05 16:09:37 +01:00
parent e302ec0fa2
commit cd70a7856b
11 changed files with 49 additions and 22 deletions

View File

@@ -50,7 +50,7 @@ jobs:
- os: ubuntu-latest - os: ubuntu-latest
variant: Clang 10 libstdc++ (Build only) variant: Clang 10 libstdc++ (Build only)
dependencies: "sudo apt install ninja-build" dependencies: "sudo apt install ninja-build"
extra-cmake-flags: "-DCMAKE_CXX_COMPILER=clang++-10 -DCMAKE_CXX_FLAGS=-stdlib=libstdc++" extra-cmake-flags: "-DCMAKE_CXX_COMPILER=clang++-10 -DCMAKE_C_COMPILER=clang-10 -DCMAKE_CXX_FLAGS=-stdlib=libstdc++"
build-only: yes build-only: yes
# Don't abort runners if a single one fails # Don't abort runners if a single one fails
fail-fast: false fail-fast: false
@@ -78,13 +78,11 @@ jobs:
if: ${{ matrix.self-host }} if: ${{ matrix.self-host }}
working-directory: ${{github.workspace}}/build working-directory: ${{github.workspace}}/build
run: | run: |
sudo cp libsnmallocshim.so libsnmallocshim-16mib.so libsnmallocshim-oe.so /usr/local/lib/ sudo cp libsnmallocshim.so libsnmallocshim-checks.so /usr/local/lib/
ninja clean ninja clean
LD_PRELOAD=/usr/local/lib/libsnmallocshim.so ninja LD_PRELOAD=/usr/local/lib/libsnmallocshim.so ninja
ninja clean ninja clean
LD_PRELOAD=/usr/local/lib/libsnmallocshim-16mib.so ninja LD_PRELOAD=/usr/local/lib/libsnmallocshim-checks.so ninja
ninja clean
LD_PRELOAD=/usr/local/lib/libsnmallocshim-oe.so ninja
qemu-crossbuild: qemu-crossbuild:
strategy: strategy:
@@ -117,6 +115,13 @@ jobs:
sudo add-apt-repository "deb http://apt.llvm.org/focal/ llvm-toolchain-focal-13 main" sudo add-apt-repository "deb http://apt.llvm.org/focal/ llvm-toolchain-focal-13 main"
sudo apt update sudo apt update
sudo apt install libstdc++-9-dev-${{ matrix.arch.name }}-cross qemu-user ninja-build clang-13 lld-13 sudo apt install libstdc++-9-dev-${{ matrix.arch.name }}-cross qemu-user ninja-build clang-13 lld-13
# The default PowerPC qemu configuration uses the wrong page size.
# Wrap it in a script that fixes this.
sudo update-binfmts --disable qemu-ppc64le
sudo sh -c 'echo ":qemu-ppc64le:M:0:\x7f\x45\x4c\x46\x02\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x15\x00:\xff\xff\xff\xff\xff\xff\xff\xfc\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\x00:`pwd`/ppc64.sh:" > /proc/sys/fs/binfmt_misc/register'
echo '#!/bin/sh' > ppc64.sh
echo '/usr/bin/qemu-ppc64le -p 65536 $@' >> ppc64.sh
chmod +x ppc64.sh
- name: Configure - name: Configure
run: > run: >
RTLD_NAME=${{ matrix.arch.rtld }} RTLD_NAME=${{ matrix.arch.rtld }}
@@ -179,6 +184,7 @@ jobs:
- name: Test - name: Test
working-directory: ${{ github.workspace }}/build working-directory: ${{ github.workspace }}/build
run: ctest -j 2 --interactive-debug-mode 0 --output-on-failure -C ${{ matrix.build-type }} run: ctest -j 2 --interactive-debug-mode 0 --output-on-failure -C ${{ matrix.build-type }}
timeout-minutes: 20
# Job to run clang-format and report errors # Job to run clang-format and report errors

View File

@@ -96,7 +96,12 @@ namespace snmalloc
else if constexpr (!pal_supports<NoAllocation, PAL>) else if constexpr (!pal_supports<NoAllocation, PAL>)
{ {
// Need at least 2 times the space to guarantee alignment. // Need at least 2 times the space to guarantee alignment.
size_t needed_size = size * 2; bool overflow;
size_t needed_size = bits::umul(size, 2, overflow);
if (overflow)
{
return nullptr;
}
// Magic number (27) for over-allocating a block of memory // Magic number (27) for over-allocating a block of memory
// These should be further refined based on experiments. // These should be further refined based on experiments.
constexpr size_t min_size = bits::one_at_bit(27); constexpr size_t min_size = bits::one_at_bit(27);

View File

@@ -31,7 +31,8 @@ namespace snmalloc
inline U* pointer_offset(T* base, size_t diff) inline U* pointer_offset(T* base, size_t diff)
{ {
SNMALLOC_ASSERT(base != nullptr); /* Avoid UB */ SNMALLOC_ASSERT(base != nullptr); /* Avoid UB */
return reinterpret_cast<U*>(reinterpret_cast<char*>(base) + diff); return reinterpret_cast<U*>(
reinterpret_cast<uintptr_t>(base) + static_cast<uintptr_t>(diff));
} }
template<enum capptr_bounds bounds, typename T> template<enum capptr_bounds bounds, typename T>

View File

@@ -512,8 +512,11 @@ namespace snmalloc
ChunkRecord* slab_record = ChunkRecord* slab_record =
reinterpret_cast<ChunkRecord*>(entry.get_metaslab()); reinterpret_cast<ChunkRecord*>(entry.get_metaslab());
slab_record->chunk = CapPtr<void, CBChunk>(p); slab_record->chunk = CapPtr<void, CBChunk>(p);
ChunkAllocator::dealloc<SharedStateHandle>( check_init([&](CoreAlloc* core_alloc) {
core_alloc->get_backend_local_state(), slab_record, slab_sizeclass); ChunkAllocator::dealloc<SharedStateHandle>(
core_alloc->get_backend_local_state(), slab_record, slab_sizeclass);
return nullptr;
});
return; return;
} }

View File

@@ -83,6 +83,13 @@ namespace snmalloc
{ {
ChunkAllocatorState& state = ChunkAllocatorState& state =
SharedStateHandle::get_slab_allocator_state(&local_state); SharedStateHandle::get_slab_allocator_state(&local_state);
if (slab_sizeclass >= NUM_SLAB_SIZES)
{
// Your address space is not big enough for this allocation!
return {nullptr, nullptr};
}
// Pop a slab // Pop a slab
auto chunk_record = state.chunk_stack[slab_sizeclass].pop(); auto chunk_record = state.chunk_stack[slab_sizeclass].pop();

View File

@@ -160,6 +160,7 @@ namespace snmalloc
template<ZeroMem zero_mem> template<ZeroMem zero_mem>
static void notify_using(void* p, size_t size) noexcept static void notify_using(void* p, size_t size) noexcept
{ {
KeepErrno e;
SNMALLOC_ASSERT( SNMALLOC_ASSERT(
is_aligned_block<page_size>(p, size) || (zero_mem == NoZero)); is_aligned_block<page_size>(p, size) || (zero_mem == NoZero));
@@ -239,7 +240,7 @@ namespace snmalloc
if (unlikely(kr != KERN_SUCCESS)) if (unlikely(kr != KERN_SUCCESS))
{ {
error("Failed to allocate memory\n"); return nullptr;
} }
return reinterpret_cast<void*>(addr); return reinterpret_cast<void*>(addr);

View File

@@ -52,7 +52,7 @@ namespace snmalloc
0); 0);
if (p == MAP_FAILED) if (p == MAP_FAILED)
PALBSD<OS>::error("Out of memory"); return nullptr;
return p; return p;
} }

View File

@@ -99,12 +99,13 @@ namespace snmalloc
static const int fd = T::anonymous_memory_fd; static const int fd = T::anonymous_memory_fd;
}; };
protected:
/** /**
* A RAII class to capture and restore errno * A RAII class to capture and restore errno
*/ */
class KeepErrno class KeepErrno
{ {
decltype(errno) cached_errno; int cached_errno;
public: public:
KeepErrno() : cached_errno(errno) {} KeepErrno() : cached_errno(errno) {}

View File

@@ -177,10 +177,6 @@ namespace snmalloc
void* ret = VirtualAlloc2FromApp( void* ret = VirtualAlloc2FromApp(
nullptr, nullptr, size, flags, PAGE_READWRITE, &param, 1); nullptr, nullptr, size, flags, PAGE_READWRITE, &param, 1);
if (ret == nullptr)
{
error("Failed to allocate memory\n");
}
return ret; return ret;
} }
# endif # endif

View File

@@ -41,7 +41,7 @@ void check_result(size_t size, size_t align, void* p, int err, bool null)
#else #else
const auto exact_size = align == 1; const auto exact_size = align == 1;
#endif #endif
if (exact_size && (alloc_size != expected_size)) if (exact_size && (alloc_size != expected_size) && (size != 0))
{ {
printf( printf(
"Usable size is %zu, but required to be %zu.\n", "Usable size is %zu, but required to be %zu.\n",
@@ -79,7 +79,10 @@ void check_result(size_t size, size_t align, void* p, int err, bool null)
} }
if (failed) if (failed)
{
printf("check_result failed! %p", p); printf("check_result failed! %p", p);
abort();
}
our_free(p); our_free(p);
} }
@@ -149,7 +152,9 @@ int main(int argc, char** argv)
{ {
const size_t size = bits::one_at_bit(sc); const size_t size = bits::one_at_bit(sc);
printf("malloc: %zu\n", size); printf("malloc: %zu\n", size);
errno = 0;
check_result(size, 1, our_malloc(size), SUCCESS, false); check_result(size, 1, our_malloc(size), SUCCESS, false);
errno = 0;
check_result(size + 1, 1, our_malloc(size + 1), SUCCESS, false); check_result(size + 1, 1, our_malloc(size + 1), SUCCESS, false);
} }
@@ -180,7 +185,7 @@ int main(int argc, char** argv)
test_realloc(our_malloc(size), size, SUCCESS, false); test_realloc(our_malloc(size), size, SUCCESS, false);
test_realloc(our_malloc(size), 0, SUCCESS, true); test_realloc(our_malloc(size), 0, SUCCESS, true);
test_realloc(nullptr, size, SUCCESS, false); test_realloc(nullptr, size, SUCCESS, false);
test_realloc(our_malloc(size), (size_t)-1, ENOMEM, true); test_realloc(our_malloc(size), ((size_t)-1) / 2, ENOMEM, true);
for (sizeclass_t sc2 = 0; sc2 < NUM_SIZECLASSES; sc2++) for (sizeclass_t sc2 = 0; sc2 < NUM_SIZECLASSES; sc2++)
{ {
const size_t size2 = sizeclass_to_size(sc2); const size_t size2 = sizeclass_to_size(sc2);
@@ -195,7 +200,7 @@ int main(int argc, char** argv)
test_realloc(our_malloc(size), size, SUCCESS, false); test_realloc(our_malloc(size), size, SUCCESS, false);
test_realloc(our_malloc(size), 0, SUCCESS, true); test_realloc(our_malloc(size), 0, SUCCESS, true);
test_realloc(nullptr, size, SUCCESS, false); test_realloc(nullptr, size, SUCCESS, false);
test_realloc(our_malloc(size), (size_t)-1, ENOMEM, true); test_realloc(our_malloc(size), ((size_t)-1) / 2, ENOMEM, true);
for (sizeclass_t sc2 = 0; sc2 < (MAX_SIZECLASS_BITS + 4); sc2++) for (sizeclass_t sc2 = 0; sc2 < (MAX_SIZECLASS_BITS + 4); sc2++)
{ {
const size_t size2 = bits::one_at_bit(sc2); const size_t size2 = bits::one_at_bit(sc2);
@@ -208,7 +213,7 @@ int main(int argc, char** argv)
test_realloc(our_malloc(64), 4194304, SUCCESS, false); test_realloc(our_malloc(64), 4194304, SUCCESS, false);
test_posix_memalign(0, 0, EINVAL, true); test_posix_memalign(0, 0, EINVAL, true);
test_posix_memalign((size_t)-1, 0, EINVAL, true); test_posix_memalign(((size_t)-1) / 2, 0, EINVAL, true);
test_posix_memalign(OS_PAGE_SIZE, sizeof(uintptr_t) / 2, EINVAL, true); test_posix_memalign(OS_PAGE_SIZE, sizeof(uintptr_t) / 2, EINVAL, true);
for (size_t align = sizeof(uintptr_t); align < MAX_SIZECLASS_SIZE * 8; for (size_t align = sizeof(uintptr_t); align < MAX_SIZECLASS_SIZE * 8;
@@ -222,7 +227,7 @@ int main(int argc, char** argv)
test_memalign(size, align, SUCCESS, false); test_memalign(size, align, SUCCESS, false);
} }
test_posix_memalign(0, align, SUCCESS, false); test_posix_memalign(0, align, SUCCESS, false);
test_posix_memalign((size_t)-1, align, ENOMEM, true); test_posix_memalign(((size_t)-1) / 2, align, ENOMEM, true);
test_posix_memalign(0, align + 1, EINVAL, true); test_posix_memalign(0, align + 1, EINVAL, true);
} }

View File

@@ -53,7 +53,9 @@ namespace test
void test_external_pointer(xoroshiro::p128r64& r) void test_external_pointer(xoroshiro::p128r64& r)
{ {
auto& alloc = ThreadAlloc::get(); auto& alloc = ThreadAlloc::get();
#ifdef NDEBUG // This is very slow on Windows at the moment. Until this is fixed, help
// CI terminate.
#if defined(NDEBUG) && !defined(_MSC_VER)
static constexpr size_t iterations = 10000000; static constexpr size_t iterations = 10000000;
#else #else
# ifdef _MSC_VER # ifdef _MSC_VER