From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR05-VI1-obe.outbound.protection.outlook.com (mail-vi1eur05on2075.outbound.protection.outlook.com [40.107.21.75]) by sourceware.org (Postfix) with ESMTPS id 526323858D28 for ; Thu, 2 Nov 2023 15:53:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 526323858D28 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 526323858D28 Authentication-Results: server2.sourceware.org; arc=pass smtp.remote-ip=40.107.21.75 ARC-Seal: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1698940411; cv=pass; b=qxqd8G3BQWAgtHdiudJz3Wenv0gK0WqEkHM+h8gzsM4k1LUJ0eQa+wRgoKhB1mjOJXV6lDcRHqLpxay9vkjhbZOjSF5edCMR2srX7Mkwf2uVipNCjfNSiABxvpAcdf6juTepvsC+GeTAGJB0FdasY5x1DnPUxPP/DdKPFBateqE= ARC-Message-Signature: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1698940411; c=relaxed/simple; bh=gjDr0CbJ9B9+uAq9UjAgKrvfy245akMdIReSB936uGs=; h=DKIM-Signature:Message-ID:Date:Subject:To:From:MIME-Version; b=KQw1+aPYPclqpPzu0f7z24OFhpOtwSz7nR11mG8ISkqW1Ronq3HKptMSvC11QjYeQaQGFcZnIngun2lsw/2mE1SzMIaUMsqUa/fN4XDjbV8vw751Yrt3J0CLnz4JK88nKak5FVNDcHOgSw7FN1FpiYg/U6SVaRDxVMv/7W3h4Rw= ARC-Authentication-Results: i=2; server2.sourceware.org ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=T0po8ykjUFY+qSDXCcKfXrFfDERVtj90q3DyvOxD4CxruRNEdnLg/4wW7BE+nDiEVnqCD7znfVMOhZXgwkZcO8PYP0a8Hk2QgGzHXscGeVV4vEoynFLN1vHx+FOAdkqrbM4yipDtELJ7cfI5gsibF1hpkUtpwjPoE/gb1US41Bl0/dkyuOhKp04oG6C6lg/1Xys1Ag+Wjt7qbOb77VhHRCzO/0SedFnnz0ChC+bA07Cv7rOF1TqDNyc0lURaijLsIwKiYf7F6d+ozTCCywuQrX90kNO5iswwdbUKE7wsd3EDPC4zlTFgZDOMQt3nAzO4NeMLLBDwPnDo68XLKc3lRA== 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=Ig3tBPQ/5TqbA0uzanUi5MOd2hzbgWFTC8snGm91I8U=; b=e3qI47uZcNCgPGZ1+WPm3ty+1/XhGA7dSLZsZ6v2H5z4M+SDcGND/+FiCPJAz6QYd5jRNHtYov0tcKHAxxu0BzrHP2VVmLKs73U+1iitE90pidETEwoL5rhcu0VRMK6po1wWCYQQd/7HVBiMVym3N+lXcd0CCFgvhwx7gMVwpNaNgwPmYMEoN2jaWdExZi6MoQbzDoBMmNcyIDDrk2RSa6TqeCP6jGF5fAD8nbWDAN0apXCnpfBj4UpU9IGkV1/1A3ty0XFW7y32eHZfIIRLtAPKZRB5HGwgTjxhilp3Po9wsp7fRYz3G0CVun+4ee+sDuRl/AsN7W0DSQHl7OYV/w== 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 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=Ig3tBPQ/5TqbA0uzanUi5MOd2hzbgWFTC8snGm91I8U=; b=uVqUkXU0YS2ntcZfFBfmZLZa9UPxOEN3+kUo9anWg0N+o7TxJAoluxMc6gzOM4phSM60YLSu1ULe8lunZcELXAK3BjWQomV93EV++1RkdNmnUlL0zQ/n0l7a8QkO9KKwk9qYMV7GgW7/8ONAI3ZRCG8/xZqUAFtkYR3ZUpnEXIEXslSqEWKU51QuQJg4HTalqPlH66CnmCHgJdzo6J9s810LDahBvlhh4HB2PXWLgrBtksueoksFAOeTUcPhiFgX1QpU3yWxcVlVN0mKqcakw5q9xJOiY1JIxfAxTHD09JRnVeLMpcO6hnJCZ1V88jAGJMdlqMbaT9CEvA1O4lt+Fg== Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com; Received: from DU2PR04MB8790.eurprd04.prod.outlook.com (2603:10a6:10:2e1::23) by VI1PR04MB9954.eurprd04.prod.outlook.com (2603:10a6:800:1d7::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6954.19; Thu, 2 Nov 2023 15:53:16 +0000 Received: from DU2PR04MB8790.eurprd04.prod.outlook.com ([fe80::d924:b650:a2ad:7b25]) by DU2PR04MB8790.eurprd04.prod.outlook.com ([fe80::d924:b650:a2ad:7b25%3]) with mapi id 15.20.6954.019; Thu, 2 Nov 2023 15:53:16 +0000 Message-ID: Date: Thu, 2 Nov 2023 16:53:14 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: [PATCH, V2 07/10] gas: synthesize CFI for hand-written asm Content-Language: en-US To: Indu Bhagat References: <20231030165137.2570939-1-indu.bhagat@oracle.com> <20231030165137.2570939-8-indu.bhagat@oracle.com> From: Jan Beulich Cc: binutils@sourceware.org In-Reply-To: <20231030165137.2570939-8-indu.bhagat@oracle.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: FR2P281CA0005.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:a::15) To DU2PR04MB8790.eurprd04.prod.outlook.com (2603:10a6:10:2e1::23) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DU2PR04MB8790:EE_|VI1PR04MB9954:EE_ X-MS-Office365-Filtering-Correlation-Id: ff2e6afa-bc31-465f-2a95-08dbdbbbd392 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: QmrukWXVJnMprbI6gPNyAkWEcGvYWDIkGvdEENePeGUIHZGasiUTYEPNTTYWuWkKuicbuNt2kjHYkwcBxIjgXn82acpV6yAb/1l0Zn+MXdtsJMIWFlguliIuvaxq2YXyD5jivxCk3/Y7zld8HkcnekURI2G2wsr1W4THABE6wIokrtTk4JdEDwZ7CuWlHsPlbTGDdflZcHHXWK4tLMaK6nlXHKEdzgBmsDGxNqdQ/nnJI2q0gq5Wr8YZABl+jnZijimWErEDgxI3kxWHfbJlUZ8WKpuJ0/RIveW1HIdi720bAco8KjMH17VFcTozF3drL54mCp9ier+g6Hjr1O9n6PaTY48r5jTgC+tmkAoNKTVK+ih0JdlThak4MxwC6IjJwGZVemymgUwya2pH2DqS53BxEiwVG/q/Xwg0LidfQraf83sBdjQe5ir7aHT9OPIGcDwCjT6r25dL5ejDo8/L7LEBvZ3cdDXpUniXNJbjejDY/pjEifD3sBeGiNOsBud8V/1ofxzyCwiKZRcfrxdNfXKcZNIb0XXwSidpkeKwip2F6PLDLbhgRgZPyEV5ZykCm8+M4DbZTcER9gtSgy270vnJbbCGZ6fdYYvczRrgOyHmzb0Mr6+w0Ts5Ws8RuiDtNSjZs/0M4GH8xbeFprt9sG0LnTVAGI+ykHS6OtGzwbU= X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DU2PR04MB8790.eurprd04.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230031)(136003)(346002)(376002)(366004)(39860400002)(396003)(230922051799003)(451199024)(64100799003)(186009)(1800799009)(83380400001)(31686004)(2616005)(6512007)(26005)(38100700002)(31696002)(36756003)(86362001)(2906002)(30864003)(5660300002)(4326008)(478600001)(53546011)(8936002)(6916009)(66556008)(66946007)(316002)(8676002)(66476007)(6486002)(41300700001)(6506007)(2004002)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?anRPalYrVktnbFpEU256d0pWQzJ0b3R5QXozOUdZVkxZcE4zaWNGbkhKYWV4?= =?utf-8?B?T0kxV2k2Q2lFWUMrQTdWZ1ZlUTRtangyMzk4czJGZExnV2o5K1owQS9VVmpY?= =?utf-8?B?clh2aXlTTE84NWFaMjYrQ3gwd3BkamdwQkpqRXpkUDJYOHJia2tVZk1JR3Br?= =?utf-8?B?NVF3Vzg5aWFDV04zOHJDTHVlN1FQL2tLSTlwQlpEN2E4RmhpeDZYUytpcjly?= =?utf-8?B?dk1jZUpZQWFqK2VONWJoVFZZK3Y3d0d6c2hOdWJFbjNaQ1M4Z0FVYSs0MFRi?= =?utf-8?B?aDdtR2J6UUM1b2NwKy9veWs0aWJlWENjbFZXV3VuRVN3VDVjS25PSUhqTW9h?= =?utf-8?B?NTE1RTk5ZTh1b3ZkeWk3N1NjZDlHL3phTzNaNFdNdnBweWdzbEVQbFAvZTYx?= =?utf-8?B?QWl5TXFPVFZ5SHJpSUoxRmVlM3VSR1g3K213KzFrNVgwOVBscC9GNDB2NVlU?= =?utf-8?B?YWhGanVPOFYrdFdyL0RFMklkWUdJb2k0RURDY2hkRStPcWZDRnQ0K2x5STQ0?= =?utf-8?B?RmVwaHR1THZlSmFGQ0FzVlR4Ty9NaGpsSk13b05XZ0J4TDZ6QWcrUStNMXRE?= =?utf-8?B?MkJOVnFkQTdGalpnbmoyazNMSWtoWUl0QUlXL1NGT0RJN1ZmMUc0WHhWblc3?= =?utf-8?B?bUl1YWFna2MwZGJiL1J3Rm9WUmxvMDBlcUNYZmt2SGQvc1BjQlVKR2NVS3hp?= =?utf-8?B?R2lTQnVoNmlTNVFNaHVxNDRJR1RmelhiTXRSMVhTQjI0aHlwUTh3VWxqSjhu?= =?utf-8?B?RGFwc1prZlltOWdlT3UzRHhQblRzcTZ5RVZXWGpZZEp6aExhU0x0UmNieGpM?= =?utf-8?B?dXo2QWtXSnIzVC9KOEJlaWNseThMTVh1TGZqVjZvb0JFS2hrMm4wblNXcVMx?= =?utf-8?B?SEc4S0NDRFZzelJUSVNkZ0w2SFNEcjJ6NWVxcThOV2NiRi9aeElaYjZ0N1Z2?= =?utf-8?B?d2xpVCtXZ3FwZWpGUVhIcnRaTlcxQytrbWRiMHo1dEJ3KzFvdExsbm45ZnRx?= =?utf-8?B?SU93YjZENjdrNUxZbWV3N01ZOXNxRHFOVnFoSUdFMmtVVUc0M09hZkZtVmln?= =?utf-8?B?WCtHSlZKWDFYdjcvdk1TUXptbjhnamthM1JGNlgxa1BHWExsYmUwM3kwUHQ3?= =?utf-8?B?VzBtRmVzV0pDMW1UU2RLQStucnF5V0kxWlN5VFcxVWtCZkVIMGtzSkt4RFRX?= =?utf-8?B?YWhDVmFmajNVOGhHQVppbG91bmpyZEZXRlkvZE83UXNMSlV6TGFaWW92d216?= =?utf-8?B?Q2E2ZVNvcjVLMFBjUW02MmtVN2RHQ2RqSFV1cVNtdDdkcmNSOVd4QThqQVIy?= =?utf-8?B?T2FNbVY5Qlh5UU1rVzkzTEhjakp0Tm9XbFpMbnIrRTRpS3kvWGtIRWdYTFZr?= =?utf-8?B?Q2J5RmVkT2xsOTFCRGZ4aWpZQ2xHdDJUV1dVMFRmSGVTS2Rvb1RtMEs2bW5V?= =?utf-8?B?b2pEaEtXVnd0QUg1SU1QdnIvVDJ2dmlkS09yYUtDWGdKMWh3c1hGNjZ4REZD?= =?utf-8?B?dFM0OVRuMXZGcHhFNmJMK1dzczBZRFd0YVpwb1pQOWl2LzNjY0hrVGdxUU5C?= =?utf-8?B?MVhOYWx6a0R3OW9LSFUxQmQwSlFRdXBDL1ROS2xndCtnU25YWmo5L1FtQzB1?= =?utf-8?B?eTQ4Yzhqd0ltVk8yOE1pSzBHVGRhUzYvNXcvTFd0Sjk2WnlDVytmYllSR09P?= =?utf-8?B?UmZvZWNmb1Jaek9SeFI0SU56Zmc3ei9CN1FxT2c2QlFpaEQrSlZXOURUdjlj?= =?utf-8?B?STFRMzE5VC9XN09vaXpGZEhpTmt5WlVDOTBjVlBONURxQnlDb3ZoNWtlSVQ4?= =?utf-8?B?Sk9ZRDJhNlVmQXhISFl5R1ZwM3BVdC9xSmJNeHc3ckZUWVl4bkxxaWkvZW5V?= =?utf-8?B?cVV3N0JtN01HNEtrRU5iUlM4SXVYSnpjOXBINTY2eHFXUHVFaHFwT1lkN1pD?= =?utf-8?B?WDdxOFM1MmJYVHQrUXVjdVpiekVKVUR3QmlVS0o4c1FYTmJoZlowTDlOS2ZG?= =?utf-8?B?eVg3RTluejRoWFdTMXI2T240Y1prZ1gzSGhRWGZBRGlpUFdtYTZZN2MzZk5S?= =?utf-8?B?QlNGV3ErYStPUnNnVnRZazhVZi9rMXZ3OWFoRlNjNDNKUGI3SUVOZ0ZqQ1Ar?= =?utf-8?Q?7IIcNviaffmw7U+n67S2EFAh4?= X-OriginatorOrg: suse.com X-MS-Exchange-CrossTenant-Network-Message-Id: ff2e6afa-bc31-465f-2a95-08dbdbbbd392 X-MS-Exchange-CrossTenant-AuthSource: DU2PR04MB8790.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Nov 2023 15:53:15.8999 (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: RuHLXhETp+7MTbbI3/gaBlCqcvvHA+D5NK5MzoJ9ZpVgA25yzhILROH6osXLcqK1WX3PZPZX3uiY2qvUrp/j7A== X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB9954 X-Spam-Status: No, score=-3028.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,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 List-Id: On 30.10.2023 17:51, Indu Bhagat wrote: > --- /dev/null > +++ b/gas/ginsn.c > @@ -0,0 +1,1225 @@ > +/* ginsn.h - GAS instruction representation. > + Copyright (C) 2023 Free Software Foundation, Inc. > + > + This file is part of GAS, the GNU Assembler. > + > + GAS is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3, or (at your option) > + any later version. > + > + GAS is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with GAS; see the file COPYING. If not, write to the Free > + Software Foundation, 51 Franklin Street - Fifth Floor, Boston, MA > + 02110-1301, USA. */ > + > +#include "as.h" > +#include "subsegs.h" > +#include "ginsn.h" > +#include "scfi.h" > + > +#ifdef TARGET_USE_GINSN > + > +const char *const ginsn_type_names[] = This looks like it wants to be static. > +{ > +#define _GINSN_TYPE_ITEM(NAME, STR) STR, > + _GINSN_TYPES > +#undef _GINSN_TYPE_ITEM > +}; > + > +const char *const ginsn_src_type_names[] = This and ... > +{ > +#define _GINSN_SRC_TYPE_ITEM(NAME, STR) STR, > + _GINSN_SRC_TYPES > +#undef _GINSN_SRC_TYPE_ITEM > +}; > +const char *const ginsn_dst_type_names[] = ... this also look so, but then they're also unused inside this CU, so when making them static the compiler would complain. If they're needed by a later patch, perhaps they should be moved there? And if entirely unused, perhaps they should both be deleted (or at least commented / #ifdef-ed out)? With the many symbols further down it's also hard to spot whether any of them should perhaps also be static. > +{ > +#define _GINSN_DST_TYPE_ITEM(NAME, STR) STR, > + _GINSN_DST_TYPES > +#undef _GINSN_DST_TYPE_ITEM > +}; > + > +static > +ginsnS *ginsn_alloc (void) > +{ > + ginsnS *ginsn = XCNEW (ginsnS); > + return ginsn; > +} > + > +static ginsnS* > +ginsn_init (enum ginsn_type type, symbolS *sym, bool real_p) > +{ > + ginsnS *ginsn = ginsn_alloc (); > + ginsn->type = type; > + ginsn->sym = sym; Is a symbol hanging off of a ginsn ever intended to be altered? If not, field and function argument would want to be pointer-to-const. > + if (real_p) > + ginsn->flags |= GINSN_F_INSN_REAL; > + return ginsn; > +} > + > +static void > +ginsn_cleanup (ginsnS **ginsnp) > +{ > + ginsnS *ginsn; > + > + if (!ginsnp || !*ginsnp) > + return; > + > + ginsn = *ginsnp; > + if (ginsn->scfi_ops) > + { > + scfi_ops_cleanup (ginsn->scfi_ops); > + ginsn->scfi_ops = NULL; > + } > + > + free (ginsn); > + ginsn = NULL; This doesn't do anything useful. Did you perhaps mean to set *ginsnp to NULL, and then also ahead of calling free()? > +} > + > +static void > +ginsn_set_src (struct ginsn_src *src, enum ginsn_src_type type, uint32_t reg, > + int32_t immdisp) I find the use of fixed-width types suspicious here: For reg, likely it wants to be unsigned int. For immdisp it's less clear: Immediates can be wider than 32 bits, and they may also be either signed ot unsigned. From an abstract perspective, assuming the immediate value actually is used for anything, I'd expect offsetT to be used, following struct expressionS' X_add_number. > +{ > + if (!src) > + return; > + > + src->type = type; > + /* Even when the use-case is SCFI, the value of reg may be > SCFI_NUM_REGS. > + E.g., in AMD64, push fs etc. */ > + src->reg = reg; > + > + if (type == GINSN_SRC_IMM || type == GINSN_SRC_INDIRECT) > + src->immdisp = immdisp; What's the point of the conditional around the assignment? > +ginsnS * > +ginsn_new_add (symbolS *sym, bool real_p, > + enum ginsn_src_type src1_type, uint32_t src1_val, int32_t src1_disp, > + enum ginsn_src_type src2_type, uint32_t src2_val, int32_t src2_disp, > + enum ginsn_dst_type dst_type, uint32_t dst_reg, int32_t dst_disp) > +{ > + ginsnS *ginsn = ginsn_init (GINSN_TYPE_ADD, sym, real_p); > + /* src info. */ > + ginsn_set_src (&ginsn->src[0], src1_type, src1_val, src1_disp); > + ginsn_set_src (&ginsn->src[1], src2_type, src2_val, src2_disp); > + /* dst info. */ > + ginsn_set_dst (&ginsn->dst, dst_type, dst_reg, dst_disp); > + > + return ginsn; > +} > + > +ginsnS * > +ginsn_new_and (symbolS *sym, bool real_p, > + enum ginsn_src_type src1_type, uint32_t src1_val, int32_t src1_disp, > + enum ginsn_src_type src2_type, uint32_t src2_val, int32_t src2_disp, > + enum ginsn_dst_type dst_type, uint32_t dst_reg, int32_t dst_disp) > +{ > + ginsnS *ginsn = ginsn_init (GINSN_TYPE_AND, sym, real_p); > + /* src info. */ > + ginsn_set_src (&ginsn->src[0], src1_type, src1_val, src1_disp); > + ginsn_set_src (&ginsn->src[1], src2_type, src2_val, src2_disp); > + ginsn_set_dst (&ginsn->dst, dst_type, dst_reg, dst_disp); > + > + return ginsn; > +} The comments in the two functions don't look overly useful to me. But if you have them, please have them be consistent across similar functions. > +ginsnS * > +ginsn_new_mov (symbolS *sym, bool real_p, > + enum ginsn_src_type src_type, uint32_t src_reg, int32_t src_disp, > + enum ginsn_dst_type dst_type, uint32_t dst_reg, int32_t dst_disp) > +{ > + ginsnS *ginsn = ginsn_init (GINSN_TYPE_MOV, sym, real_p); > + /* src info. */ > + ginsn_set_src (&ginsn->src[0], src_type, src_reg, src_disp); > + /* dst info. */ > + ginsn_set_dst (&ginsn->dst, dst_type, dst_reg, dst_disp); > + > + return ginsn; > +} As indicated before, if both src and dst can be indirect here, ... > +ginsnS * > +ginsn_new_store (symbolS *sym, bool real_p, > + enum ginsn_src_type src_type, uint32_t src_reg, > + enum ginsn_dst_type dst_type, uint32_t dst_reg, int32_t dst_disp) > +{ > + ginsnS *ginsn = ginsn_init (GINSN_TYPE_STORE, sym, real_p); > + /* src info. */ > + ginsn_set_src (&ginsn->src[0], src_type, src_reg, 0); > + /* dst info. */ > + gas_assert (dst_type == GINSN_DST_INDIRECT); > + ginsn_set_dst (&ginsn->dst, dst_type, dst_reg, dst_disp); > + > + return ginsn; > +} > + > +ginsnS * > +ginsn_new_load (symbolS *sym, bool real_p, > + enum ginsn_src_type src_type, uint32_t src_reg, int32_t src_disp, > + enum ginsn_dst_type dst_type, uint32_t dst_reg) > +{ > + ginsnS *ginsn = ginsn_init (GINSN_TYPE_LOAD, sym, real_p); > + /* src info. */ > + gas_assert (src_type == GINSN_SRC_INDIRECT); > + ginsn_set_src (&ginsn->src[0], src_type, src_reg, src_disp); > + /* dst info. */ > + ginsn_set_dst (&ginsn->dst, dst_type, dst_reg, 0); > + > + return ginsn; > +} ... I can't see what these are needed for. > +ginsnS * > +ginsn_new_sub (symbolS *sym, bool real_p, > + enum ginsn_src_type src1_type, uint32_t src1_val, int32_t src1_disp, > + enum ginsn_src_type src2_type, uint32_t src2_val, int32_t src2_disp, > + enum ginsn_dst_type dst_type, uint32_t dst_reg, int32_t dst_disp) > +{ > + ginsnS *ginsn = ginsn_init (GINSN_TYPE_SUB, sym, real_p); > + /* src info. */ > + ginsn_set_src (&ginsn->src[0], src1_type, src1_val, src1_disp); > + ginsn_set_src (&ginsn->src[1], src2_type, src2_val, src2_disp); > + /* dst info. */ > + ginsn_set_dst (&ginsn->dst, dst_type, dst_reg, dst_disp); > + > + return ginsn; > +} > + > +/* PS: Note this API does not identify the displacement values of > + src1/src2/dst. At this time, it is unnecessary for correctness to support > + the additional argument. */ And why do the displacements need tracking for add, and, and sub? > +static bool > +ginsn_indirect_jump_p (ginsnS *ginsn) > +{ > + bool ret_p = false; > + if (!ginsn) > + return ret_p; > + > + ret_p = (ginsn->type == GINSN_TYPE_JUMP > + && ginsn->src[0].type == GINSN_SRC_REG); On x86 an indirect jump can also have a memory source operand (i.e. what you call "indirect"). > +static bool > +ginsn_direct_local_jump_p (ginsnS *ginsn) > +{ > + bool ret_p = false; > + if (!ginsn) > + return ret_p; > + > + ret_p |= (ginsn->type == GINSN_TYPE_JUMP > + && ginsn->src[0].type == GINSN_SRC_SYMBOL > + && S_IS_LOCAL (ginsn->src[0].sym)); This looks fragile: You may not yet have seen the .globl or .weak directive. > +static char* > +ginsn_src_print (struct ginsn_src *src) > +{ > + size_t len = 39; > + char *src_str = XNEWVEC (char, len); > + > + memset (src_str, 0, len); > + > + if (src->type == GINSN_SRC_REG) > + { > + char *buf = XNEWVEC (char, 32); > + sprintf (buf, "%%r%d, ", ginsn_get_src_reg (src)); > + strcat (src_str, buf); > + } > + else if (src->type == GINSN_SRC_IMM) > + { > + char *buf = XNEWVEC (char, 32); > + sprintf (buf, "%d, ", ginsn_get_src_imm (src)); Here and below, if you stick to int32_t as the type, this wants to use PRId32. > +static const char* > +ginsn_type_sym_begin_end_print (ginsnS *ginsn) > +{ > + int id = 0; > + const char *ginsn_sym_strs[] > + = { "", "FUNC_BEGIN", "FUNC_END" }; static and with a 2nd const? > +static char* > +ginsn_print (ginsnS *ginsn) > +{ > + struct ginsn_src *src; > + struct ginsn_dst *dst; > + size_t len = GINSN_LISTING_LEN; > + char *ginsn_str = XNEWVEC (char, len); > + > + memset (ginsn_str, 0, len); > + > + strcpy (ginsn_str, "ginsn: "); > + > + strcat (ginsn_str, ginsn_type_names[ginsn->type]); > + strcat (ginsn_str, " "); > + > + /* For some ginsn types, no further information is printed for now. */ > + if (ginsn->type == GINSN_TYPE_CALL > + || ginsn->type == GINSN_TYPE_RETURN > + || ginsn->type == GINSN_TYPE_OTHER) > + goto end; > + else if (ginsn->type == GINSN_TYPE_SYMBOL) > + { > + if (GINSN_F_USER_LABEL_P (ginsn)) > + strncat (ginsn_str, S_GET_NAME (ginsn->sym), len - 10); > + else > + strcat (ginsn_str, ginsn_type_sym_begin_end_print (ginsn)); > + goto end; > + } > + > + /* src 1. */ > + src = ginsn_get_src1 (ginsn); > + strcat (ginsn_str, ginsn_src_print (src)); > + > + /* src 2. */ > + src = ginsn_get_src2 (ginsn); > + strcat (ginsn_str, ginsn_src_print (src)); > + > + /* dst. */ > + dst = ginsn_get_dst (ginsn); > + strcat (ginsn_str, ginsn_dst_print (dst)); > + > +end: > + gas_assert (strlen (ginsn_str) < GINSN_LISTING_LEN); This check comes too late - if it triggers, you've corrupted memory already. I wonder anyway why you construct all this "by hand", rather than using snprintf(). Jan