From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id VcO0FE96OGlFTikAWB0awg (envelope-from ) for ; Tue, 09 Dec 2025 14:36:47 -0500 Received: by simark.ca (Postfix, from userid 112) id 4F3901E0B3; Tue, 09 Dec 2025 14:36:47 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.3 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, 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 vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (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 A7F171E08D for ; Tue, 09 Dec 2025 14:36:46 -0500 (EST) Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id 380714BA23E3 for ; Tue, 9 Dec 2025 19:36:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 380714BA23E3 Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 7F8254BA540C for ; Tue, 9 Dec 2025 19:36:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7F8254BA540C Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=efficios.com Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=efficios.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 7F8254BA540C Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=158.69.221.121 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1765308975; cv=none; b=W6jheIV8aYEOq9IGkRjWpQZzI1hsN+9fM6Bjk4kzQek/28uq+DYTzwZyrVgRyXp97d0OFj1zcrbZEEQHpuLQbFwW0gNnYM5CvynMNdScwHbX95WCM4+ziMgR2XSpFl04H2KmPyHPsAK5x3VWx6ldZfWKb+FTHtgCflTHwj7SOv8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1765308975; c=relaxed/simple; bh=2kAhNYQpZp3xEn7QnUZbWu0aWJ1iMJZmrTwZJaFNyxU=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=T3ZBoriTLbv8MqxFGXwFrr5C33N8OXNR5reqrHMlRB6FoP2+FMvBAQmT862WKdIDo1oYvZs33WgyxNyzi6tej1GXnMhv+Hf6/DflgSrYgkmCyLpd0bt77b/L0kdEaWiHboNNsp+VxQEd3Rcj/+CrnR/inJfU1LqT6EA+MHqNXNg= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7F8254BA540C Received: by simark.ca (Postfix) id 1EA221E0AB; Tue, 09 Dec 2025 14:36:15 -0500 (EST) From: Simon Marchi To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH 01/11] gdb/solib-rocm: assert that host ops isn't rocm_solib_ops Date: Tue, 9 Dec 2025 14:32:05 -0500 Message-ID: <20251209193610.296085-2-simon.marchi@efficios.com> X-Mailer: git-send-email 2.52.0 In-Reply-To: <20251209193610.296085-1-simon.marchi@efficios.com> References: <20251209193610.296085-1-simon.marchi@efficios.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 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 In some fork cases, the rocm_solib_target_inferior_created observer gets called while the child inferior (passed as a parameter) already has a rocm_solib_ops installed. Since we unconditionally wrap the existing solib_ops with a new rocm_solib_ops, we can end up with a chain of multiple rocm_solib_ops, like: rocm_solib_ops -> rocm_solib_ops -> svr4_solib_ops I don't think it is technically harmful as of now (unless the process does a ton of forks and the rocm_solib_ops accumulate), but it is for sure useless. Add an assert for this in the rocm_solib_ops constructor, which reveals the cases where this happens. When a fork happens with follow-fork-mode == child and detach-on-fork on, infrun's follow_fork_inferior function directly moves the program space from the parent the child inferior, as an optimization. Coming into rocm_solib_target_inferior_created, the inferior's pspace unexpectedly already has a rocm_solib_ops pushed. Fix this locally by using an inferior_forked observer. In the scenario described above, remove the rocm_solib_ops and restore the host solib_ops as the program space's solib_ops, to make it look as if infrun didn't do this trick. This requires adding two parameters to the inferior_forked observer (detach_on_fork and follow_child). This should probably be done by infrun directly at some point. The logic being that it's fine to do an optimization, but it should look as if it didn't occur. If infrun created a brand new pspace for the child, there wouldn't be a rocm_solib_ops there already. But I prefer a local fix for now. Finally, there are also the vfork cases, where the child inferior shares the program space with its parent, and therefore the child's program space already has a rocm_solib_ops installed. Address this case by returning early (the `inf->vfork_parent != nullptr` check), because there is nothing we want to do for a vfork child anyway. Change-Id: I2e76d111e96f1e01b6799b04da9cdd6f6e2984c9 --- gdb/amd-dbgapi-target.c | 3 ++- gdb/infrun.c | 3 ++- gdb/observable.h | 8 ++++++-- gdb/solib-rocm.c | 31 +++++++++++++++++++++++++++++++ 4 files changed, 41 insertions(+), 4 deletions(-) diff --git a/gdb/amd-dbgapi-target.c b/gdb/amd-dbgapi-target.c index d296f2830063..87b834739e2d 100644 --- a/gdb/amd-dbgapi-target.c +++ b/gdb/amd-dbgapi-target.c @@ -2102,7 +2102,8 @@ amd_dbgapi_inferior_execd (inferior *exec_inf, inferior *follow_inf) static void amd_dbgapi_inferior_forked (inferior *parent_inf, inferior *child_inf, - target_waitkind fork_kind) + target_waitkind fork_kind, bool detach_on_fork, + bool follow_child) { if (child_inf != nullptr) { diff --git a/gdb/infrun.c b/gdb/infrun.c index bd114e16b80e..e0ffe95e4e45 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -645,7 +645,8 @@ holding the child stopped. Try \"set %ps\" or \"%ps\".\n"), target_follow_fork (child_inf, child_ptid, fork_kind, follow_child, detach_fork); - gdb::observers::inferior_forked.notify (parent_inf, child_inf, fork_kind); + gdb::observers::inferior_forked.notify (parent_inf, child_inf, fork_kind, + detach_fork, follow_child); /* target_follow_fork must leave the parent as the current inferior. If we want to follow the child, we make it the current one below. */ diff --git a/gdb/observable.h b/gdb/observable.h index 5f064cf1fc8c..c8cbdbad97d1 100644 --- a/gdb/observable.h +++ b/gdb/observable.h @@ -92,9 +92,13 @@ extern observable the child (because we follow only the child or we follow both), CHILD_INF is the child inferior. Otherwise, CHILD_INF is nullptr. - FORK_KIND is TARGET_WAITKIND_FORKED or TARGET_WAITKIND_VFORKED. */ + FORK_KIND is TARGET_WAITKIND_FORKED or TARGET_WAITKIND_VFORKED. + + detach_on_fork and follow_child represent the "detach-on-fork" and + "follow-fork-mode" settings. */ extern observable inferior_forked; + target_waitkind /* fork_kind */, bool /* detach_on_fork */, + bool /* follow_child */> inferior_forked; /* The shared library specified by SOLIB has been loaded. Note that when gdb calls this observer, the library's symbols probably diff --git a/gdb/solib-rocm.c b/gdb/solib-rocm.c index d97ab25df5d0..a9573f8eefde 100644 --- a/gdb/solib-rocm.c +++ b/gdb/solib-rocm.c @@ -164,8 +164,14 @@ struct rocm_solib_ops : public solib_ops explicit rocm_solib_ops (program_space *pspace, solib_ops_up host_ops) : solib_ops (pspace), m_host_ops (std::move (host_ops)) { + gdb_assert (m_host_ops != nullptr); + gdb_assert (dynamic_cast (m_host_ops.get ()) == nullptr); } + /* Release the host solib_ops. */ + solib_ops_up release_host_ops () + { return std::move (m_host_ops); } + /* The methods implemented by rocm_solib_ops. */ owning_intrusive_list current_sos () const override; void create_inferior_hook (int from_tty) const override; @@ -808,6 +814,10 @@ rocm_update_solib_list () static void rocm_solib_target_inferior_created (inferior *inf) { + /* A vfork child shares its pspace with its parent, do not touch anything. */ + if (inf->vfork_parent != nullptr) + return; + get_solib_info (inf)->solib_list.clear (); auto prev_ops = inf->pspace->release_solib_ops (); @@ -839,6 +849,24 @@ rocm_solib_target_inferior_execd (inferior *exec_inf, inferior *follow_inf) get_solib_info (exec_inf)->solib_list.clear (); } +static void +rocm_solib_target_inferior_forked (inferior *parent_inf, inferior *child_inf, + target_waitkind fork_kind, + bool detach_on_fork, bool follow_child) +{ + if (detach_on_fork && follow_child && fork_kind == TARGET_WAITKIND_FORKED) + { + /* In this particular configuration, infrun's follow_fork_inferior + function moves the parent pspace to the child directly. Remove the + existing rocm_solib_ops from the child and restore the host solib_ops, + to make it look like a brand new pspace. */ + auto rocm_ops_holder = child_inf->pspace->release_solib_ops (); + auto rocm_ops + = gdb::checked_static_cast (rocm_ops_holder.get ()); + child_inf->pspace->set_solib_ops (rocm_ops->release_host_ops ()); + } +} + INIT_GDB_FILE (rocm_solib) { /* The dependency on the amd-dbgapi exists because solib-rocm's @@ -852,4 +880,7 @@ INIT_GDB_FILE (rocm_solib) gdb::observers::inferior_execd.attach (rocm_solib_target_inferior_execd, "solib-rocm", { &get_amd_dbgapi_target_inferior_execd_observer_token () }); + + gdb::observers::inferior_forked.attach + (rocm_solib_target_inferior_forked, "solib-rocm"); } -- 2.52.0