From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 3B39A387545A for ; Thu, 11 Jul 2024 20:10:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3B39A387545A Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 3B39A387545A Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=217.140.110.172 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1720728624; cv=none; b=aW+kYOtmZRuue5kLHg/Ll55SOFe/RYzsdn6RNmEk1Y7M4Z2nVMLB+RzGKta9kph69QkoU0VAJwu/6ZxcGbz+Gccw96yUHetFC/IzHRwGkGnCGhmTYh+mjWa2xBFhwSFRYT9lH3kOkGExVyzJx+2pm/a0TBKCVoyQ9t3C2InvVgA= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1720728624; c=relaxed/simple; bh=paB7zuZQWXgE8UqqqdmbIm7IxWRH9h3+4Muie4Z/vp0=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=so9wE9xeMehStXZeeMj+nfqWoSOu7cZqovWYVWE4kIJQyrcSZf1FvhjzLU2cRkCExE44U/KVbN27+F9Grc67wjSYPdMGvTD9pQMrtvRlhV6v0mFc8pN3z2ZyzQcNQilKXZjnjYkYYY5MricGsyrLPkS14NGdugsUxPJ8TT4Dkdw= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 216BD1007; Thu, 11 Jul 2024 13:10:48 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.110.72]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 5D0A93F762; Thu, 11 Jul 2024 13:10:22 -0700 (PDT) From: Richard Sandiford To: Indu Bhagat Mail-Followup-To: Indu Bhagat ,binutils@sourceware.org, Richard.Earnshaw@arm.com, richard.sandiford@arm.com Cc: binutils@sourceware.org, Richard.Earnshaw@arm.com Subject: Re: [PATCH,V4 7/8] gas: aarch64: add experimental support for SCFI In-Reply-To: <8bc9fd54-bceb-4440-84d4-46afa8de8119@oracle.com> (Indu Bhagat's message of "Thu, 11 Jul 2024 12:07:40 -0700") References: <20240701025404.3361349-1-indu.bhagat@oracle.com> <20240701025404.3361349-8-indu.bhagat@oracle.com> <8bc9fd54-bceb-4440-84d4-46afa8de8119@oracle.com> Date: Thu, 11 Jul 2024 21:10:21 +0100 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-13.4 required=5.0 tests=BAYES_00,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Indu Bhagat writes: > On 7/11/24 06:15, Richard Sandiford wrote: >> Indu Bhagat writes: >>> On 7/1/24 12:49, Richard Sandiford wrote: >>>> Indu Bhagat writes: >>>>> +{ >>>>> + enum aarch64_operand_class opnd_class; >>>>> + unsigned int dw2reg_num = 0; >>>>> + >>>>> + opnd_class = aarch64_get_operand_class (opnd->type); >>>>> + >>>>> + switch (opnd_class) >>>>> + { >>>>> + case AARCH64_OPND_CLASS_FP_REG: >>>>> + dw2reg_num = opnd->reg.regno + 64; >>>>> + break; >>>>> + case AARCH64_OPND_CLASS_SVE_REGLIST: >>>>> + dw2reg_num = opnd->reglist.first_regno + 64; >>>>> + break; >>>>> + case AARCH64_OPND_CLASS_MODIFIED_REG: >>>>> + case AARCH64_OPND_CLASS_INT_REG: >>>>> + case AARCH64_OPND_CLASS_ADDRESS: >>>>> + /* Use a dummy register value in case of WZR, else this will be an >>>>> + incorrect dependency on REG_SP. */ >>>>> + if (!sp_allowed_p && opnd->reg.regno == REG_SP) >>>>> + dw2reg_num = GINSN_DW2_REGNUM_R1_DUMMY; >>>>> + else >>>>> + /* For GPRs of our interest (callee-saved regs, SP, FP, LR), >>>>> + DWARF register number is the same as AArch64 register number. */ >>>>> + dw2reg_num = opnd->reg.regno; >>>>> + break; >>>> >>>> I think the AARCH64_OPND_CLASS_ADDRESS case should look at opnd->addr >>>> instead. AARCH64_OPND_CLASS_MODIFIED_REG should look at opnd->shifter. >>>> >>> >>> Made the correction for AARCH64_OPND_CLASS_ADDRESS. >>> >>> But for AARCH64_OPND_CLASS_MODIFIED_REG, the register information is >>> still correct in opnd->reg.regno, IIUC. It seems to me that the only >>> information available in the opnd->shifter is how the register is >>> modified by additional work (shift, multiply etc.); This information is >>> not used by SCFI: >>> - an add/sub with two source register and destination REG_SP/ REG_FP >>> makes REG_SP/ REG_FP untraceable. So ignoring the shift amount etc does >>> not hurt SCFI correctness. >>> - Cant think of other operations where the shift amount will affect >>> SCFI correctness.. >>> >>> So I am not sure of the "AARCH64_OPND_CLASS_MODIFIED_REG should look at >>> opnd->shifter." of the review comment. >> >> It's more about type correctness. "shifter" is the data associated with >> AARCH64_OPND_CLASS_MODIFIED_REG and "reg" is the data associated with >> AARCH64_OPND_CLASS_INT_REG etc. I think it's mostly a coincidence >> that the "reg" and "shifter" alternatives of the union put the register >> at the same byte offset from the start of the structure. >> > > The shifter struct is out of the union in struct aarch64_opnd_info. Oops, yes, I shouldn't have relied on memory. Sorry for the noise. Richard