[FIRRTL] Fix use-after-free in InferReset #7273
Conversation
| nodeOp->replaceAllUsesWith(wireOp); | ||
| nodeOp.erase(); | ||
| nodeOp->removeAttr(nodeOp.getInnerSymAttrName()); |
There was a problem hiding this comment.
Should probably remove the annotations as well, e.g. if the node has an OMIR path tracker we will end up having two things with the same path tracker.
Not as important but might want to make the Node non-forcible.
There was a problem hiding this comment.
yea working on it.
There was a problem hiding this comment.
Changing the node to be non-forcable is kind of a pain as it would requiring changing the number of results, which I don't think you can do. The forcable result should be unused, so it should be cleared away with the rest of the node by CSE/Canonicalization.
There was a problem hiding this comment.
Yes you can't change results, the way it's done is a bit rough but basically creates a new one and replaces it.
And even if you did that (there's a helper, but in the detail namespace) it'd be erasing the existing node so no-go.
Thanks!!
|
Thanks for getting us past this. Sounds like some bits are still coming, but please add a test (such as in the linked issue). |
|
Okay to land? 😁 |
As @youngar explains well, a node was being deleted when it had references. Just route the node through the bounce wire instead of trying to replace it.
Closes #7225