Skip to content
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

Change maybe_resize_storage_cpu new_size arg to unsigned #52671

Closed
wants to merge 4 commits into from

Conversation

malfet
Copy link
Contributor

@malfet malfet commented Feb 23, 2021

Stack from ghstack:

Code is written with the assumption that new_size is unsigned value,
and when function is called with negative value it silently returns a nullptr rather than raise an exception.
Fix abovementioned logic by converting new_size to unsigned type and let cpu_allocator raise exception on negative alloc.

Unroll nested if blocks by returning early if new_size is 0

Add TestNN.test_adaptive_pooling_size_overflow to indirecty validate the fix.

Fixes #50960

Differential Revision: D26607549

Current code is written with the assumption that `new_size` is unsigned value,
and when function is called with negative value it silently returns a nullptr rather than raise an exception.
Fix abovementioned logic by converting new_size to unsigned type and let cpu_allocator raise exception on negative alloc.
Add `TestNN.test_adaptive_pooling_size_overflow` to indirecty validate the fix.

Fixes #50960

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Feb 23, 2021

💊 CI failures summary and remediations

As of commit 9d37713 (more details on the Dr. CI page):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

@malfet malfet requested review from mruberry, VitalyFedyunin and a team February 23, 2021 14:57
Current code is written with the assumption that `new_size` is unsigned value,
and when function is called with negative value it silently returns a nullptr rather than raise an exception.
Fix abovementioned logic by converting new_size to unsigned type and let cpu_allocator raise exception on negative alloc.
Add `TestNN.test_adaptive_pooling_size_overflow` to indirecty validate the fix.

Fixes #50960

Differential Revision: [D26607549](https://our.internmc.facebook.com/intern/diff/D26607549)

[ghstack-poisoned]
Current code is written with the assumption that `new_size` is unsigned value,
and when function is called with negative value it silently returns a nullptr rather than raise an exception.
Fix abovementioned logic by converting new_size to unsigned type and let cpu_allocator raise exception on negative alloc.
Add `TestNN.test_adaptive_pooling_size_overflow` to indirecty validate the fix.

Fixes #50960

Differential Revision: [D26607549](https://our.internmc.facebook.com/intern/diff/D26607549)

[ghstack-poisoned]
Code is written with the assumption that `new_size` is unsigned value,
and when function is called with negative value it silently returns a nullptr rather than raise an exception.
Fix abovementioned logic by converting new_size to unsigned type and let cpu_allocator raise exception on negative alloc.

Unroll nested if blocks by returning early if `new_size` is 0

Add `TestNN.test_adaptive_pooling_size_overflow` to indirecty validate the fix.

Fixes #50960

Differential Revision: [D26607549](https://our.internmc.facebook.com/intern/diff/D26607549)

[ghstack-poisoned]
@malfet malfet changed the title maybe_resize_storage_cpu to error on negative numbers Change maybe_resize_storage_cpu new_size arg to unsigned Feb 24, 2021
@facebook-github-bot
Copy link
Contributor

@malfet merged this pull request in 59ac0ff.

@facebook-github-bot facebook-github-bot deleted the gh/malfet/2/head branch February 28, 2021 15:16
aocsa pushed a commit to Quansight/pytorch that referenced this pull request Mar 15, 2021
…#52671)

Summary:
Pull Request resolved: pytorch#52671

Code is written with the assumption that new_size is unsigned value,
and when function is called with negative value it silently returns a nullptr rather than raise an exception.
Fix above-mentioned logic by converting new_size to unsigned type and let cpu_allocator raise exception on negative alloc.

Unroll nested if blocks by returning early if new_size is 0

Add TestNN.test_adaptive_pooling_size_overflow to indirecty validate the fix.

Fixes pytorch#50960

Test Plan: Imported from OSS

Reviewed By: walterddr

Differential Revision: D26607549

Pulled By: malfet

fbshipit-source-id: e3d4f7548b098f24fa5aba42d8f4e9288ece1e2e
xsacha pushed a commit to xsacha/pytorch that referenced this pull request Mar 31, 2021
…#52671)

Summary:
Pull Request resolved: pytorch#52671

Code is written with the assumption that new_size is unsigned value,
and when function is called with negative value it silently returns a nullptr rather than raise an exception.
Fix above-mentioned logic by converting new_size to unsigned type and let cpu_allocator raise exception on negative alloc.

Unroll nested if blocks by returning early if new_size is 0

Add TestNN.test_adaptive_pooling_size_overflow to indirecty validate the fix.

Fixes pytorch#50960

Test Plan: Imported from OSS

Reviewed By: walterddr

Differential Revision: D26607549

Pulled By: malfet

fbshipit-source-id: e3d4f7548b098f24fa5aba42d8f4e9288ece1e2e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants