From patchwork Fri May 29 14:10:27 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kratochvil X-Patchwork-Id: 6978 Received: (qmail 7440 invoked by alias); 29 May 2015 14:10:35 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 7417 invoked by uid 89); 29 May 2015 14:10:34 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.3 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=no 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 (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 29 May 2015 14:10:33 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 792822CD7E7; Fri, 29 May 2015 14:10:32 +0000 (UTC) Received: from host1.jankratochvil.net (ovpn-116-53.ams2.redhat.com [10.36.116.53]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t4TEASEM021915 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 29 May 2015 10:10:31 -0400 Date: Fri, 29 May 2015 16:10:27 +0200 From: Jan Kratochvil To: Yao Qi Cc: Andreas Schwab , gdb-patches@sourceware.org Subject: Re: [PATCH] Fix wrong assertions Message-ID: <20150529141027.GA8159@host1.jankratochvil.net> References: <87vbg1eg08.fsf@igel.home> <20150513140106.GB3023@host1.jankratochvil.net> <86bnh3pw61.fsf@gmail.com> <20150529113101.GA15460@host1.jankratochvil.net> <86382fpki0.fsf@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <86382fpki0.fsf@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-IsSubscribed: yes On Fri, 29 May 2015 15:43:19 +0200, Yao Qi wrote: > Jan Kratochvil writes: > > The terminology seems bogus there. > > > > "partially ambiguous" was meant the chain: > > main -> a -> -> d > > An intersection of all possible chains. > > Sounds like "partially ambiguous" is equivalent to "ambiguous". Yes, probably, I am not sure how to call it all myself. > If that is right, the assert below is too strict, isn't? Yes, it is too strict, this is why I agree with the fix by Andreas. > /* See call_site_find_chain_1 why there is no way to reach the bottom callee > PC again. In such case there must be two different code paths to reach > it, therefore some of the former determined intermediate PCs must differ > and the unambiguous chain gets shortened. */ > gdb_assert (result->callers + result->callees < result->length); > > > but that doe snot matter). Consequently its elements from the middle are > > being removed and there remains only some few unambiguous top and > > bottom ones. > > If there is no call sites removed from the chain during the intersection, > CALLERS + CALLEES == LENGTH, right? Just I expected there always has to be some site removed from the chain. I do not find obvious it does not have to. But maybe someone else finds it obvious. > in function chain_candidate, > result->length is set by the length of a chain. If this chain is the > shortest one, CALLERS + CALLEES == LENGTH otherwise, > CALLERS + CALLEES < LENGTH. Is it right? It is right now. But when one does not think about self-tail-calls then even the shortest one will get one frame removed. > If so, we need to relax the > condition in the assert and update the comments. Yes, attached with updated comment. > > I did not realize that there can be self-tail-call: > > main(0x100) -> a(0x200) -> d(0x400) > > main(0x100) -> a(0x280) -> a(0x200) -> d(0x400) > > which intersects to: > > main(0x100) -> ? -> a(0x200) -> d(0x400) > > And so if the first chain was chosen the > > main(0x100) -> a(0x200) -> d(0x400) > > then the final intersection has callers+callees==length. > > What are the definitions of CALLERS, CALLEES, top and bottom? given this example? top=CALLERS=main(0x100), therefore 1 bottom=CALLEES=d(0x400), therefore 1 top = topmost, where you can go by GDB "up" commands, also called "prev" in struct frame_info. bottom = bottommost, where you can go by GDB "down" commands, also called "next" in struct frame_info. Jan 2015-05-29 Andreas Schwab Jan Kratochvil PR symtab/18392 * dwarf2-frame-tailcall.c (pretended_chain_levels): Correct assertion. * dwarf2loc.c (chain_candidate): Likewise. diff --git a/gdb/dwarf2-frame-tailcall.c b/gdb/dwarf2-frame-tailcall.c index b412a5b..f964ab2 100644 --- a/gdb/dwarf2-frame-tailcall.c +++ b/gdb/dwarf2-frame-tailcall.c @@ -197,7 +197,7 @@ pretended_chain_levels (struct call_site_chain *chain) return chain->length; chain_levels = chain->callers + chain->callees; - gdb_assert (chain_levels < chain->length); + gdb_assert (chain_levels <= chain->length); return chain_levels; } diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c index 3aa8ddd..68d6cb4 100644 --- a/gdb/dwarf2loc.c +++ b/gdb/dwarf2loc.c @@ -825,9 +825,9 @@ chain_candidate (struct gdbarch *gdbarch, struct call_site_chain **resultp, /* See call_site_find_chain_1 why there is no way to reach the bottom callee PC again. In such case there must be two different code paths to reach - it, therefore some of the former determined intermediate PCs must differ - and the unambiguous chain gets shortened. */ - gdb_assert (result->callers + result->callees < result->length); + it. Still it may CALLERS+CALLEES==LENGTH in the case of optional + tail-call calling itself. */ + gdb_assert (result->callers + result->callees <= result->length); } /* Create and return call_site_chain for CALLER_PC and CALLEE_PC. All the