Skip to content

Fix integer modulo by zero crash in CPU EP Mod operator#27833

Merged
hariharans29 merged 2 commits intomainfrom
copilot/fix-mod-operator-divide-by-zero
Mar 30, 2026
Merged

Fix integer modulo by zero crash in CPU EP Mod operator#27833
hariharans29 merged 2 commits intomainfrom
copilot/fix-mod-operator-divide-by-zero

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 25, 2026

Description

Add a pre-check for zero values in the divisor tensor for integral types in Mod. Returns an error Status instead of hitting undefined behavior (SIGFPE / structured exception).

  • element_wise_ops.cc: Added CheckZeroDivisorImpl as a single template struct in the mod_internal namespace using if constexpr (std::is_integral<T>::value) to guard the check — no-op for non-integer types. The struct's operator() returns Status (via ORT_RETURN_IF) and is dispatched with InvokeRet<Status>. When the divisor is a constant initializer, TryGetConstantInput validates for zeros once at kernel creation time in the out-of-line constructor (using ORT_THROW_IF_ERROR), avoiding per-Compute overhead. A divisor_is_validated_constant_ flag tracks whether the one-time check was performed. In Compute, non-constant divisors are scanned via the type dispatcher (using ORT_RETURN_IF_ERROR) before calling CallModImpl, skipping the check when the constant was already validated. The Mod constructor is defined out-of-line after the mod_internal namespace to keep it contiguous.
  • element_wise_ops_test.cc: Added Mod_int8_by_zero, Mod_int32_by_zero, Mod_int64_by_zero_scalar tests covering tensor and scalar divisor cases, plus Mod_int32_by_zero_constant_initializer to exercise the TryGetConstantInput constructor path with is_initializer = true.

Motivation and Context

Integer modulo by zero is UB in C++ and causes a hardware exception that crashes the process. Float types produce NaN naturally via std::fmod, but int8/int16/int32/int64/uint* types do not. This is the same class of issue that was fixed for the Div operator in #27693, now applied to the Mod operator.


💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

Add pre-check for zero values in the divisor tensor for integral types
in the Mod operator. Returns an error instead of hitting undefined
behavior (SIGFPE / structured exception).

- When the divisor is a constant initializer, TryGetConstantInput
  validates for zeros once at kernel creation time in the constructor.
- In Compute, scans non-constant divisors before calling the mod
  dispatch, skipping the check when the constant was already validated.
- Added Mod_int8_by_zero, Mod_int32_by_zero, Mod_int64_by_zero_scalar,
  and Mod_int32_by_zero_constant_initializer tests.

Co-authored-by: edgchen1 <18449977+edgchen1@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/onnxruntime/sessions/a7664acb-d5cc-4408-9621-8e67b1bc4f23
Copy link
Copy Markdown
Contributor

@edgchen1 edgchen1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address the comments

…xpr, return Status, keep mod_internal contiguous

- Merged two template specializations into single template with if constexpr
- Changed operator() to return Status using ORT_RETURN_IF
- Use InvokeRet<Status> with ORT_THROW_IF_ERROR in constructor and
  ORT_RETURN_IF_ERROR in Compute
- Moved Mod constructor out-of-line after mod_internal namespace to keep
  the namespace contiguous

Co-authored-by: edgchen1 <18449977+edgchen1@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/onnxruntime/sessions/3fba85ca-1037-479d-8d46-4c1403a914ec
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 25, 2026

@copilot please address the comments

All three comments addressed in 04afe71.

Copilot AI requested a review from edgchen1 March 25, 2026 02:18
@edgchen1 edgchen1 marked this pull request as ready for review March 25, 2026 21:31
@hariharans29 hariharans29 enabled auto-merge (squash) March 30, 2026 17:10
@hariharans29 hariharans29 merged commit 2cdf010 into main Mar 30, 2026
103 of 106 checks passed
@hariharans29 hariharans29 deleted the copilot/fix-mod-operator-divide-by-zero branch March 30, 2026 21:17
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.

3 participants