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 4DBC53858D1E for ; Mon, 9 Jan 2023 16:48:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4DBC53858D1E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=foss.arm.com Authentication-Results: sourceware.org; spf=none smtp.mailfrom=foss.arm.com 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 CFAFAAD7; Mon, 9 Jan 2023 08:49:13 -0800 (PST) Received: from [10.2.78.76] (unknown [10.2.78.76]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 2DE723F587; Mon, 9 Jan 2023 08:48:31 -0800 (PST) Message-ID: <280ceb21-4ea7-33ae-0d87-c3f01db33a9e@foss.arm.com> Date: Mon, 9 Jan 2023 16:48:29 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [PATCH 9/15] arm: Set again stack pointer as CFA reg when popping if necessary Content-Language: en-GB To: Andrea Corallo , Andrea Corallo via Gcc-patches Cc: nd , Richard Earnshaw References: From: Richard Earnshaw In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3489.7 required=5.0 tests=BAYES_00,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,NICE_REPLY_A,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: On 09/01/2023 14:58, Andrea Corallo via Gcc-patches wrote: > Andrea Corallo via Gcc-patches writes: > >> Richard Earnshaw writes: >> >>> On 27/09/2022 16:24, Kyrylo Tkachov via Gcc-patches wrote: >>>> >>>>> -----Original Message----- >>>>> From: Andrea Corallo >>>>> Sent: Tuesday, September 27, 2022 11:06 AM >>>>> To: Kyrylo Tkachov >>>>> Cc: Andrea Corallo via Gcc-patches ; Richard >>>>> Earnshaw ; nd >>>>> Subject: Re: [PATCH 9/15] arm: Set again stack pointer as CFA reg when >>>>> popping if necessary >>>>> >>>>> Kyrylo Tkachov writes: >>>>> >>>>>> Hi Andrea, >>>>>> >>>>>>> -----Original Message----- >>>>>>> From: Gcc-patches >>>>>> bounces+kyrylo.tkachov=arm.com@gcc.gnu.org> On Behalf Of Andrea >>>>>>> Corallo via Gcc-patches >>>>>>> Sent: Friday, August 12, 2022 4:34 PM >>>>>>> To: Andrea Corallo via Gcc-patches >>>>>>> Cc: Richard Earnshaw ; nd >>>>>>> Subject: [PATCH 9/15] arm: Set again stack pointer as CFA reg when >>>>> popping >>>>>>> if necessary >>>>>>> >>>>>>> Hi all, >>>>>>> >>>>>>> this patch enables 'arm_emit_multi_reg_pop' to set again the stack >>>>>>> pointer as CFA reg when popping if this is necessary. >>>>>>> >>>>>> >>>>>> From what I can tell from similar functions this is correct, but could you >>>>> elaborate on why this change is needed for my understanding please? >>>>>> Thanks, >>>>>> Kyrill >>>>> >>>>> Hi Kyrill, >>>>> >>>>> sure, if the frame pointer was set, than it is the current CFA register. >>>>> If we request to adjust the current CFA register offset indicating it >>>>> being SP (while it's actually FP) that is indeed not correct and the >>>>> incoherence we will be detected by an assertion in the dwarf emission >>>>> machinery. >>>> Thanks, the patch is ok >>>> Kyrill >>>> >>>>> >>>>> Best Regards >>>>> >>>>> Andrea >>> >>> Hmm, wait. Why would a multi-reg pop be updating the stack pointer? >> >> Hi Richard, >> >> not sure I understand, isn't any pop updating SP by definition? > > > Back on this, > > compiling: > > ======= > int i; > > void foo (int); > > int bar() > { > foo (i); > return 0; > } > ======= > > With -march=armv8.1-m.main+fp -mbranch-protection=pac-ret+leaf -mthumb -O0 -g > > Produces the following asm for bar. > > bar: > @ args = 0, pretend = 0, frame = 0 > @ frame_needed = 1, uses_anonymous_args = 0 > pac ip, lr, sp > push {r3, r7, ip, lr} > add r7, sp, #0 > ldr r3, .L3 > ldr r3, [r3] > mov r0, r3 > bl foo > movs r3, #0 > mov r0, r3 > pop {r3, r7, ip, lr} > aut ip, lr, sp > bx lr > > The offending instruction causing the ICE (without this patch) when > emitting dwarf is "pop {r3, r7, ip, lr}". > > The current CFA reg when emitting the multipop is R7 (the frame > pointer). If is not the multipop that has the duty to restore SP as > current CFA here which other instruction should do it? > Digging a bit deeper, I'm now even more confused. arm_expand_epilogue contains (parphrasing the code): if frame_pointer_needed { if arm {} else { if adjust r7 += adjust mov sp, r7 // Reset CFA to SP } } so there should always be a move of r7 into SP, even if this is strictly redundant. I don't understand why this doesn't happen for your testcase. Can you dig a bit deeper? I wonder if we've (probably incorrectly) assumed that this function doesn't need an epilogue but can use a simple return? I don't think we should do that when authentication is needed: a simple return should really be one instruction. > Best Regards > > Andrea R.