From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id +d/6NniC1WiKKRMAWB0awg (envelope-from ) for ; Thu, 25 Sep 2025 13:57:12 -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=AJjk5TmJ; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id DAE7A1E0BA; Thu, 25 Sep 2025 13:57:12 -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 471831E04C for ; Thu, 25 Sep 2025 13:57:12 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B338C3857BB9 for ; Thu, 25 Sep 2025 17:57:11 +0000 (GMT) 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 9E829385840E for ; Thu, 25 Sep 2025 17:56:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9E829385840E 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 9E829385840E 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=1758823005; cv=none; b=x2kUFeg3oc229oFkliip8mXh4ri4XflclnuQKLiuWmTknzlCptIJt9zp20b1Hzomuj6tkm3hFE9OmmYsBzBkBYnfRNesXIEpA2WBRTPP18wAt09Gq3g9QvwRgwo+bx/wCaRNcQ+4TJVCge8QHhdvve/V3P2m6cPBSkuFl1IMn3c= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1758823005; c=relaxed/simple; bh=QzX8jaJL1RMuMttMiviq1jrHu7R3L/kaCyoUerrstMo=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=sPMOUsn8ohuF6DIPVUg/I+SpKDRo0qH73/4jic89Y0e0MmpIFeaGMu68LO/0oKPS378l/132hLtIFxuikMBDan2ro5qo1vKnmdFTSKPp8jQWheJ4JSqyWSsO+RGKSrqz1AIJV6nS7UDQt8BzXFZ+eUJwYwYuzJDrgQHckcwpmm8= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9E829385840E 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=AJjk5TmJ DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1758823005; 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=pJsUubO/89xS6UzqyinOLnluCin488K/5UBBzRhDDpY=; b=AJjk5TmJwf/Nho4pdtXm0YSrmVMyY5DLZ3bc+qqRiGIFIaBNydNDAVbAH/V0LZAU7jVPE7 Jd41U7RVAhDVSw4qEJtZLNywNntn5m5YkxRDNNNUDeyybTe7hzPfXOJMhlVqaE80X3hY1f jT56F6tsNntlEEb2hzb4na2pMsOBmtA= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-336-1BOM2nM_PpKANIipcDNqWg-1; Thu, 25 Sep 2025 13:56:44 -0400 X-MC-Unique: 1BOM2nM_PpKANIipcDNqWg-1 X-Mimecast-MFC-AGG-ID: 1BOM2nM_PpKANIipcDNqWg_1758823003 Received: by mail-wr1-f69.google.com with SMTP id ffacd0b85a97d-3ee13baf21dso1171092f8f.0 for ; Thu, 25 Sep 2025 10:56:43 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1758823002; x=1759427802; 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=MNBJQ28lsWNmVgbVLlNgDbNSKlzzgWmVjcukC/ZNyuU=; b=WjuDRd2wmY09fligds/ykntOsTOywVBfgykCsjSyWp4bFcxs1xmUzFWiBLKsgAO8y6 xmMDtmi+EYqEOq1kpdQ3k/kyok9Pf3QkYL3HAA25rES+4LtIsclbW41g93fF/E7Vdkp0 ka1Gg8xuscYlSFmzyxVqnkbLwUMV9mSXoTksjLUKPdLOP3lIriwVnS4Z+KstfeSq6m/C mQdgfVR115DUG32kpIvmmHxhOBnFC04Mb00kMd1vGiiIkbPgsCIbDuFujd2+d9cXLAit 8vOb/FjQmxEG5ZsKeZKgtWnrLnlZ1PDcbdSwbYDnaVCtPO1FG46Q/mEaqF/GK7F41Vmk baEA== X-Gm-Message-State: AOJu0YwhjwuQF25dmmo/Yshwe2HyBR7EqSxpERN7E4SzdWY5OBwEkPWR zmUKIzmxlros+SqaGpwG5MBwHx/CDv6QZJCrF55gf/FKoNUIo2/Kx8SiW4rVnGSteuwiQ1tdRZ3 gJLc+PUKkIy9kwCZTMD+evoiJIl742lXASBL4HKNvzDatbKw1xAK5tjM5IEcr4VLu2pFAgfc= X-Gm-Gg: ASbGncvpniyeqcrxa+kF3gfjgTPBScZfUq6R5KmFPrQUQG9nzL2zoHSxtQw7C6/sSNS PX7KyFSEfXs3uc5tbkswaDWo6sQ/GG0sxOYzCMqSVOLVMVZCrvb5GwyTEMx8VHPO44dMN9Z1rsR JKtZ+BgnHhw9sxj5kLtaDRUAfRT8jLJhZtu8D2TgbrsjUyJfJP068Aceuey6D25P/EETmUP7CdX pW7iq6mRcmENwo8iFWS8GMZfIWWm/SdTQdS5BpQPS1bPwJC729rsf3e1HBE43xZlmsuO1Xn369W rCWc4lmVCnK2PpiA4muqoqtf55MajF4Byjs= X-Received: by 2002:a05:6000:3102:b0:405:3028:1bce with SMTP id ffacd0b85a97d-40e4886dea7mr4466738f8f.32.1758823002518; Thu, 25 Sep 2025 10:56:42 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEs5UZKnE8On0vONarwhxkMk1aJwhKs5BDffSZwbAOoQ6oRU+fLyyLxWX766ue1m8yyklSVTQ== X-Received: by 2002:a05:6000:3102:b0:405:3028:1bce with SMTP id ffacd0b85a97d-40e4886dea7mr4466721f8f.32.1758823002107; Thu, 25 Sep 2025 10:56:42 -0700 (PDT) Received: from localhost ([31.111.84.207]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-40fc6921bcfsm4034607f8f.43.2025.09.25.10.56.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Sep 2025 10:56:41 -0700 (PDT) From: Andrew Burgess To: =?utf-8?Q?S=C3=A9bastien?= Darche Cc: gdb-patches@sourceware.org, simark@simark.ca Subject: Re: [PATCH] gdb: ensure bp_location::section is set correct to avoid an assert In-Reply-To: <7febb0c1-7bbd-45d5-8ebe-91c34bb4a6ce@efficios.com> References: <7febb0c1-7bbd-45d5-8ebe-91c34bb4a6ce@efficios.com> Date: Thu, 25 Sep 2025 18:56:40 +0100 Message-ID: <87tt0qe7qf.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: hfIYlKfiNtGi-AeMNM1kIx0X04i5y2DPAgOxLnJhkFI_1758823003 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: > Hi Andrew, > > We've noticed a regression caused by this patch on one of the amdgpu > tests (namely gdb.rocm/displaced-stepping.exp). This test attempts > to insert a breakpoint at the beginning of a GPU kernel, step, then > proceed to the end of the test. It would appear the breakpoint is > inserted but not hit by the GPU. > > > Upon investigating why the breakpoint is not set the way it should be, > I've found that code_breakpoint::add_location relies on the section > being set to deduce the proper gdbarch, by calling get_sal_arch > (breakpoint.c:8615). If the sal section is not set, then the gdbarch is > set by default to the breakpoint gdbarch, which in our case is the host's > gdbarch. This causes problem down the road leading to the breakpoint not > being hit. > > > A quick fix to the problem would be to call find_pc_section() to assign > the proper section to the pc, *after* checking for find_pc_overlay : > > > diff --git a/gdb/linespec.c b/gdb/linespec.c > index e634bf22f3c..4a4d178085c 100644 > --- a/gdb/linespec.c > +++ b/gdb/linespec.c > @@ -4120,6 +4120,10 @@ minsym_found (struct linespec_state *self, struct= =20 > objfile *objfile, > =C2=A0 =C2=A0 =C2=A0 debugging, it should reflect the SAL's pc value.=C2= =A0 */ > =C2=A0 =C2=A0sal.section =3D find_pc_overlay (sal.pc); > > +=C2=A0 if (sal.section =3D=3D nullptr) { > +=C2=A0 =C2=A0 sal.section =3D find_pc_section (sal.pc); > +=C2=A0 } > + > =C2=A0 =C2=A0if (self->maybe_add_address (objfile->pspace (), sal.pc)) > =C2=A0 =C2=A0 =C2=A0add_sal_to_sals (self, result, &sal, msymbol->natura= l_name (),=20 > false); > =C2=A0} > > > This ensures that minsym_found() finds the proper pc (instead of the > resolver, for ifuncs, just like your patch intended), then assigns > the section corresponding to that pc. Ensuring that sal.section is > consistent to the pc is, in my opinion, a more appropriate solution > than leaving its contents empty for the time being. Modifying the > other places where the section is needed (or expected to be null) may > require some further investigation. > >> I think what we need to do is update minsym_found to just use >> find_pc_overlay, which is how the symtab_and_line::section is set in >> most other cases.=C2=A0 What this actually means in practise is that the >> section field will be set to NULL (see find_pc_overlay in symfile.c). >> But given that this is how the section is computed in most other >> cases, I don't see why it should be especially problematic for this >> case.=C2=A0 In reality, I think this just means that the section is >> calculated via a call to find_pc_section when it's needed, as an >> example, see lookup_minimal_symbol_by_pc_section (minsyms.c). >> >> I do wonder if we should be doing better when creating the >> symtab_and_line, and insist that the section be calculated correctly >> at that point, but I really don't want to open that can of worms right >> now, so I think just changing minsym_found to "do it just like >> everyone else" should be good enough. > > Admittedly, this means thinking about *when* sections are stored > in the sal. Let me know what you think about this problem and if > you think of another approach. First, I don't really object to the fix you propose. There's already code like this in GDB, see the find_pc_overlay call in blockframe.c. However. I think the real fix here would be to fold the find_pc_section call into find_pc_overlay, like: struct obj_section * find_pc_overlay (CORE_ADDR pc) { struct obj_section *best_match =3D NULL; =20 if (overlay_debugging) { ... existing code here ... } else best_match =3D find_pc_section (pc); =20 return best_match; } Note that the function names might not be ideal after this change, but they'll do for now. Almost all calls to find_pc_overlay are related to setting the section within a SAL, so I think we can be comfortable in those cases that this change is OK (I say that without testing this at all). Then there's a find_pc_overlay call in blockframe.c where we already have a find_pc_section call, so I think that case is fine. There's a call in find_pc_line, this one appears at a glance to be interesting as we'd now be passing a non NULL section to find_pc_sect_line in more cases, which, following the code through, would appear as if we might trigger some additional cases. And there's a call inside memory_xfer_partial_1, but this is already guarded by an overlay_debugging check, so in this case the change I propose would have no impact. I'd definitely be interested in seeing if the above change throws up any problems in testing. Part of the reason I'd push for a wider fix is that there are lots different linespec formats, and I don't think they all pass through minsym_found (or maybe they do?). If they don't then it feels like you should be able to adjust your original test such that minsym_found isn't called, and you'll have the same incorrect gdbarch problem. So then you'll have to add a find_pc_section in _another_ place.... Anyway, the above isn't a requirement, just my thoughts. Thanks, Andrew