Re: [RFA] Fix GDB 8.3 regression crash when registers cannot be modified.

[Mark Wielaard <mark at klomp dot org>]

Hi,

On Sat, Apr 13, 2019 at 10:28:17AM +0200, Philippe Waroquiers wrote:
> This crash was detected when using GDB with the valgrind gdbserver.
> To reproduce:
> 
> valgrind sleep 10000
> 
> In another window:
> gdb
> target remote | vgdb
> p printf("make sleep print something\n")
> =>
> terminate called after throwing an instance of 'gdb_exception_RETURN_MASK_ERROR'
> Aborted
> 
> The problem is that the valgrind gdbserver does not allow to change
> registers when the inferior is blocked in a system call.
> GDB then raises an exception.  The exception causes the destructor
> of
>  typedef std::unique_ptr<infcall_suspend_state, infcall_suspend_state_deleter>
>     infcall_suspend_state_up;
> to be called.  This destructor itself tries to restore the value of
> the registers, and fails similarly.  We must catch the exception in
> the destructor to avoid crashing GDB.
> If the destructor encounters a problem, no warning is produced if
> there is an uncaught exception, as in this case, the user will already
> be informed of a problem via this exception.
> 
> With this change, no crash anymore, and all the valgrind 3.15 tests
> pass succesfully.
> 
> Note: when this patch is approved, I will push an equivalent patch
> on master, but with TRY/CATCH/e.message () replaced by
> try/catch/e.what ().
> 
> gdb/ChangeLog
> 
> struct infcall_suspend_state_deleter
> 2019-04-13  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* inferior.h (struct infcall_suspend_state_deleter):
> 	Catch exception in destructor to avoid crash.

What is the current status of this patch?
We really need something like this to enable the valgrind gdbserver
integration testing again on Fedora 30/rawhide.

Thanks,

Mark

> ---
>  gdb/inferior.h | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/inferior.h b/gdb/inferior.h
> index 2d1bb97a28..4d84afac6a 100644
> --- a/gdb/inferior.h
> +++ b/gdb/inferior.h
> @@ -68,7 +68,19 @@ struct infcall_suspend_state_deleter
>  {
>    void operator() (struct infcall_suspend_state *state) const
>    {
> -    restore_infcall_suspend_state (state);
> +    TRY
> +      {
> +	restore_infcall_suspend_state (state);
> +      }
> +    CATCH (e, RETURN_MASK_ALL)
> +      {
> +	/* If we are restoring the inferior state due to an exception,
> +	   some error message will be printed.  So, only warn the user
> +	   when we cannot restore during normal execution.  */
> +	if (!std::uncaught_exception ())
> +	  warning (_("Failed to restore inferior state: %s"), e.message);
> +      }
> +    END_CATCH
>    }
>  };
>  
> -- 
> 2.20.1