From fa9d1b13c9bc104e363fb7df292a9b964a92d730 Mon Sep 17 00:00:00 2001 From: dsengupta0628 Date: Wed, 4 Mar 2026 00:02:57 +0000 Subject: [PATCH 1/3] Fix more MT issues arising from pruneCrprArrival Signed-off-by: dsengupta0628 --- search/Search.cc | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/search/Search.cc b/search/Search.cc index f8a9c51d..6f56b625 100644 --- a/search/Search.cc +++ b/search/Search.cc @@ -1449,9 +1449,16 @@ ArrivalVisitor::pruneCrprArrivals() Arrival arrival = tag_bldr_->arrival(path_index); // Latch D->Q path uses enable min so crpr clk path min/max // does not match the path min/max. - if (delayGreater(max_arrival_max_crpr, arrival, min_max, this) - && clk_info_no_crpr->crprClkPath(this)->minMax(this) - == clk_info->crprClkPath(this)->minMax(this)) { + // Use crprClkPathRaw() rather than crprClkPath() to avoid going + // through Path::vertexPath(), which can transiently return nullptr + // during a concurrent setVertexArrivals() tag-group transition. + // The min/max is a property of the tag (tag_index_), which is + // stable for the lifetime of the ClkInfo object. + const Path *crpr_clk_path_no_crpr = clk_info_no_crpr->crprClkPathRaw(); + if (crpr_clk_path_no_crpr + && delayGreater(max_arrival_max_crpr, arrival, min_max, this) + && crpr_clk_path_no_crpr->minMax(this) + == clk_info->crprClkPathRaw()->minMax(this)) { debugPrint(debug_, "search", 3, " pruned %s", tag->to_string(this).c_str()); path_itr = path_index_map.erase(path_itr); From 0a8ccbd3c4f83c4a5e1e0493e3a51940cf68db96 Mon Sep 17 00:00:00 2001 From: dsengupta0628 Date: Wed, 4 Mar 2026 21:08:23 +0000 Subject: [PATCH 2/3] Revert "Fix more MT issues arising from pruneCrprArrival" This reverts pruneCrprArrival changes commit fa9d1b13c9bc104e363fb7df292a9b964a92d730. --- search/Search.cc | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/search/Search.cc b/search/Search.cc index 6f56b625..f8a9c51d 100644 --- a/search/Search.cc +++ b/search/Search.cc @@ -1449,16 +1449,9 @@ ArrivalVisitor::pruneCrprArrivals() Arrival arrival = tag_bldr_->arrival(path_index); // Latch D->Q path uses enable min so crpr clk path min/max // does not match the path min/max. - // Use crprClkPathRaw() rather than crprClkPath() to avoid going - // through Path::vertexPath(), which can transiently return nullptr - // during a concurrent setVertexArrivals() tag-group transition. - // The min/max is a property of the tag (tag_index_), which is - // stable for the lifetime of the ClkInfo object. - const Path *crpr_clk_path_no_crpr = clk_info_no_crpr->crprClkPathRaw(); - if (crpr_clk_path_no_crpr - && delayGreater(max_arrival_max_crpr, arrival, min_max, this) - && crpr_clk_path_no_crpr->minMax(this) - == clk_info->crprClkPathRaw()->minMax(this)) { + if (delayGreater(max_arrival_max_crpr, arrival, min_max, this) + && clk_info_no_crpr->crprClkPath(this)->minMax(this) + == clk_info->crprClkPath(this)->minMax(this)) { debugPrint(debug_, "search", 3, " pruned %s", tag->to_string(this).c_str()); path_itr = path_index_map.erase(path_itr); From 3afeba72126d526c0973bddfaaa6e526ccce39e5 Mon Sep 17 00:00:00 2001 From: dsengupta0628 Date: Wed, 4 Mar 2026 21:09:02 +0000 Subject: [PATCH 3/3] Revert "handle MT issues in setVertexArrivals, fix non-determ crash" This reverts commit for serVertexArrivals MT safe change 16c2678e514c1a7c13a2f997ecf04ab40861154a. --- graph/Graph.cc | 6 ----- include/sta/Graph.hh | 3 --- include/sta/Search.hh | 5 ---- search/Search.cc | 56 ++++--------------------------------------- 4 files changed, 5 insertions(+), 65 deletions(-) diff --git a/graph/Graph.cc b/graph/Graph.cc index 96323427..2fe4d934 100644 --- a/graph/Graph.cc +++ b/graph/Graph.cc @@ -1126,12 +1126,6 @@ Vertex::deletePaths() tag_group_index_ = tag_group_index_max; } -void -Vertex::setPathsDeferred(Path *paths) -{ - paths_ = paths; -} - bool Vertex::hasFanin() const { diff --git a/include/sta/Graph.hh b/include/sta/Graph.hh index 5adb8b63..28625854 100644 --- a/include/sta/Graph.hh +++ b/include/sta/Graph.hh @@ -264,9 +264,6 @@ public: Path *makePaths(uint32_t count); void setPaths(Path *paths); void deletePaths(); - // Set paths_ without deleting the old array. - // Caller is responsible for deferred deletion of the old array. - void setPathsDeferred(Path *paths); TagGroupIndex tagGroupIndex() const; void setTagGroupIndex(TagGroupIndex tag_index); // Slew is annotated by sdc set_annotated_transition cmd. diff --git a/include/sta/Search.hh b/include/sta/Search.hh index de2ff958..09bc1687 100644 --- a/include/sta/Search.hh +++ b/include/sta/Search.hh @@ -414,7 +414,6 @@ protected: void initVars(); void deleteTags(); void deleteTagsPrev(); - void deletePendingPaths(); void deleteUnusedTagGroups(); void seedInvalidArrivals(); void seedArrivals(); @@ -601,10 +600,6 @@ protected: std::mutex invalid_arrivals_lock_; BfsFwdIterator *arrival_iter_; ArrivalVisitor *arrival_visitor_; - // Old vertex path arrays deferred for deletion until after the parallel - // BFS level completes, preventing use-after-free in concurrent CRPR readers. - std::vector paths_pending_delete_; - std::mutex paths_pending_delete_lock_; // Some requireds exist. bool requireds_exist_; diff --git a/search/Search.cc b/search/Search.cc index f8a9c51d..248477d4 100644 --- a/search/Search.cc +++ b/search/Search.cc @@ -708,20 +708,6 @@ Search::deleteTagsPrev() for (TagGroup** tag_groups: tag_groups_prev_) delete [] tag_groups; tag_groups_prev_.clear(); - - deletePendingPaths(); -} - -// Free old vertex path arrays that were deferred during parallel BFS visits. -// Called after visitParallel completes so no thread can still hold a pointer -// into any of these arrays. -void -Search::deletePendingPaths() -{ - LockGuard lock(paths_pending_delete_lock_); - for (Path *paths : paths_pending_delete_) - delete [] paths; - paths_pending_delete_.clear(); } void @@ -2829,28 +2815,8 @@ void Search::setVertexArrivals(Vertex *vertex, TagGroupBldr *tag_bldr) { - if (tag_bldr->empty()) { - // Inline the deletePathsIncr logic using deferred deletion so that - // concurrent CRPR/latch readers that hold a pointer into the old path - // array are not left with a dangling pointer. - tnsNotifyBefore(vertex); - if (worst_slacks_) - worst_slacks_->worstSlackNotifyBefore(vertex); - TagGroup *tag_group = tagGroup(vertex); - if (tag_group) { - Path *old_paths = vertex->paths(); - // Clear the tag group index first so concurrent readers observe a - // null tag group (and return early from Path::vertexPath) before - // we touch paths_. - vertex->setTagGroupIndex(tag_group_index_max); - vertex->setPathsDeferred(nullptr); - tag_group->decrRefCount(); - if (old_paths) { - LockGuard lock(paths_pending_delete_lock_); - paths_pending_delete_.push_back(old_paths); - } - } - } + if (tag_bldr->empty()) + deletePathsIncr(vertex); else { TagGroup *prev_tag_group = tagGroup(vertex); Path *prev_paths = vertex->paths(); @@ -2861,25 +2827,13 @@ Search::setVertexArrivals(Vertex *vertex, } else { if (prev_tag_group) { - // Clear the tag group index before replacing paths so concurrent - // readers see a consistent null-tag-group state during the - // transition and do not mix the old tag group with the new array. - vertex->setTagGroupIndex(tag_group_index_max); + vertex->deletePaths(); prev_tag_group->decrRefCount(); requiredInvalid(vertex); } size_t path_count = tag_group->pathCount(); - // Allocate the new array and switch paths_ directly from old to new - // without creating a null window or freeing the old array immediately. - // This prevents concurrent CRPR/latch readers from observing either a - // null pointer or a freed (dangling) pointer. - Path *new_paths = new Path[path_count]; - vertex->setPathsDeferred(new_paths); - if (prev_paths) { - LockGuard lock(paths_pending_delete_lock_); - paths_pending_delete_.push_back(prev_paths); - } - tag_bldr->copyPaths(tag_group, new_paths); + Path *paths = vertex->makePaths(path_count); + tag_bldr->copyPaths(tag_group, paths); vertex->setTagGroupIndex(tag_group->index()); tag_group->incrRefCount(); }