Re: [PATCH] Powerpc: Add support for openat and fstatat syscalls

[will schmidt via Gdb-patches <gdb-patches@sourceware.org>]

On Mon, 2021-10-11 at 14:17 -0700, Carl Love wrote:
> Ulrich:
> 
> On Thu, 2021-10-07 at 19:52 +0200, Ulrich Weigand wrote:
> > "Carl Love" <cel@us.ibm.com> wrote on 05.10.2021 22:59:06:
> > 
> > > +  else if (syscall == 286)
> > > +    result = gdb_sys_openat;
> > 
> > This looks OK, but ...
> > 
> > > +  else if (syscall == 291)
> > > +    result = gdb_sys_fstatat64;
> > 
> > syscall 291 is actually different between 32-bit
> > and 64-bit: on 32-bit it is fstatat64, but on
> > 64-bit it is newfstatat.

Noting this comment for below.

> > 
> > Given that this routine seems to be used for
> > both flavors, it should be correct for both.
> > 
> > (Also, there seem to be many more syscalls that
> > are not handled even though they could be.  But
> > that can be left for another time I guess ...)
> 
> I changed the fstatat64 to newfstat.  I re-ran the regression tests
> on
> a Power 9 system.  The patch does seem to work correctly.  Don't have
> a
> 32-bit system to verify on.

So.. the regression tests passed with both fstatat64 and newfstat ? 
That suggests there is not a significant difference between the two
syscalls, or this particular corner is not being tested.   

> 
> Please let me know if the patch below looks acceptable.  Thanks.
> 
>                        Carl 
> 
> -------------------------------------------------------
> Powerpc: Add support for openat and fstatat syscalls
> 
> [gdb] update ppc-linux-tdep.c
> 
> Add else if syscall entries for the openat and fstatat system calls.
> ---
>  gdb/ppc-linux-tdep.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
> index 1e94922f25a..895dacd7c1b 100644
> --- a/gdb/ppc-linux-tdep.c
> +++ b/gdb/ppc-linux-tdep.c
> @@ -1391,6 +1391,10 @@ ppc_canonicalize_syscall (int syscall)
>      result = syscall += 259 - 240;
>    else if (syscall >= 250 && syscall <= 251)	/* tgkill */
>      result = syscall + 270 - 250;
> +  else if (syscall == 286)
> +    result = gdb_sys_openat;
> +  else if (syscall == 291)
> +    result = gdb_sys_newfstatat;


The above comment suggests that there needs to be some sort of if/else
logic here to handle 32- or 64- bit.  

If there is rationale for fixing for one, and leaving broken or
incorrect for the other, some commentary should be added.


Thanks
-Will



>    else if (syscall == 336)
>      result = gdb_sys_recv;
>    else if (syscall == 337)