[FIRRTL][SpecializeLayers] Fix incorrect CF leading to double free (#7200)

As a part of specializing layers we have to remove any HierPathOps which
included a reference to deleted instances.  If any member of the path
array is contained in the deleted references list, we need to delete the
op.  There was incorrect use of `continue` which caused us to continue
processing the path instead of skipping to the next path operation,
which could lead to a double free when multiple instances in the path
were removed.
This commit is contained in:
Andrew Young 2024-06-18 08:42:26 -07:00 committed by GitHub
parent d340ca137c
commit baec1a1db1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 43 additions and 7 deletions

View File

@ -655,17 +655,18 @@ struct SpecializeLayers {
for (auto hierPath : llvm::make_early_inc_range( for (auto hierPath : llvm::make_early_inc_range(
circuit.getBody().getOps<hw::HierPathOp>())) { circuit.getBody().getOps<hw::HierPathOp>())) {
auto namepath = hierPath.getNamepath().getValue(); auto namepath = hierPath.getNamepath().getValue();
for (auto ref : namepath.drop_back()) { auto shouldDelete = [&](Attribute ref) {
if (removedSyms.contains(ref)) { return removedSyms.contains(ref);
};
if (llvm::any_of(namepath.drop_back(), shouldDelete)) {
removedPaths.insert(SymbolTable::getSymbolName(hierPath)); removedPaths.insert(SymbolTable::getSymbolName(hierPath));
hierPath->erase(); hierPath->erase();
continue; continue;
} }
}
// If we deleted the target of the hierpath, we don't need to add it to // If we deleted the target of the hierpath, we don't need to add it to
// the list of removedPaths, since no annotation will be left around to // the list of removedPaths, since no annotation will be left around to
// reference this path. // reference this path.
if (removedSyms.contains(namepath.back())) if (shouldDelete(namepath.back()))
hierPath->erase(); hierPath->erase();
} }

View File

@ -371,6 +371,41 @@ firrtl.circuit "HierPathDelete" attributes {
firrtl.extmodule @Deleted() attributes {layers = [@Layer]} firrtl.extmodule @Deleted() attributes {layers = [@Layer]}
} }
// CHECK-LABEL: firrtl.circuit "HierPathDelete"
firrtl.circuit "HierPathDelete" attributes {
disable_layers = [@Layer]
} {
firrtl.layer @Layer bind { }
// CHECK-NOT: hw.hierpath private @hierpath1 [@HierPathDelete::@middle, @Middle::@leaf, @Leaf]
hw.hierpath private @hierpath1 [@HierPathDelete::@middle, @Middle::@leaf, @Leaf]
// CHECK-NOT: hw.hierpath private @hierpath2 [@HierPathDelete::@middle, @Middle::@leaf]
hw.hierpath private @hierpath2 [@HierPathDelete::@middle, @Middle::@leaf]
// CHECK-NOT: hw.hierpath private @hierpath3 [@Middle::@leaf, @Leaf]
hw.hierpath private @hierpath3 [@Middle::@leaf, @Leaf]
// CHECK-NOT: hw.hierpath private @hierpath4 [@Middle::@leaf]
hw.hierpath private @hierpath4 [@Middle::@leaf]
firrtl.module @HierPathDelete() {
firrtl.layerblock @Layer {
firrtl.instance middle sym @middle @Middle()
}
}
firrtl.module @Middle() {
firrtl.layerblock @Layer {
firrtl.instance leaf sym @leaf @Leaf()
}
}
firrtl.extmodule @Leaf()
// CHECK-NOT: hw.hierpath private @DeletedPath [@Deleted]
hw.hierpath private @DeletedPath [@Deleted]
firrtl.extmodule @Deleted() attributes {layers = [@Layer]}
}
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
// Annotations // Annotations
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//