[Relax][ONNX] Prevent Div divide-by-zero crashes#19566
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces division-by-zero checks for integer types in the ONNX frontend. It adds a helper function _const_integer_expr_has_zero to detect zero values in constant expressions and updates the Div operator implementation to either trigger a compile-time error or emit a runtime assertion. A review comment correctly identifies that a ValueError is being returned instead of raised, which would cause the frontend to fail with an unhelpful error message.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a check for integer division by zero in the ONNX frontend by adding a helper function _const_integer_expr_has_zero and updating the Div implementation. Feedback suggests making the type-checking logic more robust by handling potential AttributeError when accessing struct_info and extending the helper function to support relax.PrimValue for scalar constants.
| if isinstance(expr, relax.Constant): | ||
| return bool(_np.any(expr.data.numpy() == 0)) |
There was a problem hiding this comment.
The helper function _const_integer_expr_has_zero should also handle relax.PrimValue expressions, as they can also represent constant integer divisors in Relax. This ensures that scalar constant zero divisors are also caught during import validation.
| if isinstance(expr, relax.Constant): | |
| return bool(_np.any(expr.data.numpy() == 0)) | |
| if isinstance(expr, relax.Constant): | |
| return bool(_np.any(expr.data.numpy() == 0)) | |
| if isinstance(expr, relax.PrimValue) and isinstance(expr.value, tirx.IntImm): | |
| return int(expr.value.value) == 0 |
…aises_valueerror()
|
Div by zero is more like a runtime behavior so I think it should be handled in runtime instead of frontend. Or maybe relax/tirx passes is fine. cc @tlopex |
Hi Committers,
This PR is trying to fix issues #19541. Any suggestions would be appreciated if you are available.
Root cause:
The ONNX
Divpath in the Relax frontend did not separate two integer-divisor cases: constant zero divisors and dynamic/unknown divisors. As a result, constant integer zero divisors were not rejected during import, and dynamic integer divisors could reach runtime without a guard. When the divisor became zero at runtime, execution could trigger SIGFPE and terminate the process instead of raising a controlled error.Solution:
This PR applies a minimal, targeted fix in the ONNX frontend
Divconversion path. It introduces: import-time validation for constant integer divisors containing zero, raising ValueError early.