From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7216 invoked by alias); 13 Jun 2011 02:32:21 -0000 Received: (qmail 7208 invoked by uid 22791); 13 Jun 2011 02:32:20 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 13 Jun 2011 02:32:05 +0000 Received: (qmail 22702 invoked from network); 13 Jun 2011 02:32:03 -0000 Received: from unknown (HELO ?192.168.0.102?) (yao@127.0.0.2) by mail.codesourcery.com with ESMTPA; 13 Jun 2011 02:32:03 -0000 Message-ID: <4DF5769F.1060602@codesourcery.com> Date: Mon, 13 Jun 2011 02:32:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.17) Gecko/20110424 Lightning/1.0b2 Thunderbird/3.1.10 MIME-Version: 1.0 To: gdb-patches@sourceware.org Subject: Re: [PATCH] Fix some i386 unwinder inconcistencies References: <201106122057.p5CKvUEa030437@glazunov.sibelius.xs4all.nl> In-Reply-To: <201106122057.p5CKvUEa030437@glazunov.sibelius.xs4all.nl> 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-06/txt/msg00154.txt.bz2 On 06/13/2011 04:57 AM, Mark Kettenis wrote: > 2011-06-12 Mark Kettenis > > * i386-tdep.c (i386_epilogue_frame_cache): Simplify code. Call > get_frame_func instead of get_frame_pc to determine the code > address used to construct the frame ID. > (i386_epilogue_frame_unwind_stop_reason): Fix coding style. > (i386_epilogue_frame_this_id): Likewise. > (i386_epilogue_frame_prev_register): New function. > (i386_epilogue_frame_unwind): Use i386_epilogue_frame_prev_register. > (i386_stack_tramp_frame_sniffer): Fix coding style. > (i386_stack_tramp_frame_unwind): Use i386_epilogue_frame_prev_register. > (i386_gdbarch_init): Fix comments. > Looks like you commit two irrelevant changes (simplification and code style/comment fix) together. IMO, each commit should be a self-contained, single-purpose change. I don't know this rule applies to GDB development or not. > - /* Cache base will be %esp plus cache->sp_offset (-4). */ > - get_frame_register (this_frame, I386_ESP_REGNUM, buf); > - cache->base = extract_unsigned_integer (buf, 4, > - byte_order) + cache->sp_offset; > + cache->pc = get_frame_func (this_frame); > > - /* Cache pc will be the frame func. */ > - cache->pc = get_frame_pc (this_frame); > - > - /* The saved %esp will be at cache->base plus 8. */ I am not sure why this comment is removed, which is still valid to statement below "cache->saved_sp = cache->base + 8;", even it says nothing more than the code. > + /* At this point the stack looks as if we just entered the > + function, with the return address at the top of the > + stack. */ > + sp = get_frame_register_unsigned (this_frame, I386_ESP_REGNUM); > + cache->base = sp + cache->sp_offset; > cache->saved_sp = cache->base + 8; > - > - /* The saved %eip will be at cache->base plus 4. */ Why this comment is removed? -- Yao (齐尧)