[Bug libc/29165] New: [Regression] broken argv adjustment

[Bug libc/29165] New: [Regression] broken argv adjustment

[danglin at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29165

            Bug ID: 29165
           Summary: [Regression] broken argv adjustment
           Product: glibc
           Version: 2.36
            Status: NEW
          Severity: normal
          Priority: P2
         Component: libc
          Assignee: unassigned at sourceware dot org
          Reporter: danglin at gcc dot gnu.org
                CC: adhemerval.zanella at linaro dot org, drepper.fsp at gmail dot com
  Target Milestone: ---
              Host: hppa*-*-linux*
            Target: hppa*-*-linux*
             Build: hppa*-*-linux*

Make check fails with many segmentation faults.  For example,

dave@atlas:~/gnu/glibc/objdir$ make test t=csu/test-as-const-rtld-sizes
make -r PARALLELMFLAGS="" -C ../glibc objdir=`pwd` test
make[1]: Entering directory '/home/dave/gnu/glibc/glibc'
make subdir=csu -C csu/ ..=../
/home/dave/gnu/glibc/objdir/csu/test-as-const-rtld-sizes.out
make[2]: Entering directory '/home/dave/gnu/glibc/glibc/csu'
(gawk '{ sub(/^/, "asconst_", $2); print; }'
/home/dave/gnu/glibc/objdir/rtld-sizes.h; \
 python3 -B ../scripts/gen-as-const.py --test rtld-sizes.sym) >
/home/dave/gnu/glibc/objdir/csu/test-as-const-rtld-sizes.cT
mv -f /home/dave/gnu/glibc/objdir/csu/test-as-const-rtld-sizes.cT
/home/dave/gnu/glibc/objdir/csu/test-as-const-rtld-sizes.c
gcc /home/dave/gnu/glibc/objdir/csu/test-as-const-rtld-sizes.c -c -std=gnu11
-fgnu89-inline  -g -O2 -Wall -Wwrite-strings -Wundef -Werror
-fmerge-all-constants -frounding-math -fno-stack-protector -fno-common
-Wstrict-prototypes -Wold-style-definition -fmath-errno    -fno-pie         
-I../include -I/home/dave/gnu/glibc/objdir/csu  -I/home/dave/gnu/glibc/objdir 
-I../sysdeps/unix/sysv/linux/hppa  -I../sysdeps/hppa/nptl 
-I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux 
-I../sysdeps/nptl  -I../sysdeps/pthread  -I../sysdeps/gnu 
-I../sysdeps/unix/inet  -I../sysdeps/unix/sysv  -I../sysdeps/unix 
-I../sysdeps/posix  -I../sysdeps/hppa/hppa1.1  -I../sysdeps/wordsize-32 
-I../sysdeps/ieee754/flt-32  -I../sysdeps/ieee754/dbl-64  -I../sysdeps/hppa/fpu
 -I../sysdeps/hppa  -I../sysdeps/ieee754  -I../sysdeps/generic  -I.. -I../libio
-I. -nostdinc -isystem /usr/lib/gcc/hppa-linux-gnu/11/include -isystem
/usr/include -D_LIBC_REENTRANT -include
/home/dave/gnu/glibc/objdir/libc-modules.h -DMODULE_NAME=testsuite_internal
-include ../include/libc-symbols.h       -DTOP_NAMESPACE=glibc -o
/home/dave/gnu/glibc/objdir/csu/test-as-const-rtld-sizes.o -MD -MP -MF
/home/dave/gnu/glibc/objdir/csu/test-as-const-rtld-sizes.o.dt -MT
/home/dave/gnu/glibc/objdir/csu/test-as-const-rtld-sizes.o
gcc -o /home/dave/gnu/glibc/objdir/csu/test-as-const-rtld-sizes -nostdlib
-nostartfiles     -Wl,-z,relro  /home/dave/gnu/glibc/objdir/csu/crt1.o
/home/dave/gnu/glibc/objdir/csu/crti.o `gcc  --print-file-name=crtbegin.o`
/home/dave/gnu/glibc/objdir/csu/test-as-const-rtld-sizes.o
/home/dave/gnu/glibc/objdir/support/libsupport_nonshared.a 
-Wl,-dynamic-linker=/lib/ld.so.1
-Wl,-rpath-link=/home/dave/gnu/glibc/objdir:/home/dave/gnu/glibc/objdir/math:/home/dave/gnu/glibc/objdir/elf:/home/dave/gnu/glibc/objdir/dlfcn:/home/dave/gnu/glibc/objdir/nss:/home/dave/gnu/glibc/objdir/nis:/home/dave/gnu/glibc/objdir/rt:/home/dave/gnu/glibc/objdir/resolv:/home/dave/gnu/glibc/objdir/mathvec:/home/dave/gnu/glibc/objdir/support:/home/dave/gnu/glibc/objdir/crypt:/home/dave/gnu/glibc/objdir/nptl
-lgcc -Wl,--as-needed -lgcc_s  -Wl,--no-as-needed
/home/dave/gnu/glibc/objdir/libc.so.6
/home/dave/gnu/glibc/objdir/libc_nonshared.a -Wl,--as-needed
/home/dave/gnu/glibc/objdir/elf/ld.so -Wl,--no-as-needed -lgcc -Wl,--as-needed
-lgcc_s  -Wl,--no-as-needed `gcc  --print-file-name=crtend.o`
/home/dave/gnu/glibc/objdir/csu/crtn.o
env GCONV_PATH=/home/dave/gnu/glibc/objdir/iconvdata
LOCPATH=/home/dave/gnu/glibc/objdir/localedata LC_ALL=C  
/home/dave/gnu/glibc/objdir/elf/ld.so.1 --library-path
/home/dave/gnu/glibc/objdir:/home/dave/gnu/glibc/objdir/math:/home/dave/gnu/glibc/objdir/elf:/home/dave/gnu/glibc/objdir/dlfcn:/home/dave/gnu/glibc/objdir/nss:/home/dave/gnu/glibc/objdir/nis:/home/dave/gnu/glibc/objdir/rt:/home/dave/gnu/glibc/objdir/resolv:/home/dave/gnu/glibc/objdir/mathvec:/home/dave/gnu/glibc/objdir/support:/home/dave/gnu/glibc/objdir/crypt:/home/dave/gnu/glibc/objdir/nptl
/home/dave/gnu/glibc/objdir/csu/test-as-const-rtld-sizes  >
/home/dave/gnu/glibc/objdir/csu/test-as-const-rtld-sizes.out; \
../scripts/evaluate-test.sh csu/test-as-const-rtld-sizes $? false false >
/home/dave/gnu/glibc/objdir/csu/test-as-const-rtld-sizes.test-result
Segmentation fault (core dumped)
make[2]: Leaving directory '/home/dave/gnu/glibc/glibc/csu'
FAIL: csu/test-as-const-rtld-sizes
original exit status 139
make[1]: Leaving directory '/home/dave/gnu/glibc/glibc'

Core was generated by `/home/dave/gnu/glibc/objdir/elf/ld.so.1 --library-path
/home/dave/gnu/glibc/obj'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0xf7478b6c in _getopt_internal_r (argc=4, argv=0xf8616024,
    optstring=0x14ae9 "", longopts=0x16ec8, longind=<optimized out>,
    long_only=<optimized out>, d=<optimized out>,
--Type <RET> for more, q to quit, c to continue without paging--
    posixly_correct=<optimized out>) at getopt.c:527
527           if (d->optind != argc && !strcmp (argv[d->optind], "--"))
(gdb) disass $pc-16,$pc+16
Dump of assembler code from 0xf7478b5c to 0xf7478b7c:
   0xf7478b5c <_getopt_internal_r+300>: cmpiclr,<> 1,r15,r0
   0xf7478b60 <_getopt_internal_r+304>: b,l,n 0xf7478cb4
<_getopt_internal_r+644>,r0
   0xf7478b64 <_getopt_internal_r+308>: cmpb,=,n r3,r6,0xf7478d28
<_getopt_internal_r+760>
   0xf7478b68 <_getopt_internal_r+312>: ldw,s r3(r7),r11
=> 0xf7478b6c <_getopt_internal_r+316>: ldb 0(r11),ret0
   0xf7478b70 <_getopt_internal_r+320>: ldo -2d(ret0),ret0
   0xf7478b74 <_getopt_internal_r+324>: cmpib,<>,n 0,ret0,0xf7478d40
<_getopt_internal_r+784>
   0xf7478b78 <_getopt_internal_r+328>: ldb 1(r11),ret0
End of assembler dump.
(gdb) p/x $r11
$1 = 0x0
(gdb) p/x $r3
$2 = 0x1
(gdb) p/x $r7
$3 = 0xf8616024

This was introduced by the following change:

dave@atlas:~/gnu/glibc/glibc$ git bisect bad
ad43cac44a6860eaefcadadfb2acb349921e96bf is the first bad commit
commit ad43cac44a6860eaefcadadfb2acb349921e96bf
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Fri Jun 15 16:14:58 2018 +0100

    rtld: Use generic argv adjustment in ld.so [BZ #23293]

    When an executable is invoked as

      ./ld.so [ld.so-args] ./exe [exe-args]

    then the argv is adujusted in ld.so before calling the entry point of
    the executable so ld.so args are not visible to it.  On most targets
    this requires moving argv, env and auxv on the stack to ensure correct
    stack alignment at the entry point.  This had several issues:

    - The code for this adjustment on the stack is written in asm as part
      of the target specific ld.so _start code which is hard to maintain.

    - The adjustment is done after _dl_start returns, where it's too late
      to update GLRO(dl_auxv), as it is already readonly, so it points to
      memory that was clobbered by the adjustment. This is bug 23293.

    - _environ is also wrong in ld.so after the adjustment, but it is
      likely not used after _dl_start returns so this is not user visible.

    - _dl_argv was updated, but for this it was moved out of relro, which
      changes security properties across targets unnecessarily.

    This patch introduces a generic _dl_start_args_adjust function that
    handles the argument adjustments after ld.so processed its own args
    and before relro protection is applied.

    The same algorithm is used on all targets, _dl_skip_args is now 0, so
    existing target specific adjustment code is no longer used.  The bug
    affects aarch64, alpha, arc, arm, csky, ia64, nios2, s390-32 and sparc,
    other targets don't need the change in principle, only for consistency.

    The GNU Hurd start code relied on _dl_skip_args after dl_main returned,
    now it checks directly if args were adjusted and fixes the Hurd startup
    data accordingly.

    Follow up patches can remove _dl_skip_args and DL_ARGV_NOT_RELRO.

    Tested on aarch64-linux-gnu and cross tested on i686-gnu.

    Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

 elf/rtld.c                    | 73 +++++++++++++++++++++++++++++++++++--------
 sysdeps/mach/hurd/dl-sysdep.c | 30 ++++++++----------
 2 files changed, 73 insertions(+), 30 deletions(-)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29165] [Regression] broken argv adjustment

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29165

--- Comment #1 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
It seems hppa creates an unrelated stack frame that ld.so changes where it
saves/restore both argc and argv. Does this fix the issue:

diff --git a/sysdeps/hppa/dl-machine.h b/sysdeps/hppa/dl-machine.h
index 8c0ca32fc6..9ce17a7e2f 100644
--- a/sysdeps/hppa/dl-machine.h
+++ b/sysdeps/hppa/dl-machine.h
@@ -415,10 +415,8 @@ asm (                                                     
                \
          So, obviously, we can't just pass %sp to _dl_start.  That's   \
          okay, argv-4 will do just fine.                               \
                                                                        \
-         The pleasant part of this is that if we need to skip          \
-         arguments we can just decrement argc and move argv, because   \
-         the stack pointer is utterly unrelated to the location of     \
-         the environment and argument vectors. */                      \
+         It also mean that to get the correct argc and argv if the     \
+         program is ld.so it requires to read _dl_argc and _dl_argv. */\
                                                                        \
        /* This is always within range so we'll be okay. */             \
 "      bl      _dl_start,%rp\n"                                        \
@@ -430,22 +428,14 @@ asm (                                                    
                \
        /* Save the entry point in %r3. */                              \
 "      copy    %ret0,%r3\n"                                            \
                                                                        \
-       /* See if we were called as a command with the executable file  \
-          name as an extra leading argument. */                        \
-"      addil   LT'_dl_skip_args,%r19\n"                                \
-"      ldw     RT'_dl_skip_args(%r1),%r20\n"                           \
-"      ldw     0(%r20),%r20\n"                                         \
+"      addil   LT'_dl_argc,%r19\n"                                     \
+"      ldw     RT'_dl_argc(%r1),%r20\n"                                \
+"      ldw     0(%r20),%r25\n"                                         \
                                                                        \
-"      ldw     -40(%sp),%r25\n"        /* argc */                      \
-"      comib,= 0,%r20,.Lnofix\n"       /* FIXME: Mispredicted branch */\
-"      ldw     -44(%sp),%r24\n"        /* argv (delay slot) */         \
+"      addil   LT'_dl_argv,%r19\n"                                     \
+"      ldw     RT'_dl_argv(%r1),%r20\n"                                \
+"      ldw     0(%r20),%r24\n"                                         \
                                                                        \
-"      sub     %r25,%r20,%r25\n"                                       \
-"      stw     %r25,-40(%sp)\n"                                        \
-"      sh2add  %r20,%r24,%r24\n"                                       \
-"      stw     %r24,-44(%sp)\n"                                        \
-                                                                       \
-".Lnofix:\n"                                                           \
        /* Call _dl_init(main_map, argc, argv, envp). */                \
 "      addil   LT'_rtld_local,%r19\n"                                  \
 "      ldw     RT'_rtld_local(%r1),%r26\n"                             \
@@ -456,9 +446,6 @@ asm (                                                      
                \
 "      bl      _dl_init,%r2\n"                                         \
 "      ldo     4(%r23),%r23\n" /* delay slot */                        \
                                                                        \
-       /* Reload argc, argv to the registers start.S expects.  */      \
-"      ldw     -40(%sp),%r25\n"                                        \
-"      ldw     -44(%sp),%r24\n"                                        \
                                                                        \
        /* _dl_fini is a local function in the loader, so we construct  \
           a false OPD here and pass this to the application.  */       \


?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29165] [Regression] broken argv adjustment

[dave.anglin at bell dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=29165

--- Comment #2 from dave.anglin at bell dot net ---
Thanks.  I think we need to keep the instructions that save and restore r24 and
r25.
They will be clobbered by call to _dl_init.

Will test.

On 2022-05-24 4:26 p.m., adhemerval.zanella at linaro dot org wrote:
> https://sourceware.org/bugzilla/show_bug.cgi?id=29165
>
> --- Comment #1 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
> It seems hppa creates an unrelated stack frame that ld.so changes where it
> saves/restore both argc and argv. Does this fix the issue:
>
> diff --git a/sysdeps/hppa/dl-machine.h b/sysdeps/hppa/dl-machine.h
> index 8c0ca32fc6..9ce17a7e2f 100644
> --- a/sysdeps/hppa/dl-machine.h
> +++ b/sysdeps/hppa/dl-machine.h
> @@ -415,10 +415,8 @@ asm (
>                  \
>            So, obviously, we can't just pass %sp to _dl_start.  That's   \
>            okay, argv-4 will do just fine.                               \
>                                                                          \
> -         The pleasant part of this is that if we need to skip          \
> -         arguments we can just decrement argc and move argv, because   \
> -         the stack pointer is utterly unrelated to the location of     \
> -         the environment and argument vectors. */                      \
> +         It also mean that to get the correct argc and argv if the     \
> +         program is ld.so it requires to read _dl_argc and _dl_argv. */\
>                                                                          \
>          /* This is always within range so we'll be okay. */             \
>   "      bl      _dl_start,%rp\n"                                        \
> @@ -430,22 +428,14 @@ asm (
>                  \
>          /* Save the entry point in %r3. */                              \
>   "      copy    %ret0,%r3\n"                                            \
>                                                                          \
> -       /* See if we were called as a command with the executable file  \
> -          name as an extra leading argument. */                        \
> -"      addil   LT'_dl_skip_args,%r19\n"                                \
> -"      ldw     RT'_dl_skip_args(%r1),%r20\n"                           \
> -"      ldw     0(%r20),%r20\n"                                         \
> +"      addil   LT'_dl_argc,%r19\n"                                     \
> +"      ldw     RT'_dl_argc(%r1),%r20\n"                                \
> +"      ldw     0(%r20),%r25\n"                                         \
>                                                                          \
> -"      ldw     -40(%sp),%r25\n"        /* argc */                      \
> -"      comib,= 0,%r20,.Lnofix\n"       /* FIXME: Mispredicted branch */\
> -"      ldw     -44(%sp),%r24\n"        /* argv (delay slot) */         \
> +"      addil   LT'_dl_argv,%r19\n"                                     \
> +"      ldw     RT'_dl_argv(%r1),%r20\n"                                \
> +"      ldw     0(%r20),%r24\n"                                         \
>                                                                          \
> -"      sub     %r25,%r20,%r25\n"                                       \
> -"      stw     %r25,-40(%sp)\n"                                        \
> -"      sh2add  %r20,%r24,%r24\n"                                       \
> -"      stw     %r24,-44(%sp)\n"                                        \
> -                                                                       \
> -".Lnofix:\n"                                                           \
>          /* Call _dl_init(main_map, argc, argv, envp). */                \
>   "      addil   LT'_rtld_local,%r19\n"                                  \
>   "      ldw     RT'_rtld_local(%r1),%r26\n"                             \
> @@ -456,9 +446,6 @@ asm (
>                  \
>   "      bl      _dl_init,%r2\n"                                         \
>   "      ldo     4(%r23),%r23\n" /* delay slot */                        \
>                                                                          \
> -       /* Reload argc, argv to the registers start.S expects.  */      \
> -"      ldw     -40(%sp),%r25\n"                                        \
> -"      ldw     -44(%sp),%r24\n"                                        \
>                                                                          \
>          /* _dl_fini is a local function in the loader, so we construct  \
>             a false OPD here and pass this to the application.  */       \
>
>
> ?
>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29165] [Regression] broken argv adjustment

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29165

--- Comment #3 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
I don't think it is required since _dl_fini does not have any arguments.

(In reply to dave.anglin from comment #2)
> Thanks.  I think we need to keep the instructions that save and restore r24
> and r25.
> They will be clobbered by call to _dl_init.
> 
> Will test.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29165] [Regression] broken argv adjustment

[dave.anglin at bell dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=29165

--- Comment #4 from dave.anglin at bell dot net ---
But the comment says these are for (_start) in start.S.

On 2022-05-25 7:57 a.m., adhemerval.zanella at linaro dot org wrote:
> I don't think it is required since _dl_fini does not have any arguments.
>
> (In reply to dave.anglin from comment #2)
>> Thanks.  I think we need to keep the instructions that save and restore r24
>> and r25.
>> They will be clobbered by call to _dl_init.
>>
>> Will test.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29165] [Regression] broken argv adjustment

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29165

--- Comment #5 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
But the start above will tail call _dl_start and _dl_start will call the user
entrypoint that will eventually calls exit syscall.  So I can't see why matter
restore r24 and r25 at this point, specially since they will point to the ld.so
ones.

(In reply to dave.anglin from comment #4)
> But the comment says these are for (_start) in start.S.
> 
> On 2022-05-25 7:57 a.m., adhemerval.zanella at linaro dot org wrote:
> > I don't think it is required since _dl_fini does not have any arguments.
> >
> > (In reply to dave.anglin from comment #2)
> >> Thanks.  I think we need to keep the instructions that save and restore r24
> >> and r25.
> >> They will be clobbered by call to _dl_init.
> >>
> >> Will test.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29165] [Regression] broken argv adjustment

[dave.anglin at bell dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=29165

--- Comment #6 from dave.anglin at bell dot net ---
It looks to me as if _dl_start doesn't call the user entrypoint directly.  It
calls _start in start.S. _start
calls __libc_start_main with the argument setup shown in the comment in
sysdeps/hppa/start.S.

On 2022-05-25 8:41 a.m., adhemerval.zanella at linaro dot org wrote:
> https://sourceware.org/bugzilla/show_bug.cgi?id=29165
>
> --- Comment #5 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
> But the start above will tail call _dl_start and _dl_start will call the user
> entrypoint that will eventually calls exit syscall.  So I can't see why matter
> restore r24 and r25 at this point, specially since they will point to the ld.so
> ones.
>
> (In reply to dave.anglin from comment #4)
>> But the comment says these are for (_start) in start.S.
>>
>> On 2022-05-25 7:57 a.m., adhemerval.zanella at linaro dot org wrote:
>>> I don't think it is required since _dl_fini does not have any arguments.
>>>
>>> (In reply to dave.anglin from comment #2)
>>>> Thanks.  I think we need to keep the instructions that save and restore r24
>>>> and r25.
>>>> They will be clobbered by call to _dl_init.
>>>>
>>>> Will test.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29165] [Regression] broken argv adjustment

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29165

--- Comment #7 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
My understanding is hppa has to create the synthetic frame layout and save the
kernel passed argc/argv so the loader can proper obtain them on
_dl_sysdep_parse_arguments.  The call frame is:

  _start 
  \_ _dl_start (void *arg)
     \_ _dl_start_final (void *arg)
         \_ _dl_sysdep_start (void *arg, ...)
             \_ _dl_sysdep_parse_arguments (void **start_argptr, ...)

Once the loader setup _dl_argc and _dl_argc for the application, it does not
matter the kernel passed argc/argv (there are already processed and should not
be visible by the application).

(In reply to dave.anglin from comment #6)
> It looks to me as if _dl_start doesn't call the user entrypoint directly. 
> It calls _start in start.S. _start
> calls __libc_start_main with the argument setup shown in the comment in
> sysdeps/hppa/start.S.
> 
> On 2022-05-25 8:41 a.m., adhemerval.zanella at linaro dot org wrote:
> > https://sourceware.org/bugzilla/show_bug.cgi?id=29165
> >
> > --- Comment #5 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
> > But the start above will tail call _dl_start and _dl_start will call the user
> > entrypoint that will eventually calls exit syscall.  So I can't see why matter
> > restore r24 and r25 at this point, specially since they will point to the ld.so
> > ones.
> >
> > (In reply to dave.anglin from comment #4)
> >> But the comment says these are for (_start) in start.S.
> >>
> >> On 2022-05-25 7:57 a.m., adhemerval.zanella at linaro dot org wrote:
> >>> I don't think it is required since _dl_fini does not have any arguments.
> >>>
> >>> (In reply to dave.anglin from comment #2)
> >>>> Thanks.  I think we need to keep the instructions that save and restore r24
> >>>> and r25.
> >>>> They will be clobbered by call to _dl_init.
> >>>>
> >>>> Will test.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29165] [Regression] broken argv adjustment

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29165

--- Comment #8 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
I see now what my fix is missing, let me fix it.

(In reply to Adhemerval Zanella from comment #7)
> My understanding is hppa has to create the synthetic frame layout and save
> the kernel passed argc/argv so the loader can proper obtain them on
> _dl_sysdep_parse_arguments.  The call frame is:
> 
>   _start 
>   \_ _dl_start (void *arg)
>      \_ _dl_start_final (void *arg)
>          \_ _dl_sysdep_start (void *arg, ...)
>              \_ _dl_sysdep_parse_arguments (void **start_argptr, ...)
> 
> Once the loader setup _dl_argc and _dl_argc for the application, it does not
> matter the kernel passed argc/argv (there are already processed and should
> not be visible by the application).
> 
> (In reply to dave.anglin from comment #6)
> > It looks to me as if _dl_start doesn't call the user entrypoint directly. 
> > It calls _start in start.S. _start
> > calls __libc_start_main with the argument setup shown in the comment in
> > sysdeps/hppa/start.S.
> > 
> > On 2022-05-25 8:41 a.m., adhemerval.zanella at linaro dot org wrote:
> > > https://sourceware.org/bugzilla/show_bug.cgi?id=29165
> > >
> > > --- Comment #5 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
> > > But the start above will tail call _dl_start and _dl_start will call the user
> > > entrypoint that will eventually calls exit syscall.  So I can't see why matter
> > > restore r24 and r25 at this point, specially since they will point to the ld.so
> > > ones.
> > >
> > > (In reply to dave.anglin from comment #4)
> > >> But the comment says these are for (_start) in start.S.
> > >>
> > >> On 2022-05-25 7:57 a.m., adhemerval.zanella at linaro dot org wrote:
> > >>> I don't think it is required since _dl_fini does not have any arguments.
> > >>>
> > >>> (In reply to dave.anglin from comment #2)
> > >>>> Thanks.  I think we need to keep the instructions that save and restore r24
> > >>>> and r25.
> > >>>> They will be clobbered by call to _dl_init.
> > >>>>
> > >>>> Will test.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29165] [Regression] broken argv adjustment

[adhemerval.zanella at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29165

Adhemerval Zanella <adhemerval.zanella at linaro dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at sourceware dot org   |adhemerval.zanella at linaro dot o
                   |                            |rg
             Status|NEW                         |RESOLVED
   Target Milestone|---                         |2.36
         Resolution|---                         |FIXED

--- Comment #9 from Adhemerval Zanella <adhemerval.zanella at linaro dot org> ---
Fixed on master.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/29165] [Regression] broken argv adjustment

[sam at gentoo dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29165

Sam James <sam at gentoo dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sam at gentoo dot org

--- Comment #10 from Sam James <sam at gentoo dot org> ---
*** Bug 29237 has been marked as a duplicate of this bug. ***

-- 
You are receiving this mail because:
You are on the CC list for the bug.