From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id JP9EFR7jfl8IZAAAWB0awg (envelope-from ) for ; Thu, 08 Oct 2020 05:59:58 -0400 Received: by simark.ca (Postfix, from userid 112) id 494371EF6F; Thu, 8 Oct 2020 05:59:58 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED, MAILING_LIST_MULTI,T_DKIM_INVALID,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from 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 RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id E3FD21E58C for ; Thu, 8 Oct 2020 05:59:56 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 360F4385703D; Thu, 8 Oct 2020 09:59:56 +0000 (GMT) Received: from mail-wr1-x42d.google.com (mail-wr1-x42d.google.com [IPv6:2a00:1450:4864:20::42d]) by sourceware.org (Postfix) with ESMTPS id 64F2B3857838 for ; Thu, 8 Oct 2020 09:59:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 64F2B3857838 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-x42d.google.com with SMTP id y12so507903wrp.6 for ; Thu, 08 Oct 2020 02:59:53 -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=eeISZcicMEh+torZLAyVtD+xOhEzNzXxf+skGPPJFY0=; b=MUyrkfYr0lVTAwcsIEGWIcu4jNSEjlhbnrI5Si6htzHt0+xYBEWF/c9ktTkFp/t0jA p5cq16/tSoXHO+XUw6goRtAfY+hhvosUjodCR8rf5MJUmycsiYHY43p5XC2wAw5aGfUD v9g+TlKGrdiRJrSPTBAnz7sPz7DA9kZaC/VnP9dIBPyVyAikF0K+STzAi8IJNKXQh3Pg aQrfE1mkIycS9Eijyz9V53WO17YAOdRNivrK8gcaROTIma8OajRUDaafF3VKMLCqV1fW P88dSfcLxV3MQPPNhKq7cSDN7Hiv6leevi9Amp8AbRhluuYqkgCVWbLPZw8MMFmqLZW1 4ZBQ== 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=eeISZcicMEh+torZLAyVtD+xOhEzNzXxf+skGPPJFY0=; b=AL8N9gpK4h3DcHbhRhPxLAE+c1P250GY7m6fHzIYfi+msXEW0N89OAuikAqR4DuJ78 zKW/gQn3lxgoRLH748goooHPadpGas1FxFlxAOafEHL6gujLgyx94zXVhX5HvfFgdhpN C8kosEyO6/VCuNdEGSc969rEpuNgmlefFH55E0fnL1KGpdtbQwpJp4HhdXhJqSTb0usX Sh1pE2UB1IJ28sVkfE51IKgu4Zz0YVKRl6LGbxstNR3B6RPgztELHBHUOcHVXYDaXBPK AOV8w5++2+FTpbd6iireeAqWtbQ85lpPEBdwxGDyu8FWFcbPWff3LiVSuGuRl4f0NYT8 vuLA== X-Gm-Message-State: AOAM531+/bR94waKcTCPTHDoT3Kp9rXUoCXRfCLLvxg0PUSYWOM9Ylws 4WtCEtVApD7goeQPz8+b0wOtmFvJw73mtQ== X-Google-Smtp-Source: ABdhPJxWr+/F0o/itJbJ4TSAteQrFXz0FLvnwdJMLv2/Zk5eN7rFZtX0b4cmKfsgRKE2gMe2zpfeuw== X-Received: by 2002:a5d:424e:: with SMTP id s14mr9089943wrr.149.1602151192069; Thu, 08 Oct 2020 02:59:52 -0700 (PDT) Received: from localhost (host109-148-134-240.range109-148.btcentralplus.com. [109.148.134.240]) by smtp.gmail.com with ESMTPSA id u15sm5965840wml.21.2020.10.08.02.59.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Oct 2020 02:59:51 -0700 (PDT) Date: Thu, 8 Oct 2020 10:59:50 +0100 From: Andrew Burgess To: gdb-patches@sourceware.org Subject: Re: [PATCHv3 0/3] Restore thread and frame patches Message-ID: <20201008095950.GN605036@embecosm.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Operating-System: Linux/5.8.12-100.fc31.x86_64 (x86_64) X-Uptime: 10:56:43 up 5 days, 2:09, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] 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: , Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" Ping! Pedro - I do feel I've addressed your initial feedback, I'd love to hear what your position is on this new version, even if it's just to confirm you are still opposed to this patch. Many thanks, Andrew * Andrew Burgess [2020-09-25 23:24:06 +0100]: > This is a third attempt to get some, or maybe all of this work merged. > Or to get some idea on what might be seen as an acceptable direction > to take this work. > > I originally posted this here (v2): > > https://sourceware.org/pipermail/gdb-patches/2020-April/167984.html > > and (v1): > > https://sourceware.org/pipermail/gdb-patches/2020-February/166202.html > https://sourceware.org/pipermail/gdb-patches/2020-April/167215.html > > Changes since V2: > > - Rebase to current master. > > - Fixed minor coding style issues, and improved a comment as pointed > out by Baris. > > - Reordered the patches so #2 is now #1. I think that current #1 > which is really a restructure might be worth merging even if #2 and > #3 never get merged, hence placing it first. > > - A few minor tweaks to take acount of general code changes since v2. > > There was some initial positive feedback on some of the v1 patches, > but Pedro was not convinced: > > https://sourceware.org/pipermail/gdb-patches/2020-April/167223.html > > I'll quote his feedback here, and reply to it inline: > > > Frankly, I'm not really sure I like this. It seems like a can of worms to > > me... It's going to cause us to have to decide whether it's a good idea to > > save/restore all kinds of state in the objects hierarchy. E.g., if this is > > reasonable, then it would also be reasonable to restore the selected > > Ada task. And frames. And maybe the selected source file and line for > > list. Once we gain support for fibers, coroutines, etc. we'll > > then need to apply the same logic. Etc. And then maybe we'll need > > some way to query the selected thread of a given inferior. Etc. > > This feedback was on v1 of the patch where I changed the default > behaviour to be restore thread/frame, since v2 the default is to > maintain GDB's current behaviour and have the restore be a feature a > user must turn on. > > I think this addresses the concern Pedro raised as we no longer have > an inconsistent position, some things are restored while others are > not, instead we have a switch to allow somethings to be restored while > we lack a switch to allow other things to be restored. > > While I agree with Pedro's original feedback that a user might be > confused, "why is XXX restored, but not YYY", now they will simply be > left wondering, "Why is there no switch to restore YYY?". Though they > might wonder why the switch doesn't exist, and might be disapointed > even, I don't think it will leave them confused with the actual > behaviour of GDB. > > Further, though "restoring stuff" as a broad category clearly covers > all the things Pedro mentions, restoring of each thing will require > its own piece of work. I don't think we should prevent merging this > just because there might be some other (similar, but unrelated) part > of GDB that could also be saved and restored. > > > > > The current rule is quite simple. If you select a object then its container > > is implicitly selected. So if you select a thread, you implicitly select > > its inferior, and implicitly select its target. > > OK, but that's looking upward, a frame is in a thread, a thread is in > an inferior, an inferior is in a target. These changes are about lookig down. > > > And the first/initial container > > object is selected. > > I'm confused! Above you talk about containers as looking upward > thread -> inferior -> target, but I think you're now talking about > things as looking down, in which case talking about containers seems > like poor terminology. > > When we switch to an inferior then its containing target is > automatically selected, but there's only one of those to select. > > > It seems very natural to me that "inferior 2" ends up > > selecting the initial thread of inferior 2. I.e., normally, thread 2.1. > > I'd certainly never want to suggest you're wrong for preferring a > particular behaviour. For me though the choice of the first thread > seems pretty arbitrary, instead the idea of restoring the selected > object within a container seems more consistent. > > However, I feel I've addressed this concern by making both of the > save/restore features being off by default. > > Thoughts, or feedback on any or all of these patches is always > welcome. > > Thanks, > Andrew > > --- > > Andrew Burgess (3): > gdb: unify two copies of restore_selected_frame > gdb: Restore previously selected thread when switching inferior > gdb: Track the current frame for each thread > > gdb/ChangeLog | 53 +++ > gdb/NEWS | 21 ++ > gdb/doc/ChangeLog | 14 + > gdb/doc/gdb.texinfo | 42 ++- > gdb/frame.c | 84 +++++ > gdb/frame.h | 85 +++++ > gdb/gdbthread.h | 15 +- > gdb/inferior.c | 58 ++- > gdb/inferior.h | 10 + > gdb/infrun.c | 25 +- > gdb/testsuite/ChangeLog | 10 + > .../gdb.threads/restore-selected-frame.c | 85 +++++ > .../gdb.threads/restore-selected-frame.exp | 336 ++++++++++++++++++ > gdb/testsuite/gdb.threads/restore-thread.c | 248 +++++++++++++ > gdb/testsuite/gdb.threads/restore-thread.exp | 219 ++++++++++++ > gdb/thread.c | 128 +++---- > 16 files changed, 1331 insertions(+), 102 deletions(-) > create mode 100644 gdb/testsuite/gdb.threads/restore-selected-frame.c > create mode 100644 gdb/testsuite/gdb.threads/restore-selected-frame.exp > create mode 100644 gdb/testsuite/gdb.threads/restore-thread.c > create mode 100644 gdb/testsuite/gdb.threads/restore-thread.exp > > -- > 2.25.4 >