Skip to content

Commit

Permalink
[mlir][Transforms] Fix use-after-free in #82474 (#82504)
Browse files Browse the repository at this point in the history
When a `ModifyOperationRewrite` is committed, the operation may already
have been erased, so `OperationName` must be cached in the rewrite
object.

Note: This will no longer be needed with #81757, which adds a "cleanup"
method to `IRRewrite`.
  • Loading branch information
matthias-springer authored Feb 21, 2024
1 parent 13b0321 commit 3f732c4
Showing 1 changed file with 7 additions and 4 deletions.
11 changes: 7 additions & 4 deletions mlir/lib/Transforms/Utils/DialectConversion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -965,14 +965,14 @@ class ModifyOperationRewrite : public OperationRewrite {
ModifyOperationRewrite(ConversionPatternRewriterImpl &rewriterImpl,
Operation *op)
: OperationRewrite(Kind::ModifyOperation, rewriterImpl, op),
loc(op->getLoc()), attrs(op->getAttrDictionary()),
name(op->getName()), loc(op->getLoc()), attrs(op->getAttrDictionary()),
operands(op->operand_begin(), op->operand_end()),
successors(op->successor_begin(), op->successor_end()) {
if (OpaqueProperties prop = op->getPropertiesStorage()) {
// Make a copy of the properties.
propertiesStorage = operator new(op->getPropertiesStorageSize());
OpaqueProperties propCopy(propertiesStorage);
op->getName().initOpProperties(propCopy, /*init=*/prop);
name.initOpProperties(propCopy, /*init=*/prop);
}
}

Expand All @@ -988,7 +988,9 @@ class ModifyOperationRewrite : public OperationRewrite {
void commit() override {
if (propertiesStorage) {
OpaqueProperties propCopy(propertiesStorage);
op->getName().destroyOpProperties(propCopy);
// Note: The operation may have been erased in the mean time, so
// OperationName must be stored in this object.
name.destroyOpProperties(propCopy);
operator delete(propertiesStorage);
propertiesStorage = nullptr;
}
Expand All @@ -1003,13 +1005,14 @@ class ModifyOperationRewrite : public OperationRewrite {
if (propertiesStorage) {
OpaqueProperties propCopy(propertiesStorage);
op->copyProperties(propCopy);
op->getName().destroyOpProperties(propCopy);
name.destroyOpProperties(propCopy);
operator delete(propertiesStorage);
propertiesStorage = nullptr;
}
}

private:
OperationName name;
LocationAttr loc;
DictionaryAttr attrs;
SmallVector<Value, 8> operands;
Expand Down

0 comments on commit 3f732c4

Please sign in to comment.