Message ID | 20190909154526.11630-1-christophe.lyon@st.com |
---|---|
Headers | show |
Series | FDPIC ABI for ARM | expand |
Hi Christophe, Can you explain this in more detail - it doesn't make sense to me to force the Thumb bit during unwinding since it should already be correct, even on a Thumb-only CPU. Perhaps the kernel code that pushes an incorrect address on the stack could be fixed instead? > 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. Wilco
On 17/09/2019 13:38, Wilco Dijkstra wrote: > Hi Christophe, > > Can you explain this in more detail - it doesn't make sense to me to force the > Thumb bit during unwinding since it should already be correct, even on a > Thumb-only CPU. Perhaps the kernel code that pushes an incorrect address on > the stack could be fixed instead? > >> 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. > > Wilco > . > Indeed, I've noticed the problem mentioned by Matthew since I committed that patch. I was about to propose a fix, replacing #if (__thumb__) with #if (!__ARM_ARCH_ISA_ARM), but you are right: maybe the kernel code should be fixed instead. So far I haven't managed to reproduce a failure in FDPIC mode without this patch though... Thanks and sorry for the breakage. Christophe
On Tue, 17 Sep 2019 at 14:08, Christophe Lyon <christophe.lyon@st.com> wrote: > > On 17/09/2019 13:38, Wilco Dijkstra wrote: > > Hi Christophe, > > > > Can you explain this in more detail - it doesn't make sense to me to force the > > Thumb bit during unwinding since it should already be correct, even on a > > Thumb-only CPU. Perhaps the kernel code that pushes an incorrect address on > > the stack could be fixed instead? > > > >> 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. > > > > Wilco > > . > > > > Indeed, I've noticed the problem mentioned by Matthew since I committed that patch. > > I was about to propose a fix, replacing #if (__thumb__) with #if (!__ARM_ARCH_ISA_ARM), but you are right: maybe the kernel code should be fixed instead. > > So far I haven't managed to reproduce a failure in FDPIC mode without this patch though... > > Thanks and sorry for the breakage. > I'm having problems with the board I use for testing, so I propose to revert that patch until I have a better description of the problem it fixed. OK? Christophe > Christophe >
On 9/19/19 4:13 PM, Christophe Lyon wrote: > On Tue, 17 Sep 2019 at 14:08, Christophe Lyon <christophe.lyon@st.com> > wrote: > > > > On 17/09/2019 13:38, Wilco Dijkstra wrote: > > > Hi Christophe, > > > > > > Can you explain this in more detail - it doesn't make sense to me > to force the > > > Thumb bit during unwinding since it should already be correct, > even on a > > > Thumb-only CPU. Perhaps the kernel code that pushes an incorrect > address on > > > the stack could be fixed instead? > > > > > >> 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. > > > > > > Wilco > > > . > > > > > > > Indeed, I've noticed the problem mentioned by Matthew since I > committed that patch. > > > > I was about to propose a fix, replacing #if (__thumb__) with #if > (!__ARM_ARCH_ISA_ARM), but you are right: maybe the kernel code should > be fixed instead. > > > > So far I haven't managed to reproduce a failure in FDPIC mode > without this patch though... > > > > Thanks and sorry for the breakage. > > > > I'm having problems with the board I use for testing, so I propose to > revert that patch until I have a better description of the problem it > fixed. > OK? Ok by me as long as lives the fdpic toolchain in a usable state (barring the potential issue here) Thanks, Kyrill > > Christophe > > > Christophe > >
On 19/09/19 16:14, Kyrill Tkachov wrote: > > On 9/19/19 4:13 PM, Christophe Lyon wrote: >> On Tue, 17 Sep 2019 at 14:08, Christophe Lyon <christophe.lyon@st.com> >> wrote: >> > >> > On 17/09/2019 13:38, Wilco Dijkstra wrote: >> > > Hi Christophe, >> > > >> > > Can you explain this in more detail - it doesn't make sense to me >> to force the >> > > Thumb bit during unwinding since it should already be correct, >> even on a >> > > Thumb-only CPU. Perhaps the kernel code that pushes an incorrect >> address on >> > > the stack could be fixed instead? >> > > >> > >> 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. >> > > >> > > Wilco >> > > . >> > > >> > >> > Indeed, I've noticed the problem mentioned by Matthew since I >> committed that patch. >> > >> > I was about to propose a fix, replacing #if (__thumb__) with #if >> (!__ARM_ARCH_ISA_ARM), but you are right: maybe the kernel code should >> be fixed instead. >> > >> > So far I haven't managed to reproduce a failure in FDPIC mode >> without this patch though... >> > >> > Thanks and sorry for the breakage. >> > >> >> I'm having problems with the board I use for testing, so I propose to >> revert that patch until I have a better description of the problem it >> fixed. >> OK? > > Ok by me as long as lives the fdpic toolchain in a usable state (barring > the potential issue here) Thanks Christophe -- reverting that patch would help our internal testing a lot! MM > > Thanks, > > Kyrill > > >> >> Christophe >> >> > Christophe >> >
On Fri, 20 Sep 2019 at 14:51, Matthew Malcomson <Matthew.Malcomson@arm.com> wrote: > > On 19/09/19 16:14, Kyrill Tkachov wrote: > > > > On 9/19/19 4:13 PM, Christophe Lyon wrote: > >> On Tue, 17 Sep 2019 at 14:08, Christophe Lyon <christophe.lyon@st.com> > >> wrote: > >> > > >> > On 17/09/2019 13:38, Wilco Dijkstra wrote: > >> > > Hi Christophe, > >> > > > >> > > Can you explain this in more detail - it doesn't make sense to me > >> to force the > >> > > Thumb bit during unwinding since it should already be correct, > >> even on a > >> > > Thumb-only CPU. Perhaps the kernel code that pushes an incorrect > >> address on > >> > > the stack could be fixed instead? > >> > > > >> > >> 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. > >> > > > >> > > Wilco > >> > > . > >> > > > >> > > >> > Indeed, I've noticed the problem mentioned by Matthew since I > >> committed that patch. > >> > > >> > I was about to propose a fix, replacing #if (__thumb__) with #if > >> (!__ARM_ARCH_ISA_ARM), but you are right: maybe the kernel code should > >> be fixed instead. > >> > > >> > So far I haven't managed to reproduce a failure in FDPIC mode > >> without this patch though... > >> > > >> > Thanks and sorry for the breakage. > >> > > >> > >> I'm having problems with the board I use for testing, so I propose to > >> revert that patch until I have a better description of the problem it > >> fixed. > >> OK? > > > > Ok by me as long as lives the fdpic toolchain in a usable state (barring > > the potential issue here) > > Thanks Christophe -- reverting that patch would help our internal > testing a lot! > MM > OK, I've reverted it. Christophe > > > > Thanks, > > > > Kyrill > > > > > >> > >> Christophe > >> > >> > Christophe > >> > >