[FIRRTL] Paths to other modules can crash

[youngar]

Running the following with ./bin/firtool path.fir --output-final-mlir=path.mlir

FIRRTL version 5.1.0
circuit Test:
  public module Test:
    output p : Path
    propassign p, path("OMReferenceTarget:~Test|Foo>w")
  public module Foo:
    wire w : UInt<8>
    invalidate w

Crashes with:

firtool: /scratch/aryoung/circt/lib/Dialect/FIRRTL/Transforms/LowerClasses.cpp:137: llvm::iterator_range<const StringAttr *> (anonymous namespace)::PathInfoTable::getRootsForPassthrough(mlir::StringAttr) const: Assertion `it != altBasePathsPassthroughs.end() && "expected passthrough module to already exist"' failed.
PLEASE submit a bug report to https://github.com/llvm/circt and include the crash backtrace.
 #0 0x00000000008222c7 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /scratch/aryoung/circt/llvm/llvm/lib/Support/Unix/Signals.inc:804:13
 #1 0x0000000000820200 llvm::sys::RunSignalHandlers() /scratch/aryoung/circt/llvm/llvm/lib/Support/Signals.cpp:106:18
 #2 0x0000000000822b0a SignalHandler(int, siginfo_t*, void*) /scratch/aryoung/circt/llvm/llvm/lib/Support/Unix/Signals.inc:0:3
 #3 0x00007f8c015dbcf0 __restore_rt (/lib64/libpthread.so.0+0x12cf0)
 #4 0x00007f8c003efacf raise (/lib64/libc.so.6+0x4eacf)
 #5 0x00007f8c003c2ea5 abort (/lib64/libc.so.6+0x21ea5)
 #6 0x00007f8c003c2d79 _nl_load_domain.cold.0 (/lib64/libc.so.6+0x21d79)
 #7 0x00007f8c003e8426 (/lib64/libc.so.6+0x47426)
 #8 0x0000000000a2e94e llvm::DenseMapIterator<mlir::StringAttr, llvm::SmallVector<mlir::StringAttr, 6u>, llvm::DenseMapInfo<mlir::StringAttr, void>, llvm::detail::DenseMapPair<mlir::StringAttr, llvm::SmallVector<mlir::StringAttr, 6u>>, true>::operator->() const /scratch/aryoung/circt/llvm/llvm/include/llvm/ADT/DenseMap.h:1289:5
 #9 0x0000000000a2e94e (anonymous namespace)::PathInfoTable::getRootsForPassthrough(mlir::StringAttr) const /scratch/aryoung/circt/lib/Dialect/FIRRTL/Transforms/LowerClasses.cpp:138:29
#10 0x0000000000a2e94e PathOpConversion::matchAndRewrite(circt::firrtl::PathOp, circt::firrtl::PathOpAdaptor, mlir::ConversionPatternRewriter&) const /scratch/aryoung/circt/lib/Dialect/FIRRTL/Transforms/LowerClasses.cpp:1792:25
#11 0x0000000000a2ea8e mlir::OpConversionPattern<circt::firrtl::PathOp>::matchAndRewrite(circt::firrtl::PathOp, circt::firrtl::PathOpGenericAdaptor<llvm::ArrayRef<mlir::ValueRange>>, mlir::ConversionPatternRewriter&) const /scratch/aryoung/circt/llvm/llvm/../mlir/include/mlir/Transforms/DialectConversion.h:624:12

cc @mikeurbach, I did not reach this from real Chisel code, I was just curious what would happen.

[mikeurbach]

Thanks for filing this, we probably need to handle this gracefully. This "passthrough" code path is a special case to handle the memory extraction transforms, where we do something special to pass through an alternative basepath to modules containing memories that have been extracted. I think this IR is triggering that case because path and the thing it refers to don't have a common "owning module", so we start down this alternative basepath route, but the IR doesn't match the very special case that it was intended to handle.