[PATCH v7 1/6] elf: Add SFrame support to _dl_find_object function

[Florian Weimer]

* claudiu zissulescu-ianculescu:


> +#define DLF_SFRAME      (1 << 0)   /* SFrame stack trace data is valid.  */

Should be 1UL, so that we can use all the bits.

> @@ -33,6 +91,7 @@
>     transactions).  */
>  struct dl_find_object_internal
>  {
> +  __atomic_wide_var flags;

I don't understand what this use of flags tries to accomplish.

> +  /* Initialize object's exception handling segment and SFrame segment
> +     data.  */
> +  atomic_store_relaxed (&result->sframe, NULL);
> +  atomic_store_relaxed (&result->eh_frame, NULL);
> +#if DLFO_STRUCT_HAS_EH_COUNT
> +  atomic_store_relaxed (&result->eh_count, 0);
> +#endif
> +
>    for (const ElfW(Phdr) *ph = l->l_phdr, *ph_end = l->l_phdr + l->l_phnum;
>         ph < ph_end; ++ph)
> -    if (ph->p_type == DLFO_EH_SEGMENT_TYPE)
> +    switch (ph->p_type)
>        {
> +      case DLFO_EH_SEGMENT_TYPE:
>          atomic_store_relaxed (&result->eh_frame,
>                                (void *) (ph->p_vaddr + l->l_addr));
>  #if DLFO_STRUCT_HAS_EH_COUNT
>          atomic_store_relaxed (&result->eh_count, ph->p_memsz / 8);
>  #endif
> -        return;
> -      }
> +        read_seg |= 1;
> +        break;

read_seg == 3 should probably checked after the switch?  Or for symmetry
on the DLFO_EH_SEGMENT_TYPE case here as well?

> -  /* Object has no exception handling segment.  */
> -  atomic_store_relaxed (&result->eh_frame, NULL);
> -#if DLFO_STRUCT_HAS_EH_COUNT
> -  atomic_store_relaxed (&result->eh_count, 0);
> -#endif
> +      case DLFO_SFRAME_SEGMENT_TYPE:
> +        atomic_store_relaxed (&result->sframe,
> +                              (void *) (ph->p_vaddr + l->l_addr));
> +	__atomic_wide_var_fetch_or_low (&result->flags, DLF_SFRAME);
> +        read_seg |= 2;
> +        /* Fall through.  */
> +      default:
> +        if (read_seg == 3)
> +          return;
> +        break;
> +      }
>  }

Not sure if a flags update is needed here at all.

>  static inline void
>  _dl_find_object_to_external (struct dl_find_object_internal *internal,
>                               struct dl_find_object *external)
>  {
> -  external->dlfo_flags = 0;
> +  external->dlfo_flags = internal->flags.__value64;
>    external->dlfo_map_start = (void *) internal->map_start;
>    external->dlfo_map_end = (void *) internal->map_end;
>    external->dlfo_link_map = internal->map;
> @@ -84,6 +147,7 @@ _dl_find_object_to_external (struct dl_find_object_internal *internal,
>  # if DLFO_STRUCT_HAS_EH_COUNT
>    external->dlfo_eh_count = internal->eh_count;
>  # endif
> +  external->dlfo_sframe = internal->sframe;
>  }

You can just do

  if (internal->sframe != NULL)
    external->dlfo_flags = DLF_SFRAME;
  else
    external->dlfo_flags = 0;

I think.  No need to keep track of the flags field in the internal data
structure.

Thanks,
Florian