From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x131.google.com (mail-lf1-x131.google.com [IPv6:2a00:1450:4864:20::131]) by sourceware.org (Postfix) with ESMTPS id 3D0A0395A071 for ; Fri, 13 May 2022 12:34:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3D0A0395A071 Received: by mail-lf1-x131.google.com with SMTP id i10so14219609lfg.13 for ; Fri, 13 May 2022 05:34:45 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=0XBx1rkHnkRRJ1YXkjeKW1wwVDemm1VRQgN1g8ILp9w=; b=flPPrr4GvK3Cd6V1V4LI1DvfsT4EcA6Ms/bv8e4eJZa3GFwtyDFl+lWxEXzigKA5GN uIwmXCDuXBCrBWD+lhKFeTmWASKIUdmba666x1BcU5hxxRT60q63W+o86pTIgbS4LHvD h3rnTZQonwckz4fk/tkvHlju5R3nPYSs+DuLeWf0cnTIl5zM+demhJKNclVmyEL1ZtmV +TDmx12lyt8djwHHHqCh1vT/aVPd0gYL013lDBBbVhDcQ8PuoUohrUk1WMRGrqghRLNf oxKMRkgZ8K37iqSO/QM+sjaqgWFVDfIL2jMy59VQN4+s0xeDXGwc3ZaxsO034uJzctF7 xqNw== X-Gm-Message-State: AOAM5328zEzPP/LVISXlvmZyo7gVMXPu/1vPAlfiwfHjtv+/lAT7uRR+ dtag/oY1p0r3M7cYACa0FEQEE8uVUoKOZJ2hUm+R8/Ti80o= X-Google-Smtp-Source: ABdhPJwG1exsYz5xYbL/1QWnuxQ7KqAY2O2CDkJxqWUOEOGrtabY524NTY6T+0RHuBDDHJTBr0jat0ItgH0VdcToQYU= X-Received: by 2002:a19:7605:0:b0:473:d5f0:3bd7 with SMTP id c5-20020a197605000000b00473d5f03bd7mr3251338lff.477.1652445283331; Fri, 13 May 2022 05:34:43 -0700 (PDT) MIME-Version: 1.0 References: <257aa215-a141-6d9b-672d-ea8ce209b107@arm.com> <63f63d70-8a55-618c-06e8-22f923c39e02@arm.com> <02ebd5db-54c6-7fba-e75a-fe35cf1253af@arm.com> <763c426d-7a62-bcb9-223a-d1b5b7e8fc03@arm.com> In-Reply-To: <763c426d-7a62-bcb9-223a-d1b5b7e8fc03@arm.com> From: Yichao Yu Date: Fri, 13 May 2022 08:34:31 -0400 Message-ID: Subject: Re: Restoring pc to a different value than lr on aarch64 To: Luis Machado Cc: gdb@sourceware.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-0.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, KAM_NUMSUBJECT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 May 2022 12:34:47 -0000 On Wed, May 11, 2022 at 10:52 AM Luis Machado wrote: > > On 5/11/22 14:26, Yichao Yu wrote: > > On Tue, May 10, 2022 at 10:49 AM Luis Machado wrote: > >> > >> On 5/9/22 15:24, Yichao Yu wrote: > >>> On Mon, May 9, 2022 at 6:44 AM Luis Machado wrote: > >>>> > >>>> On 5/6/22 17:30, Yichao Yu wrote: > >>>>> On Fri, May 6, 2022 at 12:11 PM Yichao Yu wrote: > >>>>>> > >>>>>>>> Actually I misspoke for that. It seems that sp is probably fine and > >>>>>>>> the only thing missing causing pc to not work is that > >>>>>>>> aarch64_dwarf_reg_to_regnum doesn't understand the PC dwarf reg > >>>>>>>> number. It seems that the only thing needed is to add a > >>>>>>>> > >>>>>>>> + if (reg == AARCH64_DWARF_PC) > >>>>>>>> + return AARCH64_PC_REGNUM; > >>>>>>>> > >>>>>>>> to that function. > >>>>>>>> > >>>>>>> > >>>>>>> Yes, GDB always assumes the PC from the previous frame is the LR from > >>>>>>> the current frame. That is what GCC generates. > >>>>>>> > >>>>>>> If a different setup is needed, GDB needs to be taught about it. > >>>>>> > >>>>>> I agree the current code makes sense for what gcc generates. However, > >>>>>> I think given the document from arm, explicitly setting the PC value > >>>>>> in the unwind info should also work. > >>>>>> Would a patch similar to the one above be acceptable to fix this issue? > >>>>>> > >>>>>> A related issue is that gdb also seems to be ignoring the return > >>>>>> address register in CIE. There is at least one use of it in glibc[2] > >>>>>> where the return address register is set to x15 instead. > >>>>>> I've verified that gdb is currently unable to unwind after the call to > >>>>>> `strlen` from `rawmemchr` even though the return address is still in > >>>>>> x15. > >>>>>> I thought this can be fixed by chaiming that PC is RA just like the > >>>>>> fallback case but that is apparently not working... > >>>>> > >>>>> Actually this did work but the address is wrong before the value was > >>>>> written to x15... So it was just due to incorrect unwind info (the > >>>>> glibc implementation should specify how to find x15 on the entry of > >>>>> rawmemchr). > >>>>> Is the current implementation due to some edge cases? (like old > >>>>> compiler version doesn't put a valid value in the CIE for the return > >>>>> address register). It seems that many other architecture simply use > >>>>> _RA so I don't see why this would have broader problems... > >> > >> This looks like a genuine bug, one that is not hit that often given it > >> is not very common for frame to change the return address column. This > >> will need to be fixed eventually. Thanks for spotting that. > >> > >>>> > >>>> It is probably historical that this has been handled like this. If there > >>>> is a use case for having a PC column containing distinct data, then we > >>>> could support that. > >>> > >>> I couldn't find any existed code using this, but it seems that this is > >>> the intent from ARM and I really can't think of any other way to > >>> restore everything including both pc and lr so I'd like to support > >>> that at least.... > >>> > >>>> It wouldn't be as simple as that change you mentioned though, as other > >>>> parts of the code assume PC comes from the LR. > >> > >> I take that back. I investigated this further and I think this should > >> work, although it is a use case that is not so common. > >> > >>> > >>> There are indeed other unwinders (the prologue unwinder) that assumes > >>> this, which I think should be fine. Specifying the PC explicitly > >>> should only apply to the dwarf unwinder. The only other place where I > >>> can find that relies on this is aarch64_gen_return_address. It doesn't > >>> seem that this function is used in most of the other logics and if the > >>> return address column is somehow accessible from here it should also > >>> not be too hard to fix. > >> > >> I think it is only the case for the dwarf unwinder, as that is the > >> unwinder that has access to the CFI information. > > > > Sorry what did you mean by "it" here? (being able to handle PC != LR > > during unwinding?) Do I need to worry about gen_return_address? > > > > Sorry, I meant that we only need to worry about the dwarf unwinder > functions. The other code not relying on CFI information doesn't need to > be touched (prologue unwinder, aarch64_gen_return_address etc). > > At this point, we'd like to augment aarch64_dwarf_reg_to_regnum to > return a valid number for the dwarf PC register. ... and I just sent out the patch https://sourceware.org/pipermail/gdb-patches/2022-May/189035.htmlHopefully I did it more or less right (both in the email and in the patch itself) BTW, is there a way to see what gdb actually outputs during the test? Setting `verbose` doesn't seem to help. I couldn't figure out why `p $pc` prints `` when I used it directly but doesn't seem to in the test... `$pc - &main` works which is probably better and is what I'm currently using. > >>> > >>> Is there any other cases that I'm missing? I've also tested with my > >>> simple change and it seems that unwinding from normal functions still > >>> works as intended. > >> > >> Yes, I tried it on my end as well and it does work. What I was worried > >> about is that we need to adjust the LR value in some cases. > >> > >> For aarch32 we need to remove the LSB, and for aarch64 we need to unmask > >> the value of LR if it is signed through PAC. This is the reason why we > >> have a custom dwarf2_prev_register implementation. > > > > Is the aarch64-tdep code used for aarch32 tracee as well? > > > > The aarch64 code is not shared by aarch32 (that would be arm-tdep.c). I > just mentioned it because both arm-tdep.c and aarch64-tdep.c use the > same mechanism for unwinding PC. That is, they return LR instead. OK, that makes sense. > > >> gcc and clang emit the return address column as r30. If we don't specify > >> any initialization rule for PC, then the dwarf2 unwinder will go through > >> the route of fetching LR and adjusting the values. > >> > >> On the other hand, if PC is available in a CFI column, then the dwarf > >> unwinder won't call the custom hook and will instead proceed to > >> calculate PC from the CFI expression at that column, which should give > >> the result that you want. > > > > Right, and I guess that means that the custom unwind rule needs to > > take the pointer authentication into account? > > (I've never used any machines with that enabled so I'm not 100% how > > it's supposed to work....) > > It does need to take that into account. But using a custom function for > unwinding PC means we will not have access to > > > > >> We can't initialize PC to DWARF2_FRAME_REG_RA though (not yet, anyway), > >> as that would default to mapping PC to the return address column. For > >> gcc and clang, this would be x30. That would give GDB no chance of > >> adjusting the LR values if required (for PAC-signed LR values). > > > > Just so that I understand, if LR is signed, then upon return it should > > still have the signed value. (Or in the case a different register is > > used for return address, `ret xn` would not change xn value) > > Only the resulting PC should have the signed part masked off. That's > > why the unwinding of LR is correct without any post-processing logic > > as-is but the unwinding of PC needs to be treated specially. > > > That's correct. > > >> It would be nice to have a testcase for this, alongside your patch, to > >> make sure GDB is always doing the correct unwinding. > > > > The part for returning the name of PC is easy. I assume the test would > > be mainly throwing in a aarch64-xxx.{c,exp} and to check if explicitly > > specifying the PC in the unwind info result in valid unwinding. I'll > > try look for some but any examples that I can follow? > > I think gdb/testsuite/gdb.arch/arm-disp-step.{S,exp} is a good example. > Basically, some .S file that contains some random instructions and CFI > directives that we single-step over and check if GDB is following the > values of the registers correctly. The .S file gives us more control > over those CFI directives. By `.c` I actually mean using some inline assembly. But if .S is supported it is indeed much easier to use. > > This should fix the case where PC is explicitly specified but wont fix > > the case where a return column is specified. Handling that requires a > > different way to post-processing the PC. > > Correct. That is a separate bug and I'll need to investigate that. > > Thanks! > Luis