Message ID | OF2E86E349.429AFBD8-ON00258004.003BC0A8-00258004.003C135E@notes.na.collabserv.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 122245 invoked by alias); 3 Aug 2016 10:56:41 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 122218 invoked by uid 89); 3 Aug 2016 10:56:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00, SPF_PASS autolearn=ham version=3.3.2 spammy=5267, PCs, H*x:Build, v6m X-HELO: mail2.securitas-direct.com Received: from mail2.securitas-direct.com (HELO mail2.securitas-direct.com) (195.170.189.96) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 03 Aug 2016 10:56:28 +0000 Received: from mail.securitas-direct.com (unknown [91.199.64.45]) by Websense Email with ESMTPS id F4008936735B6 for <gdb-patches@sourceware.org>; Wed, 3 Aug 2016 12:56:24 +0200 (CEST) Received: from smtp.notes.na.collabserv.com ([192.155.248.74]) by mail.securitas-direct.com with ESMTP; 03 Aug 2016 12:56:25 +0200 Received: from localhost by smtp.notes.na.collabserv.com with smtp.notes.na.collabserv.com ESMTP for <gdb-patches@sourceware.org> from <fredrik.hederstierna@verisure.com>; Wed, 3 Aug 2016 10:56:12 -0000 Received: from us1a3-smtp02.a3.dal06.isc4sb.com (10.106.154.159) by smtp.notes.na.collabserv.com (10.106.227.92) with smtp.notes.na.collabserv.com ESMTP; Wed, 3 Aug 2016 10:56:11 -0000 X-IBM-Helo: us1a3-smtp02.a3.dal06.isc4sb.com X-IBM-MailFrom: fredrik.hederstierna@verisure.com X-IBM-RcptTo: qiyaoltc@gmail.com;gdb-patches@sourceware.org Received: from us1a3-mail13.a3.dal06.isc4sb.com ([10.146.77.97]) by us1a3-smtp02.a3.dal06.isc4sb.com with ESMTP id 2016080310561086-155935 ; Wed, 3 Aug 2016 10:56:10 +0000 MIME-Version: 1.0 Sensitivity: In-Reply-To: <864m73dutq.fsf@gmail.com> References: <864m73dutq.fsf@gmail.com>, <868twkekf1.fsf@gmail.com> <OF625831A6.9C918507-ON00257FFE.0029D369-00257FFE.002D2551@notes.na.collabserv.com> <OF1926033E.B8166B2F-ON00258003.003441A6-00258003.003556EF@notes.na.collabserv.com> Subject: Re: [PATCH] Fix exception unwinding for ARM Cortex-M From: "Fredrik Hederstierna" <fredrik.hederstierna@verisure.com> To: Yao Qi <qiyaoltc@gmail.com> Cc: gdb-patches@sourceware.org Date: Wed, 3 Aug 2016 10:56:11 +0000 X-LLNOutbound: False X-TNEFEvaluated: 1 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 x-cbid: 16080310-7581-0000-0000-0000003CF2FA X-IBM-ISS-SpamDetectors: Score=0.40962; BY=0; FL=0; FP=0; FZ=0; HX=0; KW=0; PH=0; SC=0.40962; ST=0; TS=0; UL=0; ISC= X-IBM-ISS-DetailInfo: BY=3.00005546; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000177; SDB=6.00739372; UDB=6.00347609; UTC=2016-08-03 10:56:12 x-cbparentid: 16080310-7582-0000-0000-0000008BC18C Message-Id: <OF2E86E349.429AFBD8-ON00258004.003BC0A8-00258004.003C135E@notes.na.collabserv.com> |
Commit Message
Fredrik Hederstierna
Aug. 3, 2016, 10:56 a.m. UTC
Hi Yao, thanks for reviewing, I attach here updated patch. I have FSF assignment papers for GCC from December 2005, maybe needs separate assignment for GDB, so I have now sent in application to FSF for GDB assignment as well. gdb/ChangeLog: 2016-08-03 Fredrik Hederstierna <fredrik.hederstierna@verisure.com> * arm-tdep.c (arm_m_addr_is_magic): New function. (arm_addr_bits_remove) (arm_m_exception_unwind_sniffer): Check correct EXC_RETURN values. --
Comments
On Wed, Aug 3, 2016 at 11:56 AM, Fredrik Hederstierna <fredrik.hederstierna@verisure.com> wrote: > > I have FSF assignment papers for GCC from December 2005, maybe needs separate assignment for GDB, > so I have now sent in application to FSF for GDB assignment as well. > Yes, you need a separate assignment for GDB. Let me know once your assignment is ready. In the meantime, you can continue posting other patches.
Hi, I'm back from the dead. Seriously though, I was a bit too busy at work to pursue on this and I didn't have a clear idea of where to start after the comments. Moreover, as the patch did "just work" at my workplace, it was a bit difficult to justify spending a lot more time digging information about target descriptions, how to determine what features are present on target, etc. So thanks to push toward the progress of this issue, apparently with your mail subject you successfully brought people that gives more specific guidance to get this done. Indeed, the patch looks really close to what I've done, to the point that I can see that, aside from a small refactoring, the changes between our patches are mainly variable name changes and added comments. You really wrote your patch from scratch? You've got rid of the various calls to `user_reg_map_name_to_regnum', though, which is nice. > Cortex-M has two stack pointers: MSP (Main Stack Pointer) and PSP (Process Stack Pointer). > This is not handled when GDB tries to backtrace in the exception stack unwinder. > Meaning for eg. Cortex-M4F its not possible to get any call-stack backtrace if setting a breakpoint in ISR, and floating-point variable was used. Actually, the info here is inaccurate: FPU context stacking and the MSP/PSP switch are two unrelated concepts. Meaning, you can fix the code to deal with the PSP, but it still won't handle FPU register unstacking. > This patch was inspired by the great work done by James-Adam Renquinha Henri, submitted April this year. Thanks :) > The next thing would then be to also add FPU context control reg FPCCR, which is needed for retrieving info on the FPU lazy stacking. > Though its complicated I think and I will try to investigate an 'arm-m-fpu.xml' profile further, if this is solution perhaps. It indeed is just a bit more complicated. Let me summarize what needs to be done. Have the ARMv7-M Architecture Reference Manual handy, see B1.5.7. - Check if lazy stacking is enabled (FPCCR.LSPEN == 1). If it's not, the case is uncomplicated, registers are stacked as usual - If lazy stacking is active (FPCCR.LSACT == 1), the extended stack has space reserved for the FPU registers (S0-S15, FPSCR), but there are not stacked, they are still in FPU registers unmodified. So that adds more random fetches from "memory" of the target, because these are memory-mapped. ARM gives some example scenarios with chronograms of some important bits with stacking information [1]. > ChangeLog entry should cover what do you change on the function level. > Please read https://sourceware.org/gdb/wiki/ContributionChecklist If only I knew that checklist! I searched for something like this, but maybe it didn't use the correct search terms, I didn't find it. [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dai0298a/DAFGGBJD.html James-Adam Renquinha Henri, Ing. jr Ingénieur d'application CIMEQ INC. Le 2016-08-03 à 06:56, Fredrik Hederstierna a écrit : > Hi Yao, > > thanks for reviewing, I attach here updated patch. > > I have FSF assignment papers for GCC from December 2005, maybe needs separate assignment for GDB, > so I have now sent in application to FSF for GDB assignment as well. > > gdb/ChangeLog: > > 2016-08-03 Fredrik Hederstierna <fredrik.hederstierna@verisure.com> > > * arm-tdep.c (arm_m_addr_is_magic): New function. > (arm_addr_bits_remove) > (arm_m_exception_unwind_sniffer): Check correct EXC_RETURN values. > > diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c > index d2661cb..1d154cc 100644 > --- a/gdb/arm-tdep.c > +++ b/gdb/arm-tdep.c > @@ -464,6 +464,61 @@ arm_pc_is_thumb (struct gdbarch *gdbarch, CORE_ADDR memaddr) > return 0; > } > > +/* Determine if the address specified equals any of > + these magic return values, called EXC_RETURN, defined > + by the ARM v6-M and v7-M architectures. > + > + From ARMv6-M Reference Manual B1.5.8 > + Table B1-5 Exception return behavior > + > + EXC_RETURN Return To Return Stack > + 0xFFFFFFF1 Handler mode Main > + 0xFFFFFFF9 Thread mode Main > + 0xFFFFFFFD Thread mode Process > + > + From ARMv7-M Reference Manual B1.5.8 > + Table B1-8 EXC_RETURN definition of exception return behavior, no FP > + > + EXC_RETURN Return To Return Stack > + 0xFFFFFFF1 Handler mode Main > + 0xFFFFFFF9 Thread mode Main > + 0xFFFFFFFD Thread mode Process > + > + Table B1-9 EXC_RETURN definition of exception return behavior, with FP > + > + EXC_RETURN Return To Return Stack Frame Type > + 0xFFFFFFE1 Handler mode Main Extended > + 0xFFFFFFE9 Thread mode Main Extended > + 0xFFFFFFED Thread mode Process Extended > + 0xFFFFFFF1 Handler mode Main Basic > + 0xFFFFFFF9 Thread mode Main Basic > + 0xFFFFFFFD Thread mode Process Basic > + > + For more details see "B1.5.8 Exception return behavior" > + in both ARMv6-M and ARMv7-M Architecture Reference Manuals. */ > + > +static int > +arm_m_addr_is_magic (CORE_ADDR addr) > +{ > + switch (addr) > + { > + /* Values from Tables in B1.5.8 the EXC_RETURN definitions of > + the exception return behavior. */ > + case 0xffffffe1: > + case 0xffffffe9: > + case 0xffffffed: > + case 0xfffffff1: > + case 0xfffffff9: > + case 0xfffffffd: > + /* Address is magic. */ > + return 1; > + > + default: > + /* Address is not magic. */ > + return 0; > + } > +} > + > /* Remove useless bits from addresses in a running program. */ > static CORE_ADDR > arm_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR val) > @@ -471,7 +526,7 @@ arm_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR val) > /* On M-profile devices, do not strip the low bit from EXC_RETURN > (the magic exception return address). */ > if (gdbarch_tdep (gdbarch)->is_m > - && (val & 0xfffffff0) =0xfffffff0) > + && arm_m_addr_is_magic (val)) > return val; > > if (arm_apcs_32) > @@ -2990,14 +3045,8 @@ arm_m_exception_unwind_sniffer (const struct frame_unwind *self, > /* No need to check is_m; this sniffer is only registered for > M-profile architectures. */ > > - /* Exception frames return to one of these magic PCs. Other values > - are not defined as of v7-M. See details in "B1.5.8 Exception > - return behavior" in "ARMv7-M Architecture Reference Manual". */ > - if (this_pc =0xfffffff1 || this_pc == 0xfffffff9 > - || this_pc =0xfffffffd) > - return 1; > - > - return 0; > + /* Check if exception frame returns to a magic PC value. */ > + return arm_m_addr_is_magic (this_pc); > } > > /* Frame unwinder for M-profile exceptions. */ > -- >
Hi James-Adam, -----James-Adam Renquinha Henri <arenquinha@cimeq.qc.ca> wrote: ----- >Hi, I'm back from the dead. Nice to see you back :) >Seriously though, I was a bit too busy at work to pursue on this and >I >didn't have a clear idea of where to start after the comments. >Moreover, >as the patch did "just work" at my workplace, it was a bit difficult >to >justify spending a lot more time digging information about target >descriptions, how to determine what features are present on target, >etc. >So thanks to push toward the progress of this issue, apparently with >your mail subject you successfully brought people that gives more >specific guidance to get this done. I tried to check the status on your submission mid-July (https://sourceware.org/ml/gdb/2016-07/msg00011.html), but since I got no directions from anyone in the community at that time, I decided to try continue myself to get this fixed. We work alot with Cortex-M4F on a daily basis, and do really need this to be fixed. >Indeed, the patch looks really close to what I've done, to the point >that I can see that, aside from a small refactoring, the changes >between >our patches are mainly variable name changes and added comments. You >really wrote your patch from scratch? You've got rid of the various >calls to `user_reg_map_name_to_regnum', though, which is nice. Yes I wrote it from scratch, but I agree my patch looks similar to yours, as its implementing according to ARM Cortex-M spec, I guess it needs to be. But it absolutely helped me to have your ideas as starting point to look at, to see where modifications needed to be done, so I do not mind if you would like to stand as co-author in the ChangeLog? If you have FSF GDB Assignment, or would like to apply for assignment, I can add your name to ChangeLog, its totally fine by me. The important thing for me is to get this fixed, and who gets the actual and final credits I do not mind. And I think it would be great if we could try to continue this work together, to get the remaining parts for Cortex-M support in GDB. The more people who are working on this, the better. I am not so experienced in GDB internals, and also seek guidance how to continue go get the last parts in place. I think both to get GDB aware of MSP and PSP, and also FPCCR needs to be a bigger change and are more complicated. My idea is to submit a bug entry in bugzilla on this, one issue for the MSP/PSP support, another one for FPCCR, so we can get track of this, and someone could be assigned. > > Cortex-M has two stack pointers: MSP (Main Stack Pointer) and PSP >(Process Stack Pointer). > > This is not handled when GDB tries to backtrace in the exception >stack unwinder. > > Meaning for eg. Cortex-M4F its not possible to get any call-stack >backtrace if setting a breakpoint in ISR, and floating-point variable >was used. > >Actually, the info here is inaccurate: FPU context stacking and the >MSP/PSP switch are two unrelated concepts. Meaning, you can fix the >code >to deal with the PSP, but it still won't handle FPU register >unstacking. Correct, my plan now was to split the patch as Yao proposed. The the MSP/PSP-fix and FPU-fix would be two different patches. > > This patch was inspired by the great work done by James-Adam >Renquinha Henri, submitted April this year. >Thanks :) > > > The next thing would then be to also add FPU context control reg >FPCCR, which is needed for retrieving info on the FPU lazy stacking. > > Though its complicated I think and I will try to investigate an >'arm-m-fpu.xml' profile further, if this is solution perhaps. > >It indeed is just a bit more complicated. Let me summarize what needs >to >be done. Have the ARMv7-M Architecture Reference Manual handy, see >B1.5.7. > >- Check if lazy stacking is enabled (FPCCR.LSPEN == 1). If it's not, >the >case is uncomplicated, registers are stacked as usual >- If lazy stacking is active (FPCCR.LSACT == 1), the extended stack >has >space reserved for the FPU registers (S0-S15, FPSCR), but there are >not >stacked, they are still in FPU registers unmodified. > >So that adds more random fetches from "memory" of the target, because > >these are memory-mapped. > >ARM gives some example scenarios with chronograms of some important >bits >with stacking information [1]. Yes, my idea was that GDB needs to be aware of FPCCR, so we need probably an new XML profile feature for this, just as for PSP/MSP. But you are correct, its way more complicated. See also ARM Application Note I'm referring to in my patch comments. > > ChangeLog entry should cover what do you change on the function >level. > > Please read https://sourceware.org/gdb/wiki/ContributionChecklist > >If only I knew that checklist! I searched for something like this, >but >maybe it didn't use the correct search terms, I didn't find it. If you want to stand as co-author in ChangeLog, its totally fine by me, just give me a hint if you have the FSF assignment done, and I also would be more than happy if you have time to continue your great work, and get the last parts of unwinding support for Cortex-M in place. Thanks, and Kind Regards, Fredrik
Hi Fredrik, >> Seriously though, I was a bit too busy at work to pursue on this and >> I >> didn't have a clear idea of where to start after the comments. >> Moreover, >> as the patch did "just work" at my workplace, it was a bit difficult >> to >> justify spending a lot more time digging information about target >> descriptions, how to determine what features are present on target, >> etc. >> So thanks to push toward the progress of this issue, apparently with >> your mail subject you successfully brought people that gives more >> specific guidance to get this done. > > I tried to check the status on your submission mid-July (https://sourceware.org/ml/gdb/2016-07/msg00011.html), > but since I got no directions from anyone in the community at that time, I decided to try continue myself to get this fixed. > We work alot with Cortex-M4F on a daily basis, and do really need this to be fixed. We do, too. It isn't fun when you try to debug a firmware using FreeRTOS and the FPU on a Cortex-M4, only to see that you cannot backtrace into suspended tasks, is it? It took quite a bit of time for us to understand where the problem lied... I worked on this bug because I could build about anything really easily in my Arch Linux VM, and I know very well how to use plain gdb. >> Indeed, the patch looks really close to what I've done, to the point >> that I can see that, aside from a small refactoring, the changes >> between >> our patches are mainly variable name changes and added comments. You >> really wrote your patch from scratch? You've got rid of the various >> calls to `user_reg_map_name_to_regnum', though, which is nice. > > Yes I wrote it from scratch, but I agree my patch looks similar to yours, as its implementing according to ARM Cortex-M spec, > I guess it needs to be. But it absolutely helped me to have your ideas as starting point to look at, to see where modifications needed to be done, > so I do not mind if you would like to stand as co-author in the ChangeLog? > If you have FSF GDB Assignment, or would like to apply for assignment, > I can add your name to ChangeLog, its totally fine by me. That is fine for me too, I don't mind at all. I never had submitted work that required an FSF submission, but I plan to do that. > And I think it would be great if we could try to continue this work together, to get the remaining parts for Cortex-M support in GDB. > The more people who are working on this, the better. Yes! :) > I am not so experienced in GDB internals, and also seek guidance how to continue go get the last parts in place. I'm not experienced at all with GDB internals either. I ran a test program under `arm-none-eabi-gdb' under plain gdb (a bit of a headache, a debugger inside a debugger), and I traced what happened upon unwinding, and fixed rather directly what I saw was wrong. I knew that what would result from that process would be rather crude, due to my inexperience. For instance, target descriptions were news for me. Guidance on that specific part (handling target description) is needed. Anyone? > My idea is to submit a bug entry in bugzilla on this, one issue for the MSP/PSP support, another one for FPCCR, so we can get track of this, and someone could be assigned. Good idea. >>> Cortex-M has two stack pointers: MSP (Main Stack Pointer) and PSP >> (Process Stack Pointer). >>> This is not handled when GDB tries to backtrace in the exception >> stack unwinder. >>> Meaning for eg. Cortex-M4F its not possible to get any call-stack >> backtrace if setting a breakpoint in ISR, and floating-point variable >> was used. >> >> Actually, the info here is inaccurate: FPU context stacking and the >> MSP/PSP switch are two unrelated concepts. Meaning, you can fix the >> code >> to deal with the PSP, but it still won't handle FPU register >> unstacking. > > Correct, my plan now was to split the patch as Yao proposed. > The the MSP/PSP-fix and FPU-fix would be two different patches. Yes. Smaller patches will help things moving forward. I wasn't sure at the time if beginning with a small patch just covering e.g. magic PC values handling would be considered useful, because in itself it fixes nothing, but it really is important to handle the MSP/PSP and the FPU cases. So I opted for a big comprehensive patch. But indeed, it's easier to review, and easier to integrate. >>> ChangeLog entry should cover what do you change on the function >> level. >>> Please read https://sourceware.org/gdb/wiki/ContributionChecklist >> >> If only I knew that checklist! I searched for something like this, >> but >> maybe it didn't use the correct search terms, I didn't find it. > > If you want to stand as co-author in ChangeLog, its totally fine by me, > just give me a hint if you have the FSF assignment done, > and I also would be more than happy if you have time to continue your great work, and get the last parts of unwinding support for Cortex-M in place. Hmm, if I understand correctly, I should e-mail one of the gdb maintainers to get the assignment to fill? You did that before; how it went? Thanks, James-Adam Renquinha Henri, Ing. jr Ingénieur d'application CIMEQ INC. Le 2016-08-04 à 03:03, Fredrik Hederstierna a écrit : > Hi James-Adam, > > -----James-Adam Renquinha Henri <arenquinha@cimeq.qc.ca> wrote: ----- >> Hi, I'm back from the dead. > > Nice to see you back :) > > >> Seriously though, I was a bit too busy at work to pursue on this and >> I >> didn't have a clear idea of where to start after the comments. >> Moreover, >> as the patch did "just work" at my workplace, it was a bit difficult >> to >> justify spending a lot more time digging information about target >> descriptions, how to determine what features are present on target, >> etc. >> So thanks to push toward the progress of this issue, apparently with >> your mail subject you successfully brought people that gives more >> specific guidance to get this done. > > I tried to check the status on your submission mid-July (https://sourceware.org/ml/gdb/2016-07/msg00011.html), > but since I got no directions from anyone in the community at that time, I decided to try continue myself to get this fixed. > We work alot with Cortex-M4F on a daily basis, and do really need this to be fixed. > > >> Indeed, the patch looks really close to what I've done, to the point >> that I can see that, aside from a small refactoring, the changes >> between >> our patches are mainly variable name changes and added comments. You >> really wrote your patch from scratch? You've got rid of the various >> calls to `user_reg_map_name_to_regnum', though, which is nice. > > Yes I wrote it from scratch, but I agree my patch looks similar to yours, as its implementing according to ARM Cortex-M spec, > I guess it needs to be. But it absolutely helped me to have your ideas as starting point to look at, to see where modifications needed to be done, > so I do not mind if you would like to stand as co-author in the ChangeLog? > If you have FSF GDB Assignment, or would like to apply for assignment, > I can add your name to ChangeLog, its totally fine by me. > > The important thing for me is to get this fixed, and who gets the actual and final credits I do not mind. > And I think it would be great if we could try to continue this work together, to get the remaining parts for Cortex-M support in GDB. > The more people who are working on this, the better. > I am not so experienced in GDB internals, and also seek guidance how to continue go get the last parts in place. > I think both to get GDB aware of MSP and PSP, and also FPCCR needs to be a bigger change and are more complicated. > My idea is to submit a bug entry in bugzilla on this, one issue for the MSP/PSP support, another one for FPCCR, so we can get track of this, and someone could be assigned. > > >>> Cortex-M has two stack pointers: MSP (Main Stack Pointer) and PSP >> (Process Stack Pointer). >>> This is not handled when GDB tries to backtrace in the exception >> stack unwinder. >>> Meaning for eg. Cortex-M4F its not possible to get any call-stack >> backtrace if setting a breakpoint in ISR, and floating-point variable >> was used. >> >> Actually, the info here is inaccurate: FPU context stacking and the >> MSP/PSP switch are two unrelated concepts. Meaning, you can fix the >> code >> to deal with the PSP, but it still won't handle FPU register >> unstacking. > > Correct, my plan now was to split the patch as Yao proposed. > The the MSP/PSP-fix and FPU-fix would be two different patches. > > >>> This patch was inspired by the great work done by James-Adam >> Renquinha Henri, submitted April this year. >> Thanks :) >> >>> The next thing would then be to also add FPU context control reg >> FPCCR, which is needed for retrieving info on the FPU lazy stacking. >>> Though its complicated I think and I will try to investigate an >> 'arm-m-fpu.xml' profile further, if this is solution perhaps. >> >> It indeed is just a bit more complicated. Let me summarize what needs >> to >> be done. Have the ARMv7-M Architecture Reference Manual handy, see >> B1.5.7. >> >> - Check if lazy stacking is enabled (FPCCR.LSPEN == 1). If it's not, >> the >> case is uncomplicated, registers are stacked as usual >> - If lazy stacking is active (FPCCR.LSACT == 1), the extended stack >> has >> space reserved for the FPU registers (S0-S15, FPSCR), but there are >> not >> stacked, they are still in FPU registers unmodified. >> >> So that adds more random fetches from "memory" of the target, because >> >> these are memory-mapped. >> >> ARM gives some example scenarios with chronograms of some important >> bits >> with stacking information [1]. > > Yes, my idea was that GDB needs to be aware of FPCCR, so we need probably an new XML profile feature for this, just as for PSP/MSP. > But you are correct, its way more complicated. See also ARM Application Note I'm referring to in my patch comments. > > >>> ChangeLog entry should cover what do you change on the function >> level. >>> Please read https://sourceware.org/gdb/wiki/ContributionChecklist >> >> If only I knew that checklist! I searched for something like this, >> but >> maybe it didn't use the correct search terms, I didn't find it. > > If you want to stand as co-author in ChangeLog, its totally fine by me, > just give me a hint if you have the FSF assignment done, > and I also would be more than happy if you have time to continue your great work, and get the last parts of unwinding support for Cortex-M in place. > > > Thanks, and Kind Regards, > Fredrik >
On Fri, Aug 5, 2016 at 8:01 PM, Adam Renquinha <arenquinha@cimeq.qc.ca> wrote: > > Hmm, if I understand correctly, I should e-mail one of the gdb maintainers > to get the assignment to fill? You did that before; how it went? You can follow this, and use the third form (request-assign.future) https://sourceware.org/gdb/wiki/ContributionChecklist#FSF_copyright_Assignment
FYI, the assignment process is done for me as well as for Fredrik. The patch can be submitted. James-Adam Renquinha Henri, Ing. jr Ingénieur d'application CIMEQ INC. Le 2016-08-09 à 05:16, Yao Qi a écrit : > On Fri, Aug 5, 2016 at 8:01 PM, Adam Renquinha <arenquinha@cimeq.qc.ca> wrote: >> >> Hmm, if I understand correctly, I should e-mail one of the gdb maintainers >> to get the assignment to fill? You did that before; how it went? > > You can follow this, and use the third form (request-assign.future) > https://sourceware.org/gdb/wiki/ContributionChecklist#FSF_copyright_Assignment >
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index d2661cb..1d154cc 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -464,6 +464,61 @@ arm_pc_is_thumb (struct gdbarch *gdbarch, CORE_ADDR memaddr) return 0; } +/* Determine if the address specified equals any of + these magic return values, called EXC_RETURN, defined + by the ARM v6-M and v7-M architectures. + + From ARMv6-M Reference Manual B1.5.8 + Table B1-5 Exception return behavior + + EXC_RETURN Return To Return Stack + 0xFFFFFFF1 Handler mode Main + 0xFFFFFFF9 Thread mode Main + 0xFFFFFFFD Thread mode Process + + From ARMv7-M Reference Manual B1.5.8 + Table B1-8 EXC_RETURN definition of exception return behavior, no FP + + EXC_RETURN Return To Return Stack + 0xFFFFFFF1 Handler mode Main + 0xFFFFFFF9 Thread mode Main + 0xFFFFFFFD Thread mode Process + + Table B1-9 EXC_RETURN definition of exception return behavior, with FP + + EXC_RETURN Return To Return Stack Frame Type + 0xFFFFFFE1 Handler mode Main Extended + 0xFFFFFFE9 Thread mode Main Extended + 0xFFFFFFED Thread mode Process Extended + 0xFFFFFFF1 Handler mode Main Basic + 0xFFFFFFF9 Thread mode Main Basic + 0xFFFFFFFD Thread mode Process Basic + + For more details see "B1.5.8 Exception return behavior" + in both ARMv6-M and ARMv7-M Architecture Reference Manuals. */ + +static int +arm_m_addr_is_magic (CORE_ADDR addr) +{ + switch (addr) + { + /* Values from Tables in B1.5.8 the EXC_RETURN definitions of + the exception return behavior. */ + case 0xffffffe1: + case 0xffffffe9: + case 0xffffffed: + case 0xfffffff1: + case 0xfffffff9: + case 0xfffffffd: + /* Address is magic. */ + return 1; + + default: + /* Address is not magic. */ + return 0; + } +} + /* Remove useless bits from addresses in a running program. */ static CORE_ADDR arm_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR val) @@ -471,7 +526,7 @@ arm_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR val) /* On M-profile devices, do not strip the low bit from EXC_RETURN (the magic exception return address). */ if (gdbarch_tdep (gdbarch)->is_m - && (val & 0xfffffff0) == 0xfffffff0) + && arm_m_addr_is_magic (val)) return val; if (arm_apcs_32) @@ -2990,14 +3045,8 @@ arm_m_exception_unwind_sniffer (const struct frame_unwind *self, /* No need to check is_m; this sniffer is only registered for M-profile architectures. */ - /* Exception frames return to one of these magic PCs. Other values - are not defined as of v7-M. See details in "B1.5.8 Exception - return behavior" in "ARMv7-M Architecture Reference Manual". */ - if (this_pc == 0xfffffff1 || this_pc == 0xfffffff9 - || this_pc == 0xfffffffd) - return 1; - - return 0; + /* Check if exception frame returns to a magic PC value. */ + return arm_m_addr_is_magic (this_pc); } /* Frame unwinder for M-profile exceptions. */