From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id DA0133858D1E for ; Wed, 8 Nov 2023 10:54:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DA0133858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org DA0133858D1E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699440878; cv=none; b=fgRGUbT5cD0L82oV9Uy/UhncHddUrgS05gS3gwkJTJzujsAbiSlONbyu6D0FbzryJEioab/uLB2b1QXi+9/fbljCtrYb+BNDG/3oeeh+CYP4Ue/PwYb29bFN0nnOftHS3dMBLngkIIVD0AkcpaXNak4t7qWgzr1wfa/QhaZTaWo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699440878; c=relaxed/simple; bh=urQj7/oU7ppHoTNMQSUZAWIeqSdMu4WNOc2ZDviwvug=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=ILMSlqxV/W4xiVfVmQhP2pFbEAShUBDBaT0POfnDFc3uxZ5y7KMDMgX0sVhaJWhifpKIr2+tagiT7T1Dpxd6IN6T7tcJ3Dj2LdxluIFOcU94HuR3mEQzc5k3lFDKLhISaljGJ/YkA/WYIHuXLiM3zRTywyTAWhGFJe5D0BAfPH8= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1699440876; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=Pg2YETG6wfdLjDIZlh9V+iyY8P9oeKbvpi9AiBIoBtQ=; b=FqFNlEEn27R7KbVS73JrbTHU1gLi+Mf+Ay7SKxFOUTrXNmVdLXeu+fEVS36Lv1tvCWM+SK CCioSRnnIBN5urlTnDXU1Thv8NR4jn2LkkbhIB6lMJMUgEIysocA1M5FtlxZmzVEP2WkOH D2v5pT02+Kxm3v14Qqi0FvnAyoFlXLs= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-548-1K2q1r5oM2CmsFI7KArzXw-1; Wed, 08 Nov 2023 05:54:35 -0500 X-MC-Unique: 1K2q1r5oM2CmsFI7KArzXw-1 Received: by mail-wr1-f70.google.com with SMTP id ffacd0b85a97d-32f820c471fso3425618f8f.0 for ; Wed, 08 Nov 2023 02:54:34 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699440874; x=1700045674; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Pg2YETG6wfdLjDIZlh9V+iyY8P9oeKbvpi9AiBIoBtQ=; b=TwbCYyKkYFFJUzCS3dG2k422aR2H45PbAPijswIL69cZLMli9k2o0sV4WMgaRqJktd Y5LW80N111k3U+mp6Rm5uE0auGkddHi0zh/bQA/cK3EbJ4/z2mClg5x4WG/8kNLbx7ug NbkyEVBg3+HNduItpFLkhgn2nONLGEqr8iEBp5SnEeBM7/TX5h+9J45pi3e4EG1Vum38 KHPdGwc4/gWc7QlOvS5ySCTnyvmp64P3aRapMKOshRDNhPmSVWjkMsB4DW8BpcsuqBIW 8aWnpOdaH2zYmMKOAmyY3e8/2TYEZcXnwz0fa0XwzV7oYhGHN9OyS8zEUGN2OqBkGfZi SOHQ== X-Gm-Message-State: AOJu0Ywyyxv1ScnqviEzsBHzWxS79266kLdBjfz8DE9pyn4gnRyMwn2j Qiy7eJBzuXbn4iHYAlP1m/OhCRuXtKnoqpc8kVaoUQsqRXTyPIIS7gGvK65gD2p8A1zm5aZZPr7 fb55Foo7ZyjbPpxdHOlSL0Q== X-Received: by 2002:a5d:524b:0:b0:32d:9d99:94e7 with SMTP id k11-20020a5d524b000000b0032d9d9994e7mr1079486wrc.49.1699440873906; Wed, 08 Nov 2023 02:54:33 -0800 (PST) X-Google-Smtp-Source: AGHT+IFedTuR9XFcFlvr53s11x1o3x7dVkPop1ZPddc+xtm/ZOfnyLUSCr1RID3JQc9q7oEO9WXy+g== X-Received: by 2002:a5d:524b:0:b0:32d:9d99:94e7 with SMTP id k11-20020a5d524b000000b0032d9d9994e7mr1079476wrc.49.1699440873479; Wed, 08 Nov 2023 02:54:33 -0800 (PST) Received: from localhost (105.226.159.143.dyn.plus.net. [143.159.226.105]) by smtp.gmail.com with ESMTPSA id ba15-20020a0560001c0f00b0032326908972sm4681802wrb.17.2023.11.08.02.54.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 08 Nov 2023 02:54:32 -0800 (PST) From: Andrew Burgess To: Carl Love , Ulrich Weigand , "gdb-patches@sourceware.org" , Keith Seitz Cc: cel@us.ibm.com Subject: Re: [PATCH 1/2, ver3] PowerPC, Fix-test-gdb.base-store.exp In-Reply-To: <77710bdab4d396d3a4d001bfb2217d00e686c823.camel@linux.ibm.com> References: <6f9c8fa20962129048384d74f6f15d1b37d255ed.camel@us.ibm.com> <76b8ed7b93608d40ab42b0538319f78eaf7d621c.camel@us.ibm.com> <87bkcyhc5g.fsf@redhat.com> <8735bded66303c8cdfacfad9bd953c582e8076f2.camel@linux.ibm.com> <87sf602wqt.fsf@redhat.com> <51fbddb2f868c778660c592a91c39677a3763197.camel@de.ibm.com> <87ttq81m09.fsf@redhat.com> <62b44b45916ee898484992d57cc7bfa2ca0bc040.camel@de.ibm.com> <77710bdab4d396d3a4d001bfb2217d00e686c823.camel@linux.ibm.com> Date: Wed, 08 Nov 2023 10:54:31 +0000 Message-ID: <874jhwtt08.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Carl Love writes: > Andrew, Ulrich: > > Per the discussions, the commit message has been updated to make it > clear that the patch fixes the DWARF register mapping which fixes two > of the test failures in step.exp. The fix exposed a dormant issue with > the signal handling on PowerPC. The underlying issue which is then > exposed causes the signal handling test sigstep.exp to fail. The test > sigstep.exp tests stepping into a signal handler. The patch also fixes > the underlying signal handler issue. > > This new verion of the patch only updates the commit log to make it > clear the patch is fixing multiple issues. > > Please let me know if the commit description is clear and acceptable. > > Thanks. > > Carl > ------------------------------------------------------------ > rs6000, Fix Linux DWARF register mapping > > Overview of issues fixed by the patch. > > The primary issue this patch fixes is the DWARF register mapping for > Linux. The changes in ppc-linux-tdep.c fix the DWARF register mapping > issues. The register mapping issue is responsible for two of the > five regression bugs seen in gdb.base/store.exp. > > Once the register mapping is fixed, an underlying issue with the unwinding > of the signal trampoline in common-code in ifrun.c was found. This > underlying bug is best described by Ulrich in the following description. > > The unwinder bug shows up on platforms where the kernel uses a trampoline > to dispatch "calls to" the signal handler (not just *returns from* the > signal handler). Many platforms use a trampoline for signal return, and > that is working fine, but the only platform I'm (Ulrich) aware of that > uses a trampoline for signal handler calls is (recent kernels for) > PowerPC. I believe the rationale for using a trampoline here > is to improve performance by avoiding unbalancing of the > branch predictor's call/return stack. > > However, on PowerPC the bug is dormant as well as it is hidden > by *another* bug that prevents correct unwinding out of the > signal trampoline. This is because the custom CFI for the > trampoline uses a register number (VSCR) that is not ever used > by compiler-generated CFI, and that particular register is > mapped to an invalid number by the current PowerPC DWARF mapper. > > The underlying unwinder bug is exposed by the "new" regression failures > in gdb.base/sigstep.exp. These failures were previously masked by > the fact that GDB was not seeing a valid frame when it tried to unwind > the frames. The sigstep.exp test is specifically testing stepping into > a signal handler. With the correct DWARF register mapping in place, > specifically the VSCR mapping, the signal trampoline code now unwinds to a > valid frame exposing the pre-existing bug in how the signal handler on > PowerPC works. The one line change infrun.c fixes the exiting bug in > the common-code for platforms that use a trampoline to dispatch calls > to the signal handler by not stopping in the SIGTRAMP_FRAME. > > Detailed description of the DWARF register mapping fix. > > The PowerPC DWARF register mapping is the same for the .eh_frame and > .debug_frame on Linux. PowerPC uses different mapping for .eh_frame and > .debug_frame on other operating systems. The current GDB support for > mapping the DWARF registers in rs6000_linux_dwarf2_reg_to_regnum and > rs6000_adjust_frame_regnum file gdb/rs6000-tdep.c is not correct for Linux. > The files have some legacy mappings for spe_acc, spefscr, EV which was > removed from GCC in 2017. > > This patch adds a two new functions rs6000_linux_dwarf2_reg_to_regnum, > and rs6000_linux_adjust_frame_regnum in file gdb/ppc-linux-tdep.c to handle > the DWARF register mappings on Linux. Function > rs6000_linux_dwarf2_reg_to_regnum is installed for both gdb_dwarf_to_regnum > and gdbarch_stab_reg_to_regnum since the mappings are the same. > > The ppc_linux_init_abi function in gdb/ppc-linux-tdep.c is updated to > call set_gdbarch_dwarf2_reg_to_regnum map the new function > rs6000_linux_dwarf2_reg_to_regnum for the architecture. Similarly, > dwarf2_frame_set_adjust_regnum is called to map > rs6000_linux_adjust_frame_regnum into the architecture. > > Additional detail on the signal handling fix. > > The specific sequence of events for handling a signal on most > architectures is as follows: > > 1) Some code is running when a signal arrives. > 2) The kernel handles the signal and dispatches to the handler. > ... > > However on PowerPC the sequence of events is: > > 1) Some code is running when a signal arrives. > 2) The kernel handles the signal and dispatches to the trampoline. > 3) The trampoline performs a normal function call to the handler. > ... > > We want the "nexti" to step into, not over, signal handlers invoked by > the kernel. This is the case for most platforms as the kernel puts a > signal trampoline frame onto the stack to handle proper return after the > handler. However, on some platforms such as PowerPC, the kernel actually > uses a trampoline to handle *invocation* of the handler. We do not > want GDB to stop in the SIGTRAMP_FRAME. The issue is fixed in function > process_event_stop_test by adding a check that the frame is not a > SIGTRAMP_FRAME to the if statement to stop in a subroutine call. This > prevents GDB from erroneously detecting the trampoline invocation as a > subroutine call. > > This patch fixes two regression test failures in gdb.base/store.exp. > > The patch then fixes an exposed, dormant, signal handling issue that > is exposed in the signal handling test gdb.base/sigstep.exp. > > The patch has been tested on Power 8 LE/BE, Power 9 LE/BE, Power 10 with > no new regressions. Note, only two of the five failures in store.exp > are fixed. The remaining three failures are fixed in a following > patch. Carl, Sorry for the slow delay, I had to find some time to play with this a little. But thanks to the expanded explanation, it's now clear what's going on. This all looks great. Approved-By: Andrew Burgess Thanks, Andrew > --- > gdb/infrun.c | 13 +++++++++++ > gdb/ppc-linux-tdep.c | 53 ++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 66 insertions(+) > > diff --git a/gdb/infrun.c b/gdb/infrun.c > index 4730d290442..922d8a6913d 100644 > --- a/gdb/infrun.c > +++ b/gdb/infrun.c > @@ -7334,8 +7334,21 @@ process_event_stop_test (struct execution_control_state *ecs) > initial outermost frame, before sp was valid, would > have code_addr == &_start. See the comment in frame_id::operator== > for more. */ > + > + /* We want "nexti" to step into, not over, signal handlers invoked > + by the kernel, therefore this subroutine check should not trigger > + for a signal handler invocation. On most platforms, this is already > + not the case, as the kernel puts a signal trampoline frame onto the > + stack to handle proper return after the handler, and therefore at this > + point, the current frame is a grandchild of the step frame, not a > + child. However, on some platforms, the kernel actually uses a > + trampoline to handle *invocation* of the handler. In that case, > + when executing the first instruction of the trampoline, this check > + would erroneously detect the trampoline invocation as a subroutine > + call. Fix this by checking for SIGTRAMP_FRAME. */ > if ((get_stack_frame_id (frame) > != ecs->event_thread->control.step_stack_frame_id) > + && get_frame_type (frame) != SIGTRAMP_FRAME > && ((frame_unwind_caller_id (get_current_frame ()) > == ecs->event_thread->control.step_stack_frame_id) > && ((ecs->event_thread->control.step_stack_frame_id > diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c > index 784dafa59db..8d975336fe5 100644 > --- a/gdb/ppc-linux-tdep.c > +++ b/gdb/ppc-linux-tdep.c > @@ -83,6 +83,7 @@ > #include "features/rs6000/powerpc-isa207-vsx64l.c" > #include "features/rs6000/powerpc-isa207-htm-vsx64l.c" > #include "features/rs6000/powerpc-e500l.c" > +#include "dwarf2/frame.h" > > /* Shared library operations for PowerPC-Linux. */ > static struct target_so_ops powerpc_so_ops; > @@ -2088,6 +2089,49 @@ ppc_linux_displaced_step_prepare (gdbarch *arch, thread_info *thread, > return per_inferior->disp_step_buf->prepare (thread, displaced_pc); > } > > +/* Convert a Dwarf 2 register number to a GDB register number for Linux. */ > + > +static int > +rs6000_linux_dwarf2_reg_to_regnum (struct gdbarch *gdbarch, int num) > +{ > + ppc_gdbarch_tdep *tdep = gdbarch_tdep(gdbarch); > + > + if (0 <= num && num <= 31) > + return tdep->ppc_gp0_regnum + num; > + else if (32 <= num && num <= 63) > + return tdep->ppc_fp0_regnum + (num - 32); > + else if (77 <= num && num < 77 + 32) > + return tdep->ppc_vr0_regnum + (num - 77); > + else > + switch (num) > + { > + case 65: > + return tdep->ppc_lr_regnum; > + case 66: > + return tdep->ppc_ctr_regnum; > + case 76: > + return tdep->ppc_xer_regnum; > + case 109: > + return tdep->ppc_vrsave_regnum; > + case 110: > + return tdep->ppc_vrsave_regnum - 1; /* vscr */ > + } > + > + /* Unknown DWARF register number. */ > + return -1; > +} > + > +/* Translate a .eh_frame register to DWARF register, or adjust a > + .debug_frame register. */ > + > +static int > +rs6000_linux_adjust_frame_regnum (struct gdbarch *gdbarch, int num, > + int eh_frame_p) > +{ > + /* Linux uses the same numbering for .debug_frame numbering as .eh_frame. */ > + return num; > +} > + > static void > ppc_linux_init_abi (struct gdbarch_info info, > struct gdbarch *gdbarch) > @@ -2135,6 +2179,15 @@ ppc_linux_init_abi (struct gdbarch_info info, > set_gdbarch_stap_is_single_operand (gdbarch, ppc_stap_is_single_operand); > set_gdbarch_stap_parse_special_token (gdbarch, > ppc_stap_parse_special_token); > + /* Linux DWARF register mapping is different from the other OSes. */ > + set_gdbarch_dwarf2_reg_to_regnum (gdbarch, > + rs6000_linux_dwarf2_reg_to_regnum); > + /* Note on Linux the mapping for the DWARF registers and the stab registers > + use the same numbers. Install rs6000_linux_dwarf2_reg_to_regnum for the > + stab register mappings as well. */ > + set_gdbarch_stab_reg_to_regnum (gdbarch, > + rs6000_linux_dwarf2_reg_to_regnum); > + dwarf2_frame_set_adjust_regnum (gdbarch, rs6000_linux_adjust_frame_regnum); > > if (tdep->wordsize == 4) > { > -- > 2.37.2