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

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

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