Re: [PATCH 2/3 v3] Process record support for PowerPC

Ulrich Weigand
2014-12-17T18:41:25Z

Wei-cheng Wang wrote:

> 2014-12-06 Wei-cheng Wang <cole945@gmail.com>
> Ulrich Weigand <uweigand@de.ibm.com>
>
> * configure.tgt (powerpc*-*-linux): Add linux-record.o to
> gdb_target_obs.
> (ppc_linux_record_tdep, ppc64_linux_record_tdep): New for linux syscall
> record.
> (ppc_canonicalize_syscall, ppc_linux_syscall_record,
> ppc_linux_record_signal, ppc_init_linux_record_tdep): New functions.
> (ppc_linux_init_abi): Set process_record, process_record_signal.
> * ppc-tdep.h (struct gdbarch_tdep): Add ppc_syscall_record and
> ppc_linux_record_tdep to gdbarch_tdep.
> (ppc_process_record): New declaration.
> * rs6000-tdep.c (ppc_record_vsr, ppc_process_record_op4,
> ppc_process_record_op19, ppc_process_record_op31,
> ppc_process_record_op59, ppc_process_record_op60,
> ppc_process_record_op63, ppc_process_record): New functions.
>
> changelog for testsuite
>
> 2014-12-06 Wei-cheng Wang <cole945@gmail.com>
>
> * lib/gdb.exp (supports_process_record): Return true for
> powerpc*-*-linux*.
> (supports_reverse): Likewise.

This looks all very good now, except that:

> +static void
> +ppc_init_linux_record_tdep (struct linux_record_tdep *record_tdep,
> + int wordsize)
> +{
> + /* Simply return if it had been initialized. */
> + if (record_tdep->size_pointer != 0)
> + return;
> +
> + /* These values are the size of the type that will be used in a system
> + call. They are obtained from Linux Kernel source. */
> +
> + record_tdep->size_pointer = wordsize;
> + record_tdep->size__old_kernel_stat = 32;
> + record_tdep->size_tms = 32;
> + record_tdep->size_loff_t = 8;
> + record_tdep->size_flock = 32;
[...]

you seem to have removed all the 32-bit struct sizes here. Your last
iteration had different values for many of those structs for wordsize 4
vs. wordsize 8, and I think we need to keep them.

I had suggested to remove the gdbarch calls, but I was thinking instead
of a structure along the lines of

if (wordsize == 8)
{
record_tdep->size_pointer = 8;
record_tdep->size__old_kernel_stat = 32;
record_tdep->size_tms = 32;
record_tdep->size_loff_t = 8;
record_tdep->size_flock = 32;
[...]
}
else if (wordsize == 4)
{
record_tdep->size_pointer = 4;
record_tdep->size__old_kernel_stat = 32;
record_tdep->size_tms = 16;
record_tdep->size_loff_t = 8;
record_tdep->size_flock = 16;
[...]
}
else
internal_error (__FILE__, __LINE_, _("unexpected wordsize"));

The patch is OK with that change.

Thanks again,
Ulrich

--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com