Fix overflows in et

[lucylq]

Summary

Check various overflows

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19057

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

• Rolling out OSDC (ARC) runners on pull & trunk workflows in PyTorch main

❌ 2 New Failures, 3 Unrelated Failures

As of commit fdfd4c5 with merge base 4a69750 ():

NEW FAILURES - The following jobs have failed:

• Cadence Build & Test / cpu-test / test-aot / test-aot (gh)
  backends/cadence/aot/tests/test_replace_ops_passes.py::TestReplaceOpsPasses::test_replace_transposed_conv_with_linear_4
• pull / unittest / macos / macos-job (gh)
  backends/xnnpack/test/ops/test_sqrt.py::TestSqrt::test_fp16_sqrt

BROKEN TRUNK - The following jobs failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

• pull / unittest / windows / windows-job (gh) (trunk failure)
  ##[error]The operation was canceled.
• pull / unittest-editable / windows / windows-job (gh) (trunk failure)
  ##[error]The operation was canceled.
• Test Metal Backend / test-metal-qwen35-moe-tiny / macos-job (gh) (trunk failure)
  /Users/ec2-user/runner/_work/executorch/executorch/pytorch/executorch/examples/models/qwen3_5_moe/main.cpp:300:3: error: use of undeclared identifier 'cudaMemGetInfo'

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[github-actions]

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

[Copilot]

Pull request overview

This PR improves integer overflow safety across ExecuTorch runtime/components by replacing potentially overflowing additions with checked arithmetic using c10::add_overflows.

Changes:

• Add overflow-safe end-offset computations in core runtime helpers (HierarchicalAllocator, ArrayRef).
• Harden flat_tensor segment validation against addition overflow.
• Add overflow-checked argument-count and sequence-length validations in CUDA/Metal backends and Qualcomm llama runner code.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
runtime/core/hierarchical_allocator.h	Uses c10::add_overflows to safely compute offset_bytes + size_bytes before bounds checks.
runtime/core/array_ref.h	Prevents N + M overflow in ArrayRef::slice bounds validation.
extension/flat_tensor/flat_tensor_data_map.cpp	Validates segment (offset + size) with overflow detection before comparing to segment end.
examples/qualcomm/oss_scripts/llama/runner/runner.cpp	Adds overflow-checked cur_pos_ + num_prompt_tokens validation vs seq_len.
examples/qualcomm/oss_scripts/llama/runner/prompt_processor.cpp	Adds overflow-checked start_pos + num_prompt_tokens validation vs context limits.
backends/cuda/runtime/cuda_backend.cpp	Prevents overflow in n_inputs + n_outputs when validating args.size().
backends/apple/metal/runtime/metal_backend.cpp	Prevents overflow in n_inputs + n_outputs when validating args.size().

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

On MSVC, c10::add_overflows' fallback implementation performs a + b for signed types, which can itself invoke signed-overflow UB before the overflow check triggers. Since cur_pos_/end_pos are int64_t, this reintroduces a platform-specific UB risk. Consider switching this check to unsigned arithmetic (e.g., validate cur_pos_ >= 0, cast operands to uint64_t/size_t, then use add_overflows) or use an explicit bounds check like cur_pos_ <= seq_len - num_prompt_tokens after normalizing types.

[Copilot]

This uses c10::add_overflows with signed int64_t operands. In this repo's MSVC fallback implementation, add_overflows computes a + b for signed types, which can trigger signed-overflow UB before detection. To keep this check safe cross-platform, prefer unsigned arithmetic (after asserting start_pos >= 0) or rewrite as a non-overflowing bounds check using wider/unsigned types.

[Copilot]

segment_end_offset is a size_t but is passed as header_.segment_base_offset + header_.segment_data_size (uint64_t). On 32-bit builds this can truncate, making the bounds check ineffective for large offsets even though seg_end is computed in uint64_t. Consider changing segment_end_offset (and related computations) to uint64_t throughout this helper to avoid truncation and remove the need for casts.

