From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3316 invoked by alias); 4 Mar 2011 03:56:52 -0000 Received: (qmail 3308 invoked by uid 22791); 4 Mar 2011 03:56:52 -0000 X-SWARE-Spam-Status: No, hits=-0.7 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,RFC_ABUSE_POST,TW_EG,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from nm8.bullet.mail.sp2.yahoo.com (HELO nm8.bullet.mail.sp2.yahoo.com) (98.139.91.78) by sourceware.org (qpsmtpd/0.43rc1) with SMTP; Fri, 04 Mar 2011 03:56:47 +0000 Received: from [98.139.91.69] by nm8.bullet.mail.sp2.yahoo.com with NNFMP; 04 Mar 2011 03:56:45 -0000 Received: from [98.139.91.49] by tm9.bullet.mail.sp2.yahoo.com with NNFMP; 04 Mar 2011 03:56:45 -0000 Received: from [127.0.0.1] by omp1049.mail.sp2.yahoo.com with NNFMP; 04 Mar 2011 03:56:45 -0000 Received: (qmail 39629 invoked by uid 60001); 4 Mar 2011 03:56:45 -0000 Message-ID: <417465.39433.qm@web112519.mail.gq1.yahoo.com> Received: from [123.238.92.94] by web112519.mail.gq1.yahoo.com via HTTP; Thu, 03 Mar 2011 19:56:44 PST References: <341905.10459.qm@web112513.mail.gq1.yahoo.com> <961842.46357.qm@web112511.mail.gq1.yahoo.com> Date: Fri, 04 Mar 2011 03:56:00 -0000 From: paawan oza Subject: Re: [PATCH] arm reversible : progress To: paawan oza , Tom Tromey , Michael Snyder Cc: "gdb@sourceware.org" In-Reply-To: <961842.46357.qm@web112511.mail.gq1.yahoo.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-IsSubscribed: yes Mailing-List: contact gdb-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sourceware.org X-SW-Source: 2011-03/txt/msg00042.txt.bz2 Hi, with reference to below mail; QEMU provides both logging and trapshooting techniques. but I am not sure why gdb has chosen logging ? moreover when gdb is running on QEMU; does it hand over the record-replay to QEMU ? Regards, Oza. ----- Original Message ---- From: paawan oza To: Tom Tromey ; Michael Snyder Cc: "gdb@sourceware.org" Sent: Fri, March 4, 2011 9:02:10 AM Subject: Re: [PATCH] arm reversible : progress Hi Tom and Michael, First of all; thanks for your comments. 1) diff will be changed as you suggested. 2) coding guidelines will be further more corrected. 3) function will be made static and prototype will be removed. 4) casting problems and space problems will be corrected. Tom: >> One idea would be to share things with opcodes/arm-dis.c somehow. I don't know if that is practical or not. Actually, I have never understood why the process record stuff doesn't share more code with the simulators. Maybe that is just too much work somehow. And, whatever happened to the QEMU-based replay approach? That seemed promising to me. Oza: if you see x86 record code; it doesnt look to be sharing many things. same case with arm because the way we need to decode the insn for record-replay and extract registers and memory and linux ABI; probably sharing code is much more clumsy and unclean than writing few K new lines. may I have some idea about QEMU based approach ? I did not get your hint to incorporate into gdb record-replay. PS: if somebody could clarify. 1) what is the API to read arm coprocessor registers ? 2) how to read SPSR value ? 3) there are couple of FIX me in code. Regards, Oza. ----- Original Message ---- From: Tom Tromey To: paawan oza Cc: "gdb@sourceware.org" Sent: Fri, March 4, 2011 12:02:44 AM Subject: Re: [PATCH] arm reversible : progress >>>>> ">" == paawan oza writes: >> I am done with the deisng for both thumb and arm framework. Thanks for your work on this. >> having some doubts as of now. I can't help with the ARM-specific stuff, but I have a few more general comments. >> diff -urN arm_new/arm-linux-tdep.c arm_orig/arm-linux-tdep.c The patch is reversed. >> - /* Enable process record */ >> - set_gdbarch_process_record(gdbarch, arm_process_record); There are a bunch of minor coding style problems in the patch. You'll have to fix them all before submitting. In the quoted lines: the comment must end with a period and two spaces, and there is a space missing before the open paren. That is, the code should look like: /* Enable process record. */ set_gdbarch_process_record (gdbarch, arm_process_record); These two problems appear frequently. >> -int arm_handle_data_proc_misc_load_str_insn (void*); I did not read closely, but I think most or maybe all of the functions declared here should be static. This applies to some other objects as well, like arm_handle_insn. Actually, in most cases, I think the code could be rearranged so that you don't need forward declarations at all. >> - else if(ARM_INSN_SIZE_BYTES == insn_size) Space after the "if" -- this happens a lot. >> - struct gdbarch_tdep *tdep = gdbarch_tdep ((struct gdbarch*) arm_insn_r-> >>gdbarch); I think you don't need the cast here. I think this occurs in a few places. >> - struct regcache *reg_cache = (struct regcache*) arm_insn_r->regcache; Or here. But if you needed something like this, there would have to be a space before the "*". >> - /* data processing insn /multiply sinsn */ >> - if(9 == arm_insn_r->decode) One thing I dislike about the x86 record code is the huge number of constants. I think this sort of thing is better if you use some symbolic name instead. One idea would be to share things with opcodes/arm-dis.c somehow. I don't know if that is practical or not. Actually, I have never understood why the process record stuff doesn't share more code with the simulators. Maybe that is just too much work somehow. And, whatever happened to the QEMU-based replay approach? That seemed promising to me. >> - arm_insn_r->arm_regs = (uint32_t*)xmalloc (sizeof(uint32_t)*3); I think it is a little clearer to use the libiberty macros, e.g.: arm_insn_r->arm_regs = XNEWVEC (uint32_t, 3); >> - uint32_t reg_val1=0,reg_val2=0; Spaces around the "=" signs. This occurs in a few places. Tom