From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2104 invoked by alias); 14 Sep 2012 08:06:02 -0000 Received: (qmail 2082 invoked by uid 22791); 14 Sep 2012 08:05:59 -0000 X-SWARE-Spam-Status: No, hits=-5.8 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,KHOP_RCVD_UNTRUST,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 14 Sep 2012 08:05:46 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q8E85ksp005184 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 14 Sep 2012 04:05:46 -0400 Received: from host2.jankratochvil.net (ovpn-113-58.phx2.redhat.com [10.3.113.58]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q8E85eTM008638 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Fri, 14 Sep 2012 04:05:43 -0400 Date: Fri, 14 Sep 2012 08:06:00 -0000 From: Jan Kratochvil To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: [patch] Code cleanup - inline -> artificial depth [Re: [patch+7.5?] Fix GDB-return into TAILCALL_FRAME (PR 14119)] Message-ID: <20120914080540.GA8584@host2.jankratochvil.net> References: <20120912180235.GA13250@host2.jankratochvil.net> <874nn13b8k.fsf@fleche.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <874nn13b8k.fsf@fleche.redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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: 2012-09/txt/msg00270.txt.bz2 On Thu, 13 Sep 2012 22:43:07 +0200, Tom Tromey wrote: > >>>>> "Jan" == Jan Kratochvil writes: > > Jan> I am not completely sure the patch in skip_inlined_frames is > Jan> appropriate for all the callers of skip_inlined_frames but it seems > Jan> so to me, frame_id of TAILCALL_FRAME is artifical the same way like > Jan> INLINE_FRAME is. > > This argument is persuasive, but the patch still makes me a bit nervous, > just because the concrete details at each call point may matter. I have stated more clear reasons in the attached patch. > It seems some comments need updating, e.g., inline_depth is overloaded > but the comment doesn't indicate this. I did not notice it before, thanks for pointing it out. Attached a pre-requisite code cleanup patch. No regressions on {x86_64,x86_64-m32,i686}-fedora18-linux-gnu. Thanks, Jan gdb/ 2012-09-14 Jan Kratochvil Code cleanup - rename 'inline' depth to 'artificial' depth. * breakpoint.c (set_momentary_breakpoint): Rename at a caller to frame_id_artificial_p, extend the comment. * dwarf2-frame-tailcall.c (tailcall_frame_this_id): Rename at a user. * frame.c (fprint_frame_id): Rename at a user, change debug output text to "artificial=". (skip_inlined_frames): Rename to ... (skip_artificial_frames): ... here. Extend the comment. (get_stack_frame_id, frame_unwind_caller_id): Rename at a caller. (frame_id_inlined_p): Rename to ... (frame_id_artificial_p): ... here. Rename at a user. (frame_id_eq, frame_id_inner, frame_unwind_caller_pc) (frame_unwind_caller_pc_if_available, frame_unwind_caller_arch): Rename at a user. * frame.h (struct frame_id): Rename inline_depth to artificial_depth. Extend the comment. (frame_id_inlined_p): Rename to ... (frame_id_artificial_p): ... here. * inline-frame.c (inline_frame_this_id): Rename at a user. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 12f20d6..b841bcd 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -8609,9 +8609,9 @@ set_momentary_breakpoint (struct gdbarch *gdbarch, struct symtab_and_line sal, { struct breakpoint *b; - /* If FRAME_ID is valid, it should be a real frame, not an inlined - one. */ - gdb_assert (!frame_id_inlined_p (frame_id)); + /* If FRAME_ID is valid, it should be a real frame, not an inlined or + tail-called one. */ + gdb_assert (!frame_id_artificial_p (frame_id)); b = set_raw_breakpoint (gdbarch, sal, type, &momentary_breakpoint_ops); b->enable_state = bp_enabled; diff --git a/gdb/dwarf2-frame-tailcall.c b/gdb/dwarf2-frame-tailcall.c index fcfeaf4..f284d98 100644 --- a/gdb/dwarf2-frame-tailcall.c +++ b/gdb/dwarf2-frame-tailcall.c @@ -223,9 +223,9 @@ tailcall_frame_this_id (struct frame_info *this_frame, void **this_cache, *this_id = get_frame_id (next_frame); (*this_id).code_addr = get_frame_pc (this_frame); (*this_id).code_addr_p = 1; - (*this_id).inline_depth = (cache->chain_levels - - existing_next_levels (this_frame, cache)); - gdb_assert ((*this_id).inline_depth > 0); + (*this_id).artificial_depth = (cache->chain_levels + - existing_next_levels (this_frame, cache)); + gdb_assert ((*this_id).artificial_depth > 0); } /* Find PC to be unwound from THIS_FRAME. THIS_FRAME must be a part of diff --git a/gdb/frame.c b/gdb/frame.c index 9ed49f6..8c44cad 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -226,8 +226,8 @@ fprint_frame_id (struct ui_file *file, struct frame_id id) fprint_field (file, "code", id.code_addr_p, id.code_addr); fprintf_unfiltered (file, ","); fprint_field (file, "special", id.special_addr_p, id.special_addr); - if (id.inline_depth) - fprintf_unfiltered (file, ",inlined=%d", id.inline_depth); + if (id.artificial_depth) + fprintf_unfiltered (file, ",artificial=%d", id.artificial_depth); fprintf_unfiltered (file, "}"); } @@ -303,11 +303,12 @@ fprint_frame (struct ui_file *file, struct frame_info *fi) fprintf_unfiltered (file, "}"); } -/* Given FRAME, return the enclosing normal frame for inlined - function frames. Otherwise return the original frame. */ +/* Given FRAME, return the enclosing frame as found in real frames read-in from + inferior memory. Skip any previous frames which were made up by GDB. + Return the original frame if no immediate previous frames exist. */ static struct frame_info * -skip_inlined_frames (struct frame_info *frame) +skip_artificial_frames (struct frame_info *frame) { while (get_frame_type (frame) == INLINE_FRAME) frame = get_prev_frame (frame); @@ -354,7 +355,7 @@ get_frame_id (struct frame_info *fi) struct frame_id get_stack_frame_id (struct frame_info *next_frame) { - return get_frame_id (skip_inlined_frames (next_frame)); + return get_frame_id (skip_artificial_frames (next_frame)); } struct frame_id @@ -367,10 +368,10 @@ frame_unwind_caller_id (struct frame_info *next_frame) returning a null_frame_id (e.g., when a caller requests the frame ID of "main()"s caller. */ - next_frame = skip_inlined_frames (next_frame); + next_frame = skip_artificial_frames (next_frame); this_frame = get_prev_frame_1 (next_frame); if (this_frame) - return get_frame_id (skip_inlined_frames (this_frame)); + return get_frame_id (skip_artificial_frames (this_frame)); else return null_frame_id; } @@ -435,12 +436,12 @@ frame_id_p (struct frame_id l) } int -frame_id_inlined_p (struct frame_id l) +frame_id_artificial_p (struct frame_id l) { if (!frame_id_p (l)) return 0; - return (l.inline_depth != 0); + return (l.artificial_depth != 0); } int @@ -472,8 +473,8 @@ frame_id_eq (struct frame_id l, struct frame_id r) /* An invalid special addr is a wild card (or unused). Otherwise if special addresses are different, the frames are different. */ eq = 0; - else if (l.inline_depth != r.inline_depth) - /* If inline depths are different, the frames must be different. */ + else if (l.artificial_depth != r.artificial_depth) + /* If artifical depths are different, the frames must be different. */ eq = 0; else /* Frames are equal. */ @@ -530,7 +531,7 @@ frame_id_inner (struct gdbarch *gdbarch, struct frame_id l, struct frame_id r) if (!l.stack_addr_p || !r.stack_addr_p) /* Like NaN, any operation involving an invalid ID always fails. */ inner = 0; - else if (l.inline_depth > r.inline_depth + else if (l.artificial_depth > r.artificial_depth && l.stack_addr == r.stack_addr && l.code_addr_p == r.code_addr_p && l.special_addr_p == r.special_addr_p @@ -706,14 +707,14 @@ frame_unwind_pc (struct frame_info *this_frame) CORE_ADDR frame_unwind_caller_pc (struct frame_info *this_frame) { - return frame_unwind_pc (skip_inlined_frames (this_frame)); + return frame_unwind_pc (skip_artificial_frames (this_frame)); } int frame_unwind_caller_pc_if_available (struct frame_info *this_frame, CORE_ADDR *pc) { - return frame_unwind_pc_if_available (skip_inlined_frames (this_frame), pc); + return frame_unwind_pc_if_available (skip_artificial_frames (this_frame), pc); } int @@ -2326,7 +2327,7 @@ frame_unwind_arch (struct frame_info *next_frame) struct gdbarch * frame_unwind_caller_arch (struct frame_info *next_frame) { - return frame_unwind_arch (skip_inlined_frames (next_frame)); + return frame_unwind_arch (skip_artificial_frames (next_frame)); } /* Stack pointer methods. */ diff --git a/gdb/frame.h b/gdb/frame.h index 532fb26..fa80663 100644 --- a/gdb/frame.h +++ b/gdb/frame.h @@ -134,9 +134,11 @@ struct frame_id unsigned int code_addr_p : 1; unsigned int special_addr_p : 1; - /* The inline depth of this frame. A frame representing a "called" - inlined function will have this set to a nonzero value. */ - int inline_depth; + /* It is non-zero for a frame made up by GDB without stack data + representation in inferior, such as INLINE_FRAME or TAILCALL_FRAME. + Caller of inlined function will have it zero, each more inner called frame + will have it increasingly one, two etc. Similarly for TAILCALL_FRAME. */ + int artificial_depth; }; /* Methods for constructing and comparing Frame IDs. */ @@ -178,9 +180,10 @@ extern struct frame_id frame_id_build_wild (CORE_ADDR stack_addr); ID. */ extern int frame_id_p (struct frame_id l); -/* Returns non-zero when L is a valid frame representing an inlined - function. */ -extern int frame_id_inlined_p (struct frame_id l); +/* Returns non-zero when L is a valid frame representing a frame made up by GDB + without stack data representation in inferior, such as INLINE_FRAME or + TAILCALL_FRAME. */ +extern int frame_id_artificial_p (struct frame_id l); /* Returns non-zero when L and R identify the same frame, or, if either L or R have a zero .func, then the same frame base. */ diff --git a/gdb/inline-frame.c b/gdb/inline-frame.c index 06de5b0..cce9ef8 100644 --- a/gdb/inline-frame.c +++ b/gdb/inline-frame.c @@ -178,7 +178,7 @@ inline_frame_this_id (struct frame_info *this_frame, func = get_frame_function (this_frame); gdb_assert (func != NULL); (*this_id).code_addr = BLOCK_START (SYMBOL_BLOCK_VALUE (func)); - (*this_id).inline_depth++; + (*this_id).artificial_depth++; } static struct value *