From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id 7777C3858D33 for ; Mon, 6 Nov 2023 18:25:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7777C3858D33 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linux.ibm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 7777C3858D33 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=148.163.158.5 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699295108; cv=none; b=TWHeyvlkheXvPZ3BSdI90p/XkXaKri2g4SRHnoLsut5/a3DNCRa9tBU79LVeghYyN+GADxKTkoAa8ms0l+39M2YxbAI1xPF5cs4TWFDgHe15b1I5IHzOz9lPAm0aJYPMCYqiPj+TTEWyy2WAw+UNi1yutQJeAP4a08V76RZ4mhI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699295108; c=relaxed/simple; bh=V1JAN26r4JkaQf/jjTrkOo3QSKJ795HtqWzcBSAxP5I=; h=DKIM-Signature:Message-ID:Subject:From:To:Date:Mime-Version; b=Se0MzrSeFjY5vLqI2MlTXN00vuILGAf/Qv22MsXoGlHm9CTzCuQOQ3d+k9aXrZQud0/JUyJ0Gj0aAEFOn82cEBlUhHK/1vANCT6PWMJp6yXzBAR3sl1n4FA6n2V7Hmvqcyfj3fd9ZAbcrJ2G+tUHhpKSmtN/GX0s3XZDeZYXMYE= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from pps.filterd (m0353722.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3A6Hhogh002327 for ; Mon, 6 Nov 2023 18:25:06 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : subject : from : to : cc : date : in-reply-to : references : content-type : mime-version : content-transfer-encoding; s=pp1; bh=1/n9IxsnCRI3+a9KrJ+yXcx6nDG4pKIP7SNH/YM8J9E=; b=bG3fwEGVrePxp7jaNr1WyWtbzPOqSxlo4lhGnd4wsFqHsR5WoGEqLyiFj7TrfWG+T++6 aAi0l01NGTfuQ4jG+ujjqIuc85VPaccaywo1HtPrutKZzrjb3HFvsNMT7Cp7h1qoQaRA tFa0ZFrOoP5ShhjjSLsOHBEv2mXCDocbF9QkZdMGNRACxANdJmFqfLowU7U5RDvQ/n3x vrNFwDH1a6gd4OVYc7VYa1XMO7oqJSv6EBGHAmY7dF3q0llGQ6844HUA1CaRtK94RSZ/ jynNqdMUSjwXI9QdoKIKmAyfg1AlWK0M7QPQrmGKEHRFQUR6O9zll4xtJ3NkF9H+psnP sA== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3u73dwkuwn-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Mon, 06 Nov 2023 18:25:05 +0000 Received: from m0353722.ppops.net (m0353722.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 3A6HwD08014482 for ; Mon, 6 Nov 2023 18:25:05 GMT Received: from ppma21.wdc07v.mail.ibm.com (5b.69.3da9.ip4.static.sl-reverse.com [169.61.105.91]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3u73dwkuwb-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 06 Nov 2023 18:25:05 +0000 Received: from pps.filterd (ppma21.wdc07v.mail.ibm.com [127.0.0.1]) by ppma21.wdc07v.mail.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 3A6GeTwe025608; Mon, 6 Nov 2023 18:25:04 GMT Received: from smtprelay03.wdc07v.mail.ibm.com ([172.16.1.70]) by ppma21.wdc07v.mail.ibm.com (PPS) with ESMTPS id 3u619nba18-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 06 Nov 2023 18:25:04 +0000 Received: from smtpav01.wdc07v.mail.ibm.com (smtpav01.wdc07v.mail.ibm.com [10.39.53.228]) by smtprelay03.wdc07v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 3A6IOxU357999742 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 6 Nov 2023 18:24:59 GMT Received: from smtpav01.wdc07v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 7FA535804B; Mon, 6 Nov 2023 18:24:59 +0000 (GMT) Received: from smtpav01.wdc07v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 7DCD058055; Mon, 6 Nov 2023 18:24:58 +0000 (GMT) Received: from wecm-9-67-110-146.wecm.ibm.com (unknown [9.67.110.146]) by smtpav01.wdc07v.mail.ibm.com (Postfix) with ESMTP; Mon, 6 Nov 2023 18:24:58 +0000 (GMT) Message-ID: <6f9791da2a19d4d89064e045581b2a08b68dad83.camel@linux.ibm.com> Subject: Re: [PATCH 1/2, ver3] PowerPC, Fix-test-gdb.base-store.exp From: Carl Love To: Ulrich Weigand , "gdb-patches@sourceware.org" , Andrew Burgess , Keith Seitz Cc: cel@us.ibm.com Date: Mon, 06 Nov 2023 10:24:57 -0800 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> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5 (3.28.5-22.el8) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: x2Zdw87_SSAON54Hhue8BpXfWYsZpLIa X-Proofpoint-GUID: 15Ckv-xVJbwCPTwHEMFYrya4SydRBdWI X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.987,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-11-06_14,2023-11-02_03,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxlogscore=930 priorityscore=1501 suspectscore=0 spamscore=0 clxscore=1015 mlxscore=0 lowpriorityscore=0 bulkscore=0 impostorscore=0 malwarescore=0 adultscore=0 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2310240000 definitions=main-2311060150 X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,GIT_PATCH_0,KAM_ASCII_DIVIDERS,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_PASS,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: Ping Andrew: Is the updated commit log OK? Carl ----------------------------------------- On Mon, 2023-10-30 at 10:25 -0700, Carl Love wrote: > 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. > --- > 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) > {