1 Commits
Author SHA1 Message Date
andrew 9590264572 Replace SIMD assembly with initialized Node16::index + valgrind client request
CI / pre-commit (push) Successful in 2m2s
CI / test (arm64, -DCMAKE_BUILD_TYPE=Debug -DMSAN_TOOLCHAIN_PATH=/opt/msan, 22, https://minio.weaselab.dev/public/aarch64/msan-toolchain-22.1.8.tar.zst, debug-arm64, ubuntu-latest-arm64) (push) Successful in 3m45s
CI / test (amd64, -DCMAKE_BUILD_TYPE=Debug -DMSAN_TOOLCHAIN_PATH=/opt/msan, 21, https://minio.weaselab.dev/public/x86_64/msan-toolchain-21.1.8.tar.zst, debug, ubuntu-latest-amd64) (push) Failing after 3m41s
CI / release (arm64, , ubuntu-latest-arm64) (push) Successful in 3m23s
CI / test (amd64, -DCMAKE_CXX_FLAGS=-DUSE_64_BIT=1, 21, , 64-bit-versions, ubuntu-latest-amd64) (push) Successful in 3m25s
CI / test (arm64, -DCMAKE_BUILD_TYPE=Debug -DMSAN_TOOLCHAIN_PATH=/opt/msan, 22, https://minio.weaselab.dev/public/aarch64/msan-toolchain-22.1.8.tar.zst, debug-arm64, ubuntu-latest-arm64) (pull_request) Successful in 3m43s
CI / test (amd64, -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, 21, , gcc, ubuntu-latest-amd64) (push) Successful in 3m35s
CI / release (arm64, , ubuntu-latest-arm64) (pull_request) Successful in 3m20s
CI / test (amd64, -DUSE_SIMD_FALLBACK=ON, 21, , simd-fallback, ubuntu-latest-amd64) (push) Successful in 3m18s
CI / release (amd64, -DMSAN_TOOLCHAIN_PATH=/opt/msan, ubuntu-latest-amd64) (push) Successful in 6m4s
CI / coverage (push) Successful in 3m50s
CI / pre-commit (pull_request) Successful in 2m0s
CI / test (amd64, -DCMAKE_BUILD_TYPE=Debug -DMSAN_TOOLCHAIN_PATH=/opt/msan, 21, https://minio.weaselab.dev/public/x86_64/msan-toolchain-21.1.8.tar.zst, debug, ubuntu-latest-amd64) (pull_request) Failing after 3m46s
CI / test (amd64, -DCMAKE_CXX_FLAGS=-DUSE_64_BIT=1, 21, , 64-bit-versions, ubuntu-latest-amd64) (pull_request) Successful in 3m20s
CI / test (amd64, -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, 21, , gcc, ubuntu-latest-amd64) (pull_request) Successful in 3m19s
CI / test (amd64, -DUSE_SIMD_FALLBACK=ON, 21, , simd-fallback, ubuntu-latest-amd64) (pull_request) Successful in 3m20s
CI / release (amd64, -DMSAN_TOOLCHAIN_PATH=/opt/msan, ubuntu-latest-amd64) (pull_request) Successful in 5m36s
CI / coverage (pull_request) Successful in 3m47s
Commit 6fed133 moved SIMD operations on Node16::index into file-level
assembly (later extended to aarch64) to avoid reading indeterminate bytes
(UB in C++). Revert that in favor of a simpler, architecture-independent
fix:

- Restore the original HAS_AVX / HAS_ARM_NEON intrinsics at the affected
  call sites, keeping the newer conflictMask16 helper and scalar fallback.
- memset Node16::index in both copy constructors (from Node3 and Node48) so
  the [numChildren, 16) tail is always initialized, making the full 16-byte
  SIMD loads well-defined. This also fixes the same latent UB in the NEON
  path and the Node16->Node16 full-array memcpy.
- Issue a VALGRIND_MAKE_MEM_UNDEFINED client request on the tail so valgrind
  still flags any read that relies on the unused slots rather than numChildren.
- Drop simd.h, simd_x86_64.S, simd_aarch64.S, and the CMake ASM wiring.
2026-08-03 17:19:43 -04:00
-18
View File
@@ -549,7 +549,6 @@ inline void Node48::copyChildrenAndKeyFrom(const Node16 &other) {
assert(numChildren == Node16::kMaxNodes); assert(numChildren == Node16::kMaxNodes);
memset(index, -1, sizeof(index)); memset(index, -1, sizeof(index));
memset(children, 0, sizeof(children)); memset(children, 0, sizeof(children));
memset(reverseIndex, 0, sizeof(reverseIndex));
const auto z = InternalVersionT::zero; const auto z = InternalVersionT::zero;
for (auto &v : childMaxVersion) { for (auto &v : childMaxVersion) {
v = z; v = z;
@@ -569,13 +568,6 @@ inline void Node48::copyChildrenAndKeyFrom(const Node16 &other) {
std::max(maxOfMax[i >> Node48::kMaxOfMaxShift], childMaxVersion[i]); std::max(maxOfMax[i >> Node48::kMaxOfMaxShift], childMaxVersion[i]);
++i; ++i;
} }
// The reverseIndex bytes beyond numChildren are initialized so that the
// SIMD loads of a full 16-byte page in scan16 are well-defined (reading
// indeterminate bytes is UB in C++). valgrind then treats them as undefined
// so that it can still flag any read that relies on them.
VALGRIND_MAKE_MEM_UNDEFINED(reverseIndex + numChildren,
sizeof(reverseIndex) -
numChildren * sizeof(reverseIndex[0]));
} }
inline void Node48::copyChildrenAndKeyFrom(const Node48 &other) { inline void Node48::copyChildrenAndKeyFrom(const Node48 &other) {
@@ -602,7 +594,6 @@ inline void Node48::copyChildrenAndKeyFrom(const Node256 &other) {
copyCommon(*this, other); copyCommon(*this, other);
memset(index, -1, sizeof(index)); memset(index, -1, sizeof(index));
memset(children, 0, sizeof(children)); memset(children, 0, sizeof(children));
memset(reverseIndex, 0, sizeof(reverseIndex));
const auto z = InternalVersionT::zero; const auto z = InternalVersionT::zero;
for (auto &v : childMaxVersion) { for (auto &v : childMaxVersion) {
v = z; v = z;
@@ -624,13 +615,6 @@ inline void Node48::copyChildrenAndKeyFrom(const Node256 &other) {
++i; ++i;
}); });
memcpy(partialKey(), &other + 1, partialKeyLen); memcpy(partialKey(), &other + 1, partialKeyLen);
// The reverseIndex bytes beyond numChildren are initialized so that the
// SIMD loads of a full 16-byte page in scan16 are well-defined (reading
// indeterminate bytes is UB in C++). valgrind then treats them as undefined
// so that it can still flag any read that relies on them.
VALGRIND_MAKE_MEM_UNDEFINED(reverseIndex + numChildren,
sizeof(reverseIndex) -
numChildren * sizeof(reverseIndex[0]));
} }
inline void Node256::copyChildrenAndKeyFrom(const Node48 &other) { inline void Node256::copyChildrenAndKeyFrom(const Node48 &other) {
@@ -2025,8 +2009,6 @@ Node *erase(Node *self, WriteContext *writeContext, bool logical) {
parent48->childMaxVersion[lastChildrenIndex] = writeContext->zero; parent48->childMaxVersion[lastChildrenIndex] = writeContext->zero;
VALGRIND_MAKE_MEM_UNDEFINED(parent48->children + lastChildrenIndex, VALGRIND_MAKE_MEM_UNDEFINED(parent48->children + lastChildrenIndex,
sizeof(parent48->children[0])); sizeof(parent48->children[0]));
VALGRIND_MAKE_MEM_UNDEFINED(parent48->reverseIndex + lastChildrenIndex,
sizeof(parent48->reverseIndex[0]));
if (needsDownsize(parent48)) { if (needsDownsize(parent48)) {
downsize(parent48, writeContext); downsize(parent48, writeContext);