[PATCH v2 2/6] elf: lose is closely tied to _dl_map_object_from_fd

[Szabolcs Nagy]

The 12/10/2020 14:57, Adhemerval Zanella via Libc-alpha wrote:
> 
> 
> On 27/11/2020 10:20, Szabolcs Nagy via Libc-alpha wrote:
> > Simple refactoring to keep failure handling next to
> > _dl_map_object_from_fd.
> 
> It is just moving the function below _dl_process_pt_gnu_property,
> and it is already close to its usage.  I don't have a strong opinion,
> but I don't see much gain either.

this function should be part of _dl_map_object_from_fd.
(it is cleaning up local state there so a change in
local variables there likely requires changes in lose
and the logic does not make sense independently).

i think historically it was separate to avoid error code
paths to blow up the code size after inlining, but now
i tried it and code size stays small: historically there
were many call sites, but now only a single call and gotos
there.

i'll rewrite this patch to just inline lose manually.

> > ---
> >  elf/dl-load.c | 48 ++++++++++++++++++++++++------------------------
> >  1 file changed, 24 insertions(+), 24 deletions(-)
> > 
> > diff --git a/elf/dl-load.c b/elf/dl-load.c
> > index f3201e7c14..21e55deb19 100644
> > --- a/elf/dl-load.c
> > +++ b/elf/dl-load.c
> > @@ -835,30 +835,6 @@ _dl_init_paths (const char *llp, const char *source)
> >  }
> >  
> >  
> > -static void
> > -__attribute__ ((noreturn, noinline))
> > -lose (int code, int fd, const char *name, char *realname, struct link_map *l,
> > -      const char *msg, struct r_debug *r, Lmid_t nsid)
> > -{
> > -  /* The file might already be closed.  */
> > -  if (fd != -1)
> > -    (void) __close_nocancel (fd);
> > -  if (l != NULL && l->l_origin != (char *) -1l)
> > -    free ((char *) l->l_origin);
> > -  free (l);
> > -  free (realname);
> > -
> > -  if (r != NULL)
> > -    {
> > -      r->r_state = RT_CONSISTENT;
> > -      _dl_debug_state ();
> > -      LIBC_PROBE (map_failed, 2, nsid, r);
> > -    }
> > -
> > -  _dl_signal_error (code, name, NULL, msg);
> > -}
> > -
> > -
> >  /* Process PT_GNU_PROPERTY program header PH in module L after
> >     PT_LOAD segments are mapped.  Only one NT_GNU_PROPERTY_TYPE_0
> >     note is handled which contains processor specific properties.  */
> > @@ -930,6 +906,30 @@ _dl_process_pt_gnu_property (struct link_map *l, const ElfW(Phdr) *ph)
> >  }
> >  
> >  
> > +static void
> > +__attribute__ ((noreturn, noinline))
> > +lose (int code, int fd, const char *name, char *realname, struct link_map *l,
> > +      const char *msg, struct r_debug *r, Lmid_t nsid)
> > +{
> > +  /* The file might already be closed.  */
> > +  if (fd != -1)
> > +    (void) __close_nocancel (fd);
> > +  if (l != NULL && l->l_origin != (char *) -1l)
> > +    free ((char *) l->l_origin);
> > +  free (l);
> > +  free (realname);
> > +
> > +  if (r != NULL)
> > +    {
> > +      r->r_state = RT_CONSISTENT;
> > +      _dl_debug_state ();
> > +      LIBC_PROBE (map_failed, 2, nsid, r);
> > +    }
> > +
> > +  _dl_signal_error (code, name, NULL, msg);
> > +}
> > +
> > +
> >  /* Map in the shared object NAME, actually located in REALNAME, and already
> >     opened on FD.  */
> >  
> > 

--