From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 8A8D7383D01B for ; Wed, 7 Jul 2021 13:24:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8A8D7383D01B Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 167DNxPL016684 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 7 Jul 2021 09:24:03 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 167DNxPL016684 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id B90751E01F; Wed, 7 Jul 2021 09:23:58 -0400 (EDT) Subject: Re: [PATCH master + 11] gdb: don't set Linux-specific displaced stepping methods in s390_gdbarch_init To: Simon Marchi , gdb-patches@sourceware.org, Philipp Rudo References: <20210707125736.2829336-1-simon.marchi@polymtl.ca> From: Simon Marchi Message-ID: Date: Wed, 7 Jul 2021 09:23:58 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <20210707125736.2829336-1-simon.marchi@polymtl.ca> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Wed, 7 Jul 2021 13:23:59 +0000 X-Spam-Status: No, score=-4.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Jul 2021 13:24:07 -0000 On 2021-07-07 8:57 a.m., Simon Marchi wrote: > 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. Hi Philipp, Since you did the s390-tdep.c / s390-linux-tdep.c split, could you please take a look at this patch, see if it looks correct to you? Thanks! Simon