Skip to content

Fix use-after-free bug in peephole pass#6037

Merged
ezyang merged 2 commits intopytorch:masterfrom
bddppq:fix-peephole-memory-bug
Mar 27, 2018
Merged

Fix use-after-free bug in peephole pass#6037
ezyang merged 2 commits intopytorch:masterfrom
bddppq:fix-peephole-memory-bug

Conversation

@bddppq
Copy link
Copy Markdown
Contributor

@bddppq bddppq commented Mar 27, 2018

@bddppq bddppq changed the title Fix use after free bug in peephole pass Fix use-after-free bug in peephole pass Mar 27, 2018
@o8ht88z00f o8ht88z00f mentioned this pull request Mar 27, 2018
Closed
apaszke
apaszke reviewed Mar 27, 2018
View reviewed changes
Comment thread torch/csrc/jit/passes/peephole.cpp Outdated
if (!destroyed) {
for (Block * sub_block : n->blocks()) {
PeepholeOptimize(sub_block);
}

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

@apaszke
Copy link
Copy Markdown
Contributor

apaszke commented Mar 27, 2018

@pytorchbot add to whitelist

@ezyang
Copy link
Copy Markdown
Contributor

ezyang commented Mar 27, 2018

Hi:

  1. Did any of our current tests catch this bug?
  2. If so, why didn't the ASAN build catch the bug?

Thanks.

ezyang
ezyang requested changes Mar 27, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Want to know if a test is necessary

@bddppq
Copy link
Copy Markdown
Contributor Author

bddppq commented Mar 27, 2018

@ezyang This is very likely covered by tests in onnx-fb-universe, however its CI is not running in ASAN mode. Considering the those tests are going to be moved soon, probably not worth beefing up its CI setup?

@ezyang
Copy link
Copy Markdown
Contributor

ezyang commented Mar 27, 2018

I wouldn't suggest updating onnx-fb-universe, but I am wondering why the peephole optimizer (it's NOT onnx) isn't being sufficiently exercised in test_jit.py. How did you discover the bug?

@bddppq
Copy link
Copy Markdown
Contributor Author

bddppq commented Mar 27, 2018

Yeah I know it's optimizing the trace. I'm saying there are plenty of export tests in onnx-fb-universe and they are going to be moved and after the move there will be asan build of them.
We were exporting a model in fbcode and it builds in asan mode.

@ezyang ezyang merged commit ebc0194 into pytorch:master Mar 27, 2018
@bddppq bddppq deleted the fix-peephole-memory-bug branch March 27, 2018 21:26
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
@bddppq @ezyang
Fix use-after-free bug in peephole pass (pytorch#6037)
f1d8229 
* Fix use after free bug in peephole pass

* Move the loop befor the switch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ezyang ezyang ezyang approved these changes

+1 more reviewer

@apaszke apaszke apaszke approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bddppq @apaszke @ezyang