[RFC PATCH 1/2] ARM: deprecate old APCS frame format

[linux@arm.linux.org.uk (Russell King - ARM Linux)]

On Mon, Jan 25, 2016 at 05:05:20PM +0000, Jean-Philippe Brucker wrote:
> GCC 5 deprecated option -mapcs-frame, which generated frames following
> the old ABI format. In preparation for a possible removal in future
> versions of GCC, deprecate use of this flag in Linux as well.
> 
> Instead of completely removing the bits that depend on APCS frame, we
> move them behind a DEPRECATED config option to smoothen the transition.
> 
> Neither AAPCS nor EABI guarantees any frame format, so only ARM_UNWIND
> should now be used for stack traces and introspection. It is already
> selected by most defconfigs.
> Furthermore, frames currently generated by GCC are different for leaf
> and non-leaf functions, which would make adaptation quite tricky.

As I mentioned on the call last week, I'm not happy with deprecating
APCS frames.  The unwinder does not always work, and each time the
unwinder fails to work, it creates more work.

The latest scenario can be found in the "[PATCH 1/2] ARM: make
virt_to_idmap() return unsigned long" thread, where we have a
failure to unwind the first oops - unwinding stops for apparently
no reason after the first frame.

So, we have no idea how tasket_action() got called.

My current idea on that oops is... I have no idea, and the only way
to get more of an idea would be to request a rebuild with thumb
disabled and frame pointers enabled, and for the problem to be
reproduced.

This is the issue: APCS frame pointer based backtracing works every
time.  The unwinder is and always has been unreliable.

GCC people need to think about this, because if they persue, we're
going to end up having to tell people to rebuild their kernels with
older GCC versions just to get debug information out of them, unless
the quality of the unwinder and unwind information can be improved.

However, having experienced the "quality" of gdb over the last five
years in userspace, I'm really not hopeful that there is a solution
that works using the unwind information: I suspect there's a great
number of cases where the unwind information is basically wrong or
otherwise broken.

The problem with unwind based solutions is the only time you know
that the unwinding information is broken is when the code goes wrong
and the unwind information has to be used.  The nice thing about the
frame-pointer based solution is that it's correctness is an inherent
requirement for the program to work, so any problem with a frame-
pointer based solution shows up as a program crash.

This is probably the root cause why the unwind based backtracing
tends to fail.

Here's the oops in question:

Unable to handle kernel NULL pointer dereference at virtual address 00000000
pgd = c0003000
[00000000] *pgd=80000800004003, *pmd=00000000
Internal error: Oops: a07 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 0 PID: 330 Comm: kworker/0:1 Not tainted 4.5.0-rc1-00001-g22a8511 #1
Hardware name: Keystone
Workqueue: ipv6_addrconf addrconf_dad_work
task: eba89500 ti: eba96000 task.ti: eba96000
PC is at tasklet_action+0xac/0x13c
LR is at _raw_spin_unlock_irqrestore+0x28/0x54
pc : [<c0026c48>]    lr : [<c0574634>]    psr: 40000133
sp : eba97d30  ip : eb400258  fp : c083aecc
r10: 0000000c  r9 : eba97dc8  r8 : 00000100
r7 : 00000008  r6 : eba96000  r5 : 00000003  r4 : c07e408c
r3 : eba97d00  r2 : eba97d00  r1 : 60000113  r0 : 00000000
Flags: nZcv  IRQs on  FIQs on  Mode SVC_32  ISA Thumb Segment kernel
Control: 30c5387d  Table: 2b9ba700  DAC: fffffffd
Process kworker/0:1 (pid: 330, stack limit = 0xeba96210)
Stack: (0xeba97d30 to 0xeba98000)
7d20:                                     000086dd ebbb9900 00000004 eba97d30
7d40: c07e4080 c08409c0 0000000a ffff8e53 c07e4100 04208060 ebbb9900 c07dce84
7d60: 00000000 0000004e 00000000 00000001 eba97dc8 eb838000 c082ad40 c0026778
7d80: c07dce84 c006754c c080f430 c07e5e84 f080400c eba97dc8 f0804000 f0805000
7da0: 00000001 c0009438 c04fc29c 20000013 ffffffff eba97dfc cc8da400 eaf54d00
...
[<c0026c48>] (tasklet_action) from [<c08409c0>] (irq_stat+0x0/0x100)
Code: 4000 e256 0020 0a00 (5004) e1a0

Obviously, tasklet_action was _not_ called by the irq_stat array!

What we can see from the stack is there was an exception frame at
0xeba97d6c, with the parent context LR = c0009438 and PC = c04fc29c.
Much more than that is just guesswork - but the point is, the kernel
_should_ be capable of giving a good backtrace.

If we can't get a good backtrace, it limits those who can diagnose
the case of the failure, which really isn't good.

What we _might_ end up having to do is to walk every word on the
kernel stack, check whether it's in a module .text or kernel .text,
and print it as a "best guess" at a backtrace, but that will be
very misleading, as registers contain pointers to literal data,
which can be located not only at the end of a function, but also
the middle of a function, and that can end up on the stack too.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.