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 2EDE83858CDA for ; Tue, 24 Oct 2023 09:30:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2EDE83858CDA 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 2EDE83858CDA 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=1698139842; cv=none; b=Ojt6AeBkm8VkG65IXZ1oUS9X9veKsA3zuXJSyHLJM1nHttaoPbkAEfP6KcZueKFxToVqvZPKjOHusjD8GIrh0ftCZFD398GMG/t+WByoVs9LkgyFY/Tjz/M3q+xPMNZMVw7Xkhj6pWDrjZEcrU6lmyOGY/Z8NoYl0hHOzWrr6I4= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698139842; c=relaxed/simple; bh=lCvim297wfBdnIDPuczUQOwx1mtIKH7VX47uzKyq3Aw=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=ny4/lofXDYQifdwYS9Nt0D3T7xsCjPt+3rlt5WH7m342eYsHF/Hs7sdaUWgGdN4uDDZKHUKFWzj5He+QWD0ExGaDhXuUZVwG0rtftrtKwCf5zcexf1PIdZESpIucXFbmOITBzCjqlz/PNXAWrewXKjP7WD0DtzRdJb8NbR8AmoU= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1698139839; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=Ka2Moeq//YQH9Y04wSW2fxbVlsVg1R25Mm8x7zjevyc=; b=dA6eocC8axoRhr3tH41PkXF1zBXY6UdVj9cbETrwqoyM4+0CkDDJ/MAx5zmH/1xzOHpcEP 9JK7KacLvHuH1Dhb/DNFimxIVdG2/SXozVId83G+sOGS/teOv7xjg/1C6aUMsc1UAVw5mm MYuJu8PoIyGA4S23NwCmmyQeNvfLoOE= Received: from mail-lf1-f69.google.com (mail-lf1-f69.google.com [209.85.167.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-230-EkZYLiEUMJy2wsOG89q2PA-1; Tue, 24 Oct 2023 05:30:38 -0400 X-MC-Unique: EkZYLiEUMJy2wsOG89q2PA-1 Received: by mail-lf1-f69.google.com with SMTP id 2adb3069b0e04-507cc15323aso4255393e87.0 for ; Tue, 24 Oct 2023 02:30:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698139837; x=1698744637; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Ka2Moeq//YQH9Y04wSW2fxbVlsVg1R25Mm8x7zjevyc=; b=aJgzsEVQcaeRtyp261nR8JmxWENq4l9AbJDy7LDwdyjeF9uj5hEWnE5nXNZ2xDVtsD rvfdpwellB9T/+KMwLdVYyBAKhPO5ZoJOAqprWjkfPyyOBimezkzknKncw2jbr1+Rd2P lVsDIqOt1Sv7xARF/wM+BgYE+fXfxI6qXcRllaa/nBTPw+vTcWhX0AyucywEU3+hYZCF GEnxBxDEkb9tblZpPsQQ0CxYSB3T3V+3gK4RCa6Qud9XCB6+ZCQ27CvwZTKECC/2HRYd Bht/k+dDQW6X7e/Bgr8JlQxZU27hZKWfO9eawJbk58GJOKkgPCHk1u6zmv/ltZsyln37 4G+w== X-Gm-Message-State: AOJu0YxaRR1Zmat2BLY4cwjvwQ0zsfBbJHxmw3qhXFlEv/CqhE7scfM5 I8vLtAUpgRmmjWzh55G3Rcwe9PusUK4Wzj57GQqVFpKv9gD2RduRHS8o5DP8LQY4vG/kevASEu6 EVLGGMBwFRB+yGkhf5xF1jA== X-Received: by 2002:a19:5051:0:b0:507:b084:d6bb with SMTP id z17-20020a195051000000b00507b084d6bbmr7658249lfj.43.1698139836672; Tue, 24 Oct 2023 02:30:36 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGW/dSYM78IOX9dmChjQCGsQtPbHaIUU7OPBJ6XHPkOS5dY2zLum/Zq4R0qMAFFm27PhssV8g== X-Received: by 2002:a19:5051:0:b0:507:b084:d6bb with SMTP id z17-20020a195051000000b00507b084d6bbmr7658231lfj.43.1698139836251; Tue, 24 Oct 2023 02:30:36 -0700 (PDT) Received: from localhost ([31.111.84.209]) by smtp.gmail.com with ESMTPSA id n25-20020a509359000000b00530bc7cf377sm7730291eda.12.2023.10.24.02.30.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 24 Oct 2023 02:30:35 -0700 (PDT) From: Andrew Burgess To: Carl Love , Carl Love , gdb-patches@sourceware.org, UlrichWeigand , keiths@redhat.com Subject: Re: [PATCH 1/2, ver2] PowerPC, Fix-test-gdb.base-store.exp In-Reply-To: <8735bded66303c8cdfacfad9bd953c582e8076f2.camel@linux.ibm.com> References: <6f9c8fa20962129048384d74f6f15d1b37d255ed.camel@us.ibm.com> <76b8ed7b93608d40ab42b0538319f78eaf7d621c.camel@us.ibm.com> <87bkcyhc5g.fsf@redhat.com> <8735bded66303c8cdfacfad9bd953c582e8076f2.camel@linux.ibm.com> Date: Tue, 24 Oct 2023 10:30:34 +0100 Message-ID: <87sf602wqt.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 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: > GDB maintainers: > > Ver2, added a missing blank line, removed an extra blank line, fixed > spelling mistakes. Removed comment in ChangeLog about fixing py- > thread-exited.exp test errors. Patch just fixes issues with the > store.exp. Did not change the comment: > >> + /* FIXME: jimb/2004-05-05: What should we do when the debug info >> + specifies registers the architecture doesn't have? Our >> + callers don't check the value we return. */ > > Will create a new patch to address the comment in all three places in > the source code. > > Note, fixing the register mapping fixes two of the five test failures > for the store.exp test. > > Patch was retested on Power10. > > This is the first patch in the series which fixes the DWWARF register > mapping and signal handling issues on PowerPC. > > Carl > > > ------------------------------------------------- > Fix Linux DWARF register mapping > > 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. > > The second issue fixed by this patch is the check for subroutine > process_event_stop_test. Need to make sure the frame is not the > SIGTRAMP_FRAME. The sequence of events on most platforms is: > > 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 "nexti" to step into, not over, signal handlers invoked > by the kernel. This is the case 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. > > 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. > > Patch has been tested on Power 8 LE/BE, Power 9 LE/BE, Power 10 with no > new regressions. Hi Carl, Sorry for being such a pain w.r.t. this patch. Honestly, it's mostly because you touched infrun.c that I'm so interested here. I left some notes here that need addressing: https://inbox.sourceware.org/gdb-patches/6f9c8fa20962129048384d74f6f15d1b37d255ed.camel@us.ibm.com/T/#m83b4f0d7da45f15c2df44344fbf3e326cf7435e3 But I wonder if you could be more specific about when you would expect to see the failures in gdb.base/store.exp. I grabbed a random Power9 Linux machine and tried running store.exp with an unpatched GDB, and see no failures. I guess there's a specific architecture/compiler combo that I need in order to see failures -- but if those details are in the above text, I'm just not seeing them. Could you give some more details about the setup needed to see failures on store.exp. And I'd like to see clearer details about which tests the infrun.c changes will fix. Thanks, Andrew > --- > gdb/infrun.c | 13 ++++++++++ > gdb/ppc-linux-tdep.c | 56 ++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 69 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..dc03430e2af 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,52 @@ 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) > + /* FIXME: jimb/2004-05-05: What should we do when the debug info > + specifies registers the architecture doesn't have? Our > + callers don't check the value we return. */ > + 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 +2182,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