| Message ID | 20190515124006.25840-14-christophe.lyon@st.com |
|---|---|
| State | New |
| Headers | show |
| Series | FDPIC ABI for ARM | expand |
Hi Christophe, On 5/15/19 1:39 PM, Christophe Lyon wrote: > Without this, when we are unwinding across a signal frame we can jump > to an even address which leads to an exception. > > This is needed in __gnu_persnality_sigframe_fdpic() when restoring the > PC from the signal frame since the PC saved by the kernel has the LSB > bit set to zero. > > 2019-XX-XX Christophe Lyon <christophe.lyon@st.com> > Mickaël Guêné <mickael.guene@st.com> > > libgcc/ > * config/arm/unwind-arm.c (_Unwind_VRS_Set): Handle v7m > architecture. > > Change-Id: Ie84de548226bcf1751e19a09e8f091fb3013ccea > > diff --git a/libgcc/config/arm/unwind-arm.c > b/libgcc/config/arm/unwind-arm.c > index 9ba73e7..ba47150 100644 > --- a/libgcc/config/arm/unwind-arm.c > +++ b/libgcc/config/arm/unwind-arm.c > @@ -199,6 +199,11 @@ _Unwind_VRS_Result _Unwind_VRS_Set > (_Unwind_Context *context, > return _UVRSR_FAILED; > > vrs->core.r[regno] = *(_uw *) valuep; > +#if defined(__ARM_ARCH_7M__) > + /* Force LSB bit since we always run thumb code. */ > + if (regno == 15) > + vrs->core.r[regno] |= 1; > +#endif Hmm, this looks quite specific. There are other architectures that are thumb-only too (6-M, 7E-M etc). Would checking for __thumb__ be better? Thanks, Kyrill > return _UVRSR_OK; > > case _UVRSC_VFP: > -- > 2.6.3 >
On Thu, 29 Aug 2019 at 17:32, Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> wrote: > > Hi Christophe, > > On 5/15/19 1:39 PM, Christophe Lyon wrote: > > Without this, when we are unwinding across a signal frame we can jump > > to an even address which leads to an exception. > > > > This is needed in __gnu_persnality_sigframe_fdpic() when restoring the > > PC from the signal frame since the PC saved by the kernel has the LSB > > bit set to zero. > > > > 2019-XX-XX Christophe Lyon <christophe.lyon@st.com> > > Mickaël Guêné <mickael.guene@st.com> > > > > libgcc/ > > * config/arm/unwind-arm.c (_Unwind_VRS_Set): Handle v7m > > architecture. > > > > Change-Id: Ie84de548226bcf1751e19a09e8f091fb3013ccea > > > > diff --git a/libgcc/config/arm/unwind-arm.c > > b/libgcc/config/arm/unwind-arm.c > > index 9ba73e7..ba47150 100644 > > --- a/libgcc/config/arm/unwind-arm.c > > +++ b/libgcc/config/arm/unwind-arm.c > > @@ -199,6 +199,11 @@ _Unwind_VRS_Result _Unwind_VRS_Set > > (_Unwind_Context *context, > > return _UVRSR_FAILED; > > > > vrs->core.r[regno] = *(_uw *) valuep; > > +#if defined(__ARM_ARCH_7M__) > > + /* Force LSB bit since we always run thumb code. */ > > + if (regno == 15) > > + vrs->core.r[regno] |= 1; > > +#endif > > Hmm, this looks quite specific. There are other architectures that are > thumb-only too (6-M, 7E-M etc). > > Would checking for __thumb__ be better? > Right. The attached updated patch also uses R_PC instead of 15. Christophe > Thanks, > > Kyrill > > > > return _UVRSR_OK; > > > > case _UVRSC_VFP: > > -- > > 2.6.3 > >
Sorry, I forgot again to cc: Ian. Thanks, Christophe On Thu, 5 Sep 2019 at 10:30, Christophe Lyon <christophe.lyon@linaro.org> wrote: > > On Thu, 29 Aug 2019 at 17:32, Kyrill Tkachov > <kyrylo.tkachov@foss.arm.com> wrote: > > > > Hi Christophe, > > > > On 5/15/19 1:39 PM, Christophe Lyon wrote: > > > Without this, when we are unwinding across a signal frame we can jump > > > to an even address which leads to an exception. > > > > > > This is needed in __gnu_persnality_sigframe_fdpic() when restoring the > > > PC from the signal frame since the PC saved by the kernel has the LSB > > > bit set to zero. > > > > > > 2019-XX-XX Christophe Lyon <christophe.lyon@st.com> > > > Mickaël Guêné <mickael.guene@st.com> > > > > > > libgcc/ > > > * config/arm/unwind-arm.c (_Unwind_VRS_Set): Handle v7m > > > architecture. > > > > > > Change-Id: Ie84de548226bcf1751e19a09e8f091fb3013ccea > > > > > > diff --git a/libgcc/config/arm/unwind-arm.c > > > b/libgcc/config/arm/unwind-arm.c > > > index 9ba73e7..ba47150 100644 > > > --- a/libgcc/config/arm/unwind-arm.c > > > +++ b/libgcc/config/arm/unwind-arm.c > > > @@ -199,6 +199,11 @@ _Unwind_VRS_Result _Unwind_VRS_Set > > > (_Unwind_Context *context, > > > return _UVRSR_FAILED; > > > > > > vrs->core.r[regno] = *(_uw *) valuep; > > > +#if defined(__ARM_ARCH_7M__) > > > + /* Force LSB bit since we always run thumb code. */ > > > + if (regno == 15) > > > + vrs->core.r[regno] |= 1; > > > +#endif > > > > Hmm, this looks quite specific. There are other architectures that are > > thumb-only too (6-M, 7E-M etc). > > > > Would checking for __thumb__ be better? > > > Right. > The attached updated patch also uses R_PC instead of 15. > > Christophe > > > Thanks, > > > > Kyrill > > > > > > > return _UVRSR_OK; > > > > > > case _UVRSC_VFP: > > > -- > > > 2.6.3 > > >
Hi Christophe, On 9/5/19 9:30 AM, Christophe Lyon wrote: > On Thu, 29 Aug 2019 at 17:32, Kyrill Tkachov > <kyrylo.tkachov@foss.arm.com> wrote: >> Hi Christophe, >> >> On 5/15/19 1:39 PM, Christophe Lyon wrote: >>> Without this, when we are unwinding across a signal frame we can jump >>> to an even address which leads to an exception. >>> >>> This is needed in __gnu_persnality_sigframe_fdpic() when restoring the >>> PC from the signal frame since the PC saved by the kernel has the LSB >>> bit set to zero. >>> >>> 2019-XX-XX Christophe Lyon <christophe.lyon@st.com> >>> Mickaël Guêné <mickael.guene@st.com> >>> >>> libgcc/ >>> * config/arm/unwind-arm.c (_Unwind_VRS_Set): Handle v7m >>> architecture. >>> >>> Change-Id: Ie84de548226bcf1751e19a09e8f091fb3013ccea >>> >>> diff --git a/libgcc/config/arm/unwind-arm.c >>> b/libgcc/config/arm/unwind-arm.c >>> index 9ba73e7..ba47150 100644 >>> --- a/libgcc/config/arm/unwind-arm.c >>> +++ b/libgcc/config/arm/unwind-arm.c >>> @@ -199,6 +199,11 @@ _Unwind_VRS_Result _Unwind_VRS_Set >>> (_Unwind_Context *context, >>> return _UVRSR_FAILED; >>> >>> vrs->core.r[regno] = *(_uw *) valuep; >>> +#if defined(__ARM_ARCH_7M__) >>> + /* Force LSB bit since we always run thumb code. */ >>> + if (regno == 15) >>> + vrs->core.r[regno] |= 1; >>> +#endif >> Hmm, this looks quite specific. There are other architectures that are >> thumb-only too (6-M, 7E-M etc). >> >> Would checking for __thumb__ be better? >> > Right. > The attached updated patch also uses R_PC instead of 15. Looks ok to me but we'll need to make sure this doesn't break non-FDPIC targets now. A bootstrap and test of an arm-none-linux-gnueabihf targeting thumb should do it. Thanks, Kyrill > > Christophe > >> Thanks, >> >> Kyrill >> >> >>> return _UVRSR_OK; >>> >>> case _UVRSC_VFP: >>> -- >>> 2.6.3 >>>
Christophe Lyon <christophe.lyon@linaro.org> writes: > Sorry, I forgot again to cc: Ian. As far as I'm concerned, it's fine for architecture maintainers to approve changes to architecture-specific files in libgcc. Ian > On Thu, 5 Sep 2019 at 10:30, Christophe Lyon <christophe.lyon@linaro.org> wrote: >> >> On Thu, 29 Aug 2019 at 17:32, Kyrill Tkachov >> <kyrylo.tkachov@foss.arm.com> wrote: >> > >> > Hi Christophe, >> > >> > On 5/15/19 1:39 PM, Christophe Lyon wrote: >> > > Without this, when we are unwinding across a signal frame we can jump >> > > to an even address which leads to an exception. >> > > >> > > This is needed in __gnu_persnality_sigframe_fdpic() when restoring the >> > > PC from the signal frame since the PC saved by the kernel has the LSB >> > > bit set to zero. >> > > >> > > 2019-XX-XX Christophe Lyon <christophe.lyon@st.com> >> > > Mickaël Guêné <mickael.guene@st.com> >> > > >> > > libgcc/ >> > > * config/arm/unwind-arm.c (_Unwind_VRS_Set): Handle v7m >> > > architecture. >> > > >> > > Change-Id: Ie84de548226bcf1751e19a09e8f091fb3013ccea >> > > >> > > diff --git a/libgcc/config/arm/unwind-arm.c >> > > b/libgcc/config/arm/unwind-arm.c >> > > index 9ba73e7..ba47150 100644 >> > > --- a/libgcc/config/arm/unwind-arm.c >> > > +++ b/libgcc/config/arm/unwind-arm.c >> > > @@ -199,6 +199,11 @@ _Unwind_VRS_Result _Unwind_VRS_Set >> > > (_Unwind_Context *context, >> > > return _UVRSR_FAILED; >> > > >> > > vrs->core.r[regno] = *(_uw *) valuep; >> > > +#if defined(__ARM_ARCH_7M__) >> > > + /* Force LSB bit since we always run thumb code. */ >> > > + if (regno == 15) >> > > + vrs->core.r[regno] |= 1; >> > > +#endif >> > >> > Hmm, this looks quite specific. There are other architectures that are >> > thumb-only too (6-M, 7E-M etc). >> > >> > Would checking for __thumb__ be better? >> > >> Right. >> The attached updated patch also uses R_PC instead of 15. >> >> Christophe >> >> > Thanks, >> > >> > Kyrill >> > >> > >> > > return _UVRSR_OK; >> > > >> > > case _UVRSC_VFP: >> > > -- >> > > 2.6.3 >> > >
On Thu, 5 Sep 2019 at 11:03, Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> wrote: > > Hi Christophe, > > On 9/5/19 9:30 AM, Christophe Lyon wrote: > > On Thu, 29 Aug 2019 at 17:32, Kyrill Tkachov > > <kyrylo.tkachov@foss.arm.com> wrote: > >> Hi Christophe, > >> > >> On 5/15/19 1:39 PM, Christophe Lyon wrote: > >>> Without this, when we are unwinding across a signal frame we can jump > >>> to an even address which leads to an exception. > >>> > >>> This is needed in __gnu_persnality_sigframe_fdpic() when restoring the > >>> PC from the signal frame since the PC saved by the kernel has the LSB > >>> bit set to zero. > >>> > >>> 2019-XX-XX Christophe Lyon <christophe.lyon@st.com> > >>> Mickaël Guêné <mickael.guene@st.com> > >>> > >>> libgcc/ > >>> * config/arm/unwind-arm.c (_Unwind_VRS_Set): Handle v7m > >>> architecture. > >>> > >>> Change-Id: Ie84de548226bcf1751e19a09e8f091fb3013ccea > >>> > >>> diff --git a/libgcc/config/arm/unwind-arm.c > >>> b/libgcc/config/arm/unwind-arm.c > >>> index 9ba73e7..ba47150 100644 > >>> --- a/libgcc/config/arm/unwind-arm.c > >>> +++ b/libgcc/config/arm/unwind-arm.c > >>> @@ -199,6 +199,11 @@ _Unwind_VRS_Result _Unwind_VRS_Set > >>> (_Unwind_Context *context, > >>> return _UVRSR_FAILED; > >>> > >>> vrs->core.r[regno] = *(_uw *) valuep; > >>> +#if defined(__ARM_ARCH_7M__) > >>> + /* Force LSB bit since we always run thumb code. */ > >>> + if (regno == 15) > >>> + vrs->core.r[regno] |= 1; > >>> +#endif > >> Hmm, this looks quite specific. There are other architectures that are > >> thumb-only too (6-M, 7E-M etc). > >> > >> Would checking for __thumb__ be better? > >> > > Right. > > The attached updated patch also uses R_PC instead of 15. > > > Looks ok to me but we'll need to make sure this doesn't break non-FDPIC > targets now. > > A bootstrap and test of an arm-none-linux-gnueabihf targeting thumb > should do it. > Bootstrap of the whole series OK, modulo the problems with the tests discussed in patch 20. (some tests became unsupported on arm-linux-gnueabihf with thumb target) Christophe > Thanks, > > Kyrill > > > > > > Christophe > > > >> Thanks, > >> > >> Kyrill > >> > >> > >>> return _UVRSR_OK; > >>> > >>> case _UVRSC_VFP: > >>> -- > >>> 2.6.3 > >>>
diff --git a/libgcc/config/arm/unwind-arm.c b/libgcc/config/arm/unwind-arm.c index 9ba73e7..ba47150 100644 --- a/libgcc/config/arm/unwind-arm.c +++ b/libgcc/config/arm/unwind-arm.c @@ -199,6 +199,11 @@ _Unwind_VRS_Result _Unwind_VRS_Set (_Unwind_Context *context, return _UVRSR_FAILED; vrs->core.r[regno] = *(_uw *) valuep; +#if defined(__ARM_ARCH_7M__) + /* Force LSB bit since we always run thumb code. */ + if (regno == 15) + vrs->core.r[regno] |= 1; +#endif return _UVRSR_OK; case _UVRSC_VFP: