From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16074 invoked by alias); 22 Sep 2011 23:32:57 -0000 Received: (qmail 16065 invoked by uid 22791); 22 Sep 2011 23:32:56 -0000 X-SWARE-Spam-Status: No, hits=-1.6 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 22 Sep 2011 23:32:42 +0000 Received: from nat-jpt.mentorg.com ([192.94.33.2] helo=PR1-MAIL.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1R6sl0-0004PH-80 from Yao_Qi@mentor.com for gdb-patches@sourceware.org; Thu, 22 Sep 2011 16:32:42 -0700 Received: from [172.30.38.254] ([172.30.38.254]) by PR1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.1830); Fri, 23 Sep 2011 08:32:40 +0900 Message-ID: <4E7BC595.50708@codesourcery.com> Date: Thu, 22 Sep 2011 23:42:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:6.0.2) Gecko/20110906 Thunderbird/6.0.2 MIME-Version: 1.0 To: gdb-patches@sourceware.org Subject: Re: [PATCH] Collect return addresses at tracepoints References: <4E7A5B0C.2020802@earthlink.net> In-Reply-To: <4E7A5B0C.2020802@earthlink.net> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes 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 X-SW-Source: 2011-09/txt/msg00419.txt.bz2 On 09/22/2011 05:45 AM, Stan Shebs wrote: Stan, I don't know the code very well, just two cents on function comment. > Index: amd64-tdep.c > =================================================================== > RCS file: /cvs/src/src/gdb/amd64-tdep.c,v > > + /* Generate a bytecode expression to get the value of the saved PC. > + Since this is supposed to run on the target independently of GDB, > + we don't have the full power of the unwinding machinery. Instead, > + make a guess as to the most likely location of the return address, > + and issue byte codes for that. */ > + > + void > + amd64_gen_return_address (struct gdbarch *gdbarch, > + struct agent_expr *ax, struct axs_value *value, > + CORE_ADDR scope) The comments on function amd64_gen_return_address and i386_gen_return_address are identical. I suggest that we move this comment to gdbarch.sh ... and write comment on function amd64_gen_return_address and i386_gen_return_address like this, /* This is the implementation of gdbarch method gen_return_address. */ > Index: gdbarch.sh > =================================================================== > RCS file: /cvs/src/src/gdb/gdbarch.sh,v > retrieving revision 1.523 > diff -p -r1.523 gdbarch.sh > *** gdbarch.sh 22 Jul 2011 15:31:50 -0000 1.523 > --- gdbarch.sh 21 Sep 2011 21:27:00 -0000 > *************** v:const char *:solib_symbols_extension:: > *** 820,825 **** > --- 820,829 ---- > # is, absolute paths include a drive name, and the backslash is > # considered a directory separator. > v:int:has_dos_based_file_system:::0:0::0 > + > + # Generate bytecodes to collect the return address in a frame. > + m:void:gen_return_address:struct agent_expr *ax, struct axs_value *value, CORE_ADDR scope:ax, value, scope::default_gen_return_address::0 > + ... here, and document the each parameter in comment. > Index: i386-tdep.c > =================================================================== > RCS file: /cvs/src/src/gdb/i386-tdep.c,v > > + /* Generate a bytecode expression to get the value of the saved PC. > + Since this is supposed to run on the target independently of GDB, > + we don't have the full power of the unwinding machinery. Instead, > + make a guess as to the most likely location of the return address, > + and issue byte codes for that. */ > + > + void > + i386_gen_return_address (struct gdbarch *gdbarch, > + struct agent_expr *ax, struct axs_value *value, > + CORE_ADDR scope) -- Yao (齐尧)