[2/2] Handle exception when getting result of inferior call

Message ID 20241028124040.19601-2-tdevries@suse.de
State New
Headers
Series [1/2,gdb/symtab] Fix dwarf version of DWO TU |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Tom de Vries Oct. 28, 2024, 12:40 p.m. UTC
  When running test-case gdb.ada/finish-var-size.exp with target board fission,
we first run into PR32315 with the finish command (as reported in that PR),
and then again with an inferior call:
...
(gdb) print pck.get(True)
gdb/dwarf2/read.c:5567: internal-error: load_full_comp_unit: \
  Assertion `! this_cu->is_debug_types' failed.
...

After the assert for the inferior call, we do:
...
Quit this debugging session? (y or n) n
  ...
Create a core file of GDB? (y or n) n
...
and run into:
...
gdb/infcall.c:1613: internal-error: call_function_by_hand_dummy: \
  Assertion `retval != NULL' failed.
...

So what happened?

Instead of aborting for the first assertion, some error is thrown in
call_thread_fsm::should_stop during the call to get_call_return_value, at
which point the fsm is already set to finished:
...
  if (stop_stack_dummy == STOP_STACK_DUMMY)
    {
      /* Done.  */
      set_finished ();

      /* Stash the return value before the dummy frame is popped and
	 registers are restored to what they were before the
	 call..  */
      return_value = get_call_return_value (&return_meta_info);
    }
...

The error is caught by run_inferior_call, and returned to
call_function_by_hand_dummy.