[lucylq]

@claude review

[claude]

Claude finished @lucylq's task in 1m 56s —— View job

---

Review of PR #19057: Fix overflows in ExecuTorch

• Read all 7 changed files
• Analyze overflow check correctness
• Review c10::add_overflows usage and type safety
• Post review feedback

---

Overall this is a good defensive hardening PR. The changes are correct and consistent in their use of c10::add_overflows to replace unchecked arithmetic in bounds checks. A few observations:

Core Runtime Changes (LGTM)

runtime/core/hierarchical_allocator.h — Clean improvement. The old code (size_bytes <= SIZE_MAX - offset_bytes) was a manual overflow check that was correct but less readable. Using c10::add_overflows and reusing end_bytes in the subsequent buffer bounds check at line 78 eliminates the redundant addition. Both operands are size_t (unsigned), so no UB concerns here.

runtime/core/array_ref.h — Same pattern with size_t operands. Straightforward and correct.

flat_tensor Data Map (Minor observation)

extension/flat_tensor/flat_tensor_data_map.cpp:78-84 — The explicit static_cast<uint64_t> on both flatbuffer values and segment_end_offset is correct and ensures the overflow check operates in a consistent unsigned 64-bit domain. The Copilot reviewer flagged a potential 32-bit truncation issue where segment_end_offset is size_t (which is 32-bit on 32-bit platforms) but gets compared against seg_end which is uint64_t. In practice, ExecuTorch targets 64-bit platforms for this code path, and the cast to uint64_t at line 84 handles the comparison correctly. If 32-bit support matters for this path, the function signature would need to change — but that's a broader concern beyond this PR.

CUDA and Metal Backends (LGTM)

backends/cuda/runtime/cuda_backend.cpp and backends/apple/metal/runtime/metal_backend.cpp — Both n_inputs and n_outputs are size_t, so c10::add_overflows uses the unsigned path with no UB. Clean and correct.

Qualcomm Llama Runner (Signed overflow on MSVC fallback)

examples/qualcomm/oss_scripts/llama/runner/runner.cpp:437-441 and prompt_processor.cpp:252-258 — These use c10::add_overflows with int64_t operands. On GCC/Clang this compiles to __builtin_add_overflow which is well-defined. On MSVC, the fallback in c10/util/safe_numerics.h performs volatile T tmp = a + b for signed types, which is technically signed overflow UB. In practice, MSVC uses two's complement and volatile prevents the optimizer from exploiting the UB, so this works. However:

• These are Qualcomm-specific runner examples that won't be compiled with MSVC in practice, so the risk is effectively zero.
• The signed overflow UB concern is a pre-existing issue in c10::add_overflows itself, not something this PR introduces.

prompt_processor.cpp:256-257 — The RHS expression static_cast<int64_t>(metadata_.context_len) - static_cast<int64_t>(metadata_.ar_len) where both fields are int32_t is safe from underflow since the cast to int64_t happens before the subtraction (no narrowing). If ar_len > context_len, the result is simply negative, and the check correctly fails. This is fine.

Pre-existing narrowing concern (not introduced by this PR): In both runner.cpp:435 and prompt_processor.cpp:247, num_prompt_tokens is assigned from .size() (which returns size_t) into int/int32_t. On 64-bit platforms, this is a narrowing conversion that could silently truncate. This PR doesn't make this worse, but it's worth noting since the overflow checks added here depend on num_prompt_tokens not being negative from truncation.

Summary

The changes are correct and improve safety against integer overflow in bounds checks across the codebase. The core runtime changes (hierarchical_allocator.h, array_ref.h, flat_tensor_data_map.cpp) and backend changes (CUDA, Metal) are clean with no concerns. The Qualcomm runner changes are also correct in practice, with the signed-add_overflows-on-MSVC concern being pre-existing and irrelevant for the Qualcomm target platform.