From 6a54fb6f96ce29afd2ecdaa0802e3e993dd07eda Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Thu, 7 May 2020 14:25:57 +0100 Subject: [PATCH] Modify std::multimap in V3Combine safely. We used to iterate the m_callMmap std::multimap by getting limit iterators from equal_range, but we also modify the same map in the loop which invalidates those limit iterators. Note this only caused actual problems if the new AstCCall inserted via 'addCall' in the loop had a memory address (which is used as the key) which fell into the range returned by equal_range, so was pretty hard to trigger. --- src/V3Combine.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/V3Combine.cpp b/src/V3Combine.cpp index 9224e0576..2f2a70423 100644 --- a/src/V3Combine.cpp +++ b/src/V3Combine.cpp @@ -86,11 +86,10 @@ public: } else { UINFO(4, " Remove " << oldfuncp << endl); } - std::pair eqrange - = m_callMmap.equal_range(oldfuncp); - for (CallMmap::iterator nextit = eqrange.first; nextit != eqrange.second;) { - CallMmap::iterator eqit = nextit++; - AstCCall* callp = eqit->second; + // Note: m_callMmap modified in loop, so not using equal_range. + for (CallMmap::iterator it = m_callMmap.find(oldfuncp); it != m_callMmap.end(); + it = m_callMmap.find(oldfuncp)) { + AstCCall* callp = it->second; if (!callp->user3()) { // !already done UINFO(4, " Called " << callp << endl); UASSERT_OBJ(callp->funcp() == oldfuncp, callp, @@ -105,8 +104,13 @@ public: } callp->user3(true); // Dead now VL_DO_DANGLING(pushDeletep(callp), callp); - m_callMmap.erase(eqit); // Fix the table } + // It is safe to unconditionally remove this entry here as the above + // 'if' would never be entered again for this entry (we set user3). + // The only other place where m_callMmap is looked up is deleteCall + // below, but that is only ever called straight after an addCall + // of the node being deleted, so it won't miss this entry. + m_callMmap.erase(it); // Fix the table } } // METHODS