This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/2] GDB process record and reverse debugging improvements for arm*-linux*
- From: Pedro Alves <palves at redhat dot com>
- To: Omair Javaid <omair dot javaid at linaro dot org>
- Cc: oza Pawandeep <oza dot pawandeep at gmail dot com>, Yao Qi <yao at codesourcery dot com>, "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>, Patch Tracking <patches at linaro dot org>
- Date: Fri, 20 Dec 2013 12:37:41 +0000
- Subject: Re: [PATCH 2/2] GDB process record and reverse debugging improvements for arm*-linux*
- Authentication-results: sourceware.org; auth=none
- References: <CANW4E-0qP8iMVh1czWuOjhwfH9Q-2aJUG=2wEzDX1mLXy3avAw at mail dot gmail dot com> <5268865F dot 6060307 at codesourcery dot com> <527C5887 dot 8070304 at linaro dot org> <5280A8CA dot 3040307 at codesourcery dot com> <CAK1A=4xwcL=PpyC-zCHSkjDadkvGQTh+d=vmWXoshFaxS-n6gg at mail dot gmail dot com> <CANW4E-08KZaKZHXqKyw+T8KdrFAU8Z2UeAiQMej2LqdCVYu8oA at mail dot gmail dot com> <52929027 dot 7020303 at linaro dot org>
On 11/24/2013 11:47 PM, Omair Javaid wrote:
> After incorporating all suggestions I am posting a final patch. Looking
> for a go ahead for commit.
>
> This patch adds support for process record/replay system call recording
> for arm targets.
>
> 2013-11-08 Omair Javaid <omair.javaid@linaro.org>
>
> * arm-linux-tdep.c (struct arm_linux_record_tdep): Declare.
> (arm_canonicalize_syscall): New function.
> (arm_all_but_pc_registers_record): New function.
> (arm_linux_syscall_record): New function.
> (arm_linux_init_abi): Add syscall recording constructs.
...
> * arm-linux-tdep.c: Include "record-full.h" and
> "linux-record.h".
These two entries are in the same file, so merge them, like:
* arm-linux-tdep.c: Include "record-full.h" and
"linux-record.h".
(struct arm_linux_record_tdep): Declare.
(arm_canonicalize_syscall): New function.
(arm_all_but_pc_registers_record): New function.
(arm_linux_syscall_record): New function.
(arm_linux_init_abi): Add syscall recording constructs.
> +/* ARM process record-replay constructs; syscall, signal etc. */
> +
> +struct linux_record_tdep arm_linux_record_tdep;
> +
> +/* arm_canonicalize_syscall maps from the native arm Linux set
> + of syscall ids into a canonical set of syscall ids used by
> + process record. */
> +
> +static enum gdb_syscall
> +arm_canonicalize_syscall (int syscall)
> +{
> + enum { sys_process_vm_writev = 377 };
> +
> + if (syscall <= gdb_sys_sched_getaffinity)
^^
Spurious space.
> + return syscall;
> + else if (syscall >= 243 && syscall <= 247 )
> + return syscall + 2;
> + else if (syscall >= 248 && syscall <= 253 )
> + return syscall + 4;
> +
> + return -1;
> +}
> +
> +}
> +
> +/* Handler for arm system call instruction and recording. */
Spurious "and" ? Otherwise I can't parse it.
> +
> +static int
> +arm_linux_syscall_record (struct regcache *regcache, unsigned long svc_number)
> +{
...
> +
> + ret = record_linux_system_call (syscall_gdb, regcache,
> + &arm_linux_record_tdep);
> + if (ret)
if (ret != 0)
> + return ret;
> +
> + arm_linux_record_tdep.ioctl_TIOCGSERIAL = 0x541E;
> + arm_linux_record_tdep.ioctl_TIOCSSERIAL = 0x541F;
...
> + arm_linux_record_tdep.ioctl_TCGETS2 = 0x802c542a;
> + arm_linux_record_tdep.ioctl_TCSETS2 = 0x402c542b;
I see a mixup of uppercase and lowercase in these hex constants.
Could you make them all lowercase please?
> + else
> + {
> + arm_record_unsupported_insn(arm_insn_r);
Space before parens.
> + ret = -1;
> + }
Otherwise looks good.
--
Pedro Alves