The 05/13/2022 16:56, Adhemerval Zanella wrote:
> On 05/05/2022 10:58, Szabolcs Nagy via Libc-alpha wrote:
> > +_dl_start_args_adjust (int skip_args)
> > +{
> > +  void **sp = (void **) (_dl_argv - skip_args - 1);
> 
> Is it fully correctly to materialize the address for 'skip_args' equal to 0?
> I don't think it would matter anyway.

skip_args == 0 should be fine (any skip_args makes sp point to argc).

> > +  /* Shuffle auxv down. */
> > +  void *a, *b; /* Use a pair of pointers for an auxv entry.  */
> > +  unsigned long a_type;
> > +  do
> > +    {
> > +      a_type = ((ElfW(auxv_t) *) (p + 1))->a_type;
> > +      a = *++p;
> > +      b = *++p;
> > +      *++sp = a;
> > +      *++sp = b;
> > +    }> +  while (a_type != AT_NULL);
> > +#endif
> 
> Maybe:
> 
>   ElfW(auxv_t) ax;
>   do
>     { 
>       p = (void**) ((uintptr_t) p + sizeof (ax));
>       sp = (void**) ((uintptr_t) sp + sizeof (ax));
>       memcpy (&ax, p, sizeof (ax));
>       memcpy (sp, &ax, sizeof (ax));
>     }
>   while (ax.a_type != AT_NULL);
> 
> Most targets will inline memcpy and if they do not we don't enable IFUNC 
> for ld.so.

i did this differently, but same in spirit.

attaching the patch i committed.

> > -      if (__libc_enable_secure || _dl_skip_args)
> > -	{
> > -	  char **newp;
> >  
> > -	  for (newp = _environ; *newp++;);
> > +      char **newp;
> > +      for (newp = _environ; *newp++;);
> >  
> > -	  if (_dl_argv[-_dl_skip_args] == (char *) p)
> > +      if (newp != p || _dl_argv[0] != orig_argv0)
> > +	{
> > +	  if (orig_argv0 == (char *) p)
> >  	    {
> >  	      if ((char *) newp != _dl_argv[0])
> >  		{
> 
> Looks ok, but I can't really voucher for hurd code.

committed the patch, if it breaks hurd we can fix/revert later.