- Notifications
You must be signed in to change notification settings - Fork 758
check for overflow in calculate_nbytes#11217
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Conversation
pytorch-botbot commented May 29, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/11217
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 Cancelled JobAs of commit b4b35e2 with merge base 5ef38d3 ( CANCELLED JOB - The following job was cancelled. Please retry:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot commented May 29, 2025
This pull request was exported from Phabricator. Differential Revision: D75544079 |
Summary: Got a fuzzer error around overflow here Differential Revision: D75544079
434c823 to 5d3972eComparefacebook-github-bot commented May 29, 2025
This pull request was exported from Phabricator. Differential Revision: D75544079 |
Summary: Pull Request resolved: pytorch#11217 Got a fuzzer error around overflow here Differential Revision: D75544079
5d3972e to 32eef53CompareSummary: Got a fuzzer error around overflow here Differential Revision: D75544079
32eef53 to 2daf3a0Comparefacebook-github-bot commented May 29, 2025
This pull request was exported from Phabricator. Differential Revision: D75544079 |
Summary: Pull Request resolved: pytorch#11217 Got a fuzzer error around overflow here Differential Revision: D75544079
2daf3a0 to 9831a5fCompareSummary: Got a fuzzer error around overflow here Differential Revision: D75544079
9831a5f to 7351b90Compare
lucylq left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this! Could you also make the same changes to tensor_layout?
executorch/runtime/core/tensor_layout.cpp
Line 19 in 14c3b31
| Result<size_t> calculate_nbytes( |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
facebook-github-bot commented May 30, 2025
@JacobSzwejbka has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
3f3b171 to aca6a6dCompareSummary: Got a fuzzer error around overflow here Reviewed By: lucylq Differential Revision: D75544079 Pulled By: JacobSzwejbka
facebook-github-bot commented May 30, 2025
This pull request was exported from Phabricator. Differential Revision: D75544079 |
Summary: Got a fuzzer error around overflow here Reviewed By: lucylq Differential Revision: D75544079 Pulled By: JacobSzwejbka
aca6a6d to 1c3dcd0Comparefacebook-github-bot commented May 30, 2025
This pull request was exported from Phabricator. Differential Revision: D75544079 |
Summary: Got a fuzzer error around overflow here Reviewed By: lucylq Differential Revision: D75544079 Pulled By: JacobSzwejbka
1c3dcd0 to e0b8b5eCompareSummary: Got a fuzzer error around overflow here Reviewed By: lucylq Differential Revision: D75544079 Pulled By: JacobSzwejbka
e8c8ad9 to 0ba6715Comparefacebook-github-bot commented Jun 2, 2025
This pull request was exported from Phabricator. Differential Revision: D75544079 |
Summary: Got a fuzzer error around overflow here Pull Request resolved: pytorch#11217 Test Plan: Imported from GitHub, without a `Test Plan:` line. Rollback Plan: Reviewed By: lucylq Differential Revision: D75544079 Pulled By: JacobSzwejbka
0ba6715 to 83e4f30CompareSummary: Got a fuzzer error around overflow here Test Plan: Imported from GitHub, without a `Test Plan:` line. Rollback Plan: Reviewed By: lucylq Differential Revision: D75544079 Pulled By: JacobSzwejbka
83e4f30 to f96f246Comparefacebook-github-bot commented Jun 3, 2025
This pull request was exported from Phabricator. Differential Revision: D75544079 |
Summary: Got a fuzzer error around overflow here Test Plan: Imported from GitHub, without a `Test Plan:` line. Rollback Plan: Reviewed By: lucylq Differential Revision: D75544079 Pulled By: JacobSzwejbka
f96f246 to 0b65bb0CompareSummary: Got a fuzzer error around overflow here Test Plan: Imported from GitHub, without a `Test Plan:` line. Rollback Plan: Reviewed By: lucylq Differential Revision: D75544079 Pulled By: JacobSzwejbka
0b65bb0 to aecdbbcComparefacebook-github-bot commented Jun 3, 2025
This pull request was exported from Phabricator. Differential Revision: D75544079 |
1 similar comment
facebook-github-bot commented Jun 3, 2025
This pull request was exported from Phabricator. Differential Revision: D75544079 |
Summary: Got a fuzzer error around overflow here Pull Request resolved: pytorch#11217 Test Plan: Imported from GitHub, without a `Test Plan:` line. Rollback Plan: Reviewed By: lucylq Differential Revision: D75544079 Pulled By: JacobSzwejbka
aecdbbc to 83f6030Compare| // Check for overflow | ||
| ET_CHECK(sizes[i] == 0 || n / sizes[i] == prev_n); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it necessary to perform a division here? could we check n >= prev_n instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah. yes...
JacobSzwejbkaJun 3, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok no it doesnt work. I can give a counter example.
To get the numbers smaller lets say Im accumulating in uint16_t and Im taking the product of int8_t::max
n: 127 prev_n: 1
n: 16129 prev_n: 127
n: 16767 prev_n: 16129 (overflowed at 65535 from logical value 2048383)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm. I'm tempted to ask if we should just import c10/util/safe_numerics.h (which, note to self, requires llvmMathExtra.h) instead of doing division then. it is annoying that it only has 64-bit safe_multiplies functions, but we could add a size_t-based one (in executorch for now if you don't want to wait for pin bump, followed by adding it in pytorch, getting a pin bump, and deleting the executorch one).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh oh!
There was an error while loading. Please reload this page.
Summary: Got a fuzzer error around overflow here Test Plan: Imported from GitHub, without a `Test Plan:` line. Rollback Plan: Reviewed By: lucylq Differential Revision: D75544079 Pulled By: JacobSzwejbka
83f6030 to bbbf9b3Comparefacebook-github-bot commented Jun 3, 2025
This pull request was exported from Phabricator. Differential Revision: D75544079 |
Summary: Got a fuzzer error around overflow here Test Plan: Imported from GitHub, without a `Test Plan:` line. Rollback Plan: Reviewed By: lucylq Differential Revision: D75544079 Pulled By: JacobSzwejbka
bbbf9b3 to 656d28eComparefacebook-github-bot commented Jun 3, 2025
This pull request was exported from Phabricator. Differential Revision: D75544079 |
Summary: Got a fuzzer error around overflow here Test Plan: Imported from GitHub, without a `Test Plan:` line. Rollback Plan: Reviewed By: lucylq Differential Revision: D75544079 Pulled By: JacobSzwejbka
656d28e to 179b1f3Comparefacebook-github-bot commented Jun 3, 2025
This pull request was exported from Phabricator. Differential Revision: D75544079 |
Summary: Got a fuzzer error around overflow here Test Plan: Imported from GitHub, without a `Test Plan:` line. Rollback Plan: Reviewed By: lucylq Differential Revision: D75544079 Pulled By: JacobSzwejbka
179b1f3 to b4b35e2Comparefacebook-github-bot commented Jun 3, 2025
This pull request was exported from Phabricator. Differential Revision: D75544079 |
93b1a0c into pytorch:mainUh oh!
There was an error while loading. Please reload this page.
Summary: Got a fuzzer error around overflow here
Differential Revision: D75544079