Skip to content

Commit

Permalink
Reland "[FIRRTL][LowerClass] Pre-allocate namespaces before caputring…
Browse files Browse the repository at this point in the history
… refs (#7102)".

This was originally added to avoid a use-after-free. However, it got
removed as part of f4920aa. It seems this is still required, even when
we process PathTrackers sequentially. I confirmed there was a
use-after-free without this change, which is fixed after adding this
back.

Closes #7327.
  • Loading branch information
mikeurbach committed Jul 16, 2024
1 parent a9ac3ae commit 766bc37
Showing 1 changed file with 6 additions and 0 deletions.
6 changes: 6 additions & 0 deletions lib/Dialect/FIRRTL/Transforms/LowerClasses.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,12 @@ PathTracker::run(CircuitOp circuit, InstanceGraph &instanceGraph,
const DenseMap<DistinctAttr, FModuleOp> &owningModules) {
SmallVector<PathTracker> trackers;

// First allocate module namespaces. Don't capture a namespace reference at
// this point since they could be invalidated when DenseMap grows.
for (auto *node : instanceGraph)
if (auto module = node->getModule<FModuleLike>())
(void)namespaces.get(module);

for (auto *node : instanceGraph)
if (auto module = node->getModule<FModuleLike>()) {
PathTracker tracker(module, namespaces, instanceGraph, symbolTable,
Expand Down

0 comments on commit 766bc37

Please sign in to comment.