Thread (2 messages) 2 messages, 2 authors, 2016-03-07

Re: [PATCH] rtc: ds1685: actually spin forever in poweroff error path

From: Josh Poimboeuf <hidden>
Date: 2016-03-07 22:44:24
Also in: linux-rtc, lkml

On Mon, Mar 07, 2016 at 04:30:50PM -0500, Joshua Kinard wrote:
On 03/07/2016 10:03, Josh Poimboeuf wrote:
quoted
objtool reports the following warnings:

  drivers/rtc/rtc-ds1685.o: warning: objtool: ds1685_rtc_work_queue()+0x0: duplicate frame pointer save
  drivers/rtc/rtc-ds1685.o: warning: objtool: ds1685_rtc_work_queue()+0x3: duplicate frame pointer setup
  drivers/rtc/rtc-ds1685.o: warning: objtool: ds1685_rtc_work_queue()+0x0: frame pointer state mismatch

The warning message needs to be improved, but what it really means in
this case is that ds1685_rtc_poweroff() has a possible code path where
it can actually fall through to the next function in the object code,
ds1685_rtc_work_queue().

The bug is caused by the use of the unreachable() macro in a place which
is actually reachable.  That causes gcc to assume that the printk()
immediately before the unreachable() macro never returns, when in fact
it does.  So gcc places the printk() at the very end of the function's
object code.  When the printk() returns, the next function starts
executing.

The surrounding comment and printk message state that the code should
spin forever, which explains the unreachable() statement.  However the
actual spin code is missing.
So this power down trick is used by both SGI O2 (IP32) and SGI Octane (IP30)
systems via this RTC chip, and I've noticed lately that the Octane has stopped
powering off via this function (it just sits and spins forever).  The O2 powers
off as expected.  When I initially wrote this driver from the original version
I found on LKML in '09, I hadn't gotten the Octane code back into a working
shape, and once that happened, I only tested the non-SMP case (fixed Octane SMP
in 4.1).  I suspect on the Octane, the use of SMP may be what is interfering
somehow, and this bug may partially explain it.  This patch doesn't fix
poweroff for me, but it's something to start from when I can get some time to
chase it down.

That said, I initially left the 'while (1);' clause out because at one point
during development, gcc yelled at me for using that at the end of the function,
so I looked at some other drivers and saw the use of 'unreachable();' and used
it instead.  Wasn't aware both of them are needed together in this instance.  I
thought 'unreachable()' evaluated out to a 'while (1)' at the end.  Seems to
actually be some kind of internal gcc trick.

How exactly did the kbuild bot trigger the above warnings?  I've only built and
tested this driver on a MIPS platform and haven't seen that particular warning
before.
Hi Joshua,

The warning was emitted by a brand new tool named objtool which does
some static object code analysis. It's currently only in linux-next, not
yet in Linus's tree.  To get the warning, you'd need to build the
linux-next tree for x86_64 with CONFIG_STACK_VALIDATION enabled.

Here's the kbuild bot warning:

  https://lkml.kernel.org/r/201603060005.PHCyifJr%fengguang.wu@intel.com


-- 
Josh
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help