Skip to content

Improve error checking of Storage._writeFile. #46207

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.

Sign up for GitHub

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

Jump to bottom
Merged
malfet merged 1 commit into from
Oct 12, 2020
Merged

Improve error checking of Storage._writeFile. #46207

malfet merged 1 commit into from
Oct 12, 2020

Conversation

@malfet
Copy link
Contributor

@malfet malfet commented Oct 12, 2020

Summary:
Cherry-pick of #46036 into release/1.7

Previously, this function didn't do error-bounds checking on the GetItem (GET_ITEM) calls, which led to issues like #46020.

A better solution would be to use pybind, but given writing the file is going to dominate bounds checking, this is strictly better.

Test Plan: Imported from OSS

@gchanan @malfet
Improve error checking of Storage._writeFile. (pytorch#46036)
0d72706 
Summary:
Pull Request resolved: pytorch#46036

Previously, this function didn't do error-bounds checking on the GetItem (GET_ITEM) calls, which led to issues like pytorch#46020.

A better solution would be to use pybind, but given writing the file is going to dominate bounds checking, this is strictly better.

Test Plan: Imported from OSS

Reviewed By: mruberry

Differential Revision: D24228370

Pulled By: gchanan

fbshipit-source-id: f5d0a3d21ff12b4380beefe1e9954fa81ea2f567
@malfet malfet mentioned this pull request Oct 12, 2020
Closed
@dr-ci
Copy link

dr-ci bot commented Oct 12, 2020 •
edited
Loading

💊 CI failures summary and remediations

As of commit 0d72706 (more details on the Dr. CI page):

  • 3/3 failures possibly* introduced in this PR
    • 2/3 non-CircleCI failure(s)

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

See CircleCI build pytorch_linux_backward_compatibility_check_test (1/1)

Step: "Run tests" (full log | diagnosis details | 🔁 rerun)

Oct 12 19:37:59 The PR is introducing backward incompatible changes to the operator library. Please contact PyTorch team to confirm whether this change is wanted or not. 
Oct 12 19:37:59 processing existing schema:  dilation(__torch__.torch.classes.quantized.Conv3dPackedParamsBase _0) -> (int[] _0) 
Oct 12 19:37:59 processing existing schema:  groups(__torch__.torch.classes.quantized.Conv3dPackedParamsBase _0) -> (int _0) 
Oct 12 19:37:59 processing existing schema:  __getstate__(__torch__.torch.classes.xnnpack.LinearOpContext _0) -> ((Tensor, Tensor?, Scalar?, Scalar?) _0) 
Oct 12 19:37:59 processing existing schema:  __setstate__(__torch__.torch.classes.xnnpack.LinearOpContext _0, (Tensor, Tensor?, Scalar?, Scalar?) _1) -> (None _0) 
Oct 12 19:37:59 processing existing schema:  __getstate__(__torch__.torch.classes.xnnpack.Conv2dOpContext _0) -> ((Tensor, Tensor?, int[], int[], int[], int, Scalar?, Scalar?) _0) 
Oct 12 19:37:59 processing existing schema:  __setstate__(__torch__.torch.classes.xnnpack.Conv2dOpContext _0, (Tensor, Tensor?, int[], int[], int[], int, Scalar?, Scalar?) _1) -> (None _0) 
Oct 12 19:37:59 schema:  preprocess(Any self, Any mod, Dict(str, Any) method_compile_spec) -> (Any mod)  found on allowlist, skipping 
Oct 12 19:37:59 schema:  compile(Any self, Any processed, Dict(str, Any) method_compile_spec) -> (Dict(str, Any) handles)  found on allowlist, skipping 
Oct 12 19:37:59 schema:  execute(Any self, Any handle, Any[] input) -> (Any[] output)  found on allowlist, skipping 
Oct 12 19:37:59 processing existing schema:  __init__(__torch__.torch.classes.dist_rpc.WorkerInfo _0, str _1, int _2) -> (None _0) 
Oct 12 19:37:59 The PR is introducing backward incompatible changes to the operator library. Please contact PyTorch team to confirm whether this change is wanted or not.  
Oct 12 19:37:59  
Oct 12 19:37:59 Broken ops: [ 
Oct 12 19:37:59 	aten::min_values(Tensor self, int[1] dim, bool keepdim=False) -> (Tensor) 
Oct 12 19:37:59 	aten::min_values.names(Tensor self, str[1] dim, bool keepdim=False) -> (Tensor) 
Oct 12 19:37:59 	aten::max_values(Tensor self, int[1] dim, bool keepdim=False) -> (Tensor) 
Oct 12 19:37:59 	aten::max_values.names(Tensor self, str[1] dim, bool keepdim=False) -> (Tensor) 
Oct 12 19:37:59 ] 
Oct 12 19:37:59 =================== sccache compilation log =================== 
Oct 12 19:37:59 + cleanup 
Oct 12 19:37:59 + retcode=1

Extra GitHub checks: 1 failed

ci.pytorch.org: 1 failed

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 on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 3 times.

@malfet malfet merged commit 440b7bd into pytorch:release/1.7 Oct 12, 2020
@malfet malfet deleted the malfet/cp-46036 branch October 12, 2020 21:59
@stas00
Copy link
Contributor

stas00 commented Oct 14, 2020

Which build do I test to check that the problem has been solved? I tried with the latest nightly 1.8.0.dev20201013-py3.8_cuda10.2.89_cudnn7.6.5_0 and the problem is still there. Thank you.

@stas00
Copy link
Contributor

stas00 commented Oct 14, 2020

OK, it works with today's build 1.8.0.dev20201014-py3.8_cuda10.2.89_cudnn7.6.5_0

Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seemethere seemethere seemethere approved these changes

@albanD albanD albanD approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@malfet @stas00 @seemethere @albanD @gchanan