From d0eae7bc3ea9ba1705eaeca111a2e9d96d5e6468 Mon Sep 17 00:00:00 2001 From: Philipp Hahn Date: Mon, 24 Mar 2025 14:02:03 +0100 Subject: [PATCH] trace: fix SIGSEGV after pop 2 Calling `trace_pop_target()` invalidates the linked list of `target_stack_node_t`s by `free()`ing the node and its `p_target`. But `p_stack_top` may still points at that now freed node. Entering the debugger will crash as it will start from `p_stack_top`. 1. Explicitly invalidate `p_target` by setting it to `NULL`. 2. Explicitly pop the top node from `p_stack_top` before calling `trace_pop_target()`. Closes: #158 Fixes: a86f3c04a525 ("trace: fix SIGSEGV after pop") Signed-off-by: Philipp Hahn --- src/remake.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/remake.c b/src/remake.c index 84cde9e20..1bddb58bf 100644 --- a/src/remake.c +++ b/src/remake.c @@ -456,7 +456,7 @@ update_file_1 (struct file *file, unsigned int depth, } DBF (DB_VERBOSE, _("File '%s' was considered already.\n")); - p_stack_top = p_stack_top->p_parent; + p_stack_top = p_stack_top ? p_stack_top->p_parent : NULL; trace_pop_target(p_call_stack); return 0; } @@ -468,12 +468,12 @@ update_file_1 (struct file *file, unsigned int depth, break; case cs_running: DBF (DB_VERBOSE, _("Still updating file '%s'.\n")); - p_stack_top = p_stack_top->p_parent; + p_stack_top = p_stack_top ? p_stack_top->p_parent : NULL; trace_pop_target(p_call_stack); return 0; case cs_finished: DBF (DB_VERBOSE, _("Finished updating file '%s'.\n")); - p_stack_top = p_stack_top->p_parent; + p_stack_top = p_stack_top ? p_stack_top->p_parent : NULL; trace_pop_target(p_call_stack); return file->update_status; default: @@ -710,7 +710,7 @@ update_file_1 (struct file *file, unsigned int depth, set_command_state (file, cs_deps_running); --depth; DBF (DB_VERBOSE, _("The prerequisites of '%s' are being made.\n")); - p_stack_top = p_stack_top->p_parent; + p_stack_top = p_stack_top ? p_stack_top->p_parent : NULL; trace_pop_target(p_call_stack); return 0; } @@ -732,7 +732,7 @@ update_file_1 (struct file *file, unsigned int depth, OS (error, NILF, _("Target '%s' not remade because of errors."), file->name); - p_stack_top = p_stack_top->p_parent; + p_stack_top = p_stack_top ? p_stack_top->p_parent : NULL; trace_pop_target(p_call_stack); return dep_status; } @@ -843,7 +843,7 @@ update_file_1 (struct file *file, unsigned int depth, } notice_finished_file (file); - p_stack_top = p_stack_top->p_parent; + p_stack_top = p_stack_top ? p_stack_top->p_parent : NULL; trace_pop_target(p_call_stack); /* Since we don't need to remake the file, convert it to use the @@ -877,7 +877,7 @@ update_file_1 (struct file *file, unsigned int depth, DBF (DB_VERBOSE, _("Recipe of '%s' is being run.\n")); if ( file->tracing & BRK_AFTER_CMD || i_debugger_stepping ) enter_debugger(p_call_stack, file, 0, DEBUG_BRKPT_AFTER_CMD); - p_stack_top = p_stack_top->p_parent; + p_stack_top = p_stack_top ? p_stack_top->p_parent : NULL; trace_pop_target(p_call_stack); return 0; } @@ -901,7 +901,7 @@ update_file_1 (struct file *file, unsigned int depth, if ( file->tracing & BRK_AFTER_CMD || i_debugger_stepping ) enter_debugger(p_call_stack, file, 0, DEBUG_BRKPT_AFTER_CMD); - p_stack_top = p_stack_top->p_parent; + p_stack_top = p_stack_top ? p_stack_top->p_parent : NULL; trace_pop_target(p_call_stack); return file->update_status; }