py/objint_longlong: Fix overflow check in mp_obj_int_get_checked. #17739
Conversation
This file contains hidden or 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 is to fix an outstanding TODO. The test cases is using a range as this will exist in all builds, but mp_obj_get_int is used in many different parts of code where an overflow is more likely to occur. Signed-off-by: Yoctopuce dev <dev@yoctopuce.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #17739 +/- ##
=======================================
Coverage 98.41% 98.41%
=======================================
Files 171 171
Lines 22210 22210
=======================================
Hits 21857 21857
Misses 353 353 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Code size report: |
|
@dpgeorge Can you help me understand what could cause zephir port to cause a syntax error when compiling |
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.
Summary
This is to fix an outstanding TODO. The test cases is using a range expression, as this is available all builds, but
mp_obj_get_intis used by various extmod where an overflow is more likely to occur.Testing
The code is pretty simple, and has been tested on unix/longlong varient.
An test case has been added in CI tests.
Trade-offs and Alternatives
I am not sure that this exception is likely to occur in the MicroPython core code ,unless the Python code includes a serious bug. Therefore, the extra code size might not be worthwhile for everyone. However, there are many hardware extensions that use this function, and in those cases, the likelihood of an overflow is much higher.