From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from eggs.gnu.org (eggs.gnu.org [IPv6:2001:470:142:3::10]) by sourceware.org (Postfix) with ESMTPS id 68583395253F for ; Fri, 6 May 2022 12:04:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 68583395253F Received: from fencepost.gnu.org ([2001:470:142:3::e]:43426) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nmwhR-0002vN-R9; Fri, 06 May 2022 08:04:45 -0400 Received: from [87.69.77.57] (port=1130 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nmwhJ-0000ur-2q; Fri, 06 May 2022 08:04:42 -0400 Date: Fri, 06 May 2022 15:04:25 +0300 Message-Id: <837d6y27g6.fsf@gnu.org> From: Eli Zaretskii To: Felix Willgerodt Cc: markus.t.metzger@intel.com, gdb-patches@sourceware.org In-Reply-To: <20220506114010.134106-10-felix.willgerodt@intel.com> (message from Felix Willgerodt via Gdb-patches on Fri, 6 May 2022 13:40:10 +0200) Subject: Re: [PATCH v4 10/10] btrace: Extend ptwrite event decoding. References: <20220506114010.134106-1-felix.willgerodt@intel.com> <20220506114010.134106-10-felix.willgerodt@intel.com> X-Spam-Status: No, score=-7.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_BARRACUDACENTRAL, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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, 06 May 2022 12:04:49 -0000 > Date: Fri, 6 May 2022 13:40:10 +0200 > From: Felix Willgerodt via Gdb-patches > > diff --git a/gdb/NEWS b/gdb/NEWS > index a72fee81550..cf807d6209e 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -331,6 +331,12 @@ GNU/Linux/OpenRISC or1k*-*-linux* > > *** Changes in GDB 11 > > +* GDB now supports printing of ptwrite payloads from the Intel Processor > + Trace during 'record instruction-history', 'record function-call-history', > + all stepping commands and in Python. Printing is customizable via a > + ptwrite listener function in Python. By default, the raw ptwrite > + payload is printed for each ptwrite that is encountered. > + This part is OK. > +@exdent @value{GDBN} output after recording the sample program in pt format: > +@smallexample > +@group > +(gdb) record instruction-history 12,14 > +12 0x000000000040074c : ptwrite %rbx > +13 [42] > +14 0x0000000000400751 : mov -0x8(%rbp),%rbx The last line is too long, and will cause overfull hbox errors when producing the printed manual. Please make it shorter, or break into 2 lines. > +@findex gdb.ptwrite.register_listener > +@defun register_listener (@var{listener}) @defun automatically inserts the function into the index, so no need for a separate @findex (here and elsewhere in the patch). > +Used to register the ptwrite listener. The listener can be any callable > +object that accepts two arguments. I think we should describe those 2 arguments, at least shortly. > +@defun default_listener (@var{payload}, @var{ip}) > +The listener function active by default. Which does what? If we don't say anything about the default behavior, how can a user decide whether to provide his/her specialized listener? > +@smallexample > +@group > +(gdb) python-interactive > +>>> class my_listener(object): > +... def __init__(self): > +... self.var = 0 > +... def __call__(self, payload, ip): > +... if gdb.selected_thread().global_num == 1: > +... self.var += 1 > +... return "counter: @{@}, ip: @{:#x@}".format(self.var, ip) This last line is too long. (And I wonder whether this example really adds something of value to what you already described above.) > +@group > +(gdb) info threads > +* 1 Thread 0x7ffff7fd8740 (LWP 25796) "ptwrite_threads" task (arg=0x0) > + at bin/ptwrite/ptwrite_threads.c:45 > + 2 Thread 0x7ffff6eb8700 (LWP 25797) "ptwrite_threads" task (arg=0x0) > + at bin/ptwrite/ptwrite_threads.c:45 These lines are also too long. > +This @value{GDBN} feature is dependent on hardware and operating system > +support and requires the Intel Processor Trace decoder library in version > +2.0.0 or newer. You say "hardware and operating system support", but say nothing about the OSes on which this can be supported. Thanks.