From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 89828 invoked by alias); 6 Jan 2020 15:55:15 -0000 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 Received: (qmail 89819 invoked by uid 89); 6 Jan 2020 15:55:14 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-16.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_STOCKGEN,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=2500, kevin, __LINE__, __line__ X-HELO: us-smtp-delivery-1.mimecast.com Received: from us-smtp-2.mimecast.com (HELO us-smtp-delivery-1.mimecast.com) (207.211.31.81) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 06 Jan 2020 15:55:13 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1578326111; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=tuldNTu62tEvGrd6HTgyceejA7WNQofS9vDGzule/L0=; b=GBSgihQkFL3/tMyVMrKCxCFrHc//8ffxnFv5oTtJ4/COV04wXv5rtLWXH/Xfanw20HcgPm IiqTbR5jcvtcbccWwkgoU2yHlb/Yaxo246pEkX0Xsyc2dPX2bnmO43KwUFuszX5/t43Sx5 4WjU5ltSA4bS7+4hTzsoAOQEAVGKHlk= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-43-nkvIV2gYMn26a0KYZBQG-g-1; Mon, 06 Jan 2020 10:55:10 -0500 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 63B3919057A3 for ; Mon, 6 Jan 2020 15:55:09 +0000 (UTC) Received: from f31-4.lan (ovpn-116-54.phx2.redhat.com [10.3.116.54]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 349595C1B0 for ; Mon, 6 Jan 2020 15:55:09 +0000 (UTC) Date: Mon, 06 Jan 2020 15:55:00 -0000 From: Kevin Buettner To: gdb-patches@sourceware.org Subject: Re: [PATCH v2] Avoid infinite recursion in find_pc_sect_line Message-ID: <20200106085508.32a5a022@f31-4.lan> In-Reply-To: <20191204215439.1748221-1-kevinb@redhat.com> References: <20191204215439.1748221-1-kevinb@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2020-01/txt/msg00116.txt.bz2 Ping. On Wed, 4 Dec 2019 14:54:39 -0700 Kevin Buettner wrote: > A patch somewhat like this patch has been in Fedora GDB for well over > a decade. The Fedora patch was written by Jan Kratochvil. The Fedora > version prints a warning and attempts to continue. This version will > error out, fatally. An earlier version of this patch was more like > the Fedora version than this one. Simon Marchi recommended use of an > assertion to test for the infinite recursion; I decided to use an > explicit test (with an "if" statement) along with a call to > internal_error() if the condition is met. This way, I could include > a plea to file a bug report. > > It was motivated by a customer reported bug (back in 2006!) which > showed infinite mutual recursion between find_pc_sect_line and > find_pc_line. Here is a portion of the backtrace from the bug report: > > (gdb) bt > #0 0x00000000004450a4 in lookup_minimal_symbol_by_pc_section ( > pc=251700325328, section=0x570f500) at gdb/minsyms.c:484 > #1 0x00000000004bbfb2 in find_pc_sect_line (pc=251700325328, > section=0x570f500, notcurrent=0) at gdb/symtab.c:2057 > #2 0x00000000004bc480 in find_pc_line (pc=251700325328, notcurrent=0) > at gdb/symtab.c:2232 > #3 0x00000000004bc1ff in find_pc_sect_line (pc=251700325328, > section=0x570f500, notcurrent=0) at gdb/symtab.c:2081 > > ... (lots and lots of the same two functions with the same parameters) > > #1070 0x00000000004bc480 in find_pc_line (pc=251700325328, notcurrent=0) > at gdb/symtab.c:2232 > #1071 0x00000000004bc1ff in find_pc_sect_line (pc=251700325328, > section=0x570f500, notcurrent=0) at gdb/symtab.c:2081 > #1072 0x00000000004bc480 in find_pc_line (pc=251700325328, notcurrent=0) > at gdb/symtab.c:2232 > #1073 0x00000000004bc1ff in find_pc_sect_line (pc=251700325328, > section=0x570f500, notcurrent=0) at gdb/symtab.c:2081 > #1074 0x00000000004bc480 in find_pc_line (pc=251700325328, notcurrent=0) > at gdb/symtab.c:2232 > #1075 0x00000000004bc1ff in find_pc_sect_line (pc=251696794399, > section=0x59b0df8, notcurrent=0) at gdb/symtab.c:2081 > #1076 0x00000000004bc480 in find_pc_line (pc=251696794399, notcurrent=0) > at gdb/symtab.c:2232 > #1077 0x000000000055550e in find_frame_sal (frame=0xb3f3e0, sal=0x7fff1d1a8200) > at gdb/frame.c:1392 > #1078 0x00000000004d86fd in set_current_sal_from_frame (frame=0x1648, center=1) > at gdb/stack.c:379 > #1079 0x00000000004cf137 in normal_stop () at gdb/infrun.c:3147 > ... > > The test case was a large application. Attempts were made to make a > small(er) test case, but those attempts were not successful. > Therefore, I cannot provide a new test for this patch. > > That said, we ought to guard against recursively calling > find_pc_sect_line (via find_pc_line) with the identical PC value that > it had been called with. Should this happen, infinite recursion (as > shown in the above backtrace) is the result. This patch prevents > that from happening. > > If this should happens, there is a bug somewhere, perhaps in GDB, perhaps > in some other part of the toolchain or a library. We error out fatally > with a message briefly describing the condition along with a plea to file > a bug report. > > I spent some time looking at the surrounding code and commentary which > handle the case of PC being in a stub/trampoline. It first appeared > in the public GDB repository in April, 1999. The ChangeLog entry for > this commit is from 1998-12-31. The relevant portion is: > > (find_pc_sect_line): Return correct information if pc is in import > or export stub (trampoline). > > What's remarkable about the overall ChangeLog entry is that it's over > 2500+ lines long! I believe that this was part of the infamous "HP > merge" (in which insufficient due diligence was given in accepting > a large batch of changes from an outside source). In the years that > followed, much of this code was either significantly revised or > outright removed. > > For this particular case, I'm grateful that extensive comments were > provided by "RT". (I haven't been able to figure out who RT is/was.) > I've decided against attempting to revise this stub/trampoline handling > code any further than adding Jan's test which prevents an obvious case > of infinite recursion. > > I've tested on Fedora 31, x86-64. I see no regressions. I've also > searched the logfile for the new message, but as expected, no message > was found (which is good). > > gdb/ChangeLog: > > * symtab.c (find_pc_sect_line): Add check which prevents infinite > recursion. > > Change-Id: I595470be6ab5f61ca7e4e9e70c61a252c0deaeaa > --- > gdb/symtab.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/gdb/symtab.c b/gdb/symtab.c > index 5c33fbf9ab..95020d843c 100644 > --- a/gdb/symtab.c > +++ b/gdb/symtab.c > @@ -3167,7 +3167,17 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent) > ; > /* fall through */ > else > - return find_pc_line (BMSYMBOL_VALUE_ADDRESS (mfunsym), 0); > + { > + /* Detect an obvious case of infinite recursion. If this > + should occur, we'd like to know about it, so error out, > + fatally. */ > + if (BMSYMBOL_VALUE_ADDRESS (mfunsym) == pc) > + internal_error (__FILE__, __LINE__, > + _("Infinite recursion detected in find_pc_sect_line;" > + "please file a bug report")); > + > + return find_pc_line (BMSYMBOL_VALUE_ADDRESS (mfunsym), 0); > + } > } > > symtab_and_line val; > -- > 2.23.0 >