From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 29891385702B for ; Thu, 8 Jun 2023 13:17:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 29891385702B Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1686230270; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=gheLUH2oiulBdFpEW4cf+2bHFg51h89THGJRoMRTNgg=; b=LmFku44OYPesrdan+jF8vUMj6LPz4NM7NZSUnwYgE2P1T1pcSo/WC5xXXY/FzByagpt/wv 7gSlaap1tlXTJQ3ra6d5ipscfgjBhu/cMkkVZgxYI8JwYJArgOeTksOw4gC0+a/O0IPJXX UHEXBeKAzEEwpglds1hvr8W8nap56Oo= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-413-a64DfSNCMyqa2L2DSz0RLg-1; Thu, 08 Jun 2023 09:17:48 -0400 X-MC-Unique: a64DfSNCMyqa2L2DSz0RLg-1 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-3f72720c592so3769335e9.2 for ; Thu, 08 Jun 2023 06:17:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686230267; x=1688822267; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=gheLUH2oiulBdFpEW4cf+2bHFg51h89THGJRoMRTNgg=; b=kaHLIUCuL1cN4A24mR3bHJou3o6qO1wfjT7YeYHjdvnGCKXNXH53XkM9t49QTEvwpu OabXd/tKstqupGiQ0kw56wgJEUKZfnQDWpXg8D8+eyVmJQTyp9RhMWvyaEtIeoadM4m+ mx/FMt8+3KZ549z9oc6CpWbi4fjZzB9xfMUYP1+zTpyMMIwA+6jhyiiSMPXUakOwDfN5 k7PhEJBLT/4BHgXJWz/7Rl1Bo1VVGGyejnUuc3OJX+ERPeTE+HjmQaAyH+XAA3UB92Pv guatHPmhx/qqp0i9pOIZ9QMCMxDdITZVgYih5k5XmHRDzlsRWjUoOYSLgv/zpcH5Ihuu NjoA== X-Gm-Message-State: AC+VfDyddi3/l8WuFclBDHrniovbuFdBK15mjgyIhrjQnBLDH/BvCEHu yD6d9BW2C5jYxpPR54aAQ0yJMQNK5uNl+3rh1ZpHc3zeW4/NOmdGreqBrhmQm+El+ScAmjzX/Ke /606BUl9JO2Z1qXaPA2kW/nceskZYHw== X-Received: by 2002:a05:6000:1cc5:b0:30a:bf2b:e03b with SMTP id bf5-20020a0560001cc500b0030abf2be03bmr6800536wrb.1.1686230267146; Thu, 08 Jun 2023 06:17:47 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ5a5vPiMESxxFIh84Zd2R7qOtyXD9Sd+6V0N089dfv2jCT2iQqpLiy2k4ojJ8gJxv9BbZk3fQ== X-Received: by 2002:a05:6000:1cc5:b0:30a:bf2b:e03b with SMTP id bf5-20020a0560001cc500b0030abf2be03bmr6800516wrb.1.1686230266812; Thu, 08 Jun 2023 06:17:46 -0700 (PDT) Received: from localhost (11.72.115.87.dyn.plus.net. [87.115.72.11]) by smtp.gmail.com with ESMTPSA id k17-20020a5d4291000000b003047ea78b42sm1598608wrq.43.2023.06.08.06.17.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Jun 2023 06:17:46 -0700 (PDT) From: Andrew Burgess To: Pedro Alves , gdb-patches@sourceware.org Subject: Re: [PATCH 17/31] Introduce GDB_THREAD_OPTION_EXIT thread option, fix step-over-thread-exit In-Reply-To: <20221212203101.1034916-18-pedro@palves.net> References: <20221212203101.1034916-1-pedro@palves.net> <20221212203101.1034916-18-pedro@palves.net> Date: Thu, 08 Jun 2023 14:17:45 +0100 Message-ID: <87r0qmytxi.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Pedro Alves writes: > When stepping over a breakpoint with displaced stepping, GDB needs to > be informed if the stepped thread exits, otherwise the displaced > stepping buffer that was allocated to that thread leaks, and this can > result in deadlock, with other threads waiting for their turn to > displaced step, but their turn never comes. > > Similarly, when stepping over a breakpoint in line, GDB also needs to > be informed if the stepped thread exits, so that is can clear the step > over state and re-resume threads. > > This commit makes it possible for GDB to ask the target to report > thread exit events for a given thread, using the new "thread options" > mechanism introduced by a previous patch. > > This only adds the core bits. Following patches in the series will > teach the Linux backends (native & gdbserver) to handle the > GDB_THREAD_OPTION_EXIT option, and then a later patch will make use of > these thread exit events to clean up displaced stepping and inline > stepping state properly. LGTM. Reviewed-By: Andrew Burgess Thanks, Andrew > > Change-Id: I96b719fdf7fee94709e98bb3a90751d8134f3a38 > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27338 > --- > gdb/infrun.c | 15 ++++++++++----- > gdb/remote.c | 9 +++++++++ > gdb/target/target.c | 1 + > gdb/target/target.h | 4 ++++ > 4 files changed, 24 insertions(+), 5 deletions(-) > > diff --git a/gdb/infrun.c b/gdb/infrun.c > index 6fdffb31884..e47e3c688e7 100644 > --- a/gdb/infrun.c > +++ b/gdb/infrun.c > @@ -2445,24 +2445,29 @@ do_target_resume (ptid_t resume_ptid, bool step, enum gdb_signal sig) > else > target_pass_signals (signal_pass); > > - /* Request that the target report thread-{created,cloned} events in > - the following situations: > + /* Request that the target report thread-{created,cloned,exited} > + events in the following situations: > > - If we are performing an in-line step-over-breakpoint, then we > will remove a breakpoint from the target and only run the > current thread. We don't want any new thread (spawned by the > - step) to start running, as it might miss the breakpoint. > + step) to start running, as it might miss the breakpoint. We > + need to clear the step-over state if the stepped thread exits, > + so we also enable thread-exit events. > > - If we are stepping over a breakpoint out of line (displaced > stepping) then we won't remove a breakpoint from the target, > but, if the step spawns a new clone thread, then we will need > to fixup the $pc address in the clone child too, so we need it > - to start stopped. > + to start stopped. We need to release the displaced stepping > + buffer if the stepped thread exits, so we also enable > + thread-exit events. > */ > if (step_over_info_valid_p () > || displaced_step_in_progress_thread (tp)) > { > - gdb_thread_options options = GDB_THREAD_OPTION_CLONE; > + gdb_thread_options options > + = GDB_THREAD_OPTION_CLONE | GDB_THREAD_OPTION_EXIT; > if (target_supports_set_thread_options (options)) > tp->set_thread_options (options); > else > diff --git a/gdb/remote.c b/gdb/remote.c > index f7ab8523fd5..0d2b7c09a07 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -3998,6 +3998,15 @@ remote_target::update_thread_list () > if (has_single_non_exited_thread (tp->inf)) > continue; > > + /* Do not remove the thread if we've requested to be > + notified of its exit. For example, the thread may be > + displaced stepping, infrun will need to handle the > + exit event, and displaced stepping info is recorded > + in the thread object. If we deleted the thread now, > + we'd lose that info. */ > + if ((tp->thread_options () & GDB_THREAD_OPTION_EXIT) != 0) > + continue; > + > /* Not found. */ > delete_thread (tp); > } > diff --git a/gdb/target/target.c b/gdb/target/target.c > index 453167a2ad4..2dc033e63b7 100644 > --- a/gdb/target/target.c > +++ b/gdb/target/target.c > @@ -196,6 +196,7 @@ to_string (gdb_thread_options options) > { > static constexpr gdb_thread_options::string_mapping mapping[] = { > MAP_ENUM_FLAG (GDB_THREAD_OPTION_CLONE), > + MAP_ENUM_FLAG (GDB_THREAD_OPTION_EXIT), > }; > return options.to_string (mapping); > } > diff --git a/gdb/target/target.h b/gdb/target/target.h > index 139e371e8d8..4e8839c5667 100644 > --- a/gdb/target/target.h > +++ b/gdb/target/target.h > @@ -34,6 +34,10 @@ enum gdb_thread_option : unsigned > /* Tell the target to report TARGET_WAITKIND_THREAD_CLONED events > for the thread. */ > GDB_THREAD_OPTION_CLONE = 1 << 0, > + > + /* Tell the target to report TARGET_WAITKIND_THREAD_EXIT events for > + the thread. */ > + GDB_THREAD_OPTION_EXIT = 1 << 1, > }; > > DEF_ENUM_FLAGS_TYPE (enum gdb_thread_option, gdb_thread_options); > -- > 2.36.0