From patchwork Sun Nov 17 19:40:11 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Simon Marchi (Code Review)" X-Patchwork-Id: 35996 Received: (qmail 58016 invoked by alias); 17 Nov 2019 19:40:17 -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 58005 invoked by uid 89); 17 Nov 2019 19:40:17 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_STOCKGEN autolearn=ham version=3.3.1 spammy=remarkable, infrun.c, infrunc, UD:infrun.c X-HELO: mx1.osci.io Received: from polly.osci.io (HELO mx1.osci.io) (8.43.85.229) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 17 Nov 2019 19:40:15 +0000 Received: by mx1.osci.io (Postfix, from userid 994) id ED9E320300; Sun, 17 Nov 2019 14:40:13 -0500 (EST) Received: from gnutoolchain-gerrit.osci.io (gnutoolchain-gerrit.osci.io [IPv6:2620:52:3:1:5054:ff:fe06:16ca]) by mx1.osci.io (Postfix) with ESMTP id EA3912032B for ; Sun, 17 Nov 2019 14:40:11 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by gnutoolchain-gerrit.osci.io (Postfix) with ESMTP id D9A172816F for ; Sun, 17 Nov 2019 14:40:11 -0500 (EST) X-Gerrit-PatchSet: 1 Date: Sun, 17 Nov 2019 14:40:11 -0500 From: "Kevin Buettner (Code Review)" To: gdb-patches@sourceware.org Message-ID: Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange Subject: [review] Avoid infinite recursion in find_pc_sect_line X-Gerrit-Change-Id: I595470be6ab5f61ca7e4e9e70c61a252c0deaeaa X-Gerrit-Change-Number: 682 X-Gerrit-ChangeURL: X-Gerrit-Commit: 23229a89912a2e567f1d1a4ebb7ecc5c12f86c0c References: Reply-To: kevinb@redhat.com, gdb-patches@sourceware.org MIME-Version: 1.0 Content-Disposition: inline User-Agent: Gerrit/3.0.3-79-g83ff7f88f1 Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/682 ...................................................................... Avoid infinite recursion in find_pc_sect_line This is a patch which has been in Fedora GDB for well over a decade. It was written by Jan Kratochvil, though I've made some minor changes to the comment and warning message. 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. Also, it seems likely that if this case should happen, there is a bug somewhere, possibly in the C library or in some other part of the toolchain, so we print a warning message asking the user to file a bug report. We don't error out here because falling through may produce a satisfactory result. 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 --- M gdb/symtab.c 1 file changed, 10 insertions(+), 0 deletions(-) diff --git a/gdb/symtab.c b/gdb/symtab.c index 0064800..8a01317 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -3142,6 +3142,16 @@ SYMBOL_LINKAGE_NAME (msymbol)); */ ; /* fall through */ + else if (BMSYMBOL_VALUE_ADDRESS (mfunsym) == pc) + { + /* Avoid infinite (mutual) recursion between + find_pc_sect_line and find_pc_line. */ + warning ("Could not dereference stub/trampoline %s (0x%s). " + "Please file a bug report.", + MSYMBOL_LINKAGE_NAME (msymbol.minsym), + paddress(target_gdbarch (), pc)); + /* Fall through. */ + } else return find_pc_line (BMSYMBOL_VALUE_ADDRESS (mfunsym), 0); }