From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x329.google.com (mail-wm1-x329.google.com [IPv6:2a00:1450:4864:20::329]) by sourceware.org (Postfix) with ESMTPS id 560CB3858034 for ; Tue, 2 Mar 2021 12:17:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 560CB3858034 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wm1-x329.google.com with SMTP id o2so2000979wme.5 for ; Tue, 02 Mar 2021 04:17:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=TledtB/lpnon4x1L5wF/K6a1094fO8lqQR/vQwZjiLE=; b=QH1rTekdWW5QqPCpVaj8ikEObO0+U26FU+lqeYRLsjaUmpyI1JK3f4WXfuE1fA3lCz TifNqG/Z2GoevHxl9WPmPEeT0NqpVwXXW30zX/JLvLfPHqzd/E2Ye1BCBO/CEN2aW4+J cCsY/BM39wtSbe5aN80ODKL96gnBHamI25Mk/ljkyulUjpBC+kdqeGBAtyUuG2+Ndgik sGZmDvcGM9ebVuue1vAcxaOgdkjEorW65dzRe3Wy0NAgAo3IaUf841Q4QzbTYLN0rckK KuqqjpK9wbiIITmBELOfurj9RoCDFx9Bz/HzFg2HDWUA5tJH2Qu6B56hokfdxURyip5H LotQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=TledtB/lpnon4x1L5wF/K6a1094fO8lqQR/vQwZjiLE=; b=j54rXeEACMx0NER7g7Ouzzpdueh4PKLnMmypAnDQoJgpArpF7aFggCTqfri3vlARpx vSxsgModbUy91o7kNpynVvomg9wDH3KaXfvlOgdVWsszUBp6QG+vZXaShtnnNBNaIccT t0Q3MViDsN/MBPwLVITAENdNJol4mcTeQtffLWjpC8S+DkzSr5PNOPlch97Syk6zKhh4 leL0SUjzGKPTnQASnleqLiKuKnhwGDXG9hK8Jrob/jfyslfKzN0/wiWnXwJYFHhgz0du a5wL0POJjuX3LCK9+7Esj6/oRY1QsDnSgfCGo95eEv72pH7YMMa7C+176y+nFkiVZtDu DT2g== X-Gm-Message-State: AOAM532IOluTo2X5ugcCQ0fQiF0WoCYzWUkJagkkyqrRjDh/1x3pZ0c8 JeOgYI/6poJqddG7Rx/Sqo9GdA== X-Google-Smtp-Source: ABdhPJwQUFyT5NG6FPKOPKMGxRW4omfVca+wUMKWPME0h65YYjqF4Yz/R0la05ONBAvoTks4J0gnOg== X-Received: by 2002:a1c:9854:: with SMTP id a81mr3600991wme.19.1614687473321; Tue, 02 Mar 2021 04:17:53 -0800 (PST) Received: from localhost (host86-186-80-154.range86-186.btcentralplus.com. [86.186.80.154]) by smtp.gmail.com with ESMTPSA id 4sm2578952wma.0.2021.03.02.04.17.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Mar 2021 04:17:52 -0800 (PST) Date: Tue, 2 Mar 2021 12:17:51 +0000 From: Andrew Burgess To: "Saiapova, Natalia" Cc: 'Kevin Buettner' , Natalia Saiapova via Gdb-patches Subject: Re: [PATCH v2 4/6] gdb/infrun: in condition evaluation wait only for the current inferior. Message-ID: <20210302121751.GR265215@embecosm.com> References: <20201009112719.629-1-natalia.saiapova@intel.com> <20201009112719.629-5-natalia.saiapova@intel.com> <20201018184429.06921e24@f32-m1.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Operating-System: Linux/5.8.13-100.fc31.x86_64 (x86_64) X-Uptime: 12:14:50 up 83 days, 16:59, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Tue, 02 Mar 2021 12:17:56 -0000 * Saiapova, Natalia via Gdb-patches [2020-10-26 12:11:06 +0000]: > Hi Kevin, > > Thank you for looking into this! See my answer below. > > Regards, > Natalia > > -----Original Message----- > > From: Kevin Buettner > > Sent: Monday, October 19, 2020 4:44 AM > > To: Natalia Saiapova via Gdb-patches > > Cc: Saiapova, Natalia > > Subject: Re: [PATCH v2 4/6] gdb/infrun: in condition evaluation wait only for the > > current inferior. > > > > Hi Natalia, > > > > I have a question about this commit. See below. > > > > On Fri, 9 Oct 2020 13:27:16 +0200 > > Natalia Saiapova via Gdb-patches wrote: > > > > > In the middle of a condition evaluation, to successfully finish the > > > inferior call we need to wait for events from the current thread. > > > Otherwise, a pending event from another thread might be taken and > > > the inferior call is abandoned. > > > > > > gdb/ChangeLog: > > > 2020-08-26 Natalia Saiapova > > > Tankut Baris Aktemur > > > > > > * infrun.c (do_target_wait): Match an inferior PID with the > > > expected PID. > > > (fetch_inferior_event): In condition evaluation, wait for the > > > event from the current inferior. > > > > > > Co-authored-by: Natalia Saiapova > > > Co-authored-by: Tankut Baris Aktemur > > > > > > Signed-off-by: Natalia Saiapova > > > --- > > > gdb/infrun.c | 17 +++++++++++++---- > > > 1 file changed, 13 insertions(+), 4 deletions(-) > > > > > > diff --git a/gdb/infrun.c b/gdb/infrun.c > > > index 91271c2ddbe..d260eb6e3a7 100644 > > > --- a/gdb/infrun.c > > > +++ b/gdb/infrun.c > > > @@ -3565,10 +3565,11 @@ do_target_wait (ptid_t wait_ptid, > > execution_control_state *ecs, > > > polling the rest of the inferior list starting from that one in a > > > circular fashion until the whole list is polled once. */ > > > > > > - auto inferior_matches = [&wait_ptid] (inferior *inf) > > > + ptid_t wait_pid_ptid {wait_ptid.pid ()}; > > > + auto inferior_matches = [&wait_pid_ptid] (inferior *inf) > > > { > > > - return (inf->process_target () != NULL > > > - && ptid_t (inf->pid).matches (wait_ptid)); > > > + return (inf->process_target () != nullptr > > > + && ptid_t (inf->pid).matches (wait_pid_ptid)); > > > }; > > > > I'm puzzled by the changes above. Can you explain what this is > > about? (And perhaps add a comment...) > > When we come here from the infcall inside the condition, the > wait_ptid has a form {non-zero, non-zero, zero}, which corresponds > to the thread started the infcall. So, when we construct a ptid via > ptid_t (inf->pid) in inferior_matches function, it does not match > the wait_ptid. That leads to ecs->ws.kind = TARGET_WAITKIND_IGNORE > and we are not getting the event. But we wait for an event from the > thread which has started the infcall, so until we get one we are in > an infinite loop. > > This code looks like the intention here is to find a matching > inferior, so we are interested only in the PID part of the > wait_ptid. Thus, I made the change. Does this make sense to you? This describes what, but not why. You earlier change in user_visible_resume_ptid, which I'm guessing relates to this change returned the inferior_ptid. But that's not the only path by which we might return inferior_ptid. So its not clear why this change runs into problems, but other cases don't. Also I agree with Kevin that this probably needs explaining with comments in the code. Thanks, Andrew > > > > > > > > > /* First see how many matching inferiors we have. */ > > > @@ -3906,7 +3907,15 @@ fetch_inferior_event () > > > = make_scoped_restore (&execution_direction, > > > target_execution_direction ()); > > > > > > - if (!do_target_wait (minus_one_ptid, ecs, TARGET_WNOHANG)) > > > + ptid_t waiton_ptid = minus_one_ptid; > > > + > > > + /* If the thread is in the middle of the condition evaluation, > > > + wait for the event from the current inferior. */ > > > + if (inferior_ptid != null_ptid > > > + && inferior_thread ()->control.in_cond_eval) > > > + waiton_ptid = inferior_ptid; > > > + > > > + if (!do_target_wait (waiton_ptid, ecs, TARGET_WNOHANG)) > > > return; > > > > > > gdb_assert (ecs->ws.kind != TARGET_WAITKIND_IGNORE); > > > -- > > > 2.17.1 > > > > > > Intel Deutschland GmbH > > > Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany > > > Tel: +49 89 99 8853-0, www.intel.de > > > Managing Directors: Christin Eisenschmid, Gary Kershaw > > > Chairperson of the Supervisory Board: Nicole Lau > > > Registered Office: Munich > > > Commercial Register: Amtsgericht Muenchen HRB 186928 > > > > > Intel Deutschland GmbH > Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany > Tel: +49 89 99 8853-0, www.intel.de > Managing Directors: Christin Eisenschmid, Gary Kershaw > Chairperson of the Supervisory Board: Nicole Lau > Registered Office: Munich > Commercial Register: Amtsgericht Muenchen HRB 186928 >