From 74b10a3219e44ba2585e3f7226a6455d41e92c1b Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 7 Jul 2021 08:57:36 -0400 Subject: [PATCH] gdb: don't set Linux-specific displaced stepping methods in s390_gdbarch_init According to bug 28056, running an s390x binary gives: (gdb) run Starting program: /usr/bin/ls /home/ubuntu/tmp/gdb-11.0.90.20210705/gdb/linux-tdep.c:2550: internal-error: displaced_step_prepare_status linux_displaced_step_prepare(gdbarch*, thread_info*, CORE_ADDR&): Assertion `gdbarch_data->num_disp_step_buffers > 0' failed. This is because the s390 architecture registers some Linux-specific displaced stepping callbacks in the OS-agnostic s390_gdbarch_init: set_gdbarch_displaced_step_prepare (gdbarch, linux_displaced_step_prepare); set_gdbarch_displaced_step_finish (gdbarch, linux_displaced_step_finish); set_gdbarch_displaced_step_restore_all_in_ptid (gdbarch, linux_displaced_step_restore_all_in_ptid); But then the Linux-specific s390_linux_init_abi_any passes num_disp_step_buffers=0 to linux_init_abi: linux_init_abi (info, gdbarch, 0); The problem happens when linux_displaced_step_prepare is called for the first time. It tries to allocate the displaced stepping buffers, but sees that the number of displaced stepping buffers for that architecture is 0, which is unexpected / invalid. s390_gdbarch_init should not register the linux_* callbacks, that is expected to be done by linux_init_abi. If debugging a bare-metal s390 program, or an s390 program on another OS GDB doesn't know about, we wouldn't want to use them. We would either register no callbacks, if displaced stepping isn't supported, or register a different set of callbacks if we wanted to support displaced stepping in those cases. The commit that refactored the displaced stepping machinery and introduced these set_gdbarch_displaced_step_* calls is 187b041e2514 ("gdb: move displaced stepping logic to gdbarch, allow starting concurrent displaced steps"). However, even before that, s390_gdbarch_init did: set_gdbarch_displaced_step_location (gdbarch, linux_displaced_step_location); ... which already seemed wrong. The Linux-specific callback was used even for non-Linux system. Maybe that was on purpose, because it would also happen to work in some other non-Linux case, or maybe it was simply a mistake. I'll assume that this was a small mistake when s390-tdep.{h,c} where factored out of s390-linux-tdep.c, in d6e589456475 ("s390: Split up s390-linux-tdep.c into two files"). Fix this by removing the setting of these displaced step callbacks from s390_gdbarch_init. Instead, pass num_disp_step_buffers=1 to linux_init_abi, in s390_linux_init_abi_any. Doing so will cause linux_init_abi to register these same callbacks. It will also mean that when debugging a bare-metal s390 executable or an executable on another OS that GDB doesn't know about, gdbarch_displaced_step_prepare won't be set, so displaced stepping won't be used. This patch will need to be merged in the gdb-11-branch, since this is a GDB 11 regression, so here's the ChangeLog entry: gdb/ChangeLog: * s390-linux-tdep.c (s390_linux_init_abi_any): Pass 1 (number of displaced stepping buffers to linux_init_abi. * s390-tdep.c (s390_gdbarch_init): Don't set the Linux-specific displaced-stepping gdbarch callbacks. Change-Id: Ieab2f8990c78fde845ce7378d6fd4ee2833800d5 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28056 --- gdb/s390-linux-tdep.c | 2 +- gdb/s390-tdep.c | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c index 04a4ff8df34..1f40e10e3d3 100644 --- a/gdb/s390-linux-tdep.c +++ b/gdb/s390-linux-tdep.c @@ -1120,7 +1120,7 @@ s390_linux_init_abi_any (struct gdbarch_info info, struct gdbarch *gdbarch) tdep->s390_syscall_record = s390_linux_syscall_record; - linux_init_abi (info, gdbarch, 0); + linux_init_abi (info, gdbarch, 1); /* Register handling. */ set_gdbarch_core_read_description (gdbarch, s390_core_read_description); diff --git a/gdb/s390-tdep.c b/gdb/s390-tdep.c index 2579ee82b20..d3bb2bacbb4 100644 --- a/gdb/s390-tdep.c +++ b/gdb/s390-tdep.c @@ -7050,10 +7050,6 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) set_gdbarch_displaced_step_copy_insn (gdbarch, s390_displaced_step_copy_insn); set_gdbarch_displaced_step_fixup (gdbarch, s390_displaced_step_fixup); - set_gdbarch_displaced_step_prepare (gdbarch, linux_displaced_step_prepare); - set_gdbarch_displaced_step_finish (gdbarch, linux_displaced_step_finish); - set_gdbarch_displaced_step_restore_all_in_ptid - (gdbarch, linux_displaced_step_restore_all_in_ptid); set_gdbarch_displaced_step_hw_singlestep (gdbarch, s390_displaced_step_hw_singlestep); set_gdbarch_software_single_step (gdbarch, s390_software_single_step); set_gdbarch_max_insn_length (gdbarch, S390_MAX_INSTR_SIZE); -- 2.30.2