From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1983) id E8DC1395443F; Fri, 6 May 2022 17:46:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E8DC1395443F Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Carl Love To: gdb-cvs@sourceware.org Subject: [binutils-gdb] PowerPC: bp-permanent.exp, kill-after-signal fix X-Act-Checkin: binutils-gdb X-Git-Author: Carl Love X-Git-Refname: refs/heads/master X-Git-Oldrev: 0ee8858e7aeca5ba5f702204daad2ddd290ef229 X-Git-Newrev: dd9cd55e990bcc9f8448cac38d242d53974b3604 Message-Id: <20220506174634.E8DC1395443F@sourceware.org> Date: Fri, 6 May 2022 17:46:34 +0000 (GMT) X-BeenThere: gdb-cvs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 May 2022 17:46:35 -0000 https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3Ddd9cd55e990b= cc9f8448cac38d242d53974b3604 commit dd9cd55e990bcc9f8448cac38d242d53974b3604 Author: Carl Love Date: Fri May 6 17:45:58 2022 +0000 PowerPC: bp-permanent.exp, kill-after-signal fix =20 The break point after the stepi on Intel is the entry point of the user signal handler function test_signal_handler. The code at the break poi= nt looks like: =20 0x : endbr64 =20 On PowerPC with a Linux 5.9 kernel or latter, the address where gdb sto= ps after the stepi is in the vdso code inserted by the kernel. The code a= t the breakpoint looks like: =20 0x <__kernel_start_sigtramp_rt64>: bctrl =20 This is different from other architectures. As discussed below, recent kernel changes involving the vdso for PowerPC have been made changes to= the signal handler code flow. PowerPC is now stopping in function __kernel_start_sigtramp_rt64. PowerPC now requires an additional stepi= to reach the user signal handler unlike other architectures. =20 The bp-permanent.exp and kill-after-signal tests run fine on PowerPC wi= th an kernel that is older than Linux 5.9. =20 The PowerPC 64 signal handler was updated by the Linux kernel 5.9-rc1: =20 commit id: 0138ba5783ae0dcc799ad401a1e8ac8333790df9 powerpc/64/signal: Balance return predictor stack in signal trampol= ine =20 An additional change to the PowerPC 64 signal handler was made in Linux kernel version 5.11-rc7 : =20 commit id: 24321ac668e452a4942598533d267805f291fdc9 powerpc/64/signal: Fix regression in __kernel_sigtramp_rt64() sema= ntics =20 The first kernel change, puts code into the user space signal handler (= in the vdso) as a performance optimization to prevent the call/return stack from getting out of balance. The patch ensures that the entire user/kernel/vdso cycle is balanced with the addition of the "brctl" instruction. =20 The second patch, fixes the semantics of __kernel_sigtramp_rt64(). A n= ew symbol is introduced to serve as the jump target from the kernel to the trampoline which now consists of two parts. =20 The above changes for PowerPC signal handler, causes gdb to stop in the kernel code not the user signal handler as expected. The kernel dispat= ches to the vdso code which in turn calls into the signal handler. PowerPC = is special in that the kernel is using a vdso instruction (bctrl) to enter= the signal handler. =20 I do not have access to a system with the first patch but not the secon= d. I did test on Power 9 with the Linux 5.15.0-27-generic kernel. Both tests fa= il on this Power 9 system. The two tests also fail on Power 10 with the Linux 5.14.0-70.9.1.el9_0.ppc64le kernel. =20 The following patch fixes the issue by checking if gdb stopped at "sign= al handler called". If gdb stopped there, the tests verifies gdb is in th= e kernel function __kernel_start_sigtramp_rt64 then does an additional stepi to = reach the user signal handler. With the patch below, the tests run without error= s on both the Power 9 and Power 10 systems with out any failures. Diff: --- gdb/testsuite/gdb.base/bp-permanent.exp | 25 +++++++++++++++++++++ gdb/testsuite/gdb.base/kill-after-signal.exp | 33 ++++++++++++++++++++++++= +++- 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/gdb/testsuite/gdb.base/bp-permanent.exp b/gdb/testsuite/gdb.ba= se/bp-permanent.exp index 8be46e96238..21b0bc7bb2d 100644 --- a/gdb/testsuite/gdb.base/bp-permanent.exp +++ b/gdb/testsuite/gdb.base/bp-permanent.exp @@ -260,6 +260,31 @@ proc test {always_inserted sw_watchpoint} { -re "Program received signal SIGTRAP.*$gdb_prompt $" { fail $test } + -re ".*signal handler called.*$gdb_prompt $" { + # PowerPC Linux kernel patchs: + # commit: 0138ba5783ae0dcc799ad401a1e8ac8333790df9 + # powerpc/64/signal: Balance return predictor + # stack in signal trampoline. + # + # The kernel places an additional brctl instruction + # in the vdso to call the user hadler. + # + # commit 24321ac668e452a4942598533d267805f291fdc9 + # powerpc/64/signal: Fix regression in + # __kernel_sigtramp_rt64() semantics + # + # Updates the semantics of __kernel_sigtramp_rt64(). + # It adds a new symbol to serve as a jump target from + # the kernel to the trampoline. + # + # The net result of these changes is that gdb stops + # at __kernel_start_sigtramp_rt64. Need to do one + # more stepi to reach the expected location in the user + # signal handler. + gdb_test "p \$pc" ".*__kernel_start_sigtramp_rt64.*" \ + "in kernel code" + gdb_test "stepi" "handler .*" $test + } -re "handler .*$gdb_prompt $" { pass $test } diff --git a/gdb/testsuite/gdb.base/kill-after-signal.exp b/gdb/testsuite/g= db.base/kill-after-signal.exp index 09f5cbc39a6..fcbec9a1c2e 100644 --- a/gdb/testsuite/gdb.base/kill-after-signal.exp +++ b/gdb/testsuite/gdb.base/kill-after-signal.exp @@ -36,7 +36,38 @@ if ![runto_main] { } =20 gdb_test "continue" "Program received signal SIGUSR1, .*" -gdb_test "stepi" "\r\nhandler .*" + +set test "handler" +gdb_test_multiple "stepi" $test { + -re "\r\nhandler .*" { + pass $test + } + -re ".*signal handler called.*$gdb_prompt $" { + # PowerPC Linux kernel patchs: + # commit: 0138ba5783ae0dcc799ad401a1e8ac8333790df9 + # powerpc/64/signal: Balance return predictor + # stack in signal trampoline. + # + # The kernel places an additional brctl instruction + # in the vdso to call the user hadler. + # + # commit 24321ac668e452a4942598533d267805f291fdc9 + # powerpc/64/signal: Fix regression in + # __kernel_sigtramp_rt64() semantics + # + # Updates the semantics of __kernel_sigtramp_rt64(). + # It adds a new symbol to serve as a jump target from + # the kernel to the trampoline. + # + # The net result of these changes is that gdb stops + # at __kernel_start_sigtramp_rt64. Need to do one + # more stepi to reach the expected location in the user + # signal handler. + gdb_test "p \$pc" ".*__kernel_start_sigtramp_rt64.*" "in kernel code" + gdb_test "stepi" "\r\nhandler .*" $test + } +} + gdb_test_multiple "kill" "kill" { -re "Kill the program being debugged\\? \\(y or n\\) $" { gdb_test "y" "\\\[Inferior $decimal \\(.*\\) killed\\\]" "kill"