Skip to content

Conversation

@jade-cho
Copy link
Contributor

Details:

  • Fix int64_t overflow sink issue

Tickets:

  • 174701

@jade-cho jade-cho requested review from a team as code owners October 23, 2025 09:44
@github-actions github-actions bot added the category: GPU OpenVINO GPU plugin label Oct 23, 2025
if (is_number(lhs) && is_number(rhs)) {
auto rhs_val = as_number<int64_t>(rhs);
OPENVINO_ASSERT(rhs_val != 0, "Modulo by zero detected in operator%");
if (rhs_val == 1 || rhs_val == -1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any overflow issue here, is this really not a false positive?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To get INT64_MIN % -1, the program first does INT64_MIN / -1, which gives +2^63. But that number is too big for an int64_t, so it overflows. We need to handle 1 and -1 anyway. Previously, we used strings for exceptions, but now it's changed to use numbers. Indeed, I don’t think this issue will actually happen, but Coverity still flags it as a problem.

@isanghao isanghao added this pull request to the merge queue Oct 27, 2025
@isanghao isanghao removed this pull request from the merge queue due to a manual request Oct 27, 2025
+ Fix int64_t overflow sink
@jade-cho jade-cho force-pushed the fix_coverity_issue3 branch from eb911bf to 0815f08 Compare October 27, 2025 02:30
Copy link
Contributor

@isanghao isanghao left a comment

Choose a reason for hiding this comment

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

LGTM

@isanghao isanghao added this pull request to the merge queue Oct 27, 2025
Merged via the queue into openvinotoolkit:master with commit 5a51295 Oct 27, 2025
187 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: GPU OpenVINO GPU plugin

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants