Re: [PATCH 3/4] libgcc: Split FDE search code from PT_GNU_EH_FRAME lookup

[Jakub Jelinek via Libc-alpha <libc-alpha@sourceware.org>]

On Wed, Nov 03, 2021 at 05:28:48PM +0100, Florian Weimer wrote:
> @@ -383,12 +376,34 @@ _Unwind_IteratePhdrCallback (struct dl_phdr_info *info, size_t size, void *ptr)
>  # endif
>  #endif
>  
> -  _Unwind_Ptr dbase = unw_eh_callback_data_dbase (data);
> +  return 1;
> +}
> +
> +/* Result type of find_fde_tail below.  */
> +struct find_fde_tail_result
> +{
> +  const fde *entry;
> +  void *func;
> +};
> +
> +/* Find the FDE for the program counter PC, in a previously located
> +   PT_GNU_EH_FRAME data region.  */
> +static struct find_fde_tail_result
> +find_fde_tail (_Unwind_Ptr pc,
> +	       const struct unw_eh_frame_hdr *hdr,
> +	       _Unwind_Ptr dbase)

I think returning a struct like find_fde_tail_result can work nicely
on certain targets, but on many others the psABI forces such returns through
stack etc.
Wouldn't it be better to return const fde * instead of
struct find_fde_tail_result, pass in struct dwarf_eh_bases *bases
as another argument to find_fde_tail, just return NULL on the failure
cases and return some fde pointer and set bases->func on success?

> +{
> +  const unsigned char *p = (const unsigned char *) (hdr + 1);
> +  _Unwind_Ptr eh_frame;
> +  struct object ob;
> +
> +  if (hdr->version != 1)
> +    return (struct find_fde_tail_result) { NULL, };

If really returning a struct, I would have preferred { NULL, NULL }
in these cases, but see above.

> +	  if (pc < table[mid].initial_loc + data_base + range)
> +	    return (struct find_fde_tail_result) { f, func };
> +	  else
> +	    return (struct find_fde_tail_result) { NULL, func };

This case was returning NULL fde and non-NULL func?  What it would be good
for, the caller just throws that away.

Otherwise LGTM.

	Jakub