From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12300 invoked by alias); 20 Dec 2013 12:37:49 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 12285 invoked by uid 89); 20 Dec 2013 12:37:48 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 20 Dec 2013 12:37:48 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rBKCbi9f004215 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 20 Dec 2013 07:37:44 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id rBKCbf7Z008981; Fri, 20 Dec 2013 07:37:42 -0500 Message-ID: <52B43A15.2070302@redhat.com> Date: Fri, 20 Dec 2013 12:37:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Omair Javaid CC: oza Pawandeep , Yao Qi , "gdb-patches@sourceware.org" , Patch Tracking Subject: Re: [PATCH 2/2] GDB process record and reverse debugging improvements for arm*-linux* References: <5268865F.6060307@codesourcery.com> <527C5887.8070304@linaro.org> <5280A8CA.3040307@codesourcery.com> <52929027.7020303@linaro.org> In-Reply-To: <52929027.7020303@linaro.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-12/txt/msg00826.txt.bz2 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 > > * 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