Skip to content

Commit 9f7c067

Browse files
litghostkmurray
authored andcommitted
Fix bug in add_subtree_to_route_tree that can result in double free.
Previous code potentially added a rt_node to two linked_rt_edge's, resulting in a double free when freeing the tree. Signed-off-by: Keith Rothman <[email protected]>
1 parent b8ee6af commit 9f7c067

File tree

1 file changed

+10
-4
lines changed

1 file changed

+10
-4
lines changed

vpr/src/route/route_tree_timing.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,7 @@ add_subtree_to_route_tree(t_heap* hptr, t_rt_node** sink_rt_node_ptr) {
287287
downstream_rt_node = sink_rt_node;
288288

289289
std::unordered_set<int> main_branch_visited;
290+
std::unordered_set<int> all_visited;
290291
inode = hptr->u.prev.node;
291292
t_edge_size iedge = hptr->u.prev.edge;
292293
short iswitch = device_ctx.rr_nodes[inode].edge_switch(iedge);
@@ -297,9 +298,15 @@ add_subtree_to_route_tree(t_heap* hptr, t_rt_node** sink_rt_node_ptr) {
297298

298299
while (rr_node_to_rt_node[inode] == nullptr) { //Not connected to existing routing
299300
main_branch_visited.insert(inode);
301+
all_visited.insert(inode);
300302

301303
linked_rt_edge = alloc_linked_rt_edge();
302304
linked_rt_edge->child = downstream_rt_node;
305+
306+
// Also mark downstream_rt_node->inode as visited to prevent
307+
// add_non_configurable_to_route_tree from potentially adding
308+
// downstream_rt_node to the rt tree twice.
309+
all_visited.insert(downstream_rt_node->inode);
303310
linked_rt_edge->iswitch = iswitch;
304311
linked_rt_edge->next = nullptr;
305312

@@ -340,7 +347,6 @@ add_subtree_to_route_tree(t_heap* hptr, t_rt_node** sink_rt_node_ptr) {
340347

341348
//Expand (recursively) each of the main-branch nodes adding any
342349
//non-configurably connected nodes
343-
std::unordered_set<int> all_visited = main_branch_visited;
344350
for (int rr_node : main_branch_visited) {
345351
add_non_configurable_to_route_tree(rr_node, false, all_visited);
346352
}
@@ -499,11 +505,11 @@ update_unbuffered_ancestors_C_downstream(t_rt_node* start_of_new_path_rt_node) {
499505

500506
/* Having set the value of C_downstream_addition, we must check whethere the parent switch
501507
* is a buffered or unbuffered switch with the if statement below. If the parent switch is
502-
* a buffered switch, then the parent node's downsteam capacitance is increased by the
508+
* a buffered switch, then the parent node's downsteam capacitance is increased by the
503509
* value of the parent switch's internal capacitance in the if statement below.
504-
* Correspondingly, the ancestors' downstream capacitance will be updated by the same
510+
* Correspondingly, the ancestors' downstream capacitance will be updated by the same
505511
* value through the while loop. Otherwise, if the parent switch is unbuffered, then
506-
* the if statement will be ignored, and the parent and ancestral nodes' downstream
512+
* the if statement will be ignored, and the parent and ancestral nodes' downstream
507513
* capacitance will be increased by the sum of the child's downstream capacitance with
508514
* the internal capacitance of the parent switch in the while loop/.*/
509515

0 commit comments

Comments
 (0)