Scarfski/bgfx
1
0
Fork 0
mirror of https://github.com/bkaradzic/bgfx.git synced 2026-06-08 03:13:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix UBSan undefined-behavior hits in bgfx_p.h (#3688)

Browse Source 
* Default-initialize SortKey members to eliminate UBSan bool hit

SortKey data members had no initializers; the only initialization path
was an explicit reset() call. Reading m_hasAlphaRef (bool) before
reset() ran surfaced under BabylonNative's -fno-sanitize-recover=all
UBSan as 'load of value 190, which is not a valid value for type bool'.
Integer siblings had the same hazard, but UBSan only flags bools.

Add in-class default initializers matching reset()'s values. Zero
runtime cost where reset() immediately follows. No API/ABI change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Avoid shift-by-width UB in BitMaskToIndexIteratorT

UBSan on macOS flags the shifts at bgfx_p.h:854 and :862. bx::countTrailingZeros returns the type width (e.g. 32 for uint32_t) when its argument is zero, and that value was fed directly into the shift operator, producing shift-by-width UB.

Guard each shift against a zero mask so the width-valued ntz is never used as a shift exponent. Behavior is preserved for every input that previously avoided UB, and callers see isDone() == true in exactly the same iterations as before.

* Address review feedback: tighter BitMask ternary + SortKey reset() in EncoderImpl ctor

Per review on bkaradzic/bgfx#3688:

- BitMask: use branchless ternary (0 == _mask ? 0 : _mask >> ntz) in place of early-return guard.

- SortKey: drop in-class default initializers; initialize via m_key.reset() in EncoderImpl() ctor, which is the narrower location where the UB read originates.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Gary Hsu
2026-04-24 14:48:41 -07:00
committed by GitHub
parent 461622ab26
commit ac54f48d4d
1 changed files with 11 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
src/bgfx_p.h
View File

@@ -849,18 +849,22 @@ namespace bgfx
struct BitMaskToIndexIteratorT
{
BitMaskToIndexIteratorT(MaskT _mask)
: mask(0)
, idx(0)
{
const uint8_t ntz = bx::countTrailingZeros(_mask);
mask = _mask >> ntz;
const uint8_t ntz = bx::countTrailingZeros<MaskT>(_mask);
mask = 0 == _mask ? MaskT(0) : MaskT(_mask >> ntz);
idx = ntz;
}
void next()
{
// operator>> promotes to int, so we need to cast back:
const uint8_t ntzPlus1 = bx::countTrailingZeros<MaskT>(mask>>1) + 1;
mask >>= ntzPlus1;
idx += ntzPlus1;
mask = MaskT(mask >> 1);
idx += 1;
const uint8_t ntz = bx::countTrailingZeros<MaskT>(mask);
mask = 0 == mask ? MaskT(0) : MaskT(mask >> ntz);
idx += ntz;
}
bool isDone() const
@@ -2749,6 +2753,7 @@ namespace bgfx
// clear all bytes (inclusively the padding) before we start.
bx::memSet(&m_bind, 0, sizeof(m_bind) );
m_key.reset();
m_discard = false;
m_draw.clear(BGFX_DISCARD_ALL);
m_compute.clear(BGFX_DISCARD_ALL);