On 2024/3/22 17:52, Russell King (Oracle) wrote: > On Fri, Mar 22, 2024 at 09:24:20AM +0000, David Laight wrote: >> From: Russell King >>> Sent: 22 March 2024 00:09 >>> >>> On Thu, Mar 21, 2024 at 11:43:41PM +0100, Ard Biesheuvel wrote: >>>> Given that this particular issue would just disappear if the compiler >>>> would just insert a BRK after the BL, I'd prefer to explore first >>>> whether we can get this fixed on the compiler side. >>> >>> Arm32 doesn't have a BRK instruction. What would be appropriate after >>> the no-return BL would be OS specific. >> >> It would need to depend on what was being compiled. > > Yes, but as for the rest... > >> For the kernel it could be much the same as BUG(). >> (Probably without any extra data.) >> I suspect that arm32 could use 'swi' in kernel space, >> but you wouldn't want to use that in userspace. >> >> Looks like armv5 has a bkpt instruction - could that be used? >> Or does the kernel need to support armv4? >> >> The last arm I wrote anything for was a strongarm. > > Thank you David, but remember - I have programmed 32-bit Arm since 1992, > and wrote the majority of the 32-bit Arm kernel support. I think I know > what I'm walking about by now. > > The compiler can't do the same as BUG() - that is a kernel specific > construct and not an architecture one. It is an undefined instruction > specifically chosen to be undefined on both 32-bit and 16-bit Arm ISAs. > > As for your idea of using "swi" in kernel space, no that's never going > to happen - to shoe-horn that into the SWI exception path for the sake > of the compiler would be totally idiotic - it would cause userspace > performance regressions for something that never happens. Moreover, > with EABI the "comment" field in the "swi" instruction is ignored so > all SWIs under EABI are treated the same. So no, that's not going to > work without causing inefficiencies - again - for a case that will > likely never happen. > > Whereas we already provide an abort() function because iirc the > compiler used to emit branches to that due to noreturn functions. If > correct, there's previous convention for doing this - and abort() is > still exists in the kernel and in userspace since it's part of ANSI > C. This would be a more reliable and portable solution, but probably > not for embedded platforms - and that's probably why it got removed. > > There isn't going to be a single solution to this which satisfies > everyone, and I don't blame the compiler people for deciding to > basically give up with putting any instruction after a call to a > no-return function - because there isn't an instruction defined in > the architecture that _could_ be put there that would work everywhere. > If the compiler inserts (a branch to 'abort') behind (no-return BL) that does not apply to ARM32 embedded platforms, do you think the "[PATCH v3] ARM: unwind: improve unwinders for noreturn case" submitted the day before yesterday can be used as a complementary solution? 2) we're unwinding a frame that has been created because of a branch, where the PC points at the next instruction _after_ that callsite. When we hit the second type of frame, "pc-1" is closer to callsite, and no problem is introduced. In addition, the issue of the 'noreturn' can be solved.