From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x132.google.com (mail-lf1-x132.google.com [IPv6:2a00:1450:4864:20::132]) by sourceware.org (Postfix) with ESMTPS id 66A333846030 for ; Wed, 11 May 2022 13:26:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 66A333846030 Received: by mail-lf1-x132.google.com with SMTP id h29so3601300lfj.2 for ; Wed, 11 May 2022 06:26:28 -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=pRuukaYmknxCvc8WAFrzQhmX91OzQTeokJBMe2yC83s=; b=cFxyi4CFfFwjlDhD+cRgv+BnVhAVceCyQQpzqo5M0dB1T4uKH2k7RLyCe2Qxv0BBV4 Il3kDtNRBDx8/SZw6AcNn2ClOCNEv/n/8PMEZtXNHNhyijuNKhRuEmYgOXmAnzZIMBdk EIZYLD93W56TnKwa6X5jdTMdBUIXDDti8HE5juOQY6WZQzP2FZxInUS1qoxhUIahXLYU O//TWd7yiY6zX7Yk1y/FkTIvONQJRDiEKqjAhlMYtTSRHr1Nf7chZQnpkZQgtUQn1Mb+ O8NsBGFpE5nrIR+tC+2R7dRHaoBC1sDhA/CcyZIuNOLa3LonugFrm1JyPU1i2PzTzS28 D8hw== X-Gm-Message-State: AOAM530ybUY2N66UVcp23qn1Psw5JVIW9S1TEaEp2z5e3gZLrOnnPz63 ZMqN3KFrn9KJU3uFr+QlGBUKrCfOfH9bJwYtnDQFM3LN9EM= X-Google-Smtp-Source: ABdhPJwnxus6Jtu0lIH3mfsh8z/HaXW5sLuqM+9pjOOlxhUedf8AM7EmVJFmDPICb6+6IcBeKaRIfkCsTKXXFKJ8YFs= X-Received: by 2002:a19:6b19:0:b0:471:f39c:7d1d with SMTP id d25-20020a196b19000000b00471f39c7d1dmr20426247lfa.443.1652275585960; Wed, 11 May 2022 06:26:25 -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> In-Reply-To: <02ebd5db-54c6-7fba-e75a-fe35cf1253af@arm.com> From: Yichao Yu Date: Wed, 11 May 2022 09:26:17 -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=-1.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.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Wed, 11 May 2022 13:26:31 -0000 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? > > > > 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? > 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....) > 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. > 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? 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. > Could you please send a patch to gdb-patches@? Yes, sure. Mainly want to first figure out what's the expected solution here. > > > >> PC is probably specified > >> as DWARF2_FRAME_REG_SAME_VALUE (as far as I remember), so it will cause > >> some issues during unwinding. > > > > I don't believe this is the case. AFAICT, the only way gcc emits debug > > info to specify that pc is in lr without relying on architecture > > detail is that it specifies the return address column is 30 in the > > CIE. Currently, on aarch64, the return address column is basically > > never used but if the how for PC is changed to RA then it seems that > > gdb does pick it up correctly. > > > >> There is a comment in gdb/dwarf2/expr.c about some odd cases. For example: > >> > >> > >> /* GCC, in its infinite wisdom decided to not provide unwind > >> information for registers that are "same value". Since > >> DWARF2 (3 draft 7) doesn't define such behavior, said > >> registers are actually undefined (which is different to CFI > >> "undefined"). Code above issues a complaint about this. > >> Here just fudge the books, assume GCC, and that the value is > >> more inner on the stack. */ > >> > >> > >>> > >>> > >>>> > >>>> [2] https://github.com/bminor/glibc/blob/b92a49359f33a461db080a33940d73f47c756126/sysdeps/aarch64/rawmemchr.S#L34 > >>>> > >>>>>>> > >>>>>>> According to aadwarf64[1], > >>>>>>> > >>>>>>>> having both LR and PC columns is useful for describing asynchronously created stack frames. A DWARF expression may use this register to restore the context in case of a signal context. > >>>>>>> > >>>>>>> so assume the intention is that if I explicitly unwind the pc in > >>>>>>> addition to lr, it should work. I tried to do that, and also to set > >>>>>>> return address column to 32, as well as trying to mark the frame as > >>>>>>> signal frame but none of them seems to work. Is there any way for gdb > >>>>>>> to honer the explicit unwinding of pc? > >>>>>>> > >>>>>>> Also it seems that the sp is also card coded to be cfa. My code also > >>>>>>> contains explicit saving and restoring of that as well so if that's > >>>>>>> the case (haven't tested yet) it would be a problem too... > >>>>>>> > >>>>>>> Would it be possible to not use this hard-coded logic if the frame > >>>>>>> contains explicit override of the pc value? > >>>>>>> > >>>>>>> Yichao > >>>>>>> > >>>>>>> A bit more about the actual code. This is done as part of runtime > >>>>>>> patching code. The actual restoration of lr is done by returning to a > >>>>>>> runtime allocated stub that restores lr and directly branch back to > >>>>>>> the return location. After returning, all registers values are > >>>>>>> restored back to their previous one. The stack pointer is also > >>>>>>> switched out since we cannot rely on how much stack space the call > >>>>>>> site has available. > >>>>> > >>>>> This seems to work in a similar way as signal handler. GDB needs to be > >>>>> taught where to find the registers so it can properly unwind things. > >>>>> > >>>>>>> > >>>>>>> [1] https://github.com/ARM-software/abi-aa/blob/8a7b266879c60ca1c76e94ebb279b2dac60ed6a5/aadwarf64/aadwarf64.rst#note-9 > >>>>> > >> >