Re: [PATCH] Fix gdb.cp/gdb2495.exp on powerpc64le

[Tom de Vries <tdevries@suse.de>]

On 11/18/22 17:43, Tom de Vries via Gdb-patches wrote:
> From: Tom de Vries <tdevries@loganberry-1.arch.suse.de>
> 
> On powerpc64le-linux I ran into this FAIL:
> ...
> (gdb) p exceptions.throw_function()^M
> terminate called after throwing an instance of 'int'^M
> ^M
> Program received signal SIGABRT, Aborted.^M
> 0x00007ffff7979838 in raise () from /lib64/libc.so.6^M
> The program being debugged was signaled while in a function called from GDB.^M
> GDB remains in the frame where the signal was received.^M
> To change this behavior use "set unwindonsignal on".^M
> Evaluation of the expression containing the function^M
> (SimpleException::throw_function()) will be abandoned.^M
> When the function is done executing, GDB will silently stop.^M
> (gdb) FAIL: gdb.cp/gdb2495.exp: call a function that raises an exception \
>    without a handler.
> ...
> 
> The following happens:
> - we start an inferior call
> - an internal breakpoint is set on the global entry point of std::terminate
> - the inferior call uses the local entry point
> - the breakpoint is not triggered
> - we run into std::terminate
> 
> We can fix this by simply adding the missing gdbarch_skip_entrypoint call in
> create_std_terminate_master_breakpoint, but we try to do this a bit more
> generic, by:
> - adding a variant of function create_internal_breakpoint which takes a
>    minimal symbol instead of an address as argument
> - in the new function:
>    - using both gdbarch_convert_from_func_ptr_addr and gdbarch_skip_entrypoint
>    - documenting why we don't need to use gdbarch_addr_bits_remove
>    - adding a note about possibly
>      needing gdbarch_deprecated_function_start_offset.
> - using the new function in:
>    - create_std_terminate_master_breakpoint
>    - create_exception_master_breakpoint_hook, which currently uses only
>      gdbarch_convert_from_func_ptr_addr.
> 
> Note: we could use the new function in more locations in breakpoint.c, but
> as we're not aware of any related failures, we declare this out of scope for
> this patch.
> 
> Tested on x86_64-linux, powerpc64le-linux.
> 
> Co-Authored-By: Ulrich Weigand <uweigand@de.ibm.com>
> PR tdep/29793
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29793

I've pushed this, with a tested-by tag added.

Thanks,
- Tom

> ---
>   gdb/breakpoint.c | 42 +++++++++++++++++++++++++++++++++---------
>   1 file changed, 33 insertions(+), 9 deletions(-)
> 
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 490708938ec..578d45b15af 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -3283,6 +3283,8 @@ set_breakpoint_number (int internal, struct breakpoint *b)
>       }
>   }
>   
> +/* Create a TYPE breakpoint on ADDRESS from an object file with GDBARCH.  */
> +
>   static struct breakpoint *
>   create_internal_breakpoint (struct gdbarch *gdbarch,
>   			    CORE_ADDR address, enum bptype type)
> @@ -3295,6 +3297,33 @@ create_internal_breakpoint (struct gdbarch *gdbarch,
>     return add_to_breakpoint_chain (std::move (b));
>   }
>   
> +/* Create a TYPE breakpoint on minimal symbol MSYM from an object file with
> +   GDBARCH.  */
> +
> +static struct breakpoint *
> +create_internal_breakpoint (struct gdbarch *gdbarch,
> +			    struct bound_minimal_symbol &msym, enum bptype type)
> +{
> +  CORE_ADDR address;
> +
> +  address = msym.value_address ();
> +
> +  address = gdbarch_convert_from_func_ptr_addr
> +    (gdbarch, address, current_inferior ()->top_target ());
> +
> +  /* Note that we're not using gdbarch_addr_bits_remove here, because that's
> +     related to addresses in $pc.  We're getting the address from the
> +     minimal symbol table.  */
> +
> +  /* Is gdbarch_deprecated_function_start_offset needed here?  Or is that dealt
> +     with elsewhere?  Needs testing on vax.  */
> +
> +  if (gdbarch_skip_entrypoint_p (gdbarch))
> +    address = gdbarch_skip_entrypoint (gdbarch, address);
> +
> +  return create_internal_breakpoint (gdbarch, address, type);
> +}
> +
>   static const char *const longjmp_names[] =
>     {
>       "longjmp", "_longjmp", "siglongjmp", "_siglongjmp"
> @@ -3544,8 +3573,6 @@ create_std_terminate_master_breakpoint (void)
>   
>     for (struct program_space *pspace : program_spaces)
>       {
> -      CORE_ADDR addr;
> -
>         set_current_program_space (pspace);
>   
>         for (objfile *objfile : current_program_space->objfiles ())
> @@ -3573,8 +3600,8 @@ create_std_terminate_master_breakpoint (void)
>   	      bp_objfile_data->terminate_msym = m;
>   	    }
>   
> -	  addr = bp_objfile_data->terminate_msym.value_address ();
> -	  b = create_internal_breakpoint (objfile->arch (), addr,
> +	  b = create_internal_breakpoint (objfile->arch (),
> +					  bp_objfile_data->terminate_msym,
>   					  bp_std_terminate_master);
>   	  b->locspec = new_explicit_location_spec_function (func_name);
>   	  b->enable_state = bp_disabled;
> @@ -3643,7 +3670,6 @@ create_exception_master_breakpoint_hook (objfile *objfile)
>     struct breakpoint *b;
>     struct gdbarch *gdbarch;
>     struct breakpoint_objfile_data *bp_objfile_data;
> -  CORE_ADDR addr;
>   
>     bp_objfile_data = get_breakpoint_objfile_data (objfile);
>   
> @@ -3666,10 +3692,8 @@ create_exception_master_breakpoint_hook (objfile *objfile)
>         bp_objfile_data->exception_msym = debug_hook;
>       }
>   
> -  addr = bp_objfile_data->exception_msym.value_address ();
> -  addr = gdbarch_convert_from_func_ptr_addr
> -    (gdbarch, addr, current_inferior ()->top_target ());
> -  b = create_internal_breakpoint (gdbarch, addr, bp_exception_master);
> +  b = create_internal_breakpoint (gdbarch, bp_objfile_data->exception_msym,
> +				  bp_exception_master);
>     b->locspec = new_explicit_location_spec_function (func_name);
>     b->enable_state = bp_disabled;
>   
> 
> base-commit: f9f88aede3bb84efd088a59a5f6bccb3a6bb6516