Fix overflow/crash in tf.range when limits is large #51359
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR tries to address the issue raised in 46913 where tf.range (and implicitly tf.keras.layers.RepeatVector) will overflow/crash when limits is large. The reason of the overflow is that while calculating the size within the kernel, the conditional statements comes with `int64 = cond ? int64 : double` will implicitly convert to double first and then cast back to int64, causing the overflow and crash. This PR fixes the issue by casting to int64 in both selections within the conditional statements first. This PR fixes 46913. Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
mihaimaruseac
requested changes
Aug 9, 2021
Comment on lines
75
to
79
| ? static_cast<int64>( | ||
| (std::abs(limit - start) + std::abs(delta) - 1) / | ||
| std::abs(delta)) | ||
| : static_cast<int64>( | ||
| std::ceil(std::abs((limit - start) / delta)))); |
There was a problem hiding this comment.
Let's separate each branch to a separate expression to ease in readability.
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
|
Thanks @mihaimaruseac . The PR has been updated. |
mihaimaruseac
approved these changes
Aug 17, 2021
google-ml-butler
bot
added
kokoro:force-run
gbaned
added
ready to pull
mihaimaruseac
pushed a commit
that referenced
this pull request
Oct 25, 2021
PiperOrigin-RevId: 391529518 Change-Id: Ie3db4ae6d3c0f3dc88404e1dbdc22f7d03cbeb3b
mihaimaruseac
pushed a commit
that referenced
this pull request
Oct 25, 2021
PiperOrigin-RevId: 391529518 Change-Id: Ie3db4ae6d3c0f3dc88404e1dbdc22f7d03cbeb3b
mihaimaruseac
pushed a commit
that referenced
this pull request
Oct 25, 2021
PiperOrigin-RevId: 391529518 Change-Id: Ie3db4ae6d3c0f3dc88404e1dbdc22f7d03cbeb3b
mihaimaruseac
added a commit
that referenced
this pull request
Oct 25, 2021
…1d297dbba90390d5482b76113899-on-r2.6 Merge pull request #51359 from yongtang:46913-range-overflow
mihaimaruseac
added a commit
that referenced
this pull request
Oct 25, 2021
…1d297dbba90390d5482b76113899-on-r2.5 Merge pull request #51359 from yongtang:46913-range-overflow
mihaimaruseac
added a commit
that referenced
this pull request
Oct 25, 2021
…1d297dbba90390d5482b76113899-on-r2.4 Merge pull request #51359 from yongtang:46913-range-overflow
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR tries to address the issue raised in #46913 where
tf.range (and implicitly tf.keras.layers.RepeatVector)
will overflow/crash when limits is large.
The reason of the overflow is that while calculating
the size within the kernel, the conditional statements
comes with
int64 = cond ? int64 : doublewill implicitlyconvert to double first and then cast back to int64, causing
the overflow and crash.
This PR fixes the issue by casting to int64 in both selections
within the conditional statements first.
This PR fixes #46913.
Signed-off-by: Yong Tang yong.tang.github@outlook.com