From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR05-DB8-obe.outbound.protection.outlook.com (mail-db8eur05on2077.outbound.protection.outlook.com [40.107.20.77]) by sourceware.org (Postfix) with ESMTPS id ED4F43858D37 for ; Thu, 2 Nov 2023 12:28:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org ED4F43858D37 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 ED4F43858D37 Authentication-Results: server2.sourceware.org; arc=pass smtp.remote-ip=40.107.20.77 ARC-Seal: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1698928112; cv=pass; b=ZxcwJDGOrT2/Im8TwHgu0QadopSoluI46f2KMaYXQX2pYBmysjr4ii5zEOnO6YWA20f+IIgnhHFP4lcYCDAsfd+IDN9ZJRDDLyu3oZnm32HgTjuHAatZY97CwUbnYnkZrVnzB6L7QyTpw9iw7NNKPRjZS33qtI36YGq/7r6VliY= ARC-Message-Signature: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1698928112; c=relaxed/simple; bh=hCPYK231wVsmrUW1SDR+kRfqDel2Wsb35KqQnqQZ/lk=; h=DKIM-Signature:Message-ID:Date:Subject:To:From:MIME-Version; b=bT1+6PAbPYzK9yb/QDMqn4oKTaFEs8kegW3I6C2t3HLTRHLtkOm57SYnRz+ElMlfrZVMnlj+TcqzedRbZY2lKc6Gw9yOxi/VkBNBaK/uVeNN53zajGaqWQMMeFh7yBHdzbwuzuVm7qiVxy79Z9GeM7PQtbhiNYX60Ld6c1FR2bE= ARC-Authentication-Results: i=2; server2.sourceware.org ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=WP+WWOdgvZkyg7XDNRULFPuKGw8U2O/ZOdgbaSvUagG2OoZmcHXP1p29iWvbGIX4YKZ7cEq2ILbGh3m3AeVOaDw9+AnYdmmnwbTgHHnzSD9etQgyPzytIAifFbnLJIlTg1PpojpPAFN5pkn1R+vw8y2hCMRzwclocualtug02vAL2yIZEt5lQD4PBIjq3R6gfQMLhXTS6G95yAliiETX7u8o3cbXAD0QZAJQdutIKJ/HY1k93QobTfs0ZHAtiq53PcMae/7fR0Jsf6J+ehy+TYZFijdJlbInm77cH5JwJ82UokrO+/ZOXEDcweFtoiqt8GiMJgXK7myN2gDpUgF5Aw== 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=qjtUhmIeeF4ZV5qZUSxxgdJhKYVnVQq6MuBgz35SPUk=; b=E2+t8mbu2+hEbSj7yc3gnjn3zEkpLgmkhD58BNQ4cfq6l0rNqhjIuhqPl5GIFebjsAwX692TUqFJylEqJT11O87QH/kpIuxKQmG/49+A0lyDoj/mDdpYATeboHX/XqLvPWcowZH2UpM29L396VaurL95HA55GJ2/XMoBfiRfFNzWBjxgRU8FM81HP8zWKxS6+lPZ4kXtKs3mqkRX4yKj9gA348MF7+/0aawiD1Wl7scF6iwdreyL+oDAUTXqzXG/ukUIOyUSCHbeEtBBHhiEq0TaYdbmhkyE8V8fTl2XfV9gWrRL6jF54KmcY170fpq/kcyHNGyrX92NcWpzGChxGw== 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=qjtUhmIeeF4ZV5qZUSxxgdJhKYVnVQq6MuBgz35SPUk=; b=q7DgNLf6AUvFu8Yht1aCZTeUDi2XCCm4zEXg0wCZStkMSWnMYjVsTVPGl7YggZfYwS6LTOUOgECVBvbXC2CtPhcg3AJPqjBrYUHdGUCAqyPtTtZeo2HccDoI9MOrmbbD/c/JZNfOcxt3F39S2eJaPsCPr0mo7QjQhg29dSm5wlu5gY639xPRGSzYoTWgewSH3Mediz5e/T89y+1pHAMg18aVwVaXEg+wTDrfhYV99xIrAMsgt15aF6bAe+QTDYTKVlj+t3SFOLZ7I2ulf4UR/PAlG5K2oSOFl4JtP63gJJBQ8Ne6tf/JOWRSrUJQ6VGjq8j8RFP0c1LK1woBThhP9g== 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 AM8PR04MB7252.eurprd04.prod.outlook.com (2603:10a6:20b:1da::22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6954.18; Thu, 2 Nov 2023 12:28:18 +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 12:28:18 +0000 Message-ID: <13294625-0ab1-dfa6-08dd-d5c1ec541ec7@suse.com> Date: Thu, 2 Nov 2023 13:28:16 +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 09/10] gas: testsuite: add a x86_64 testsuite for SCFI Content-Language: en-US To: Indu Bhagat Cc: binutils@sourceware.org References: <20231030165137.2570939-1-indu.bhagat@oracle.com> <20231030165137.2570939-10-indu.bhagat@oracle.com> <33168ad4-97c6-6028-7055-2f21434ccfd1@suse.com> From: Jan Beulich In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: FR3P281CA0209.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:a5::8) To DU2PR04MB8790.eurprd04.prod.outlook.com (2603:10a6:10:2e1::23) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DU2PR04MB8790:EE_|AM8PR04MB7252:EE_ X-MS-Office365-Filtering-Correlation-Id: dd371555-85be-4106-f604-08dbdb9f31d3 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: SwGS744+Q92YECs8945UcPIbR453RrNqp5+VXeMVKzkPYKfbS67Fx6TcllV3ixu1sWDWIgHjkhWpTtiW6I6C/QuRNYdEk6+OsSb6Z5de3rjKE0HNdjLw+4n2CMB+auo/Xg0qnOXhfBMXm1+3wz/cPmc4/dGP/Z0v1wt6WTjBy8SPjenNW92bVi4GqoX+8nLs0lLD5t/eATPgBxC61zT+TXJlwF76qFAB/y32F4GnOPQrdHfVSLYcR/JDqGND8l/WvyeT6Fmir9auGDgew6tjNk/FZEwJSMvqyhWc9vv9sGn3sV0MA/ef359+wN0RvIauBVDtQ8kwIKzYBgHAYpHpmEwjTcJ4X2kUaMHwY6E76cfy7NNmHXQu7wdwj4XjsmTiNB/kO6clswsM0flhamXz8XtPr2KQj1c0YU+PnPtDHFc0Xg2TTVSbqxJKxOegxLLHJk6ZezGuj1b6vbxEQIMSUPlwGASaZ0f8lGGRb/bE2bM9iwC9fBVFZsXRYZHfKNP0fNkOefxV6Cm070SqK3dS4gG/JDxycatK9NSUDG++HtbiSznnNLwOLGqZ1p928OehhkPgSwuYRhVRDPzBDADAlqx1vC+05GYJZaD/B1kGluynid3nuQ/6xJBZnPKPOvPZMOvE7SVfs9xR5pDNXFkxYkBvy5dPMVRKrRzuwRKFgPXiI49/r6W7r1Pn+voye3++EJbtYuKvQWTLwZrHEpCv0A== 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)(346002)(39860400002)(136003)(396003)(366004)(376002)(230173577357003)(230273577357003)(230922051799003)(1800799009)(186009)(451199024)(64100799003)(6916009)(316002)(30864003)(86362001)(31696002)(2616005)(66476007)(2906002)(83380400001)(26005)(66556008)(66946007)(6506007)(53546011)(36756003)(478600001)(41300700001)(38100700002)(6512007)(8676002)(4326008)(8936002)(31686004)(6486002)(5660300002)(2004002)(43740500002)(45980500001);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UzJQQ0FCZGhEclBGNG5uUE9YNkgwRkh4Z0Q4SjlXb09MTzE3SHBDU3UvQTFn?= =?utf-8?B?eE44VkpvSXI0NnIzQlRvQU9ZN1JQZDU5T3UxRjdhZnJnSm1vQmZTSDlHNjNW?= =?utf-8?B?SFBxbzlPT1NXWmM0WXFBOUpWMXZQbHpsZ0hxaDFKaXJKVnZtSndZOVVHWUdn?= =?utf-8?B?dWlzMjFtR2UvOXhoWGdNL2N0bVEyUmN4NGM4NEs4WkF2ZEM3T0p4S3BsWlBV?= =?utf-8?B?azllOTZqemk1SVl5bEw2MlNqbDRja05VOFJEbmdLTXpnWEZ4Yk8xT2hDQitQ?= =?utf-8?B?cURKRHZ4MlBXMWFSQStEZDhRVkt4M1BDNmxRUTZQTEt6UmttZm91Qmtud0Ev?= =?utf-8?B?RDREUW1oalROa2M4aEVicDhPb3E4YzNTTnVhWGdaSi81SUQzaXQ4Z0lWRlpR?= =?utf-8?B?dFNuczAvUHlIaUovR0Z6VmlTK3BML1FyWXJGWXlZalFEVGlUb2NZNGk5WGVm?= =?utf-8?B?T0ZETWl6OHpCeFdDWFBCZGtDNVQ0WEVSdGN6Q21JSFh2UG1iZWxQa0pWdXJV?= =?utf-8?B?WHQxSGR5OUt6S1ErWkZVa3B0MVRmNG03MGtEeC9OaldqTTJodDZmd2dqSTZ6?= =?utf-8?B?RTYxQjlieTRuNGpoQW94Y0trN0tRcGdYUGNiU1VGTnU2aU5xbEx2d3ByNUpE?= =?utf-8?B?ZytYSHZwQjNkVkt1VXA4eFBRa28rbXQ2ek85QjNwdUtIeUd4N1cyUnpZeW54?= =?utf-8?B?eEpyekZ3VVJRanRjNTRSem9LTFlZVDRxbDBDcWJPdExmVUtHR3JnS2VyZHhW?= =?utf-8?B?QjdrRGRKWWNVeWo4U01YcSsyZFpCSldoakw4dzJmN1FGQVJoQjZxZGhQakRa?= =?utf-8?B?ZFpNcTU5a1c5cENZUU4yNDFDdEx0ZS9JL3lvaUVXUkEwL2JVbjZBWVdOdFR6?= =?utf-8?B?MUQwVFAxSU5DSDdLaitiMFQ5c3lTajlNL3ZQWi9iMjM3eCtpMXJ6bVgxQTRV?= =?utf-8?B?MitOSmNRRFZmZlhEZW1mSmlpbzNDbFpzZHhyU0U5ZUFBQWtHSyt4YU10SXRk?= =?utf-8?B?cWZvVWlLdFc5d2JCd2FRamZEcnRHNUszcVE5WWdnb3NtajJhZkJVc0I4eHZp?= =?utf-8?B?Z3QwYncyM1V3dFM0a0NncUpYOHc0eWtXOUt4TkhiZU92Tk1VbzU1cm5lSElU?= =?utf-8?B?b1dkVnl6dmg3SW43S09TeU1ibW0zM0pJWHBlUVA0Z3NLSmh2U05oUVl5b3JM?= =?utf-8?B?ZnE2RG1Zem5xa2dzSjlIcG1CdjRMMFBndUM2MmI2U2dCQU8raUV6ckxrQ2d5?= =?utf-8?B?QlROZWQ1dUFFK0UxT2tWTVZ3NndyYk9najVVYmY4L3k5WkFpb1A1ZEErZTRi?= =?utf-8?B?V3c5OFA3WDRzWFdVYTdFSzFyTjA5RDE5anFaK25Ib3FVaHluMWpaMS9OZWhU?= =?utf-8?B?ZStaNElEOGNNTXQ3c0k2Z3RDMzROd1FkdVVVa3lqbXQrRHBEd1ZwU2FIcCtz?= =?utf-8?B?ejZLOUF2T2tmbkRGL0NxYW1Nb2xSYXYvZVBrQ2hvQjUxa0U1UEhuVEVTYXZ1?= =?utf-8?B?L2QxTThKcFN0QWllZkVMNldEdU1STGhRZFJYYTlGRENXTEFoK3A1bHFnOXg2?= =?utf-8?B?Nzh6d2FJUXMrTjF1TTNLZ1RzNk1EdVBrUlFZWWZOdG5FaDhOaHBuLytJdmZN?= =?utf-8?B?T3p6azJyUGwvMHdUY1hTRzR0eko4bVcwOVJHV25JWFYwL3Z3MUFDZDhPM0pu?= =?utf-8?B?a0ZBOTlKSkk5WGg0MDJOLytBbDdIM0pxbjk2UzZpZTFFanZrd3FHQUFicG1s?= =?utf-8?B?aWw3cUNPYUt0TmppM1pQWStua1N1aGpyc0NqNmVYR0VpMGNaUEFVZG5NTGx6?= =?utf-8?B?U2ExSDRvL2h4Mkg5WGNPQVVLWGpFZDlKWjdib00yMmI1ZkkzakNCTW1mQ3pH?= =?utf-8?B?aFFRbkF0UjR1UkIwZ1dVaE5UbE5xZ0ZwKzNESElBUmVtTUJ0Q2RwRWg4Vmg3?= =?utf-8?B?a1lDdks3eTd2cmtnc05NeHJoT29YRG9PUStWQXNSSGZXOEswcUViOUtiUUpQ?= =?utf-8?B?S0VhaGhVRnQ1a0pNRFU2a3BvMXY3WEl6SVh2Vm5kS1hOM0NVbXNSVXFmQ0VL?= =?utf-8?B?bVptOVJJYytIb2g1U3VqQ2ZWb2p1cWRiNW42aHBPYWN2blo0SGVLM2N4akJm?= =?utf-8?Q?CRLBfqX0kSpLpfankMUN1z5Ph?= X-OriginatorOrg: suse.com X-MS-Exchange-CrossTenant-Network-Message-Id: dd371555-85be-4106-f604-08dbdb9f31d3 X-MS-Exchange-CrossTenant-AuthSource: DU2PR04MB8790.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Nov 2023 12:28:18.8388 (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: SNN+e/SgoUOm+hHwMmKH9921Z+f3YX+2OE1ugSha+1lpO0c5JwU0bUVlL/vy/l07JUXZGOjITbwsjWw6RoIZig== X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM8PR04MB7252 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 01.11.2023 07:24, Indu Bhagat wrote: > On 10/31/23 09:13, Jan Beulich wrote: >> On 30.10.2023 17:51, Indu Bhagat wrote: >>> --- /dev/null >>> +++ b/gas/testsuite/gas/scfi/x86_64/scfi-add-2.s >>> @@ -0,0 +1,48 @@ >>> + .section .rodata >>> + .type simd_cmp_op, @object >>> + .size simd_cmp_op, 8 >>> +simd_cmp_op: >>> + .long 2 >>> + .zero 4 >>> + >>> +# Testcase for add instruction. >>> +# add reg, reg instruction >>> + .text >>> + .globl foo >>> + .type foo, @function >>> +foo: >>> + .cfi_startproc >>> + pushq %rbp >>> + .cfi_def_cfa_offset 16 >>> + .cfi_offset 6, -16 >>> + movq %rsp, %rbp >>> + .cfi_def_cfa_register 6 >>> + pushq %r12 >>> + .cfi_offset 12, -24 >>> + mov %rsp, %r12 >> >> You copy %rsp to %r12 here, and ... >> >>> +# Stack manipulation is permitted if the base register for >>> +# tracking CFA has been changed to FP. >>> + addq %rdx, %rsp >>> + addq %rsp, %rax >>> +# Some add instructions may access the stack indirectly. Such >>> +# accesses do not make REG_FP untraceable. >>> + addl %eax, -84(%rbp) >>> +# Other kind of add instructions should not error out in the >>> +# x86_64 -> ginsn translator >>> + addq $simd_cmp_op+8, %rdx >>> + addl %edx, -32(%rsp) >>> + addl $1, fb_low_counter(,%rbx,4) >>> + mov %r12, %rsp >> >> ... you restore it here, but both without any .cfi_* annotation. >> It is therefore unclear whether this is in any way related to ... >> > > There are no .cfi_* annotations for the %rsp updates here because the > CFA tracking has already been switched to REG_FP based tracking. From > the perspective of DWARF CFI, this usages of the stack do not need to > tracked. If unwinding happens from any instruction in the above > sequence, we already have the correct and complete unwind information. > >>> +# Popping a callee-saved register. >>> +# RSP must be traceable. >>> + pop %r12 >>> + .cfi_restore 12 >> >> ... what the comment says about these. >> > > The comment here means that hand-written programs may use stack for > their local usage etc. but must ensure that before or in the epilogue, > rsp is restored to the desired value (before register restores via pop > instructions). And how exactly do you tell that the "mov %r12, %rsp" is a restore, not the setting of an arbitrary new %rsp value? > This is not a special handling in the SCFI machinery but it is what is > needed to ensure correctness of the function. For asm not adhering to > the ABI/calling conventions, SCFI cannot be used. But the calling convention doesn't go as far as dictating local frame layout in a function, I don't think? >>> + leave >>> + .cfi_def_cfa_register 7 >>> + .cfi_restore 6 >> >> Using numbers here isn't very helpful, I'm afraid. >> > > I am not sure I understand. The DWARF register numbers and the offset > values are required by the semantics of those CFI directives. Offsets are (largely) unavoidable to be numerics, yes. But .cfi_* directives support register names, and using them is (imo) far better than Dwarf register numbers - you may have memorized them by now, but others likely won't. >>> --- /dev/null >>> +++ b/gas/testsuite/gas/scfi/x86_64/scfi-asm-marker-3.l >>> @@ -0,0 +1,2 @@ >>> +.*Assembler messages: >>> +.*9: Warning: --scfi=all ignores some user-specified CFI directives >> >> Is repeating this for (about?) every test really necessary / useful? >> If multiple passes for every test were to make sense to me, I'd expect >> one pass with SCFI and one pass without, where the directives then >> take effect. And then the same set of expectations should match. >> > > re: is repeating this useful? Strictly speaking no. _But_, I think as > the code evolves, we may add more diagnostics to the SCFI machinery. > Explicitly checking for the set of warnings helps us ensure no new > warnings show up where they are not expected. In a way, it is a stricter > check and ensures no unintented slippage. Well, okay, that's fair. You didn't respond to the other part of my comment, though. >>> --- /dev/null >>> +++ b/gas/testsuite/gas/scfi/x86_64/scfi-bp-sp-2.s >>> @@ -0,0 +1,49 @@ >>> +# Testcase for switching between sp/fp based CFA. >>> + .text >>> + .globl foo >>> + .type foo, @function >>> +foo: >>> + .cfi_startproc >>> + pushq %r14 >>> + .cfi_def_cfa_offset 16 >>> + .cfi_offset 14, -16 >>> + pushq %r13 >>> + .cfi_def_cfa_offset 24 >>> + .cfi_offset 13, -24 >>> + pushq %r12 >>> + .cfi_def_cfa_offset 32 >>> + .cfi_offset 12, -32 >>> + pushq %rbp >>> + .cfi_def_cfa_offset 40 >>> + .cfi_offset 6, -40 >>> + pushq %rbx >>> + .cfi_def_cfa_offset 48 >>> + .cfi_offset 3, -48 >>> + movq %rdi, %rbx >>> + subq $32, %rsp >>> + .cfi_def_cfa_offset 80 >>> +# This mov does not switch CFA tracking to REG_FP, because there has already >>> +# been stack usage between here and the push %rbp >>> + movq %rsp, %rbp >> >> Yet another constraint? >> > > Not sure I understand. This is not a constraint for SCFI. A pattern > where the user does: > push %rbp > .. more stack usage.. > movq %rsp, %rbp > is not using frame pointer for stack tracing, but simply as a scratch > register. Why might it not be? We're talking about hand-written assembly, and assembly writers are free to e.g. first push all (necessary) callee- saved register and then copy %rsp into %rbp. For a function with many stack locals this has the benefit of allowing more of them to be accessed via (%rbp). >>> --- /dev/null >>> +++ b/gas/testsuite/gas/scfi/x86_64/scfi-cfg-1.d >>> @@ -0,0 +1,36 @@ >>> +#as: --scfi -W >>> +#objdump: -Wf >>> +#name: Synthesize CFI in presence of control flow 1 >>> +#... >>> +Contents of the .eh_frame section: >>> + >>> +00000000 0+0014 0+0000 CIE >>> + Version: 1 >>> + Augmentation: "zR" >>> + Code alignment factor: 1 >>> + Data alignment factor: -8 >>> + Return address column: 16 >>> + Augmentation data: [01][abc] >>> + DW_CFA_def_cfa: r7 \(rsp\) ofs 8 >>> + DW_CFA_offset: r16 \(rip\) at cfa-8 >>> + DW_CFA_nop >>> + DW_CFA_nop >>> + >>> +0+0018 0+0024 0000001c FDE cie=00000000 pc=0+0000..0+003a >>> + DW_CFA_advance_loc: 1 to 0+0001 >>> + DW_CFA_def_cfa_offset: 16 >>> + DW_CFA_offset: r3 \(rbx\) at cfa-16 >>> + DW_CFA_advance_loc: 37 to 0+0026 >>> + DW_CFA_remember_state >>> + DW_CFA_advance_loc: 1 to 0+0027 >>> + DW_CFA_restore: r3 \(rbx\) >>> + DW_CFA_def_cfa_offset: 8 >>> + DW_CFA_advance_loc: 1 to 0+0028 >>> + DW_CFA_restore_state >>> + DW_CFA_advance_loc: 9 to 0+0031 >>> + DW_CFA_restore: r3 \(rbx\) >>> + DW_CFA_def_cfa_offset: 8 >>> + DW_CFA_nop >>> +#... >>> + >>> +#pass >> >> Seeing this recurring pattern (the last three lines): Why not just #pass? >> > > I thought adding #... is clearer as it indicates that the test knowingly > skips lines thereafter. Well, #pass alone already says exactly this. If you didn't expect further output lines, you'd _omit_ #pass. >>> --- /dev/null >>> +++ b/gas/testsuite/gas/scfi/x86_64/scfi-cfg-1.s >>> @@ -0,0 +1,47 @@ >>> +# Testcase with one dominator bb and two exit bbs >>> +# Something like for: return ferror (f) || fclose (f) != 0; >>> + .text >>> + .section .rodata.str1.1,"aMS",@progbits,1 >>> +.LC0: >>> + .string "w" >>> +.LC1: >>> + .string "conftest.out" >>> + .section .text.startup,"ax",@progbits >>> + .p2align 4 >>> + .globl main >>> + .type main, @function >>> +main: >>> +.LFB11: >> >> Coming back to "hand-written assembly": The above looks very much like it >> was compiler output (earlier tests did, too, but it's perhaps more >> prominent here). That's not necessarily what a human might write, and >> hence I wonder about the overall coverage that can be gained that way. >> > > Yes, its inspired by compiler generated output. I ran into this when > running some tests. Its still a good basic cfg test for SCFI - a > function with two return paths. You understand though that I used this only as (sutiable) example. My point being that hand-written assembly frequently doesn't resemble compiler- genertaed code. Otherwise, i.e. if the resulting code wasn't to be different, why would one write assembly code in nthe first place? >>> --- /dev/null >>> +++ b/gas/testsuite/gas/scfi/x86_64/scfi-diag-1.s >>> @@ -0,0 +1,23 @@ >>> +# Testcase for REG_FP based CFA >>> +# and using REG_FP as scratch. >>> + .text >>> + .globl foo >>> + .type foo, @function >>> +foo: >>> + .cfi_startproc >>> + pushq %rbp >>> + .cfi_def_cfa_offset 16 >>> + .cfi_offset 6, -16 >>> + movq %rsp, %rbp >>> + .cfi_def_cfa_register 6 >>> +# The following add causes REG_FP to become untraceable >>> + addq %rax, %rbp >> >> But this isn't a problem as long as FP isn't further used. Indeed ... >> >>> + .cfi_def_cfa_register 7 >>> + pop %rbp >> >> ... its original value is restored right afterwards. Yet another constraint? >> > > Hmm. I need some clarification on the statement "Yet another constraint". By "constraint" I mean assumptions you make on the way assembly code is written, for your machinery to be usable. The more such assumptions, the smaller the set of code "eligible" to (future) use of your work. Hence also why I think all such "constraints" need to be spelled out in a single place, such that one can - verify that everything meeting these constraints actually also works, - check up front whether one's assembly code is actually suitable for enabling --scfi. >>> --- /dev/null >>> +++ b/gas/testsuite/gas/scfi/x86_64/scfi-fp-diag-2.s >>> @@ -0,0 +1,55 @@ >>> +# Testcase for a diagnostic around assymetrical restore >>> +# Testcase inspired by byte_insert_op1 in libiberty >>> +# False positive for the diagnostic >>> +.type foo, @function >>> +foo: >>> +.LFB10: >>> + .cfi_startproc >>> + endbr64 >>> + pushq %rbp >>> + .cfi_def_cfa_offset 16 >>> + .cfi_offset 6, -16 >>> + movq %rsp, %rbp >>> + .cfi_def_cfa_register 6 >>> + pushq %r12 >>> + pushq %rbx >>> + subq $24, %rsp >>> + .cfi_offset 12, -24 >>> + .cfi_offset 3, -32 >>> + movl %edi, -20(%rbp) >>> + movq %rsi, -32(%rbp) >>> + movl %edx, -24(%rbp) >>> + movq %rcx, -40(%rbp) >>> +# The assembler cannot differentiate that the following >>> +# mov to %rbx is not a true restore operation, but simply >>> +# %rbx register usage as a scratch reg of some sort. >>> +# The assembler merely warns of a possible assymetric restore operation >>> +# In this case, its noise for the user unfortunately. >>> + movq -40(%rbp), %rbx >>> + movq -40(%rbp), %rax >>> + leaq 3(%rax), %r12 >>> + jmp .L563 >>> +.L564: >>> + subq $1, %rbx >>> + subq $1, %r12 >>> + movzbl (%rbx), %eax >>> + movb %al, (%r12) >>> +.L563: >>> + cmpq -32(%rbp), %rbx >>> + jne .L564 >> >> But this is pretty common usage. Imo this (the false positive warning) >> cannot remain like this. >> > > The warning of "Warning: SCFI: asymetrical register restore" was added > to alert user if they do something like > pushq %rbx > pushq %r12 > ... > popq %rbx > popq %r12 > i.e., asymmetric save and restore. Now the user may do a restore to any > register and use what was saved on stack as scratch regiser; the only > way to differentiate between a "true reg restore in epilogue" vs "reg > restored for scratch purposes" is to know whether its the epilogue. GAS > will not be able to determine that easily. > > We could remove the warning altogether. I just thought it provides > users with some protection / validation of hand-written asm. I'm in favor of such assisting warnings, but they're useful only if there are few false positives and few false negatives. Too many warnings makes people ignore them all (or shut them off), while too few makes people mistrust them being helpful. >>> --- /dev/null >>> +++ b/gas/testsuite/gas/scfi/x86_64/scfi-x86-64.exp >>> @@ -0,0 +1,103 @@ >>> +# Copyright (C) 2022-2023 Free Software Foundation, Inc. >>> + >>> +# This program 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 of the License, or >>> +# (at your option) any later version. >>> +# >>> +# This program 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 this program; if not, write to the Free Software >>> +# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. >>> + >>> +if { ![is_elf_format] } then { >>> + return >>> +} >>> + >>> +# common tests >>> +if { ([istarget "x86_64-*-*"]) } then { >>> + >>> + global ASFLAGS >>> + set old_ASFLAGS "$ASFLAGS" >>> + >>> + run_dump_test "scfi-cfi-label-1" >>> + run_list_test "scfi-cfi-label-1" "--scfi --warn" >>> + >>> + run_list_test "scfi-diag-1" "--scfi" >>> + run_list_test "scfi-fp-diag-2" "--scfi" >>> + run_list_test "scfi-diag-2" "--scfi" >>> + >>> + run_list_test "scfi-unsupported-1" "--32 --scfi" >> >> Perhaps a 2nd run with --x32, unless (see above) you choose to support >> that ABI as well? >> > > At the moment, the plan was to next work on --scfi=inline and add > support for aarch64/aapcs64 ABIs. No plans to add support for x32 > unless a need arises. But that's precisely my point: As long as you don't support --x32, you want to check that its use is properly diagnosed (just like use of --32 is). Jan