From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id CXCgIx6y42hBiSEAWB0awg (envelope-from ) for ; Mon, 06 Oct 2025 08:12:14 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=C+/SBIFE; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 8D5161E0B6; Mon, 06 Oct 2025 08:12:14 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id C77891E047 for ; Mon, 06 Oct 2025 08:12:13 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 385F43858C98 for ; Mon, 6 Oct 2025 12:12:13 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 385F43858C98 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=C+/SBIFE Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 4EB543858D1E for ; Mon, 6 Oct 2025 12:11:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4EB543858D1E Authentication-Results: sourceware.org; dmarc=pass (p=quarantine 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 4EB543858D1E 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=1759752690; cv=none; b=xpdNykIA7y4J34FQCpOpzsYoRFIMpvCI9yYmLyxtjQnCeuhHPXiGmwinAHfQ1+xheHcnLa0c5/f5EMAr/rw60ZoUzwxOpqTuM0IV6WhWBNZGbvSkl+cWvKZAGhKO8Cw5GvNAFeGMtjO2Cin3/lfLoY/y+FLem+Y/cyeeUAwaLUU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1759752690; c=relaxed/simple; bh=+wa6hp1zYsNCgp6GOSGpVBxRaI8cXoCiI71vxnXgJ9o=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=Io8sqmP86r/ArE3rW7SMmm4/eXHKANrCE0GRVNxN9l53O3XKhM457zx8EHEzfT6yTONrQWZDn36hm/2FgRugbFmHHX8rsJ9e928W3e5BkrsfLu/lcrWDa4QJfl/pekbR2XVlyz+J9UTyTatY8nXgkjDcO7QPZBw1ZKzvaXppqBc= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4EB543858D1E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1759752690; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=yivjBH1Bs1CKv2pRBYMrQN2qKXqIaPFHkhYV0GQxcH4=; b=C+/SBIFE2iBAjPlfZoKnxcm31M/tWQSIPOqW25IxG4OD70ZMZWPFr7Lpf+ho5AdZ5UZKP6 kYDrwtapUnAJd14zyvrDKxaDEc8FGVitOlHlWqySqUHqtfYZdSNoFVivhH8fojxk8stqcO 6YxSMH7ABFNAT+sXtRXhLIm6MaIjb7A= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-80-rIkTLfO2OcWV0pe_LU0Ulw-1; Mon, 06 Oct 2025 08:11:28 -0400 X-MC-Unique: rIkTLfO2OcWV0pe_LU0Ulw-1 X-Mimecast-MFC-AGG-ID: rIkTLfO2OcWV0pe_LU0Ulw_1759752688 Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-46e35baddc1so25789005e9.2 for ; Mon, 06 Oct 2025 05:11:28 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1759752687; x=1760357487; h=content-transfer-encoding:mime-version:message-id:date:references :in-reply-to:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=5CBJ7n85r9WdkeVnHZXsbyF376VIcxtVyOF7ZxZOGy0=; b=bV5QXPUSRg7TlKYJT2cWhhPWF8UP1VQUljOzi0kr4XGsENjpWTxrt6yA8BUTkXZ23a +bTOk2s5tNIER3pJkiAz5Fp32EkN+6aD0sSeDwJ+bHWHGFnQJCa4W7H+RvZJbuMlvevF x6EqyZj5lbP7dhgY5FdJxeSq2PyL0THwFyeW1R+2OGKqbCqZbeICuPYxBK3ZtbSr+Vsa BJ47sRGPSTHj8m8s/uUVY/8JNFoSD8o3yvfCzzinnx3QarfxIlsD3sL3vyAStwlEouRI CIVhUtIZEtT+ZUvLoLvz78zmxppNdiDfGZYD67FxSG+SvjNufJ2x7BjC4fQOPX2nMDst lQJw== X-Gm-Message-State: AOJu0YzWInZ3KZXr1Cv3mMvHwwR+ii7HFRlakpgA6VPYpPWwtHz2HqP5 nYdkE1KOVgQM2KdpOI1CD2roA0EtFHNnVQXh5yXPUeE0byEX65Pzskn696TV+8+GuMifMi+gYV3 aS/PRdCavFxtvbRSODcI4nWeis/D8gt2EFGAAPxjv5SA3de71IkL60BGjIi0Vt/pvKQC/mCg= X-Gm-Gg: ASbGncutim0xsVlZbWqYAJES+qFAr7ok4B902O166bqhi7uE45phz9e6qopUf52w+iu u5nCWcRFqMIRes0bS0Fj48v3RU9Leo52rarlfrLTyIXmF4YG1+OG+kAeQr4nCr5RlLrlCcApfdI CXvyKn1/GzPQoVE/OxsR7t3gTwPQ0gU4uZ25NHpfHv4XMVs9QQpUiyTb0Mq8LtwvD6TFWc7BL8n tQEx2DodfuGoFtKYCTmK556Gtw4jQmql4fWTLHY52KUxaYLwLe/NpC/1fW9C3DLCJT6ME+i8mqd 3NhOGf2cvL+1hyDiOnpx79BU3cvchBzRzShaRIom X-Received: by 2002:a05:600c:3b0a:b0:456:1b6f:c888 with SMTP id 5b1f17b1804b1-46e71140bbemr72343605e9.23.1759752687149; Mon, 06 Oct 2025 05:11:27 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEY0pIyhh7/MxYVkozsTkpuly5FKq+YVnVFgSEk5IumSdE8LAMEGt54dwAbxwBNTCzLWtwemQ== X-Received: by 2002:a05:600c:3b0a:b0:456:1b6f:c888 with SMTP id 5b1f17b1804b1-46e71140bbemr72343375e9.23.1759752686579; Mon, 06 Oct 2025 05:11:26 -0700 (PDT) Received: from localhost ([31.111.84.207]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-46e61a0204fsm256693755e9.14.2025.10.06.05.11.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 Oct 2025 05:11:26 -0700 (PDT) From: Andrew Burgess To: =?utf-8?Q?S=C3=A9bastien?= Darche , simark@simark.ca Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] gdb: ensure bp_location::section is set correct to avoid an assert In-Reply-To: <6c31b667-db2d-453e-9597-9fe011c4766e@efficios.com> References: <7febb0c1-7bbd-45d5-8ebe-91c34bb4a6ce@efficios.com> <87tt0qe7qf.fsf@redhat.com> <87ldm2dxcl.fsf@redhat.com> <6c31b667-db2d-453e-9597-9fe011c4766e@efficios.com> Date: Mon, 06 Oct 2025 13:11:24 +0100 Message-ID: <87jz186xhv.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: Ntfy0C4LavshcXX9r6QuMnhqS50pJHSg0SNEirTCKzk_1759752688 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org S=C3=A9bastien Darche writes: > On 9/25/25 17:40, Andrew Burgess wrote: >> Maybe the answer is as simple as moving the .section assignment into the >> earlier if block, something like: >>=20 >> if (is_function && want_start_sal) >> { >> sal =3D find_function_start_sal (func_addr, NULL, self->funfirstl= ine); >>=20 >> /* This breakpoint is for the ifunc case, FUNC_ADDR is can be >> anywhere, in a completely different section to MSYMBOL, or eve= n >> in a different objfile! >>=20 >> TODO: I haven't checked, maybe find_function_start_sal already >> fills this stuff in for us? Or maybe it could be made too? >> For now I'm assuming all we have is an address, but this needs >> checking. */ >> sal.section =3D find_pc_overlay (func_addr); >> if (sal.section =3D=3D nullptr) >> sal.section =3D find_pc_section (func_addr); >> } >> else >> { >> sal.objfile =3D objfile; >> sal.msymbol =3D msymbol; >> /* Store func_addr, not the minsym's address in case this was an >> =09 ifunc that hasn't been resolved yet. */ >> if (is_function) >> =09sal.pc =3D func_addr; >> else >> =09sal.pc =3D msymbol->value_address (objfile); >> sal.pspace =3D current_program_space; >>=20 >> /* We can assign the section based on MSYMBOL here because the >> breakpoint is actually being placed at (or near) MSYMBOL. */ >> sal.section =3D msymbol->obj_section (objfile); >> } >>=20 > > To answer your question on whether find_function_start_sal does fill=20 > this for us : it depends. It manages to do it on amd64 but not on amdgpu. > > By default, the sal does not contain a valid section. It's only when we= =20 > try to adjust the pc past the prologue (skip_prologue_sal) that a=20 > section is computed for the pc at the start of the function. If we do=20 > have a prologue, then we assign that section (symtab.c:3914). If not=20 > (and that is the case on amdgpu), then we're left with an empty=20 > sal.section. I would say the behavior is not really consistent. > > I would agree it could be made to. >> Does this look like a valid path forward maybe? > > Your solution seems to work for the gnu-ifunc test and fixes the=20 > regression for gdb.rocm/displaced-stepping.exp - so I'd say it's a good= =20 > aproach. I am not familiar with overlays, so I can't really judge if the= =20 > change would impact how they are handled. > > I think it would be best to ensure find_function_start_sal has a=20 > consistent behavior across architectures. I'll submit a small patch=20 > which should address this. This would also at least reduce the chance=20 > for another bug like this to appear somewhere else : > Hi, I'm proposing the patch below. You should double check that this still addresses the issue you're seeing with the amdgpu target. Given Simon's concerns, I do wonder if there might still be some issues with this related to overlay debugging, but without any way to test it, and no known overlay users, I think we can probably just ignore that for now. If this fixes your regression, then maybe we should merge this, and figure any other issues out later? Thanks, Andrew --- commit 6ffea587445eeacf8b2962de6d3b00d6efa98213 Author: Andrew Burgess Date: Mon Oct 6 10:27:08 2025 +0100 gdb: fixes for setting the section in minsym_found =20 After this commit: =20 commit 6f7ad2381ae72aa592ada4a0921265aa3292b1fa Date: Wed Sep 3 19:57:42 2025 +0100 =20 gdb: ensure bp_location::section is set correct to avoid an asser= t =20 Some issues were reported as a result of the bp_location::section being left as NULL by the call to find_pc_overlay that was introduced. See this thread: =20 https://inbox.sourceware.org/gdb-patches/7febb0c1-7bbd-45d5-8ebe-91c3= 4bb4a6ce@efficios.com =20 The problem was that code_breakpoint::add_location relies on the section being set in order to deduce the gdbarch. If the section is not set then the gdbarch is deduced using the breakpoint's gdbarch. =20 The bug was reported by the ROCm engineers, who have inferiors running mixed host and GPU code, and so rely on the section being set in order to establish the correct architecture for a specific address. =20 During discussion in the above thread Simon pointed out that the change made in the above commit might not be correct anyway for overlay debugging (does that even work, or is it used any more?), as the commit relies on establishing a section by calling find_pc_overlay. However, when presented with multiple possible sections, find_pc_overlay cannot know which section to select, and so just picks one. This could be different from the section of the minimal_symbol we already had to hand. =20 This patch I think should (at least) resolve the issues the ROCm engineers are seeing. =20 Instead of always calling find_pc_overlay I have moved the section assignment inside the if/then/else blocks with the following reasoning. =20 In the 'else' block, this is the non-function or non-ifunc case, the address used is based on the msymbol's address, and so should be in the same section. In this case we can use the msymbol's section. =20 In the 'if' block things are more complicated. This could be the ifunc case, in which case func_addr could have been adjusted to a different section, or even different objfile. =20 Further, when we call find_function_start_sal, we pass in just an address, so the SAL being returned isn't going to consider which overlay section the original msymbol was from, which could cause problems for overlay debugging maybe? =20 Anyway, I'm ignoring that for now, as fixing that would be a whole big thing. So I'm proposing that, if find_function_start_sal returns a symtab_and_line with a section set, then we use that section. Otherwise, we can try to figure out a section. diff --git a/gdb/linespec.c b/gdb/linespec.c index 2ddc495babf..4d9c5ac26f3 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -4083,13 +4083,16 @@ minsym_found (struct linespec_state *self, struct o= bjfile *objfile, =20 CORE_ADDR func_addr; bool is_function =3D msymbol_is_function (objfile, msymbol, &func_addr); + bool is_ifunc =3D false; =20 if (is_function) { const char *msym_name =3D msymbol->linkage_name (); =20 - if (msymbol->type () =3D=3D mst_text_gnu_ifunc -=09 || msymbol->type () =3D=3D mst_data_gnu_ifunc) + is_ifunc =3D (msymbol->type () =3D=3D mst_text_gnu_ifunc +=09=09 || msymbol->type () =3D=3D mst_data_gnu_ifunc); + + if (is_ifunc) =09want_start_sal =3D gnu_ifunc_resolve_name (msym_name, &func_addr); else =09want_start_sal =3D true; @@ -4098,7 +4101,32 @@ minsym_found (struct linespec_state *self, struct ob= jfile *objfile, symtab_and_line sal; =20 if (is_function && want_start_sal) - sal =3D find_function_start_sal (func_addr, NULL, self->funfirstline); + { + sal =3D find_function_start_sal (func_addr, NULL, self->funfirstline= ); + + /* If SAL already has a section then we'll use that. If not, then w= e +=09 can try to find a section. + +=09 In the ifunc case though we cannot rely on the section of MSYMBOL, +=09 the ifunc target could be in a different section, or even a +=09 different objfile, from the original MSYMBOL. For this case, we +=09 fall back to looking up a section based on FUNC_ADDR. + +=09 For the non-ifunc case, we can use the section of MSYMBOL, as +=09 that's how we filled in FUNC_ADDR, so they should be in the same +=09 section. */ + if (sal.section =3D=3D nullptr) +=09{ +=09 if (!is_ifunc) +=09 sal.section =3D msymbol->obj_section (objfile); +=09 else +=09 { +=09 sal.section =3D find_pc_overlay (func_addr); +=09 if (sal.section =3D=3D nullptr) +=09=09sal.section =3D find_pc_section (func_addr); +=09 } +=09} + } else { sal.objfile =3D objfile; @@ -4110,14 +4138,13 @@ minsym_found (struct linespec_state *self, struct o= bjfile *objfile, else =09sal.pc =3D msymbol->value_address (objfile); sal.pspace =3D current_program_space; - } =20 - /* Don't use the section from the msymbol, the code above might have - adjusted FUNC_ADDR, in which case the msymbol's section might not be - the section containing FUNC_ADDR. It might not even be in the same - objfile. As the section is primarily to assist with overlay - debugging, it should reflect the SAL's pc value. */ - sal.section =3D find_pc_overlay (sal.pc); + /* We can assign the section based on MSYMBOL here because the +=09 breakpoint is actually being placed at (or near) MSYMBOL. Note, +=09 this is not a path where ifunc resolution can have occurred, which +=09 could adjust FUNC_ADDR significantly. */ + sal.section =3D msymbol->obj_section (objfile); + } =20 if (self->maybe_add_address (objfile->pspace (), sal.pc)) add_sal_to_sals (self, result, &sal, msymbol->natural_name (), false);