The current version requires clang-format-9. This now getting hard to get.
This commit moves it to the clang-format-15, which is the latest in 22.04.
Also, updates clang-tidy to 15 as well.
All the checks and mitigations have been placed under feature flags.
These can be controlled by defining
SNMALLOC_CHECK_CLIENT_MITIGATIONS
This can take a term that represents the mitigations that should be enabled.
E.g.
-DSNMALLOC_CHECK_CLIENT_MITIGATIONS=nochecks+random_pagemap
The CMake uses this to build numerous versions of the LD_PRELOAD library and
tests to allow individual features to be benchmarked.
Co-authored-by: Nathaniel Wesley Filardo <nfilardo@microsoft.com>
* Move Morello CI to track default release
- Log some details of the build environment
- Remove workarounds overcome by events
* Morello CI: parameterize run queue and boot env
* Morello CI to run as a non-root user
For reasons unrelated to snmalloc, it's become more convenient to engage
in a little white lie, as it were, that the CI jobs are not `root` on
the worker nodes. So I'm testing changes on the cluster orchestration
goo to run the github runner as a non-root user. However, much as with
GitHub's own runners, the runner user is in the `wheel`, and `root` will
have no password, so we can still `su` up to `root` when needed.
Of course, when we are already root, we can `su` to anyone we like,
including `root`, so these changes are compatible with both the old and
new world order and have been tested with both.
This uses VirtualBox instead of xhyve. It might be slower, but should
be more reliable.
Tests run on FreeBSD, NetBSD, and OpenBSD. Only the FreeBSD ones are
passing at the moment, the others will keep running but aren't added as
dependencies for the action used to guard commits.
See src/snmalloc/README.md for an explanation of the layers.
Some other cleanups on the way:
Fine-grained stats support is now gone.
It's been broken for two years, it depends on iostream (which then
causes linker failures with libstdc++) and it's collecting the wrong
stats for the new design. After discussion with @mjp41, it's better to
remove it and introduce new stats support later, rather than keep broken
code in the main branch.
Tracing was controlled with a preprocessor macro, now there's also a
CMake option.
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.
Modernise and tidy the CMake a bit:
- Use generator expressions for a lot of conditionals so that things
are more reliable with multi-config generators (and less verbose).
- Remove C as a needed language. None of the code was C but we were
using C to test if headers worked. This was fragile because a build
with `CMAKE_CXX_COMPILER` set might have checked things compiled with
the system C compiler and then failed when the specified C++ compiler
used different headers.
- Rename the `BACKTRACE_HEADER` macro to `SNMALLOC_BACKTRACE_HEADER`.
This is exposed into code that consumes snmalloc and so should be
'namespaced' (to the degree that's possible with C macros).
- Clean up the options and use dependent options to hide options
that are not always relevant.
- Use functions instead of macros for better variable scoping.
- Factor out some duplicated bits into functions.
- Update to the latest way of telling CMake to use C++17 or C++20.
- Migrate everything that's setting global properties to setting only
per-target properties.
- Link with -nostdlib++ if it's available. If it isn't, fall back to
enabling the C language and linking with the C compiler.
- Make the per-test log messages verbose outputs. These kept scrolling
important messages off the top of the screen for me.
- Make building as a header-only library a public option.
- Add install targets that install all of the headers and provide a
config option. This works with the header-only configuration for
integration with things like vcpkg.
- Fix a missing `#endif` in the `malloc_useable_size` check. This was
failing co compile on all platforms because of the missing `#endif`.
- Bump the minimum version to 3.14 so that we have access to
target_link_options. This is necessary to use generator expressions
for linker flags.
- Make the linker error if the shim libraries depend on symbols that
are not defined in the explicitly-provided libraries.
- Make the old-Ubuntu CI jobs use C++17 explicitly (previously CMake
was silently ignoring the fact that the compiler didn't support C++20)
- Fix errors found by the more aggressive linking mode.
With these changes, it's now possible to install snmalloc and then, in
another project, do something like this:
```cmake
find_package(snmalloc CONFIG REQUIRED)
target_link_libraries(t1 snmalloc::snmalloc)
target_link_libraries(t2 snmalloc::snmallocshim-static)
```
In this example, `t1` gets all of the compile flags necessary to include
snmalloc headers for its build configuration. `t2` is additionally
linked to the snmalloc static shim library.
The code was able to use pthread destructors rather than C++ thread
local destructors. This removes the dependence on a C++ .so on linux.
However, this is not stable on other platforms such as Apple. Where the
C++ thread local state can be cleared before the pthread destructor
runs.
- 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`.
Use a matrix containing objects rather than a matrix containing keys
that we try to use to define arrays of things.
Fix a typo in ARCH. Too much Arm made it AARCH.
Also add a check that the test programs are under about ten megabytes
(they're currently around one on platforms that put inline statics full
of zeroes into BSS and around 270 on ones that don't).
Fixes#339
The naming is more than a little confusing. AIUI, in Ubuntu's terminology, the
`powerpc` packages are for 32-bit machines, the `ppc64-powerpc` packages are for
Big Endian 64-bit machines, and the `ppc64el` packages are for Little Endian
64-bit machines.
Everyone seems to have agreed that the long-term answer for 64-bit PowerPC is
Little Endian (Debian maintains an unofficial BE port, but Canonical/Ubuntu,
RedHat, and SUSE all seem to have standardized on LE).
For maximal confusion, the appropriate triple's first component is, however,
`powerpc64le`.
The status badge should report the most recent status for the master
branch, not the most recent actions run.
llvm.sh wasn't updated when trunk moved to 14 so was failing to fetch clang-13 (which is now in the llvm-toolchain-focal-13 repo, not the llvm-toolchain-focal repo). Duplicate the correct logic here rather than relying on the external script.
A few highlights relative to our existing CI:
- Add a FreeBSD 12.2 and 13.0 runner so we have some FreeBSD CI.
- Windows builds use msbuild with the Visual-Studio-provided clang toolchain to test clang
- The matrix builds describe the axes of the matrix, not all points.
- The Arm builds now cross-compile with a native clang and run the tests with qemu, rather than running the compiler, linker, and ctest all with qemu.
This also includes a fix for one of the tests that was doing `static_cast<unsigned int>(1) << 36`, which is undefined behaviour and was sometimes causing qemu to hang. There is now an assert to catch this in the future.