Then in call_function_by_hand_dummy, we do:
...
    if (call_thread->state != THREAD_EXITED)
      {
	/* The FSM should still be the same.  */
	gdb_assert (call_thread->thread_fsm () == sm);

	if (call_thread->thread_fsm ()->finished_p ())
	  {
	    struct value *retval;

	    infcall_debug_printf ("call completed");
...
and end up in the "call completed" branch, asserting that
call_thread->thread_fsm ()->return_value != NULL.

Indeed the call completed successfully, but we ran into an exception fetching
the return value of the call, so it's not there.

Fix this by handling the case that an exception was returned by
run_inferior_call in the "call completed" branch, getting us instead:
...
(gdb) print pck.get(True)
gdb/dwarf2/read.c:5567: internal-error: load_full_comp_unit: \
  Assertion `! this_cu->is_debug_types' failed.
  ...
Quit this debugging session? (y or n) n
  ...
Create a core file of GDB? (y or n) n
Command aborted.
(gdb)
...

Tested on aarch64-linux.

PR gdb/32316
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32316
---
 gdb/infcall.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)
  

Comments

Tom de Vries Jan. 20, 2025, 9:01 a.m. UTC | #1
On 10/28/24 13:40, Tom de Vries wrote:
> When running test-case gdb.ada/finish-var-size.exp with target board fission,
> we first run into PR32315 with the finish command (as reported in that PR),
> and then again with an inferior call:
> ...
> (gdb) print pck.get(True)
> gdb/dwarf2/read.c:5567: internal-error: load_full_comp_unit: \
>    Assertion `! this_cu->is_debug_types' failed.
> ...
> 
> After the assert for the inferior call, we do:
> ...
> Quit this debugging session? (y or n) n
>    ...
> Create a core file of GDB? (y or n) n
> ...
> and run into:
> ...
> gdb/infcall.c:1613: internal-error: call_function_by_hand_dummy: \
>    Assertion `retval != NULL' failed.
> ...
> 
> So what happened?
> 
> Instead of aborting for the first assertion, some error is thrown in
> call_thread_fsm::should_stop during the call to get_call_return_value, at
> which point the fsm is already set to finished:
> ...
>    if (stop_stack_dummy == STOP_STACK_DUMMY)
>      {
>        /* Done.  */
>        set_finished ();
> 
>        /* Stash the return value before the dummy frame is popped and
> 	 registers are restored to what they were before the
> 	 call..  */
>        return_value = get_call_return_value (&return_meta_info);
>      }
> ...
> 
> The error is caught by run_inferior_call, and returned to
> call_function_by_hand_dummy.
> 
> Then in call_function_by_hand_dummy, we do:
> ...
>      if (call_thread->state != THREAD_EXITED)
>        {
> 	/* The FSM should still be the same.  */
> 	gdb_assert (call_thread->thread_fsm () == sm);
> 
> 	if (call_thread->thread_fsm ()->finished_p ())
> 	  {
> 	    struct value *retval;
> 
> 	    infcall_debug_printf ("call completed");
> ...
> and end up in the "call completed" branch, asserting that
> call_thread->thread_fsm ()->return_value != NULL.
> 
> Indeed the call completed successfully, but we ran into an exception fetching
> the return value of the call, so it's not there.
> 
> Fix this by handling the case that an exception was returned by
> run_inferior_call in the "call completed" branch, getting us instead:
> ...
> (gdb) print pck.get(True)
> gdb/dwarf2/read.c:5567: internal-error: load_full_comp_unit: \
>    Assertion `! this_cu->is_debug_types' failed.
>    ...
> Quit this debugging session? (y or n) n
>    ...
> Create a core file of GDB? (y or n) n
> Command aborted.
> (gdb)
> ...
> 

Ping.

Thanks,
- Tom

> Tested on aarch64-linux.
> 
> PR gdb/32316
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32316
> ---
>   gdb/infcall.c | 32 ++++++++++++++++++--------------
>   1 file changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/gdb/infcall.c b/gdb/infcall.c
> index 02e3b385bd7..4998b933520 100644
> --- a/gdb/infcall.c
> +++ b/gdb/infcall.c
> @@ -1610,25 +1610,29 @@ call_function_by_hand_dummy (struct value *function,
>   
>   	    maybe_remove_breakpoints ();
>   
> -	    gdb_assert (retval != NULL);
> -
>   	    /* Destruct the pass-by-ref argument clones.  */
>   	    call_destructors (dtors_to_invoke, default_return_type);
>   
> -	    return retval;
> +	    if (e.reason >= 0)
> +	      {
> +		gdb_assert (retval != NULL);
> +		return retval;
> +	      }
>   	  }
>   	else
> -	  infcall_debug_printf ("call did not complete");
> +	  {
> +	    infcall_debug_printf ("call did not complete");
>   
> -	/* Didn't complete.  Clean up / destroy the call FSM, and restore the
> -	   previous state machine, and handle the error.  */
> -	{
> -	  std::unique_ptr<thread_fsm> finalizing
> -	    = call_thread->release_thread_fsm ();
> -	  call_thread->set_thread_fsm (std::move (saved_sm));
> +	    /* Didn't complete.  Clean up / destroy the call FSM, and restore
> +	       the previous state machine, and handle the error.  */
> +	    {
> +	      std::unique_ptr<thread_fsm> finalizing
> +		= call_thread->release_thread_fsm ();
> +	      call_thread->set_thread_fsm (std::move (saved_sm));
>   
> -	  finalizing->clean_up (call_thread.get ());
> -	}
> +	      finalizing->clean_up (call_thread.get ());
> +	    }
> +	  }
>         }
>     }
>   
> @@ -1638,8 +1642,8 @@ call_function_by_hand_dummy (struct value *function,
>       {
>         const char *name = get_function_name (funaddr,
>   					    name_buf, sizeof (name_buf));
> -
> -      discard_infcall_control_state (inf_status.release ());
> +      if (inf_status != nullptr)
> +	discard_infcall_control_state (inf_status.release ());
>   
>         /* We could discard the dummy frame here if the program exited,
>   	 but it will get garbage collected the next time the program is
  

Patch

diff --git a/gdb/infcall.c b/gdb/infcall.c
index 02e3b385bd7..4998b933520 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -1610,25 +1610,29 @@  call_function_by_hand_dummy (struct value *function,
 
 	    maybe_remove_breakpoints ();
 
-	    gdb_assert (retval != NULL);
-
 	    /* Destruct the pass-by-ref argument clones.  */
 	    call_destructors (dtors_to_invoke, default_return_type);
 
-	    return retval;
+	    if (e.reason >= 0)
+	      {
+		gdb_assert (retval != NULL);
+		return retval;
+	      }
 	  }
 	else
-	  infcall_debug_printf ("call did not complete");
+	  {
+	    infcall_debug_printf ("call did not complete");
 
-	/* Didn't complete.  Clean up / destroy the call FSM, and restore the
-	   previous state machine, and handle the error.  */
-	{
-	  std::unique_ptr<thread_fsm> finalizing
-	    = call_thread->release_thread_fsm ();
-	  call_thread->set_thread_fsm (std::move (saved_sm));
+	    /* Didn't complete.  Clean up / destroy the call FSM, and restore
+	       the previous state machine, and handle the error.  */
+	    {
+	      std::unique_ptr<thread_fsm> finalizing
+		= call_thread->release_thread_fsm ();
+	      call_thread->set_thread_fsm (std::move (saved_sm));
 
-	  finalizing->clean_up (call_thread.get ());
-	}
+	      finalizing->clean_up (call_thread.get ());
+	    }
+	  }
       }
   }
 
@@ -1638,8 +1642,8 @@  call_function_by_hand_dummy (struct value *function,
     {
       const char *name = get_function_name (funaddr,
 					    name_buf, sizeof (name_buf));
-
-      discard_infcall_control_state (inf_status.release ());
+      if (inf_status != nullptr)
+	discard_infcall_control_state (inf_status.release ());
 
       /* We could discard the dummy frame here if the program exited,
 	 but it will get garbage collected the next time the program is