Re: [PATCH v2] ARM: unwind: improve unwinders for noreturn case

[Jiangfeng Xiao]

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.