From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x730.google.com (mail-qk1-x730.google.com [IPv6:2607:f8b0:4864:20::730]) by sourceware.org (Postfix) with ESMTPS id D76233950C11 for ; Tue, 14 Apr 2020 21:31:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org D76233950C11 Received: by mail-qk1-x730.google.com with SMTP id v7so15158200qkc.0 for ; Tue, 14 Apr 2020 14:31:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=O9yAHZ92a6e0+qDdEBGH9vFy3DoigMMthn5X2wgU6EA=; b=GLOGWdyvm1opTM9yN8MfBW177NtD5VCwp+MJnb3+OreZPx6IDfI5g5tWzJQVb9CTf3 zI/0fl6QludxfZ7jLebp5D17m/FimYw5VFhHsDumzf1lOeG1cS1eINMMYPxD01V1G/qd GXsRbFeJ895qmLR9GTAYzB0B++GAu46x1vgI6ty469igFexYNSCAsMRqjGTl4ZHC0A5x Yx+dUdHQFcf6OJLH+RmX8mCGND02GvNMMpvAKBW3xTj+Nv1PIbIjCUnG9bszM5u6ZmJb s0rj0l9O6GiI9tI5JYuYMOxhpGz9V96hrVP3WLqt8KLlS87SivCLB4vGdEXFCXtk4Za/ Ddew== X-Gm-Message-State: AGi0PuadBb3sVGGlxAFqidXX5V7uR4XblQhNCKtkj8NOnAIrvMN5076w Cp5rBh9z8yRD9B5j272QRMWlCSbLUzY= X-Google-Smtp-Source: APiQypIB0skQlyUVGaNG88u5pvLCh/mQ+wfP5dZ8DHRFwA/WXtxa++JDNg90/lfGMexI5/s+SWD48g== X-Received: by 2002:a37:6cc5:: with SMTP id h188mr23752335qkc.389.1586899909792; Tue, 14 Apr 2020 14:31:49 -0700 (PDT) Received: from localhost.localdomain ([191.249.229.71]) by smtp.gmail.com with ESMTPSA id y188sm8152814qkd.35.2020.04.14.14.31.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Apr 2020 14:31:48 -0700 (PDT) From: Luis Machado To: gdb-patches@sourceware.org Cc: tromey@adacore.com Subject: [PATCH] Fix inline frame unwinding breakage Date: Tue, 14 Apr 2020 18:31:37 -0300 Message-Id: <20200414213137.24015-1-luis.machado@linaro.org> X-Mailer: git-send-email 2.17.1 X-Spam-Status: No, score=-21.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 Apr 2020 21:31:53 -0000 There has been some breakage for aarch64-linux, arm-linux and s390-linux in terms of inline frame unwinding. There may be other targets, but these are the ones i'm aware of. The following testcases started to show numerous failures and trigger internal errors in GDB after commit 1009d92fc621bc4d017029b90a5bfab16e17fde5, "Find tailcall frames before inline frames". gdb.opt/inline-break.exp gdb.opt/inline-cmds.exp gdb.python/py-frame-inline.exp gdb.reverse/insn-reverse.exp The internal errors were of this kind: binutils-gdb/gdb/frame.c:579: internal-error: frame_id get_frame_id(frame_info*): Assertion `fi->level == 0' failed. After a lengthy investigation to try and find the cause of these assertions, it seems we're dealing with some fragile/poorly documented code to handle inline frames and we are attempting to unwind from this fragile section of code. Before commit 1009d92fc621bc4d017029b90a5bfab16e17fde5, the tailcall sniffer was invoked from dwarf2_frame_prev_register. By the time we invoke the dwarf2_frame_prev_register function, we've probably already calculated the frame id (via compute_frame_id). After said commit, the call to dwarf2_tailcall_sniffer_first was moved to dwarf2_frame_cache. This is very early in a frame creation process, and we're still calculating the frame ID (so compute_frame_id is in the call stack). This would be fine for regular frames, but the above testcases all deal with some inline frames. The particularity of inline frames is that their frame ID's depend on the previous frame's ID, and the previous frame's ID relies in the inline frame's registers. So it is a bit of a messy situation. We have comments in various parts of the code warning about some of these particularities. In the case of dwarf2_tailcall_sniffer_first, we attempt to unwind the PC, which goes through various functions until we eventually invoke frame_unwind_got_register. This function will eventually attempt to create a lazy value for a particular register, and this lazy value will require a valid frame ID. Since the inline frame doesn't have a valid frame ID yet (remember we're still calculating the previous frame's ID so we can tell what the inline frame ID is) we will call compute_frame_id for the inline frame (level 0). We'll eventually hit the assertion above, inside get_frame_id: -- /* If we haven't computed the frame id yet, then it must be that this is the current frame. Compute it now, and stash the result. The IDs of other frames are computed as soon as they're created, in order to detect cycles. See get_prev_frame_if_no_cycle. */ gdb_assert (fi->level == 0); -- It seems to me we shouldn't have reached this assertion without having the inline frame ID already calculated. In fact, it seems we even start recursing a bit when we invoke get_prev_frame_always within inline_frame_this_id. But a check makes us quit the recursion and proceed to compute the id. Here's the call stack for context: <<<< recursion >>>> #1 0x0000aaaaaae1d098 in get_prev_frame_always (this_frame=0xaaaaab85a670) at ../../../repos/binutils-gdb/gdb/frame.c:2124 <<< recursion >>> at ../../../repos/binutils-gdb/gdb/inline-frame.c:165 at ../../../repos/binutils-gdb/gdb/dwarf2/frame.c:1296 at ../../../repos/binutils-gdb/gdb/aarch64-tdep.c:1114 at ../../../repos/binutils-gdb/gdb/dwarf2/frame.c:1316 at ../../../repos/binutils-gdb/gdb/dwarf2/frame-tailcall.c:388 at ../../../repos/binutils-gdb/gdb/dwarf2/frame.c:1218 <<<< first call >>>> #22 0x0000aaaaaae1d098 in get_prev_frame_always (this_frame=0xaaaaab85a670) at ../../../repos/binutils-gdb/gdb/frame.c:2124 The following patch addresses this by using a function that unwinds the PC from the next (inline) frame directly as opposed to creating a lazy value that is bound to the next frame's ID (still not computed). I've validated this for aarch64-linux and x86_64-linux by running the testsuite. Tromey, would you mind checking if this suits your problematic core file tailcall scenario? gdb/ChangeLog: 2020-04-14 Luis Machado * dwarf2/frame-tailcall.c (dwarf2_tailcall_sniffer_first): Use get_frame_register instead of gdbarch_unwind_pc. --- gdb/dwarf2/frame-tailcall.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gdb/dwarf2/frame-tailcall.c b/gdb/dwarf2/frame-tailcall.c index 2d219f13f9..01bb134a5c 100644 --- a/gdb/dwarf2/frame-tailcall.c +++ b/gdb/dwarf2/frame-tailcall.c @@ -385,7 +385,9 @@ dwarf2_tailcall_sniffer_first (struct frame_info *this_frame, prev_gdbarch = frame_unwind_arch (this_frame); /* Simulate frame_unwind_pc without setting this_frame->prev_pc.p. */ - prev_pc = gdbarch_unwind_pc (prev_gdbarch, this_frame); + get_frame_register (this_frame, gdbarch_pc_regnum (prev_gdbarch), + (gdb_byte *) &prev_pc); + prev_pc = gdbarch_addr_bits_remove (prev_gdbarch, prev_pc); /* call_site_find_chain can throw an exception. */ chain = call_site_find_chain (prev_gdbarch, prev_pc, this_pc); -- 2.17.1