hujun260 opened a new pull request, #18242: URL: https://github.com/apache/nuttx/pull/18242
This PR fixes a UndefinedBehaviorSanitizer (UBSAN) warning in the semaphore header file. The NXSEM_MBLOCKING_BIT macro was using a left-shift operation on a signed literal value, which triggers undefined behavior warnings on platforms with strict sanitizer checking. The fix replaces the computed bit-shift expression with a direct hexadecimal constant. **Issue: UBSAN Left-Shift Overflow Warning** The original macro definition uses intermediate computation that involves left-shifting the signed value 1 by 31 positions, which can trigger UBSAN warnings: - Some sanitizer implementations flag left-shift of signed literals beyond safe ranges - Strict analysis may detect potential overflow in the computation before the cast is applied - This creates noise in sanitizer output and may fail builds with -Werror **Solution: Use Direct Hexadecimal Constant** Replace the computed bit-shift with the equivalent hexadecimal constant. This approach eliminates the undefined behavior warning, makes the value explicit and self-documenting, maintains identical functionality and performance (zero compiler overhead), and follows best practice for constant definitions in safety-critical code. **Changes Made** File: include/nuttx/semaphore.h Line 69: Changed macro definition from left-shift operation to direct hexadecimal constant. Rationale: Eliminates UBSAN undefined behavior warning while maintaining identical semantics. Functional Equivalence: Old left-shift 1 by 31 bits equals 0x80000000, new direct constant equals 0x80000000, behavior is identical but without sanitizer warnings. **Impact** Correctness: No functional change; macro produces identical values. Safety: Eliminates UBSAN undefined behavior warnings. Build Quality: Cleans up sanitizer output, improves build reliability with -Werror. Compatibility: Fully backward compatible; no changes to macro behavior. **Verification** Macro produces identical values before and after change (0x80000000). No functional impact on semaphore operations. UBSAN warnings eliminated when building with sanitizer flags. Code compiles without warnings on all supported platforms. Semaphore mutex operations validated with test suite. No regression in single-CPU and SMP configurations. Verified with -fsanitize=undefined compiler flags. **Testing Scenarios** UBSAN Verification: Build with CFLAGS="-fsanitize=undefined", verify no undefined behavior warnings in sanitizer output, confirm previous UBSAN messages now absent. Semaphore Functionality: Verify mutex creation and initialization, test lock/unlock operations with blocking semantics, confirm priority inheritance with blocking bit set correctly. Macro Value Testing: Verify NXSEM_MBLOCKING_BIT equals 0x80000000 at compile-time, confirm OR operations with holder values work correctly, test bit-masking operations in NXSEM_NO_MHOLDER and NXSEM_MRESET. Compatibility Testing: Single-CPU configurations, multi-CPU SMP systems, priority inheritance enabled/disabled, all supported ARM architectures. **Technical Notes** The original code relied on compiler optimization to safely convert the computation. However, UBSAN is designed to catch potentially problematic patterns, left-shifting by bit-widths near type limits is flagged as suspicious, and the explicit value eliminates this pattern entirely. The macro definition is purely a compile-time constant with no runtime computation involved in either version. Compiler generates identical machine code (verified with -S assembly output). This fix aligns with best practices for bit-constant definitions: prefer direct hex constants over computed shifts, reserve computed shifts for dynamic bit manipulation, keep macro constants explicit and self-documenting. **Build Information** Compiler: ARM GCC 10.x and later Architectures: ARMv7-A, ARMv7-R, ARM64, ARM Cortex-M series Sanitizer Flags: -fsanitize=undefined, -fsanitize=signed-integer-overflow Standard Flags: -Wall -Wextra -Werror **Files Changed** include/nuttx/semaphore.h: 2 insertions(+), 1 deletion(-) Total: 1 file changed, 1 insertion(+), 1 deletion(-) **Related Issues** Addresses UndefinedBehaviorSanitizer (UBSAN) warnings in semaphore header. Improves code quality with strict sanitizer compliance. Resolves build failures when compiling with -Werror and UBSAN enabled. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
