[XPTI][SYCL] Fix undefined behavior in hash_t::bit_count()

[uditagarwal97]

Problem

xpti::hash_t::bit_count() computed the number of bits needed to represent a value as:

return static_cast<unsigned>(std::log2(value)) + 1;

This has two defects, both flagged by UndefinedBehaviorSanitizer:

1. value == 0 is undefined behaviour. std::log2(0) is -inf, and converting -inf to unsigned is UB. On x86-64 the conversion yields 0x80000000, so the function returned 0x80000001, which is then used as a shift count in compact()/compact_short() — UB a second time. A zero line or column is the common case, not a corner case: sycl/include/sycl/detail/code_location.hpp defaults both to 0, so this fires on ordinary tracing paths.

2. Imprecision for large values. The FIXME above the function already documented it: not every uint64_t is representable as double, so log2 can round up and the result is one too large. Measured against an exact integer implementation, every value from 2^49-1 upward is off by one (e.g. 2^49-1: 50 vs the correct 49; UINT64_MAX: 65 vs 64).

UBSan diagnostic (build configured with -DLLVM_USE_SANITIZER=Address;Undefined; the runtime is built with -fno-sanitize-recover=all, so this aborts the process):

include/xpti/xpti_data_types.h:157:34: runtime error: -inf is outside the range of
representable values of type 'unsigned int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior

It reproduced in 46 SYCL unit tests — any test that emits a tracepoint with an unknown line/column.

Fix

Replace the floating-point computation with an integer loop, and keep the historical "a zero field still occupies one bit" behaviour so the packing in compact()/compact_short() does not change shape:

if (value == 0)
  return 1;
unsigned count = 0;
while (value) {
  ++count;
  value >>= 1;
}
return count;

Returning 1 for 0 matters: with 0 the neighbouring fields would shift into the zero field's place, and (file=1,func=1,line=0,col=1) and (file=1,func=1,line=1,col=0) would collide. Over a small sampled domain, that variant raised the number of colliding pairs from 4016 to 6339, so it is kept at 1.

Impact on hash values

Values change only where the old code was UB or imprecise:

input	old	new
0	0x80000001 (UB)	1
1 .. 2^48	same	same
2^49-1 .. UINT64_MAX	one too large	exact

---

Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com

[Copilot]

🟡 Changes recommended

Full-width values still cause undefined 64-bit shifts in packing paths, and regression coverage is missing.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Replaces floating-point bit counting in XPTI hashing with exact integer arithmetic.

Changes:

• Handles zero without undefined conversion.
• Computes exact bit counts for large integers.
• Preserves one-bit allocation for zero-valued fields.

File summaries

File	Description
xpti/include/xpti/xpti_data_types.h	Implements integer-based hash_t::bit_count().

Review details

Suppressed comments (1)

xpti/include/xpti/xpti_data_types.h:163

• For inputs at or above 2^63, this returns 64, but compact() and compact_short() immediately use the result as the shift count of a uint64_t. A shift by the type width is still undefined, so the advertised UINT64_MAX case remains undefined in the packing path. Please handle a 64-bit field explicitly before shifting (for example, clear the accumulated value before OR-ing the full-width field), or constrain and validate the accepted input range.

    return count;

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.