Skip to content

Check for overflow in StorageView::reserve before allocation - #2094

Merged
jordimas merged 1 commit into
OpenNMT:masterfrom
infinityscroll:fix/storage-view-byte-overflow-20260830
Aug 30, 2026
Merged

Check for overflow in StorageView::reserve before allocation#2094
jordimas merged 1 commit into
OpenNMT:masterfrom
infinityscroll:fix/storage-view-byte-overflow-20260830

Conversation

@infinityscroll

@infinityscroll infinityscroll commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Reject negative reserve requests before the existing capacity fast path.
  • Check the element-count-to-byte-count multiplication before allocation, using division to avoid signed overflow.
  • Validate before releasing the current buffer, so a rejected request preserves its contents and shape.
  • Exercise FLOAT16/FLOAT32 byte-count boundaries, negative sizes, preservation after rejection, and zero/smaller/equal-capacity no-ops.

Validation

Built and tested on macOS arm64, AppleClang 21, Debug, Ruy backend, two build jobs:

cmake -S . -B build -DCMAKE_POLICY_VERSION_MINIMUM=3.5 \
  -DCMAKE_BUILD_TYPE=Debug -DWITH_MKL=OFF -DWITH_ACCELERATE=OFF \
  -DWITH_RUY=ON -DOPENMP_RUNTIME=NONE -DBUILD_TESTS=ON \
  -DBUILD_CLI=OFF -DBUILD_SHARED_LIBS=ON
cmake --build build --target ctranslate2_test --parallel 2
build/tests/ctranslate2_test tests/data \
  '--gtest_filter=StorageViewTest.*:CPU/StorageViewDeviceTest.*'
build/tests/ctranslate2_test tests/data --gtest_brief=1
  • Targeted storage tests: 11/11 passed.
  • Before the follow-up correction, the same tests against the original Check for overflow in StorageView::reserve byte size #2092 implementation reproduced the negative-size and buffer-preservation failures (9 passed, 2 failed).
  • Full local suite: 192 passed, 1 skipped, 3 failed (196 total). The failures are CPU/OpDeviceFPTest.Gemm/float32, GemmBias/float32, and GemmResidual/float32. The same three numerical failures were present in the Check for overflow in StorageView::reserve byte size #2092 baseline before the follow-up correction.
  • git diff --check passes. Linux/MKL, CUDA, and Windows have not been tested locally; upstream CI is still needed.

AI assistance disclosure

OpenAI Codex assisted with preparing the follow-up correction and regression tests.

Reject negative sizes before the capacity fast path, check byte-count multiplication before releasing storage or allocating, and add safe overflow, preservation, and no-op regression tests.
@infinityscroll
infinityscroll force-pushed the fix/storage-view-byte-overflow-20260830 branch from 3227a01 to b817e51 Compare August 30, 2026 16:14
@infinityscroll

infinityscroll commented Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

@jordimas Could you review it when you have a chance?

@jordimas

Copy link
Copy Markdown
Collaborator

@jordimas Could you review it when you have a chance?

Looks good. Thanks for your contribution

@jordimas
jordimas merged commit af0453f into OpenNMT:master Aug 30, 2026
22 checks passed
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.

2 participants