From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id to8qEkLiu19/TgAAWB0awg (envelope-from ) for ; Mon, 23 Nov 2020 11:24:34 -0500 Received: by simark.ca (Postfix, from userid 112) id 3FC9B1F0AB; Mon, 23 Nov 2020 11:24:34 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=0.2 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_NONE,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.2 Received: from sourceware.org (unknown [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 AC25F1E58E for ; Mon, 23 Nov 2020 11:24:33 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 085D7386F818; Mon, 23 Nov 2020 16:24:33 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 085D7386F818 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1606148673; bh=RAMv7AstLkWr/TU5/BYHzdHD7dyL97+mASgtjyRlMig=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=GqjTJ6hPQl+BkCibEPIcFOSNeVSfJhBm/xO9WuQ/lxbr0M/EC5uDCYbmc1Yeedkky pFsgYt2gEUdwBzo8m7GNLACXNJnDn6WjKBuZeDslknUqV+SBzGkvegqa3eSTqF/0LO STA6pW4KYYzNqEteo5lLERSwM5WuUgUlh/J8X+64= Received: from mail-qv1-xf2b.google.com (mail-qv1-xf2b.google.com [IPv6:2607:f8b0:4864:20::f2b]) by sourceware.org (Postfix) with ESMTPS id A3D003857C71 for ; Mon, 23 Nov 2020 16:24:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A3D003857C71 Received: by mail-qv1-xf2b.google.com with SMTP id p12so6862254qvj.13 for ; Mon, 23 Nov 2020 08:24:30 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=RAMv7AstLkWr/TU5/BYHzdHD7dyL97+mASgtjyRlMig=; b=HJvjIyJIHLz7eSDViNRMQukNwHiY8NLQgCUUXW2ZKEJU7c4esw3Jy28K5Xy9RBcA71 Daij620Dsqi1WuUbpWQnVIM0bmbGdObCo3lOS/S/rkwfaVrP46oIoy/ti+pZCkwFFgWb xFS1JQBzLLfcYnDMjc9tLvQuY1zKYh9f59qrJWtYUI8B/l4DmD08rcAqxqtjmpmGoTzu 8Cx+Bjnk5LHtmhObxQQ6VZiksaExKgbyiD3RDrTG4narNgsphQiwmqnLi0gHlj8TgW6L CKMB/YbtyNq31huFF8/kY3xXvqKmgxI0qKBR9tz2Dk7qt6DrwHx6bJSVuKjibl9Z47Tl etSw== X-Gm-Message-State: AOAM530U/0CAvbvq/VjvGR8GQl0sUswKPhisGVUuj7BdQyt1Pu+QoQY3 kDFZcvvWDfe+eQOSXD1pBScv0w42cteZKg== X-Google-Smtp-Source: ABdhPJwdR3xUfFQOBHicQR4gI+BtsLiNFtpY54HRQAf7nO62uUup0WXuTA5H0RC1ElmhJlIfUKIvUg== X-Received: by 2002:a0c:fbac:: with SMTP id m12mr62880qvp.52.1606148669473; Mon, 23 Nov 2020 08:24:29 -0800 (PST) Received: from ?IPv6:2804:7f0:8284:370e:18fe:65d7:a5d3:1acf? ([2804:7f0:8284:370e:18fe:65d7:a5d3:1acf]) by smtp.gmail.com with ESMTPSA id y1sm10191999qky.63.2020.11.23.08.24.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 23 Nov 2020 08:24:28 -0800 (PST) Subject: Re: [PATCH 1/2] gdb: introduce new 'maint flush ' prefix command To: Andrew Burgess , gdb-patches@sourceware.org References: Message-ID: Date: Mon, 23 Nov 2020 13:24:25 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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: , From: Luis Machado via Gdb-patches Reply-To: Luis Machado Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" On 11/23/20 7:39 AM, Andrew Burgess wrote: > We currently have two flushing commands 'flushregs' and 'maint > flush-symbol-cache'. I'm planning to add at least one more so I > thought it might be nice if we bundled these together into one place. > > And so I created the 'maint flush ' command prefix. Currently there > are two commands: > > (gdb) maint flush symbol-cache > (gdb) maint flush register-cache > > Unfortunately, even though both of the existing flush commands are > maintenance commands, I don't know how keen we about deleting existing > commands for fear of breaking things in the wild. So, both of the > existing flush commands 'maint flush-symbol-cache' and 'flushregs' are > still around as aliases to the new commands. > > I've updated the testsuite to use the new command syntax, and updated > the documentation too. Should we keep testing the alias in some way though? I see the testcases have been updated to only use the new command. We should certify that the old ones still exist and work? If we plan to drop the old commands at some point (I think it makes sense), we should probably mark them as deprecated with a set removal date. > > gdb/ChangeLog: > > * NEWS: Mention new commands. > * cli/cli-cmds.c (maintenanceflushlist): Define. > * cli/cli-cmds.h (maintenanceflushlist): Declare. > * maint.c (_initialize_maint_cmds): Initialise > maintenanceflushlist. > * regcache.c: Add 'cli/cli-cmds.h' include. > (reg_flush_command): Add header comment. > (_initialize_regcache): Create new 'maint flush register-cache' > command, make 'flushregs' an alias. > * symtab.c: Add 'cli/cli-cmds.h' include. > (_initialize_symtab): Create new 'maint flush symbol-cache' > command, make old command an alias. > > gdb/doc/ChangeLog: > > * gdb.texinfo (Symbols): Document 'maint flush symbol-cache'. > (Maintenance Commands): Document 'maint flush register-cache'. > > gdb/testsuite/ChangeLog: > > * gdb.base/c-linkage-name.exp: Update to use new 'maint flush ...' > commands. > * gdb.base/killed-outside.exp: Likewise. > * gdb.opt/inline-bt.exp: Likewise. > * gdb.perf/gmonster-null-lookup.py: Likewise. > * gdb.perf/gmonster-print-cerr.py: Likewise. > * gdb.perf/gmonster-ptype-string.py: Likewise. > * gdb.python/py-unwind.exp: Likewise. > --- > gdb/ChangeLog | 15 ++++++++++++++ > gdb/NEWS | 5 +++++ > gdb/cli/cli-cmds.c | 4 ++++ > gdb/cli/cli-cmds.h | 4 ++++ > gdb/doc/ChangeLog | 5 +++++ > gdb/doc/gdb.texinfo | 20 +++++++++++++++---- > gdb/maint.c | 5 +++++ > gdb/regcache.c | 11 ++++++++-- > gdb/symtab.c | 8 ++++++-- > gdb/testsuite/ChangeLog | 11 ++++++++++ > gdb/testsuite/gdb.base/c-linkage-name.exp | 2 +- > gdb/testsuite/gdb.base/killed-outside.exp | 2 +- > gdb/testsuite/gdb.opt/inline-bt.exp | 2 +- > .../gdb.perf/gmonster-null-lookup.py | 2 +- > gdb/testsuite/gdb.perf/gmonster-print-cerr.py | 2 +- > .../gdb.perf/gmonster-ptype-string.py | 2 +- > gdb/testsuite/gdb.python/py-unwind.exp | 2 +- > 17 files changed, 87 insertions(+), 15 deletions(-) > > diff --git a/gdb/NEWS b/gdb/NEWS > index d1f721c3953..b07b054223f 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -27,6 +27,11 @@ set debug event-loop > show debug event-loop > Control the display of debug output about GDB's event loop. > > +maintenance flush symbol-cache > +maintenance flush register-cache > + These new commands are equivalent to the already existing command > + 'maintenance flush-symbol-cache' and 'flushregs' respectively. > + Not a native speaker, but "already existing" sounds a bit verbose. Would existing suffice? Also, probably "existing commands" rather than "existing command"? > * Changed commands > > break [PROBE_MODIFIER] [LOCATION] [thread THREADNUM] > diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c > index 54822fad481..88c83cd6319 100644 > --- a/gdb/cli/cli-cmds.c > +++ b/gdb/cli/cli-cmds.c > @@ -151,6 +151,10 @@ struct cmd_list_element *maintenanceprintlist; > > struct cmd_list_element *maintenancechecklist; > > +/* Chain containing all defined "maintenance flush" subcommands. */ > + > +struct cmd_list_element *maintenanceflushlist; > + > struct cmd_list_element *setprintlist; > > struct cmd_list_element *showprintlist; > diff --git a/gdb/cli/cli-cmds.h b/gdb/cli/cli-cmds.h > index 1d641520bf5..976cea07806 100644 > --- a/gdb/cli/cli-cmds.h > +++ b/gdb/cli/cli-cmds.h > @@ -89,6 +89,10 @@ extern struct cmd_list_element *maintenanceinfolist; > > extern struct cmd_list_element *maintenanceprintlist; > > +/* Chain containing all defined "maintenance flush" subcommands. */ > + > +extern struct cmd_list_element *maintenanceflushlist; > + > extern struct cmd_list_element *setprintlist; > > extern struct cmd_list_element *showprintlist; > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index 01dcac941c2..3745aa52973 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -19503,12 +19503,17 @@ > Print symbol cache usage statistics. > This helps determine how well the cache is being utilized. > > +@kindex maint flush symbol-cache > @kindex maint flush-symbol-cache > @cindex symbol cache, flushing > +@item maint flush symbol-cache > @item maint flush-symbol-cache > -Flush the contents of the symbol cache, all entries are removed. > -This command is useful when debugging the symbol cache. > -It is also useful when collecting performance data. > +Flush the contents of the symbol cache, all entries are removed. This > +command is useful when debugging the symbol cache. It is also useful > +when collecting performance data. The command @code{maint > +flush-symbol-cache} is an alias for @code{maint flush symbol-cache} > +that exists for backward compatibility with older versions of > +@value{GDBN}. > > @end table > > @@ -38859,9 +38864,16 @@ > restore internal > @end smallexample > > +@kindex maint flush register-cache > @kindex flushregs > +@cindex register cache, flushing > +@item maint flush register-cache > @item flushregs > -This command forces @value{GDBN} to flush its internal register cache. > +Flush the contents of the register cache and as a consequence the > +frame cache. This command is useful when debugging issues related to > +register fetching, or frame unwinding. The command @code{flushregs} > +is an alias for @code{maint flush register-cache} that exists for > +backward compatibility with older versions of @value{GDBN}. > > @kindex maint print objfiles > @cindex info for known object files > diff --git a/gdb/maint.c b/gdb/maint.c > index e8cdda3da0e..5b83cf2ae72 100644 > --- a/gdb/maint.c > +++ b/gdb/maint.c > @@ -1083,6 +1083,11 @@ lists all sections from all object files, including shared libraries."), > &maintenanceprintlist, "maintenance print ", 0, > &maintenancelist); > > + add_basic_prefix_cmd ("flush", class_maintenance, > + _("Maintenance command for flushing GDB internal caches."), > + &maintenanceflushlist, "maintenance flush ", 0, > + &maintenancelist); > + > add_basic_prefix_cmd ("set", class_maintenance, _("\ > Set GDB internal variables used by the GDB maintainer.\n\ > Configure variables internal to GDB that aid in GDB's maintenance"), > diff --git a/gdb/regcache.c b/gdb/regcache.c > index 91d3202b94b..7985df0a776 100644 > --- a/gdb/regcache.c > +++ b/gdb/regcache.c > @@ -30,6 +30,7 @@ > #include "observable.h" > #include "regset.h" > #include > +#include "cli/cli-cmds.h" > > /* > * DATA STRUCTURE > @@ -1382,6 +1383,8 @@ regcache::debug_print_register (const char *func, int regno) > fprintf_unfiltered (gdb_stdlog, "\n"); > } > > +/* Implement 'maint flush register-cache' command. */ > + Implement "the ... command"? > static void > reg_flush_command (const char *command, int from_tty) > { > @@ -2082,8 +2085,12 @@ _initialize_regcache () > gdb::observers::target_changed.attach (regcache_observer_target_changed); > gdb::observers::thread_ptid_changed.attach (regcache_thread_ptid_changed); > > - add_com ("flushregs", class_maintenance, reg_flush_command, > - _("Force gdb to flush its register cache (maintainer command).")); > + add_cmd ("register-cache", class_maintenance, reg_flush_command, > + _("Force gdb to flush its register and frame cache."), > + &maintenanceflushlist); > + /* This alias exists for backwards compatibility. */ > + add_com_alias ("flushregs", "maintenance flush register-cache", > + class_maintenance, 0); > > #if GDB_SELF_TEST > selftests::register_test ("get_thread_arch_aspace_regcache", > diff --git a/gdb/symtab.c b/gdb/symtab.c > index dccc3d1e237..b7d0e70f4a7 100644 > --- a/gdb/symtab.c > +++ b/gdb/symtab.c > @@ -42,6 +42,7 @@ > #include "addrmap.h" > #include "cli/cli-utils.h" > #include "cli/cli-style.h" > +#include "cli/cli-cmds.h" > #include "fnmatch.h" > #include "hashtab.h" > #include "typeprint.h" > @@ -6929,10 +6930,13 @@ If zero then the symbol cache is disabled."), > _("Print symbol cache statistics for each program space."), > &maintenanceprintlist); > > - add_cmd ("flush-symbol-cache", class_maintenance, > + add_cmd ("symbol-cache", class_maintenance, > maintenance_flush_symbol_cache, > _("Flush the symbol cache for each program space."), > - &maintenancelist); > + &maintenanceflushlist); > + /* This alias exists for backwards compatibility. */ > + add_alias_cmd ("flush-symbol-cache", "flush symbol-cache", class_maintenance, 0, > + &maintenancelist); > > gdb::observers::executable_changed.attach (symtab_observer_executable_changed); > gdb::observers::new_objfile.attach (symtab_new_objfile_observer); > diff --git a/gdb/testsuite/gdb.base/c-linkage-name.exp b/gdb/testsuite/gdb.base/c-linkage-name.exp > index 6b0a014949e..c6b3b509df6 100644 > --- a/gdb/testsuite/gdb.base/c-linkage-name.exp > +++ b/gdb/testsuite/gdb.base/c-linkage-name.exp > @@ -64,7 +64,7 @@ gdb_test "maint info symtabs" "\{ symtab \[^\r\n\]*c-linkage-name-2.c.*" > > # Flush the symbol cache to prevent the lookup to return the same as before. > > -gdb_test "maint flush-symbol-cache" > +gdb_test "maint flush symbol-cache" > > # Try to print MUNDANE using its linkage name again, after partial > # symtab expansion. > diff --git a/gdb/testsuite/gdb.base/killed-outside.exp b/gdb/testsuite/gdb.base/killed-outside.exp > index 3e20ad67cee..645b41f4867 100644 > --- a/gdb/testsuite/gdb.base/killed-outside.exp > +++ b/gdb/testsuite/gdb.base/killed-outside.exp > @@ -115,7 +115,7 @@ with_test_prefix "stepi" { > # other commands would trigger. > with_test_prefix "registers" { > test { > - gdb_test "flushregs" ".*" > + gdb_test "maint flush register-cache" ".*" > gdb_test "info threads" ".*" > } > } > diff --git a/gdb/testsuite/gdb.opt/inline-bt.exp b/gdb/testsuite/gdb.opt/inline-bt.exp > index d428c396359..109627c2306 100644 > --- a/gdb/testsuite/gdb.opt/inline-bt.exp > +++ b/gdb/testsuite/gdb.opt/inline-bt.exp > @@ -61,7 +61,7 @@ gdb_test "info frame" ".*inlined into frame.*" "func2 inlined (3)" > # function. > gdb_test_no_output "set backtrace limit 2" > # Force flushing the frame cache. > -gdb_test "flushregs" "Register cache flushed." > +gdb_test "maint flush register-cache" "Register cache flushed." > gdb_test "up" "#1 .*func1.*" "up from bar (4)" > gdb_test "info frame" ".*in func1.*" "info frame still works" > # Verify the user visible limit works as expected. > diff --git a/gdb/testsuite/gdb.perf/gmonster-null-lookup.py b/gdb/testsuite/gdb.perf/gmonster-null-lookup.py > index eaf4b11c9f8..f4ce1ea55e9 100644 > --- a/gdb/testsuite/gdb.perf/gmonster-null-lookup.py > +++ b/gdb/testsuite/gdb.perf/gmonster-null-lookup.py > @@ -40,7 +40,7 @@ class NullLookup(perftest.TestCaseWithBasicMeasurements): > utils.safe_execute("mt expand-symtabs") > iteration = 5 > while iteration > 0: > - utils.safe_execute("mt flush-symbol-cache") > + utils.safe_execute("mt flush symbol-cache") > func = lambda: utils.safe_execute("p symbol_not_found") > self.measure.measure(func, run) > iteration -= 1 > diff --git a/gdb/testsuite/gdb.perf/gmonster-print-cerr.py b/gdb/testsuite/gdb.perf/gmonster-print-cerr.py > index 796380dcacd..adee2e601aa 100644 > --- a/gdb/testsuite/gdb.perf/gmonster-print-cerr.py > +++ b/gdb/testsuite/gdb.perf/gmonster-print-cerr.py > @@ -46,7 +46,7 @@ class PrintCerr(perftest.TestCaseWithBasicMeasurements): > utils.runto_main() > iteration = 5 > while iteration > 0: > - utils.safe_execute("mt flush-symbol-cache") > + utils.safe_execute("mt flush symbol-cache") > func = lambda: utils.safe_execute("print gm_std::cerr") > self.measure.measure(func, run) > iteration -= 1 > diff --git a/gdb/testsuite/gdb.perf/gmonster-ptype-string.py b/gdb/testsuite/gdb.perf/gmonster-ptype-string.py > index 78fa3dfd432..aa5513547e5 100644 > --- a/gdb/testsuite/gdb.perf/gmonster-ptype-string.py > +++ b/gdb/testsuite/gdb.perf/gmonster-ptype-string.py > @@ -41,7 +41,7 @@ class GmonsterPtypeString(perftest.TestCaseWithBasicMeasurements): > utils.safe_execute("mt expand-symtabs") > iteration = 5 > while iteration > 0: > - utils.safe_execute("mt flush-symbol-cache") > + utils.safe_execute("mt flush symbol-cache") > func1 = lambda: utils.safe_execute("ptype hello") > func = lambda: utils.run_n_times(2, func1) > self.measure.measure(func, run) > diff --git a/gdb/testsuite/gdb.python/py-unwind.exp b/gdb/testsuite/gdb.python/py-unwind.exp > index 4ef63bf965c..e8ae8632a8e 100644 > --- a/gdb/testsuite/gdb.python/py-unwind.exp > +++ b/gdb/testsuite/gdb.python/py-unwind.exp > @@ -57,4 +57,4 @@ gdb_test_sequence "where" "Backtrace restored by unwinder" { > } > > # Check that the Python unwinder frames can be flushed / released. > -gdb_test "flushregs" "Register cache flushed\\." "flush frames" > +gdb_test "maint flush register-cache" "Register cache flushed\\." "flush frames" > Otherwise LGTM.