From 09cf98d55c555ddd672e08d847e2f9ded0673424 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Karad=C5=BEi=C4=87?= Date: Mon, 5 Oct 2026 08:55:08 -0700 Subject: [PATCH] Added ASan/UBSan to CI, and fixed issues it found. (#445) --- .github/workflows/main.yml | 16 +++++-- include/bx/bitarray.h | 3 -- include/bx/inline/bitarray.inl | 19 ++++---- include/bx/inline/math.inl | 17 ++++++- include/bx/inline/readerwriter.inl | 7 ++- include/bx/inline/simd_ni.inl | 6 ++- scripts/toolchain.lua | 19 ++++++++ src/bitarray.cpp | 17 +++---- src/dtoa.cpp | 2 +- src/settings.cpp | 7 ++- tests/bitarray_test.cpp | 73 ++++++++++++++++++++++++++++++ 11 files changed, 154 insertions(+), 32 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 67d7570..537701a 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -85,8 +85,9 @@ jobs: { compiler: gcc, genie: linux-gcc, bindir: linux64_gcc, config: release, binsuffix: Release }, { compiler: clang, genie: linux-clang, bindir: linux64_clang, config: debug, binsuffix: Debug }, { compiler: clang, genie: linux-clang, bindir: linux64_clang, config: release, binsuffix: Release }, + { compiler: clang, genie: linux-clang, bindir: linux64_clang, config: debug, binsuffix: Debug, genie-options: --with-sanitizers, suffix: -sanitizers }, ] - name: linux-${{ matrix.compiler }}-${{ matrix.config }}64 + name: linux-${{ matrix.compiler }}-${{ matrix.config }}64${{ matrix.suffix }} runs-on: ubuntu-24.04 steps: - name: Checkout bx @@ -97,9 +98,12 @@ jobs: - name: Build run: | cd bx - tools/bin/linux/genie --gcc=${{ matrix.genie }} gmake + tools/bin/linux/genie ${{ matrix.genie-options }} --gcc=${{ matrix.genie }} gmake make -R -C .build/projects/gmake-${{ matrix.genie }} config=${{ matrix.config }}64 -j$(nproc) - name: Check + env: + ASAN_OPTIONS: halt_on_error=1 + UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=1 run: | cd bx ".build/${{ matrix.bindir }}/bin/bx.test${{ matrix.binsuffix }}" -d yes @@ -110,8 +114,9 @@ jobs: include: [ { runs-on: macos-15, arch: osx-arm64, bindir: osx-arm64, config: debug, binsuffix: Debug }, { runs-on: macos-15, arch: osx-arm64, bindir: osx-arm64, config: release, binsuffix: Release }, + { runs-on: macos-15, arch: osx-arm64, bindir: osx-arm64, config: debug, binsuffix: Debug, genie-options: --with-sanitizers, suffix: -sanitizers }, ] - name: osx-${{ matrix.arch }}-${{ matrix.config }} + name: osx-${{ matrix.arch }}-${{ matrix.config }}${{ matrix.suffix }} runs-on: ${{ matrix.runs-on }} steps: - name: Checkout bx @@ -122,9 +127,12 @@ jobs: - name: Build run: | cd bx - tools/bin/darwin/genie --gcc=${{ matrix.arch }} gmake + tools/bin/darwin/genie ${{ matrix.genie-options }} --gcc=${{ matrix.arch }} gmake make -C .build/projects/gmake-${{ matrix.arch }} config=${{ matrix.config }} -j$(sysctl -n hw.physicalcpu) - name: Check + env: + ASAN_OPTIONS: halt_on_error=1 + UBSAN_OPTIONS: print_stacktrace=1:halt_on_error=1 run: | cd bx ".build/${{ matrix.bindir }}/bin/bx.test${{ matrix.binsuffix }}" -d yes diff --git a/include/bx/bitarray.h b/include/bx/bitarray.h index f3e5a1e..95d0c6b 100644 --- a/include/bx/bitarray.h +++ b/include/bx/bitarray.h @@ -247,9 +247,6 @@ namespace bx /// Default constructor. Constructs an empty bit array. BitArrayBaseT(); - /// Destructor. Releases storage. - ~BitArrayBaseT(); - /// Constructs a bit array by copying from a bit array view. /// /// @param[in] _src Source bit array view to copy from. diff --git a/include/bx/inline/bitarray.inl b/include/bx/inline/bitarray.inl index a7eaf2d..b0cf00a 100644 --- a/include/bx/inline/bitarray.inl +++ b/include/bx/inline/bitarray.inl @@ -251,11 +251,17 @@ namespace bx template inline void BitArrayBaseT::set(const BitArrayViewBase& _src) { - if (this != &_src) + if (this == &_src) { - this->setCount(_src.getCount() ); + return; + } - const uint32_t sizeInBytes = min(this->getCapacity(), _src.getCapacity() ) / 8; + this->setCount(_src.getCount() ); + + const uint32_t sizeInBytes = min(this->getCapacity(), _src.getCapacity() ) / 8; + + if (0 != sizeInBytes) + { bx::memCopy(this->m_ptr, _src.getPtr(), sizeInBytes); } } @@ -278,12 +284,6 @@ namespace bx MutableBitArrayView::set(_beginBit, _endBit, _value); } - template - inline BitArrayBaseT::~BitArrayBaseT() - { - this->clear(); - } - template template inline BitArrayBaseT& BitArrayBaseT::operator=(const BitArrayBaseT& _rhs) @@ -400,7 +400,6 @@ namespace bx { if (NULL != this->m_storage.getAllocator() ) { - this->clear(); this->m_storage.setCapacity(0, 0); } diff --git a/include/bx/inline/math.inl b/include/bx/inline/math.inl index d2d171d..4aa7ed4 100644 --- a/include/bx/inline/math.inl +++ b/include/bx/inline/math.inl @@ -444,7 +444,10 @@ BX_FP_PRECISE_BEGIN() const float scaled = _a * (2.0f*kInvPi); const float real = floor(scaled); const float xx = _a - real * kPiHalf; - const int32_t bits = int32_t(real) & 3; + const int32_t bits = isFinite(real) + ? int32_t(mod(real, 4.0f) ) & 3 + : 0 + ; constexpr float kSinC2 = -0.16666667163372039794921875f; constexpr float kSinC4 = 8.333347737789154052734375e-3f; @@ -702,6 +705,18 @@ BX_FP_PRECISE_BEGIN() return 0.0f; } + constexpr float expMax = log(kFloatLargest); + + if (_a >= expMax) + { + return kFloatInfinity; + } + + if (isNan(_a) ) + { + return _a; + } + constexpr float kExpC0 = 1.66666666666666019037e-01f; constexpr float kExpC1 = -2.77777777770155933842e-03f; constexpr float kExpC2 = 6.61375632143793436117e-05f; diff --git a/include/bx/inline/readerwriter.inl b/include/bx/inline/readerwriter.inl index 1e754fd..b3d657f 100644 --- a/include/bx/inline/readerwriter.inl +++ b/include/bx/inline/readerwriter.inl @@ -246,7 +246,12 @@ namespace bx int64_t remainder = m_size-m_pos; int32_t size = int32_t(max(0, min(_size, remainder, INT32_MAX) ) ); - memCopy(&m_data[m_pos], _data, size); + + if (0 < size) + { + memCopy(&m_data[m_pos], _data, size); + } + m_pos += size; m_top = max(m_top, m_pos); if (size != _size) diff --git a/include/bx/inline/simd_ni.inl b/include/bx/inline/simd_ni.inl index e1c7c09..82aca59 100644 --- a/include/bx/inline/simd_ni.inl +++ b/include/bx/inline/simd_ni.inl @@ -856,6 +856,9 @@ BX_FP_PRECISE_BEGIN() const Ty near_result = simd_f32_add(_a, one); const Ty exp_min = simd_splat(-87.33654475f); const Ty clamp_mask = simd_f32_cmple(_a, exp_min); + const Ty exp_max = simd_splat(88.72283905f); + const Ty inf = simd_splat(kFloatInfinity); + const Ty inf_mask = simd_f32_cmpge(_a, exp_max); const Ty kC0 = simd_splat( 1.66666666666666019037e-01f); const Ty kC1 = simd_splat(-2.77777777770155933842e-03f); const Ty kC2 = simd_splat( 6.61375632143793436117e-05f); @@ -887,7 +890,8 @@ BX_FP_PRECISE_BEGIN() const Ty ikk = simd_f32_ftoi_trunc(kk); const Ty raw = simd_f32_ldexp_ni(tmp6, ikk); const Ty rc = simd_selb(clamp_mask, zero, raw); - const Ty result = simd_selb(near_mask, near_result, rc); + const Ty ri = simd_selb(inf_mask, inf, rc); + const Ty result = simd_selb(near_mask, near_result, ri); return result; } diff --git a/scripts/toolchain.lua b/scripts/toolchain.lua index cf93be2..dab67b1 100644 --- a/scripts/toolchain.lua +++ b/scripts/toolchain.lua @@ -171,6 +171,11 @@ function toolchain(_buildDir, _libDir) description = "Use AVX extension.", } + newoption { + trigger = "with-sanitizers", + description = "Build with address and undefined behavior sanitizers (gcc/clang).", + } + -- Avoid error when invoking genie --help. if (_ACTION == nil) then return false end @@ -516,6 +521,20 @@ function toolchain(_buildDir, _libDir) crtNone() end + if _OPTIONS["with-sanitizers"] then + configuration { "linux-* or osx*" } + buildoptions { + "-fsanitize=address,undefined", + "-fno-sanitize-recover=all", + "-fno-omit-frame-pointer", + } + linkoptions { + "-fsanitize=address,undefined", + } + + configuration {} + end + flags { "NoPCH", "NativeWChar", diff --git a/src/bitarray.cpp b/src/bitarray.cpp index 77dfe2b..f20ca97 100644 --- a/src/bitarray.cpp +++ b/src/bitarray.cpp @@ -54,7 +54,7 @@ namespace bx const simd64_t zero = simd64_zero(); const simd64_t allOnes = simd64_splat(UINT64_MAX); const simd64_t maskB = simd64_x64_sll(allOnes, int(shiftB) ); - const simd64_t maskE = simd64_x64_srl(allOnes, int(64-shiftE) ); + const simd64_t maskE = simd64_x64_srl(allOnes, int( (64-shiftE) & 63) ); const simd64_t valVec = simd64_splat(uint64_t(_value) ); const simd64_t orbits = simd64_u64_sub(zero, valVec); const simd64_t orbitsB = simd64_and(maskB, orbits); @@ -132,7 +132,7 @@ namespace bx const uint32_t shiftE = shiftEVec.u32; const simd64_t allOnes = simd64_splat(UINT64_MAX); const simd64_t maskB = simd64_x64_sll(allOnes, int(shiftB) ); - const simd64_t maskE = simd64_x64_srl(allOnes, int(64-shiftE) ); + const simd64_t maskE = simd64_x64_srl(allOnes, int( (64-shiftE) & 63) ); if (idxB == idxE) { @@ -180,8 +180,7 @@ namespace bx const uint32_t shiftE = shiftEVec.u32; const simd64_t allOnes = simd64_splat(UINT64_MAX); const simd64_t maskB = simd64_x64_sll(allOnes, int(shiftB) ); - const simd64_t tmp0 = simd64_x64_srl(allOnes, int(63-shiftE) ); - const simd64_t maskE = simd64_x64_srl(tmp0, 1); + const simd64_t maskE = simd64_x64_srl(allOnes, int( (64-shiftE) & 63) ); if (idxB == idxE) { @@ -235,8 +234,7 @@ namespace bx const uint32_t shiftE = shiftEVec.u32; const simd64_t allOnes = simd64_splat(UINT64_MAX); const simd64_t maskB = simd64_x64_sll(allOnes, int(shiftB) ); - const simd64_t tmp0 = simd64_x64_srl(allOnes, int(63-shiftE) ); - const simd64_t maskE = simd64_x64_srl(tmp0, 1); + const simd64_t maskE = simd64_x64_srl(allOnes, int( (64-shiftE) & 63) ); if (idxB == idxE) { @@ -290,8 +288,7 @@ namespace bx const uint32_t shiftE = shiftEVec.u32; const simd64_t allOnes = simd64_splat(UINT64_MAX); const simd64_t maskB = simd64_x64_sll(allOnes, int(shiftB) ); - const simd64_t tmp0 = simd64_x64_srl(allOnes, int(63-shiftE) ); - const simd64_t maskE = simd64_x64_srl(tmp0, 1); + const simd64_t maskE = simd64_x64_srl(allOnes, int( (64-shiftE) & 63) ); if (idxB == idxE) { @@ -362,7 +359,7 @@ namespace bx _ptr[idxB] = replaceBits(_ptr[idxB], shiftB, maskB, _bits); const uint64_t bitsE = _bits >> (64 - shiftB); - const uint64_t maskE = UINT64_MAX >> (64 - shiftE); + const uint64_t maskE = UINT64_MAX >> ( (64 - shiftE) & 63); _ptr[idxE] = replaceBits(_ptr[idxE], 0, maskE, bitsE); } @@ -389,7 +386,7 @@ namespace bx const uint64_t maskB = UINT64_MAX >> shiftB; const uint64_t bitsB = extractBits(_ptr[idxB], shiftB, maskB); - const uint64_t maskE = UINT64_MAX >> (64 - shiftE); + const uint64_t maskE = UINT64_MAX >> ( (64 - shiftE) & 63); const uint64_t bitsE = extractBits(_ptr[idxE], 0, maskE); return (bitsE << (64-shiftB) ) | bitsB; diff --git a/src/dtoa.cpp b/src/dtoa.cpp index b903c4f..885c488 100644 --- a/src/dtoa.cpp +++ b/src/dtoa.cpp @@ -478,7 +478,7 @@ namespace bx return 0; } - _max = toString(_dst + 1, _max - 1, asUnsigned(-_value), _base, _separator); + _max = toString(_dst + 1, _max - 1, 0 - asUnsigned(_value), _base, _separator); if (_max == 0) { diff --git a/src/settings.cpp b/src/settings.cpp index b8fc821..7c8bdce 100644 --- a/src/settings.cpp +++ b/src/settings.cpp @@ -149,7 +149,12 @@ namespace bx const FilePath uri(_name); const StringView path(strTrim(uri.getPath(), "/") ); - _propertyName = uri.getFileName(); + const StringView slash = strRFind(_name, '/'); + + _propertyName = slash.isEmpty() + ? _name + : StringView(slash.getPtr()+1, _name.getTerm() ) + ; if (path.isEmpty() ) { diff --git a/tests/bitarray_test.cpp b/tests/bitarray_test.cpp index 4029cb9..4ff7ee6 100644 --- a/tests/bitarray_test.cpp +++ b/tests/bitarray_test.cpp @@ -903,3 +903,76 @@ TEST_CASE("MippedBitArray-dynamic", "[container]") REQUIRE(65 == array.findClear(64, 300) ); REQUIRE(bx::kInvalid == array.findClear(64, 65) ); } + +TEST_CASE("BitArray-ranges-ending-on-word-boundary", "[container]") +{ + uint64_t words[4]; + bx::MutableBitArrayView bs(words, sizeof(words), 256); + + bs.clear(); + bs.set(0, 64, true); + REQUIRE(UINT64_MAX == words[0]); + REQUIRE(0 == words[1]); + + bs.clear(); + bs.set(10, 192, true); + REQUIRE( (UINT64_MAX >> 10) == (words[0] >> 10) ); + REQUIRE(UINT64_MAX == words[1]); + REQUIRE(UINT64_MAX == words[2]); + REQUIRE(0 == words[3]); + REQUIRE(182 == bs.countBits(10, 192) ); + REQUIRE(64 == bs.countBits(128, 192) ); + + bs.set(0, 256, true); + bs.set(0, 256, false); + REQUIRE(bs.testNone() ); + + bs.set(70, true); + REQUIRE( bs.testAny(64, 128) ); + REQUIRE(!bs.testNone(64, 128) ); + REQUIRE(!bs.testAll(64, 128) ); + REQUIRE( bs.testNone(0, 64) ); + REQUIRE(!bs.testAny(0, 64) ); + REQUIRE(1 == bs.countBits(64, 128) ); + + bs.set(127, true); + REQUIRE( bs.testAny(64, 128) ); + REQUIRE(2 == bs.countBits(64, 128) ); + + bs.set(64, 128, true); + REQUIRE( bs.testAll(64, 128) ); + REQUIRE(!bs.testAll(0, 128) ); + + bs.set(0, 256, true); + REQUIRE( bs.testAll(0, 256) ); + REQUIRE(256 == bs.countBits(0, 256) ); + + bs.set(255, false); + REQUIRE(!bs.testAll(0, 256) ); + REQUIRE( bs.testAny(192, 256) ); + REQUIRE( bs.testAll(192, 255) ); + + bs.set(192, 256, false); + REQUIRE( bs.testNone(192, 256) ); + REQUIRE(!bs.testAny(192, 256) ); + REQUIRE( bs.testAll(0, 192) ); + + bs.clear(); + bs.set(60, 124, UINT64_MAX); + REQUIRE(0xf == (words[0] >> 60) ); + REQUIRE( (UINT64_MAX >> 4) == words[1]); + REQUIRE(UINT64_MAX == bs.get(60, 124) ); + + bs.clear(); + bs.set(64, 128, UINT64_C(0x1234567890abcdef) ); + REQUIRE(UINT64_C(0x1234567890abcdef) == words[1]); + REQUIRE(UINT64_C(0x1234567890abcdef) == bs.get(64, 128) ); + REQUIRE(0 == words[0]); + REQUIRE(0 == words[2]); + + bs.clear(); + bs.set(100, 164, UINT64_MAX); + REQUIRE(UINT64_MAX == bs.get(100, 164) ); + REQUIRE( (UINT64_MAX >> 36) == (words[1] >> 36) ); + REQUIRE( (UINT64_MAX >> 28) == words[2]); +}