From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from de-smtp-delivery-102.mimecast.com (de-smtp-delivery-102.mimecast.com [194.104.111.102]) by sourceware.org (Postfix) with ESMTPS id DCA8E3857423 for ; Thu, 26 May 2022 15:15:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DCA8E3857423 Received: from EUR05-DB8-obe.outbound.protection.outlook.com (mail-db8eur05lp2113.outbound.protection.outlook.com [104.47.17.113]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id de-mta-26-KJ4CKYNGO2iFmLku8Xv0CQ-1; Thu, 26 May 2022 17:15:55 +0200 X-MC-Unique: KJ4CKYNGO2iFmLku8Xv0CQ-1 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=VugTjZ0z5ybkbA+kM2wGDfbic1RlQP9x5l1sKR5UV0QIYyjhth7qo2VzryYSMte7daNdVkpXovqkhIAiRKfkgG4KkZEzlO55fyjVHnHj75AT8ev3J0q+3WHJ5IjDTgeD3n2szy2mCXP5pBkvPdVNfXhohfLahPv+U8MZa5OqS0f5bsJpVbuVUbOWBSDvxpV+KRkqPcKwQIQOmBXL8TFYKszVZgvPnEq2LYpPRjc6cx4rYnPiQmw6BT7LnX+cIqpP3sP0MT6m51V2/XkhCsk3iLbhcz2JOelJ7sMF2HfzqSJJ2FVP6RLjxfaWqAKa6wpi49X59oo9VLFJHltZJrICOA== 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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=Sl9AE9EFIU3c2dNxRXaTN0PadxU+CsIaTexcLQfd0gQ=; b=gp4MQd8VaRY5Xfw8RzcTS5jt+pB3MbBw3V++q0NgFwMX1y/MebNGHn7mWRFIbLNvaqA+XXvB1HQ5EztVU8zsSggcie1rtvxr1UGJ4jwcWXdA8Uug6/73jT08r9fea8is+Y0fvUlsYI0FZJE8BVMUdSqWOIWEBq0VNk4hzU/SNDPcwE55tUOyeI52wBDjJzJusvuljS0onFNL/+HX4X/BEC5pIG5cbjnTfRQQxJrTtMuE/3mQqmS/lzQl+wBhouLPnS977hYlRvWVgCStl1Gq87IjeL4w3Sk7b55XpeLjuLHJZkwRR+S6RvFynFaEvjHk2J0jOjppo3JVkAb/da6Ksg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none Received: from VE1PR04MB6560.eurprd04.prod.outlook.com (2603:10a6:803:122::25) by AM6PR04MB5992.eurprd04.prod.outlook.com (2603:10a6:20b:93::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5293.13; Thu, 26 May 2022 15:15:54 +0000 Received: from VE1PR04MB6560.eurprd04.prod.outlook.com ([fe80::dfa:a64a:432f:e26b]) by VE1PR04MB6560.eurprd04.prod.outlook.com ([fe80::dfa:a64a:432f:e26b%7]) with mapi id 15.20.5293.013; Thu, 26 May 2022 15:15:53 +0000 Message-ID: <692a257c-0ace-1ea3-0413-063543e32453@suse.com> Date: Thu, 26 May 2022 17:15:52 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [PATCH] opcodes/i386: remove trailing whitespace from insns with zero operands Content-Language: en-US To: Andrew Burgess References: <20220526124520.2650720-1-aburgess@redhat.com> Cc: binutils@sourceware.org From: Jan Beulich In-Reply-To: <20220526124520.2650720-1-aburgess@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: AS9PR06CA0560.eurprd06.prod.outlook.com (2603:10a6:20b:485::10) To VE1PR04MB6560.eurprd04.prod.outlook.com (2603:10a6:803:122::25) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: c1837ee2-3dcc-4f6a-b225-08da3f2aa045 X-MS-TrafficTypeDiagnostic: AM6PR04MB5992:EE_ X-Microsoft-Antispam-PRVS: X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 760Kd6eWKRRAHPZMHWX/V0jXRVNYYKE4otz/y8McRcyaeiRGvHhgTTBxTnccaAQzS2Q6gDbS2R/l+djT4MShpsnh0wRAlcYRfqJ9rAZ3/+9QJ2gtFUMz5ZrUz5XxZ8WvzHlHrst+B6KVJ1EovdIl3Vt2UqBnA9Hhc/iBJPehzJu0vqvrEXh5uBZTEdtQGb19Rlk3eEal3apv/XqjnaDcVnKkkY5iPsMF1H7KQgfGqO6s8rDhnDJf0yO2/KvQTN5uBm2xIuTuOuVKjw/hLowygx8SHia8YRIUsafa5ZUHfo+tsw6XDnG6zYeMlmIWvaRdMit56i6CFV2Jn0Q12ELL4eExyi1Q2lznbmWW6wEs0UIfSwHnE1jPMJ+4R2iRvlewVtTX04nXBwIxJ7C5T7oBwEnYMN4iU4hovJ0NJKi+QSlgX6z5hrQYMreaFGPZRfs0cYbIbCpPR+OrJa2PZ/0q0ryIpSIYHdDNx232hVCRnNDEpHlhFx/QZ+DjiSNzPl6WP949hQ34IkafGeyINKJ4MsJOZ0GajOvnhXp9BRg46W2+QQlYKoC0YjrJyRoy3rwBJvgBlsB8JyNs85m2kgqUF52nyzhjyKTxlwHpmBSCEh5faJ2g0UDfdq+1R3wwvT/iwSqeu/uok5axzgJht8iMZz2N0BHP53WPj+FD/cXcS9GnJoAsHs6yHZwhKLnFp5+fJobdpc8UMSICZ6iNsjRsK0sEBFb+yd7HylLdwM0dp3I8shbPZQz4BECKVcwSf0D23Vzplx8mxxLgoi5qW06UhokmEeIWmB8av0TenqamUK/zBLvP4UF13CZFhkGnjAn0 X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:VE1PR04MB6560.eurprd04.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230001)(366004)(53546011)(31686004)(6506007)(316002)(38100700002)(6916009)(83380400001)(36756003)(66946007)(66476007)(8676002)(66556008)(2616005)(2906002)(186003)(4326008)(5660300002)(966005)(8936002)(86362001)(6512007)(31696002)(508600001)(6486002)(45980500001)(43740500002); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 2 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ckt0bmw3WUxmVWJnR09hQzRZc2twd3c5ZTJYZWI1UmJZSjl2cDZCRHdhTVhP?= =?utf-8?B?bFlrbFR5MGcwM2ovRzlTQmpMUHJuRlEyNmsvdHNwK05aL0h1QTVZQ3FTRXM1?= =?utf-8?B?Q2pJdUR3NzFVWVBYVktYdzFrSDkzcVgwVWRKM3pQd1NXWUpwcW5pZ1A2eU9k?= =?utf-8?B?OG51bUllVXlVRFZqNFh1VTRaZHBBYVdnRWJnaDAreWorSFRzSk1FRWNyL1gz?= =?utf-8?B?cEs1OUN3cmZmTkdDV0wwdEpualMyUlppS0pLMTNIcEhLODlEUGdvVEZhWXRZ?= =?utf-8?B?UmoyVVRkdVpqQ3F1Q3BoMmIrNkdRaGxCWGlRRER2UmlrVERGdUllRkxadVV3?= =?utf-8?B?YXhzVXVvMW1PaWJaMk9xTzZCeG1KUmJ6RmhTdjFuMTJhaW9saW81M2dWTXF6?= =?utf-8?B?aFovYjZpbW8zVFVObm9HSFRTQVFvWC9aVG1VZ1NBMG1hbk1tRVFQQm1NOEZ0?= =?utf-8?B?OU42cDNFWlBZbmJ1K01raytMM05iQzI5K1dqMkl0M05QSWNHLzIxNGNHWEtV?= =?utf-8?B?Q3NTb3NXYklzU1liVlF6UWN0eEdCMG1lblkwUE1CaWs4MWluOTV0MkpMM05V?= =?utf-8?B?YjVOVXJHWk9QNXgxbkcvWXNhbVNDT3MyczJvd2F4SnlEcHA5Q0F3aEI2QlVM?= =?utf-8?B?SUsxdGtab0pCTDEraG1BREZzMnppS2ljOS9HN09jNEZMUUV1cUFRRFpFZVJs?= =?utf-8?B?WU5OMVFUUEF2aCtMQzdvbFBXL3RGaS9iMTRnTVRnUi9VQm4zMlM5L01GU1JM?= =?utf-8?B?Uit3WTRwam04M1k3UGp6bVpHU01YQVpCRVlwL2Q3aGZiaGl6TEp4OTh2UEpO?= =?utf-8?B?ZUpOU01lMFE2Q0w1bWlPbktWdVQxc2hjdkxXdEpFcWFwdFJEalZrQVZ6UWdu?= =?utf-8?B?enpYQ1QraWVPMnpaNTFGaVh5M1o0dFpYbGEyU2JUcXJRb3FXY1ZtOGFEd21K?= =?utf-8?B?T3VMY2xaZitTNnVHakt0U28xd3Y5WmxJNDJ4RjNGM2hORmVXbDMrWkttUURE?= =?utf-8?B?Nm1hWk8vY3RrYk0rZDZyU1JGRTcwN1FpK04zMDRvclB5UkptSCtmZTJGL0Nv?= =?utf-8?B?YUVOeHg3UjBCNjhQdm43MVhORU45eGFSK3U5YUZNY1pzNk55QXM4VWQ5aGMw?= =?utf-8?B?aythSW03K25adForQkNGM0NiVEVtWkpESi8xaHg3eHgvdCtQTk1HaDBBYitz?= =?utf-8?B?aDJUSGNWWnhtZlB0VFgwSmZnWENPOStMbDlIQUtNZTNHZDNvL1U5cVlONWF0?= =?utf-8?B?clBZYmZsK2FKc0c0bGZsKzUrKzFZQ0hxSlBUL0Rwb2xPTURBOWdoay9mZm1p?= =?utf-8?B?K3VHUm1IQnFFU01YdFNJMnhRVWF3bFA2blQwRHA4SFk3YmsyZ2plWk1Pa2NK?= =?utf-8?B?dnBzR212ZStFaUtwUlpDcTI0UDIzYmNBZ3BDUDROeWlvRmFaMENLSm1qdUVr?= =?utf-8?B?c1FDMXVhRnFGb1ZQa3lUVk1qRzk1LzNrcWltQkZRWUNjMWRoQkh2TVRTVENI?= =?utf-8?B?dXhHSUJuUGFrMnFXaEh6eTVWOXJvSUpmMis4aDRySkdueC94dVRLQ0xaNVNQ?= =?utf-8?B?eGN6dXJnRVJPS2k2MWxkalR2VWhjRzBselEyUzR6R2ZqUDFFS25BNEVLMWRM?= =?utf-8?B?VkZBaFBkNlVuTzk0OE5oaUtkMUsvblU0N0orNTliblVHRWt1SitrVFYzK1Zt?= =?utf-8?B?bFN0NzBTTGdmTG9pUlNTRC85U2g2WWJVamJtamlzKzJnUmRqVnFYQWM5ZTMx?= =?utf-8?B?T2NXK3E5ZnV1Zk1uZnFGeHA2L2ZWL3NxL1dWd1JMejJGSHYvZkJTdWFkeDJm?= =?utf-8?B?c21mamRxM2VtR0oyNVJjVFFzR0RqYnZ5a0QybStjOEpRM1l0WGo2SHBBbW9p?= =?utf-8?B?YXRRbk1vdmV6Skh2VGxHUDkrbDFVQ05XS0JxaWh6TWNWL2JNWjJYdXIrYXlv?= =?utf-8?B?RkpPZjJaQmhKOHBPUEx5YVB2ZHhKcld5VDVBMTNSc0pWMUdCMkxONnZKRnJK?= =?utf-8?B?NklMRkZFc2VnbFBUY1FHczFEMlB4Z0cxUkhyNTBrb0dkeFc2dDJpSW4vdXhj?= =?utf-8?B?WWljdWo2ZmFvWlRjclY2RDJEdlZFVGMwWUg3d2o4NzR6T2liNU9LbFNQY0VZ?= =?utf-8?B?aGZXSHZvRUpMMDhXRkt5RlJLUzdzcFlqWGkvUG1Ra2JVQW5YVTAyOWprajZJ?= =?utf-8?B?bWtRSlpZVTMzOW5nZXRRbUl0elBSU2lUUUJuRklqK1EzbjFucXlKNldzVXJt?= =?utf-8?B?bXA5Z0hpRHdxNU1wdmQ2MTRiZXkyWlpGc1VtVTVEYjRHSFJaK3hKWkxndGFJ?= =?utf-8?B?T3ArU0Y4Nmtqakd6YnNQM25KYXdOZ3Z2TVNlL01MTVoxUzdOWG9TVUZLS3R4?= =?utf-8?Q?DSw+JJXccwn7PpzZHFa50XJEn7lECXRpEWJgQW8i2E853?= X-MS-Exchange-AntiSpam-MessageData-1: rCUJhvvcXX1rxw== X-OriginatorOrg: suse.com X-MS-Exchange-CrossTenant-Network-Message-Id: c1837ee2-3dcc-4f6a-b225-08da3f2aa045 X-MS-Exchange-CrossTenant-AuthSource: VE1PR04MB6560.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 May 2022 15:15:53.8163 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: NHXFb2eX2qqaGF7obLeYMyOIiuDqJ1fORaY1PM+Z1FfrQ8PYPzf1YMY63gWbujnu/Z++Ko2leLl5UIwTRhmDIg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR04MB5992 X-Spam-Status: No, score=-3032.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 26 May 2022 15:15:59 -0000 On 26.05.2022 14:45, Andrew Burgess via Binutils wrote: > While working on another patch[1] I had need to touch this code in > i386-dis.c: > > ins->obufp = ins->mnemonicendp; > for (i = strlen (ins->obuf) + prefix_length; i < 6; i++) > oappend (ins, " "); > oappend (ins, " "); > (*ins->info->fprintf_styled_func) > (ins->info->stream, dis_style_mnemonic, "%s", ins->obuf); > > What this code does is add whitespace after the instruction mnemonic > and before the instruction operands. > > The problem I ran into when working on this code can be seen by > assembling this input file: > > .text > nop > retq > > Now, when I disassemble, here's the output. I've replaced trailing > whitespace with '_' so that the issue is clearer: > > Disassembly of section .text: > > 0000000000000000 <.text>: > 0: 90 nop > 1: c3 retq___ > > Notice that there's no trailing whitespace after 'nop', but there are > three spaces after 'retq'! > > What happens is that instruction mnemonics are emitted into a buffer > instr_info::obuf, then instr_info::mnemonicendp is setup to point to > the '\0' character at the end of the mnemonic. > > When we emit the whitespace, this is then added starting at the > mnemonicendp position. Lets consider 'retq', first the buffer is > setup like this: > > 'r' 'e' 't' 'q' '\0' > > Then we add whitespace characters at the '\0', converting the buffer > to this: > > 'r' 'e' 't' 'q' ' ' ' ' ' ' '\0' > > However, 'nop' is actually an alias for 'xchg %rax,%rax', so, > initially, the buffer is setup like this: > > 'x' 'c' 'h' 'g' '\0' > > Then in NOP_Fixup we spot that we have an instruction that is an alias > for 'nop', and adjust the buffer to this: > > 'n' 'o' 'p' '\0' '\0' > > The second '\0' is left over from the original buffer contents. > However, when we rewrite the buffer, we don't afjust mnemonicendp, > which still points at the second '\0' character. > > Now, when we insert whitespace we get: > > 'n' 'o' 'p' '\0' ' ' ' ' ' ' ' ' '\0' > > Notice the whitespace is inserted after the first '\0', so, when we > print the buffer, the whitespace is not printed. > > The fix for this is pretty easy, I can change NOP_Fixup to adjust > mnemonicendp, but now a bunch of tests start failing, we now produce > whitespace after the 'nop', which the tests don't expect. > > So, I could update the tests to expect the whitespace.... > > ...except I'm not a fan of trailing whitespace, so I'd really rather > not. > > Turns out, I can pretty easily update the whitespace emitting code to > spot instructions that have zero operands and just not emit any > whitespace in this case. So this is what I've done. I appreciate this; I never understood what the trailing blanks were good for (and I hadn't noticed the NOP anomaly so far). I'll leave approving of the change to H.J., though, just indicating here that all looks good to me. > I've left in the fix for NOP_Fixup, I think updating mnemonicendp is > probably a good thing, though this is not really required any more. I agree with keeping it. > I've then updated all the tests that I saw failing to adjust the > expected patterns to account for the change in whitespace. You could have left alone the ones using " *", reducing overall patch size, but since you went farther then that - even better. > For reviewing, start with opcodes/i386-dis.c, that's the actual change > to how whitespace is emitted. If you're happy with that then > everything else is testsuite updates. I think this doesn't belong in the commit message, but would better go ... > [1] https://sourceware.org/pipermail/binutils/2022-April/120610.html > --- ... below such a marker. Jan