From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x32e.google.com (mail-wm1-x32e.google.com [IPv6:2a00:1450:4864:20::32e]) by sourceware.org (Postfix) with ESMTPS id 347363851C2C for ; Fri, 25 Sep 2020 22:24:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 347363851C2C 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-x32e.google.com with SMTP id e2so909501wme.1 for ; Fri, 25 Sep 2020 15:24:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=62zAnERYKfOEay+5fplxXPxBXK4H9qs4ih8ZBT+LU9c=; b=gtAbIyQoVTFI/9n/3/pyLNMNHcs0FZcvL1vKi3Y793S5hifcg8j08Gl7GfTxkChKno DaEAO9U1ozdyoQsiqzorfSfqduInzKJ7ZiLbQo5BuSrA1P8H3DSr0lDXasHXuPLI3T1N F2xmcIphOTJLo9bvybDzidtc2hwucgVBZZ5oZ/vEbrqokiPBpSvpOAKduhQDXGeTPxii KxN6TrNJgOQ2Q26oF7Pn8Ph7ggjCIr4w9gB95oFa1otfKaEsojMP2CTKGvak+G7uSPIT MstVV8r3dD8wq6OgCyDU449zf7HhsLZrQJwfI/F36qXo1D8eI8Eks1uWsZz7aipHF9Kl z1DQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=62zAnERYKfOEay+5fplxXPxBXK4H9qs4ih8ZBT+LU9c=; b=Hb1uWmtkEvdXq4HvFXhgkZamJ43bUsxGagGPoe3vFRnjtqIrw3qfdhniYP1pb8Rd9A 8thRMJu7ZtTzouNVNAD5y5vreKPJjOkVLvi3bsr5kAd6PKljmBQvQef+o+QBh8oMjGVW pOeM8GGai6aqzcQ02uN8ch6/l6X5H3HJZa+ALR0jlzQjYTUrjYabISEwfd8BwUQAYmGr QqoWl2ilgNpFltHcNfMuw67XiLW3uZ++PD81WpljSp8tOMG9rk3Oyi6wUe5+Nn4Dx7ba o8EmcWGuZeJrNEWZ6fEQf2ClFCC5rzDp0RRCKIKdcQUTRucZ1SgWGgZT1y4Xtk2ZV4H0 xEEQ== X-Gm-Message-State: AOAM532f6g7k0Mr58ulpbbUOCnczoLHm9mKkUHlzF8v0WKATeoRJCrCj /13nZuGJD4U8QWiwqzp0sv87/L+XbB1+OQ== X-Google-Smtp-Source: ABdhPJzJd8HMU6WN86+lxssBBSgAXNXRGd9MMA60A4tL5gJrJXHNnNpqPDy6Ntz98PQka9QOEkyLsg== X-Received: by 2002:a1c:9d43:: with SMTP id g64mr778575wme.16.1601072655807; Fri, 25 Sep 2020 15:24:15 -0700 (PDT) Received: from localhost (host109-151-23-62.range109-151.btcentralplus.com. [109.151.23.62]) by smtp.gmail.com with ESMTPSA id 92sm4730715wra.19.2020.09.25.15.24.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 25 Sep 2020 15:24:15 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Subject: [PATCHv3 0/3] Restore thread and frame patches Date: Fri, 25 Sep 2020 23:24:06 +0100 Message-Id: X-Mailer: git-send-email 2.25.4 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.8 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, 25 Sep 2020 22:24:19 -0000 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