DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com DE314C058EA9
Date: Wed, 30 Aug 2017 23:42:09 -0500
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: "H. Peter Anvin" <hpa@zytor.com>
Cc: arnd@arndb.de, linux-kernel@vger.kernel.org, mingo@kernel.org,
        fengguang.wu@intel.com, torvalds@linux-foundation.org,
        tglx@linutronix.de, peterz@infradead.org,
        linux-tip-commits@vger.kernel.org
Subject: Re: [tip:x86/asm] objtool: Handle GCC stack pointer adjustment bug
Message-ID: <20170831044209.4hodx2dasad66yab@treble>
References: <6a41a96884c725e7f05413bb7df40cfe824b2444.1504028945.git.jpoimboe@redhat.com>
 <tip-dd88a0a0c8615417fe6b4285769b5b772de87279@git.kernel.org>
 <e6eb6a11-ad65-dccb-2c79-485e58e316a2@zytor.com>
 <20170830201413.gx4in2wnz5yypzrc@treble>
 <04d4f019-44a6-e01c-128d-aa31df9dd5dc@zytor.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <04d4f019-44a6-e01c-128d-aa31df9dd5dc@zytor.com>
User-Agent: Mutt/1.6.0.1 (2016-04-01)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4681
Lines: 109

On Wed, Aug 30, 2017 at 04:39:42PM -0700, H. Peter Anvin wrote:
> On 08/30/17 13:14, Josh Poimboeuf wrote:
> > On Wed, Aug 30, 2017 at 12:23:24PM -0700, H. Peter Anvin wrote:
> >> On 08/30/17 02:43, tip-bot for Josh Poimboeuf wrote:
> >>>
> >>> Those warnings are caused by an unusual GCC non-optimization where it
> >>> uses an intermediate register to adjust the stack pointer.  It does:
> >>>
> >>>   lea    0x8(%rsp), %rcx
> >>>   ...
> >>>   mov    %rcx, %rsp
> >>>
> >>> Instead of the obvious:
> >>>
> >>>   add    $0x8, %rsp
> >>>
> >>> It makes no sense to use an intermediate register, so I opened a GCC bug
> >>> to track it:
> >>>
> >>>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81813
> >>>
> >>> But it's not exactly a high-priority bug and it looks like we'll be
> >>> stuck with this issue for a while.  So for now we have to track register
> >>> values when they're loaded with stack pointer offsets.
> >>>
> >>
> >> This seems like a good reason to try to extract this information from
> >> the DWARF data *if available*?
> > 
> > Well, I haven't ruled that out for the future, but in this case,
> > integrating DWARF would be a lot more work than this relatively simple
> > patch.
> > 
> > If we did go that route, it could be tricky deciding when to trust
> > DWARF vs. when to trust objtool's reverse engineering.
> > 
> > Another (vague) idea I'm thinking about is to write a GCC plugin which
> > annotates the object files in a way that would help objtool become more
> > GCC-ignorant.  If it worked, this approach would be more powerful and
> > less error-prone than relying on DWARF.
> > 
> > Depending on how much work we can offload to the plugin, it might also
> > help make it easier to port objtool to other arches and compilers (e.g.,
> > clang).
> > 
> > I'm not 100% sold on that idea either, because it still requires objtool
> > to trust the compiler to some extent.  But I think it would be worth it
> > because it would make the objtool code simpler, more portable, more
> > robust, and easier to maintain (so I don't always have to stay on top of
> > all of GCC's latest optimizations).
> > 
> > In the meantime, objtool's current design is working fine (for now).  I
> > haven't found any issues it can't handle (yet).
> > 
> 
> Reverse engineering this way is at least NP-complete, and quite possibly
> undecidable.

Well, in practice, the reverse engineering already works very well.
Much to my complete surprise, admittedly.  And there are ways to "prove"
its accuracy over time with runtime sanity checks.

But I think we can agree that it would be wise to improve the current
approach.

> A gcc plugin would tie the kernel *way* harder to gcc than it is now

Actually, I would expect that moving GCC-specific pieces to a GCC plugin
would *decouple* the kernel from GCC, because there would then be a
compiler-independent interface to objtool.  A similar plugin could be
written for clang.  (all with the usual disclaimer: "if it works", of
course)

And I would hope/expect that the plugin would be less affected by new
and unusual GCC optimizations than objtool currently is, because it
would be much easier for the plugin to understand those changes by just
reading the RTL, instead of us trying to blindly decipher each new
pattern we find in the object code.

> and it seems incredibly unlikely that you would come up with something
> simpler and more reliable than a DWARF parser.

I'm not so sure about that.  I think it's just a matter of reading RTL
from a GCC plugin and providing some hints in a special section.  Of
course, the devil's in the details.

> What you *can* do, of course, is cross-correlate the two, and *way*
> more importantly, you cover assembly.

But how do you decide when to trust DWARF and when not?  We can't just
say "trust DWARF in everything but inline asm" because there's no way to
delineate inline asm just by reading the object code.

And I've heard some talk of buggy DWARF output in various versions of
GCC, of which we would could do nothing about if we blindly trusted
DWARF.  I don't know if those reports are accurate, but we would be at
the mercy of tooling bugs.  And in my experience, GCC doesn't seem to
prioritize the fixing of such "minor" issues (with the above patch being
an example).

There are also some control flow quirks which objtool struggles with,
for which a GCC plugin would be *so* much nicer.  Examples: noreturn
functions, switch statement jump tables, DRAP stack alignments, sibling
calls, KASAN/UBSAN/gcov issues, unreachable instructions, cold/unlikely
subfunctions, unusual stack pointer update patterns.  Those are the
things that keep me up at night.

-- 
Josh