Skip to content

Fix signed integer overflow UB in BSNumber and BitHacks for INT_MIN#122

Open
CodeReclaimers wants to merge 1 commit intodavideberly:masterfrom
CodeReclaimers:fix/bsnumber-signed-overflow
Open

Fix signed integer overflow UB in BSNumber and BitHacks for INT_MIN#122
CodeReclaimers wants to merge 1 commit intodavideberly:masterfrom
CodeReclaimers:fix/bsnumber-signed-overflow

Conversation

@CodeReclaimers
Copy link
Contributor

BSNumber(int32_t) and BSNumber(int64_t) negate negative inputs via number = -number, which is undefined behavior for INT32_MIN/INT64_MIN. Fix: compute the magnitude in unsigned arithmetic.

BitHacks::GetTrailingBit(int32_t) has the same issue with value & -value for INT32_MIN, and the uint32_t overload delegated to it. Fix: reverse the delegation so uint32_t does the work using ~value + 1u, and int32_t delegates to uint32_t.

Test: build with -fsanitize=undefined and construct BSNumber from INT32_MIN and INT64_MIN.

Test (CMakeLists.txt)
cmake_minimum_required(VERSION 3.14)
project(test_issue_8_2 LANGUAGES CXX)
set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(GTE_ROOT "${CMAKE_CURRENT_SOURCE_DIR}/../../GTE")
add_executable(test_issue_8_2 test_issue_8_2.cpp)
target_include_directories(test_issue_8_2 PRIVATE "${GTE_ROOT}")
Test (test_issue_8_2.cpp)
#include <cstdlib>
#include <cstdint>
#include <climits>
#include <cmath>
#include <Mathematics/BSNumber.h>
#include <Mathematics/UIntegerAP32.h>

using BSN = gte::BSNumber<gte::UIntegerAP32>;

int main()
{
    BSN bn32(INT32_MIN);
    BSN bn64(INT64_MIN);
    double v32 = static_cast<double>(bn32);
    double v64 = static_cast<double>(bn64);
    bool ok = (v32 == static_cast<double>(INT32_MIN))
           && (v64 == static_cast<double>(INT64_MIN));
    return ok ? EXIT_SUCCESS : EXIT_FAILURE;
}

…alues

BSNumber(int32_t) and BSNumber(int64_t) negated negative inputs via
`number = -number`, which is undefined behavior for INT32_MIN/INT64_MIN.
Fix: use unsigned arithmetic for the magnitude.

BitHacks::GetTrailingBit(int32_t) used `value & -value` which also
overflows for INT32_MIN, and the uint32_t overload delegated to it.
Fix: reverse delegation so uint32_t does the work using ~value + 1u.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant