circt icon indicating copy to clipboard operation
circt copied to clipboard

[FIRRTL] InferResets: deleting op used to drive reset network results in use-after-free

Open youngar opened this issue 2 months ago • 0 comments

firrtl.circuit "MovableNodeShouldDominate" {
  firrtl.module @MovableNodeShouldDominate(in %clock: !firrtl.clock) {
    %child_clock = firrtl.instance child @Child(in clock: !firrtl.clock)
    firrtl.connect %child_clock, %clock : !firrtl.clock
    %ui1 = firrtl.constant 1 : !firrtl.uint<1>
    %0 = firrtl.asAsyncReset %ui1 : (!firrtl.uint<1>) -> !firrtl.asyncreset
    %localReset = firrtl.node %0 {annotations = [{class = "sifive.enterprise.firrtl.FullAsyncResetAnnotation"}]} : !firrtl.asyncreset
  }
  firrtl.module @Child(in %clock: !firrtl.clock) {
    %reg = firrtl.reg %clock : !firrtl.clock, !firrtl.uint<8>
  }
}
./bin/circt-opt --pass-pipeline='builtin.module(firrtl.circuit(firrtl-infer-resets))' reset.mlir

InferResets needs to wire the result of the NodeOp to the instance of Child, and to do so replaces it with a wire. This results in a dangling reference to the deleted node stored in the ResetDomain.

Running it under valgrind yields many errors, such as the one below, but my line numbers won't line up with main due to a lot of printf debugging:

==176563== Invalid read of size 8
==176563==    at 0x1C35A53: mlir::Value::getLoc() const (llvm/mlir/lib/IR/Value.cpp:0)
==176563==    by 0x12A6F98: implementAsyncReset (lib/Dialect/FIRRTL/Transforms/InferResets.cpp:1688)
==176563==    by 0x12A6F98: implementAsyncReset (lib/Dialect/FIRRTL/Transforms/InferResets.cpp:1647)
==176563==    by 0x12A6F98: (anonymous namespace)::InferResetsPass::runOnOperationInner() (lib/Dialect/FIRRTL/Transforms/InferResets.cpp:545)
==176563==    by 0x12A4465: (anonymous namespace)::InferResetsPass::runOnOperation() (lib/Dialect/FIRRTL/Transforms/InferResets.cpp:513)

==176563==  Address 0x613f688 is 40 bytes inside a block of size 128 free'd
==176563==    at 0x4C39A93: free (vg_replace_malloc.c:872)
==176563==    by 0x18E466D: llvm::iplist_impl<llvm::simple_ilist<mlir::Operation>, llvm::ilist_traits<mlir::Operation> >::erase(llvm::ilist_iterator<llvm::ilist_detail::node_options<mlir::Operation, true, false, void, false>, false, false>) (ilist.h:205)
==176563==    by 0x12A7755: erase (OpDefinition.h:131)
==176563==    by 0x12A7755: implementAsyncReset (lib/Dialect/FIRRTL/Transforms/InferResets.cpp:1742)
==176563==    by 0x12A7755: implementAsyncReset (lib/Dialect/FIRRTL/Transforms/InferResets.cpp:1647)
==176563==    by 0x12A7755: (anonymous namespace)::InferResetsPass::runOnOperationInner() (lib/Dialect/FIRRTL/Transforms/InferResets.cpp:545)
==176563==    by 0x12A4465: (anonymous namespace)::InferResetsPass::runOnOperation() (lib/Dialect/FIRRTL/Transforms/InferResets.cpp:513)

youngar avatar Jun 21 '24 06:06 youngar