From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by sourceware.org (Postfix) with ESMTPS id 688893890005 for ; Fri, 13 Aug 2021 13:36:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 688893890005 X-IronPort-AV: E=McAfee;i="6200,9189,10074"; a="212444742" X-IronPort-AV: E=Sophos;i="5.84,319,1620716400"; d="scan'208";a="212444742" Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Aug 2021 06:36:37 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.84,319,1620716400"; d="scan'208";a="571803015" Received: from fmsmsx602.amr.corp.intel.com ([10.18.126.82]) by orsmga004.jf.intel.com with ESMTP; 13 Aug 2021 06:36:37 -0700 Received: from fmsmsx606.amr.corp.intel.com (10.18.126.86) by fmsmsx602.amr.corp.intel.com (10.18.126.82) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.10; Fri, 13 Aug 2021 06:36:36 -0700 Received: from fmsedg601.ED.cps.intel.com (10.1.192.135) by fmsmsx606.amr.corp.intel.com (10.18.126.86) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.10 via Frontend Transport; Fri, 13 Aug 2021 06:36:36 -0700 Received: from NAM10-BN7-obe.outbound.protection.outlook.com (104.47.70.109) by edgegateway.intel.com (192.55.55.70) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2242.10; Fri, 13 Aug 2021 06:36:36 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=XxLqbwZqDeyUEyL/NyeRYGebaMbr373+LYxis79r0xC1oi44bZGXkGCD1sIMGQsLyujdDY8ehpYl6uyfYZ/vXsEJMkESfsWGA1R7rFIbFNM+ZtMeFoPaxOBMcAPJr9EfHojHKU98idR2D/WR+4f2QOQcSpWuL4MqLg9fLbxY07VjlG74o7kV7YTsdQTF4iUN1Ad83SzsvVmyBfdwY2p4CrzZZT2mIsrLhqBExXzP0nRCCQeOjnJ6pByVkRYyjhSl41MEJrx6VC91KEsUZcxcjChhZcinwiGOkiZDcaiPcpm87iPnMdUef9NXYn0Dbm7fQrNBi0AOF0++ZHejRsAVEg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=dbx1vgPQEtL+sRX4h0TD5fcda0+GtywkTpnx3tRH2/4=; b=Y8X0ii6n5sQMfFy0NJGgu/bJYgu1DcJfjMehNxM0R0XBWGtII0jnY3hFB4BQd0t0BzQxoejmIduixKXdOrRjz4eLM+MZEe+SWvJxa41B0gEKRp9kpf1MSJOjadV/g2GOeJOyE5soX+CtVXedjIW+jcmpibnM2DxqcGGVcX2lTkMtg0oBfjjcJakxd+t/oxZzIQw9F/bFIPQbvKZttFOXez1Brh5WTXEIhIn8kZVTbS1G/uNtN1CETCX7nJKjlF0lQ9TbIg0EXVzf2wOuYMe0WDVAwyLj8t6hYEQv3RyVMJsCv3IrgAdEJh+3xUEj4yBkhZVtvC3bdz/nd0irhh3iCA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Received: from DM8PR11MB5749.namprd11.prod.outlook.com (2603:10b6:8:10::15) by DM6PR11MB4219.namprd11.prod.outlook.com (2603:10b6:5:14e::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4394.20; Fri, 13 Aug 2021 13:36:34 +0000 Received: from DM8PR11MB5749.namprd11.prod.outlook.com ([fe80::392c:516a:cc52:963]) by DM8PR11MB5749.namprd11.prod.outlook.com ([fe80::392c:516a:cc52:963%9]) with mapi id 15.20.4415.016; Fri, 13 Aug 2021 13:36:34 +0000 From: "Metzger, Markus T" To: "Willgerodt, Felix" CC: "gdb-patches@sourceware.org" Subject: RE: [PATCH v3 12/12] btrace: Extend ptwrite event decoding. Thread-Topic: [PATCH v3 12/12] btrace: Extend ptwrite event decoding. Thread-Index: AQHXYoPIA11xeH3HqkW7RfzJKFSQaqtxucgQ Date: Fri, 13 Aug 2021 13:36:34 +0000 Message-ID: References: <20210616074205.1129553-1-felix.willgerodt@intel.com> <20210616074205.1129553-13-felix.willgerodt@intel.com> In-Reply-To: <20210616074205.1129553-13-felix.willgerodt@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-reaction: no-action dlp-version: 11.5.1.3 dlp-product: dlpe-windows x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 9d96b4f4-192b-44c4-6135-08d95e5f5e64 x-ms-traffictypediagnostic: DM6PR11MB4219: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: EVO/0GrEpAlkcE9oZDhy5lbn5jt3Tkcu7OL9p92G2MkS32xN+QgA0VapqijdkodnXvkEhvMrVUaNwrCTN9Du0oPec0vA6oY14+ChEHIxaRza62XrN0OeOlW+ZH8KvsRwqF/H57uheTWmucI9rk6F8wNmYn0Fw4hYYMmHsNhusSDdV793x3MLzeLX0hglDl+BAWVQng3AYZ3gAM9ayUXYPn/VcGjiUXv1NfEaRwedH2mFmqRg1TpYGxfOehgc7I9G5l91Ez0cyWWtoh4S97FbnqK2IIwe+MhyWNhR48B4EBVuBxTVa7RWCX529bAfLAVnbfbhb1PudztTzMgvlX7I4Zikl4/uE3zK8SmTQKHee9uEowij7UWLArhU9KL7LDWX8bnZRjBrIIpdhqFxmctu/IloY8kEZbsu8w84ug8bGL1BU3RxuBJ8jwG5fqObYB3Pd2zp6d6O5iKlI5plXxjsEKIMb4QptKvEpItOc2Z6vqkf3vX4AtMugC9skrMly/KFez4Aqg6h28D+cxaMHICZVupMuM/v52vGoGmLvaMxNt6ulBNiFJmgNtDH+CidcNb9horvo2zTYY4K8dzYDrSU5ns4FaPLZBZ4BfvqGrcD3EptX8tSGspHYGptfyYhG3NoNphzFK0zi7A77TF8Pmt4KixWYzr9AT/hkwtGmJt36Kmtn7WQRByj0E20VEp+11wklUHrRPOwxloQN93ZG9Kneg== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM8PR11MB5749.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(396003)(376002)(366004)(346002)(136003)(39860400002)(64756008)(7696005)(66476007)(8936002)(66556008)(76116006)(478600001)(26005)(5660300002)(6506007)(8676002)(86362001)(66446008)(66946007)(186003)(83380400001)(6636002)(33656002)(122000001)(52536014)(6862004)(2906002)(38070700005)(55016002)(4326008)(38100700002)(71200400001)(9686003)(316002); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?2Twx/5WELD941gypAHJyGWT5hfsKXXmx8ebN3qdE7/L6Lrornr01A8yHAC0h?= =?us-ascii?Q?ISALTLefleUq9lV8jFS2cC0dO3UUr2doGrNqu1Ftf3CYCjba6B+4a3PCBdtw?= =?us-ascii?Q?64bOkCtCuzMKxJyx99lyP6OWb5ivaj6nipKNeBnFgh1ITnEV4ShTld0sZV7r?= =?us-ascii?Q?dD8hWaeWPCA7IZ5+wRuwn2Ouu+wu3cxv8iiycJJ5veiIYNeExrmV+fbIhVJ5?= =?us-ascii?Q?NrdHGixlFo9EbcAkPj6YrHOgJgIO1oBHUnrOSnTTj17QnZY3cCFqZG1+ZxLn?= =?us-ascii?Q?ovZweowE3308nJW3j8rx+rpuNHzPnazeRYRndFszkJx6D284TJz3w5M/B8kk?= =?us-ascii?Q?sNxZ17E0plr1wdxbW0M+EzJyH7aOSCsJfCU7tptYbBN+Uhi4sK9efGqvjQ0F?= =?us-ascii?Q?o1MIYJPmth2xZnfsNEKdHDUBH3qE89NTGLRgNl9oMxBpnsVN6WvVHCXWPOJz?= =?us-ascii?Q?s9CGOnihHpjRjRHMHLg7kn5ceyHDrf3DdgNOJHe/JToO6UI/Pz9vKB7YMMPz?= =?us-ascii?Q?KSKS1n6djh4Wa3SX5BVW/rCJelaLmrnZ/8U4ENiYyB6BM/G9e0Q+JMI067ia?= =?us-ascii?Q?SVy4kJW8+tyj6vC5ELt6G4uWJ6IKK8b6uREzwrvU0MjeEa6m7FefemyXcR8k?= =?us-ascii?Q?eAI5+zPC4vnY/dtXyv19Ra9niE40FcikWhOyJrIy3Vv+IuXuqT7OYfqYl4mk?= =?us-ascii?Q?Vb4/Y6jo5GaNXXBnYemw5wcrh+AoDXntjNs6vUhrEKVcCRp36EFn1N5b0ZZi?= =?us-ascii?Q?w8Xt4z1d4n/mPzXn55odPpMcxigTADuhN1GWif/ADNoOTLEuhEPTNyyozvBM?= =?us-ascii?Q?eGgTOKgqIBHPRFK2e75I6uRE35CEx6SFRIF9FuErmLLNCpeo0jBWU0vW93wM?= =?us-ascii?Q?ptcCO/9adoEYkjk8wvOGnC3+H9PzFba9k5D6ISeYKaGxrSJpy3zE6hgrCLe2?= =?us-ascii?Q?KGYftXwURQ2l6zKzqCgGG8Oqs0LOOEkQ/RPy1uTOq6drZY1o9mCYYwlGSxxx?= =?us-ascii?Q?lfLNoNIDLON0tavT0xkY5xGV/TDcoOz9Gy+kexWtPMYMvBKDM+RGCXgGtAqb?= =?us-ascii?Q?QOApS1Vrzn9LS1d4hAAWaUxrj01EtFFiLMTjNbfH6e9AslR+yy3BLuPiLMCP?= =?us-ascii?Q?w87lGUaCVgFciO67FE7UC63rlp9lIaReTgVQU2FuducWG3VtkkTdW9rocf3M?= =?us-ascii?Q?HFuDRqNbEdC/FEv+SehjPwO2Fr+QPEWxIgROaHFBW7xbEZ9n05jCz5HoZ3gF?= =?us-ascii?Q?UjgP5aDqYyU9iBx5+6B/NQx1WSo97xTzKNJcteFVkSFqDE7r2dE0kiFM2GLT?= =?us-ascii?Q?XxY=3D?= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM8PR11MB5749.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 9d96b4f4-192b-44c4-6135-08d95e5f5e64 X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Aug 2021 13:36:34.7171 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: 0CGZxnOEN/yMzS7cmhpT2dHV4lRlUuKZly6YI2vKO9theVEaIwymz8MoHuDQdXMQWW+7XiZXcRXGO2xlf0iMvzW2SHSYVoRiFhsRWDCWB1Y= X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB4219 X-OriginatorOrg: intel.com Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00, DKIM_INVALID, DKIM_SIGNED, KAM_DMARC_NONE, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=no 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, 13 Aug 2021 13:36:50 -0000 Thanks, Felix, >+ case ptev_ptwrite: >+ { >+ uint64_t *ip =3D nullptr; >+ gdb::unique_xmalloc_ptr ptw_string =3D nullptr; >+ struct btrace_insn ptw_insn; >+ btrace_insn_flags flags; We're meanwhile declaring variables where they are initialized and used. T= he rest of the code has not been updated but new code should follow this. >+ >+ /* Lookup the ip if available. */ >+ if (event.ip_suppressed =3D=3D 0) >+ ip =3D &event.variant.ptwrite.ip; >+ else if (!btinfo->functions.empty () >+ && !btinfo->functions.back ().insn.empty ()) >+ ip =3D &btinfo->functions.back ().insn.back ().pc; This isn't necessary; libipt will fill in the IP. It suffices to check for= EVENT.IP_SUPPRESSED. >+ >+ if (btinfo->ptw_callback_fun !=3D nullptr) This is probably the first check we should do, i.e. if it is nullptr, break= right at the beginning. >+ ptw_string =3D btinfo->ptw_callback_fun ( >+ &event.variant.ptwrite.payload, ip, >+ btinfo->ptw_listener); The indentation looks a bit odd. You may break before '=3D btinfo->...' i= f the line is getting too long, otherwise. >+ >+ if (ptw_string =3D=3D nullptr) >+ break; >+ >+ btinfo->aux_data.emplace_back (ptw_string.get ()); >+ >+ if (!btinfo->functions.empty () >+ && !btinfo->functions.back ().insn.empty ()) >+ flags =3D btinfo->functions.back ().insn.back ().flags; >+ else >+ flags =3D 0; >+ >+ /* Update insn list with ptw payload insn. */ We should probably start by memset()ing ptw_insn to all-zero. >+If an inferior uses the instruction, @value{GDBN} inserts the raw payload= value Maybe add 'by default' to stress that this behavior can be overwritten, whi= ch is documented below. >+load_lib gdb-python.exp >+ >+if { [skip_btrace_pt_tests] } { >+ unsupported "Target does not support record btrace pt." >+ return -1 >+} >+ >+if { [skip_btrace_ptw_tests] } { >+ unsupported "Hardware does not support ptwrite instructions." >+ return -1 >+} >+ >+# Test libipt version (must be >=3D 2.0.0). >+if {[require_libipt_version 2 0 0]} { >+ unsupported "Libipt doesn't support ptwrite decoding." >+ return -1 >+} We shouldn't need to check the patch version. See comments on a previous p= atch regarding feature checking. We can probably combine it with skip_btra= ce_ptw_tests. >+### 1. Default testrun >+ >+# Setup recording >+gdb_test_no_output "set record instruction-history-size unlimited" "Defau= lt: set >unlimited" >+gdb_test_no_output "record btrace pt" "Default: record btrace pt" >+gdb_test "next" ".*" "Default: first next" >+gdb_test "next" ".*" "Default: second next" You may use with_test_prefix to add the "Default: " prefix to the test outp= ut. This groups tests and makes it a bit more readable IMHO. For the abov= e group, the first two wouldn't need any additional test name string. And = if you combined the latter two into "next 2", it wouldn't either. >+# Test payload printing during stepping >+gdb_test "record goto 10" "No such instruction\." >+gdb_test "record goto 9" ".*ptwrite64.* at .*ptwrite.c:23.*" >+gdb_test "stepi" ".*\\\[42\\\].*" >+gdb_test "reverse-stepi" ".*\\\[42\\\].*" >+gdb_test "continue" ".*\\\[42\\\].*\\\[43\\\].*" >+gdb_test "reverse-continue" ".*\\\[43\\\].*\\\[42\\\].*" Between [42] and [43] we wouldn't get any other output we need to ignore, w= ould we? There's multi_line to capture multiple lines of output. >+# Test auxiliary type in python >+gdb_test_multiline "auxiliary type in python" \ >+ "python" "" \ >+ "h =3D gdb.current_recording().instruction_history" "" \ >+ "for insn in h: print(insn)" "" \ >+ "end" [multi_line \ >+ ".*RecordAuxiliary.*" \ >+ ".*RecordAuxiliary.*" \ >+ ] Could we inspect the auxiliary records to see if we get the correct strings= ? Also, it would be nice to print the surrounding instructions instead of = ignoring all the output. If we just printed the instruction_history, wouldn't we get output very sim= ilar to the CLI? >+### 2. Test listener registration >+### 2.1 Custom listener >+ >+gdb_test_multiline "Custom: register listener in python" \ >+ "python" "" \ >+ "def my_listener(payload, ip):" "" \ >+ " if payload =3D=3D 66:" "" \ >+ " return \"payload: {0}, ip: {1:#x}\".format(payload, ip)" "" \ >+ " else:" "" \ >+ " return None" "" \ >+ "import gdb.ptwrite" "" \ >+ "gdb.ptwrite.register_listener(my_listener)" "" \ >+ "end" "" >+ >+gdb_test "record instruction-history 1" [multi_line \ >+ ".*\[0-9\]+\t $hex :\tptwrite %\[a-z\]+" \ >+ "\[0-9\]+\t \\\[payload: 66, ip: $hex\\\]" \ >+ ".*\[0-9\]+\t $hex :\tptwrite %\[a-z\]+" \ >+ "\[0-9\]+\t $hex :.*" \ What output do we need to ignore here at the beginning of every other line? >+# Run a test on the target to see if it supports ptwrite instructions. >+# Return 0 if so, 1 if it does not. Based on 'check_vmx_hw_available' >+# from the GCC testsuite. See above comments. I think we should simply record a single ptwrite insid= e main() and check that GDB can decode the trace without errors - and check= 'info record' that we actually decoded something. Regards, Markus. Intel Deutschland GmbH Registered Address: Am Campeon 10, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva = Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928