From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x435.google.com (mail-wr1-x435.google.com [IPv6:2a00:1450:4864:20::435]) by sourceware.org (Postfix) with ESMTPS id CDA40385BF9D for ; Fri, 14 May 2021 12:49:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org CDA40385BF9D 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-wr1-x435.google.com with SMTP id x5so29936467wrv.13 for ; Fri, 14 May 2021 05:49:28 -0700 (PDT) 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=TzLGY4PIAKHGeZb4wLOX6K/mI6VRCMcKwmAPqkRk7m8=; b=VzD+BsLceBO0OEOrmHTpVYdDrwK8GJ+/fG4hw2j3JmXmSpRLVyCFqfhQZf726m5zEq 7pIISkBNV8188ThtU/fBiMGjuxqWlGvVxvdY9rUoLrTWuMAwJfbgfkCEO58ObaCJqffw G/kHEGbugpGkh9tFGJOGeJk4UjOvxYtTlJAdYodhi583j6/P8BnX9TcscbTdRrrz9YV9 ve+dmTfEm3ThxaTCZPTvOD5UeLOz+tqCHItOKSNirwFDzFaWNARZYPZIBcereNvb2iqt Tj4iR/Ki/i2jUuUO31Blj9v+i46hVMlk16eZNJZe4cKssQ38uUT7tWIgz26iR8myde4S cdBQ== 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=TzLGY4PIAKHGeZb4wLOX6K/mI6VRCMcKwmAPqkRk7m8=; b=rukG5czojPjmMWNHlzSdBvE+8usMi7zc5EaCyVwdhZYgG87d6fuKK326WZT/mOpVFn oNlEdvIzTr4KGJziN7ha51KNvcSbs9W4wDVTm7/9pFgF8UkbG3l6CFX6deuhv7d69wdl pgc9lEj153Dsd/+LgJrpz/k3rgKjG2qqja3aVV0d7QNYyS82P3WhatbrTsoEmnmIq87O Zo/MdtVFhPqzi4y/shlDPiU6VLw+1EuQcL1jWkITiZdYI8AKfQdH0fInYT5kS3agdNRa +IPXTWk/a1HLklJmSCE52D976oNajz7nnqt0H5jFTmEIBonX/F+csGbbSzZcN5T+NH3g WaAA== X-Gm-Message-State: AOAM532Iysp1SJA3QuGvtVE7N8dOEDRdXIMwRhi9I+uZwTSHbe+8K/uq /8+WYjkDMDPWT0r5sW9QatiGdnvl+f6jJw== X-Google-Smtp-Source: ABdhPJz+3mxqKtCRydp6B3q9Bpvbhcg32uY9wCihjwHkqImWMB6ZFs+F82eDbERC59RyAxsge3dpPA== X-Received: by 2002:adf:f3cb:: with SMTP id g11mr1038262wrp.75.1620996567977; Fri, 14 May 2021 05:49:27 -0700 (PDT) Received: from localhost (host109-151-46-70.range109-151.btcentralplus.com. [109.151.46.70]) by smtp.gmail.com with ESMTPSA id p2sm681397wrj.10.2021.05.14.05.49.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 14 May 2021 05:49:27 -0700 (PDT) Date: Fri, 14 May 2021 13:49:26 +0100 From: Andrew Burgess To: Simon Marchi Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 2/2] gdb: avoid sending unnecessary wildcard vCont actions Message-ID: <20210514124926.GK3067949@embecosm.com> References: <0b31590c3358c4fbdf4522c45164470bea323102.1620743949.git.andrew.burgess@embecosm.com> <9f349591-9eba-7bc0-2fd2-d590f0c2a700@polymtl.ca> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9f349591-9eba-7bc0-2fd2-d590f0c2a700@polymtl.ca> X-Operating-System: Linux/5.8.18-100.fc31.x86_64 (x86_64) X-Uptime: 13:40:55 up 11 days, 1:35, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Spam-Status: No, score=-6.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, 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: Fri, 14 May 2021 12:49:33 -0000 * Simon Marchi [2021-05-13 13:55:05 -0400]: > On 2021-05-11 10:39 a.m., Andrew Burgess wrote: > > Looking at remote_target::commit_resumed the algorithm for sending > > wildcard actions starts by assuming that we can send a wildcard > > action, and then looks for pending events, or resume requests, that > > prevent wildcard actions (either global, or inferior specific). > > > > This is fine, and there's no real problems with this, but it does lead > > to one slight weirdness, if we are in non-stop mode, and > > displaced-stepping is off, then, when we do a step, all threads should > > be stopped, and we should step only the one thread that we wish to > > have move on. However, in the situation where there the inferior only > > has a single thread then we actually end up sending a packet like > > this: > > > > [remote] Sending packet: $vCont;s:pPID.TID;c#XX > > > > Notice the trailing ';c'. > > > > Now, there's nothing technically wrong here, we are correctly asking > > the one and only thread PID.TID to perform a step, there are no other > > threads, so telling everyone else to continue is harmless. > > > > However, this annoys me. It annoys me because we know we want only > > the one, and only one, thread PID.TID to step, why send over redundant > > information! > > > > This commit fixes this tiny blip by keeping a counter of the number of > > threads that could be resumed in each inferior. We count up as we see > > threads that need to be resumed, and then count down when non-wildcard > > actions are sent (e.g. the s:pPID.TID in our example above). Then, > > when we are considering sending wildcard actions, we only send one if > > there are any threads that will be impacted by the wildcard action. > > > > Now, in the above situation we send out > > > > [remote] Sending packet: $vCont;s:pPID.TID#XX > > > > There should be no change in behaviour when considering gdb/gdbserver > > working together after this commit. > > The code looks correct, but in my opinion this little oddity is not > significant enough to justify making this code more complex (it's > already complex enough as it is). Thanks for the feedback. I spotted this oddity when looking at gdb/27830 [1], this bug boils down to what should happen when a new thread is started. Right now we (almost) always assume that any new thread should be set running, but this just isn't true. If we are single stepping a thread in non-stop mode, and with displaced stepping off, then, should a new thread appear we should report the new thread as created/stopped rather than setting it running. Solving this for native targets is easy enough, but we don't really have an easy/obvious way in the remote protocol to specify how new threads should be handled (well, we have QThreadEvents, which might be the solution). Anyway, I wondered if it was possible to infer from the last vCont packet how new threads should be handled, which got me looking at these packets... Given I don't, right now, know that fixing this is a requirement, I'll hold off pushing this until either (a) I know it's required, or (b) I can figure out a simpler way to have write the logic so we can send the exactly correct contents. Thanks, Andrew [1] https://sourceware.org/bugzilla/show_bug.cgi?id=27830