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 88E303858C1F for ; Mon, 9 Jan 2023 15:57:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 88E303858C1F 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 078FB1042; Mon, 9 Jan 2023 07:58:00 -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 59B653F587; Mon, 9 Jan 2023 07:57:17 -0800 (PST) Message-ID: Date: Mon, 9 Jan 2023 15:57:16 +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? Ah, OK. I think this is a special case, though, because in this specific case the frame pointer (r7) and the stack pointer point to the same place. This means that in the epilogue we don't start by restoring SP from FP (at which point we tell the dwarf code that the frame is back in SP again). For example, if I have: int i; void foo (int, int*); int bar() { int j[10]; foo (i,j); return 0; } then the epilogue sequence starts with: adds r7, r7, #40 .cfi_def_cfa_offset 8 mov sp, r7 .cfi_def_cfa_register 13 And then the pop works correctly as-is. But I'm not convinced that this is enough anyway, you cause the compiler to output a directive that changes the CFA pointer back to r13, but you don't output anything that changes the CFA offset. So I think this means that the CFA state machine ends up pointing to the wrong location, but it's hard to tell as you haven't shown the CFA directives in your example above. > > Best Regards > > Andrea R.