Re: [PATCH 20/24] objtool: Another static block fail

From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: David Woodhouse <dwmw2@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-kernel@vger.kernel.org, Dave Hansen <dave.hansen@intel.com>,
	Ashok Raj <ashok.raj@intel.com>,
	Tim Chen <tim.c.chen@linux.intel.com>,
	Andy Lutomirski <luto@kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Greg KH <gregkh@linuxfoundation.org>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Andi Kleen <ak@linux.intel.com>,
	Arjan Van De Ven <arjan.van.de.ven@intel.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Jun Nakajima <jun.nakajima@intel.com>,
	Asit Mallick <asit.k.mallick@intel.com>,
	Jason Baron <jbaron@akamai.com>
Subject: Re: [PATCH 20/24] objtool: Another static block fail
Date: Tue, 30 Jan 2018 21:12:21 -0600	[thread overview]
Message-ID: <20180131031221.7k2igskvrpsphc2u@treble> (raw)
In-Reply-To: <20180130095653.GZ2269@hirez.programming.kicks-ass.net>

On Tue, Jan 30, 2018 at 10:56:53AM +0100, Peter Zijlstra wrote:
> On Mon, Jan 29, 2018 at 04:52:53PM -0600, Josh Poimboeuf wrote:
> > On Tue, Jan 23, 2018 at 04:25:59PM +0100, Peter Zijlstra wrote:
> > > I've observed GCC generate:
> > > 
> > >   sym:
> > >      NOP/JMP 1f	(static_branch)
> > >      JMP 2f
> > >   1: /* crud */
> > >      JMP 3f
> > >   2: /* other crud */
> > > 
> > >   3: RETQ
> > > 
> > > 
> > > This means we need to follow unconditional jumps; be conservative and
> > > only follow if its a unique jump.
> > > 
> > > (I've not yet figured out which CONFIG option is responsible for this,
> > >  a normal defconfig build does not generate crap like this)
> > > 
> > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > 
> > Any chance we can just add a compiler barrier to the assertion macro and
> > avoid all this grow_static_blocks() mess?  It seems a bit... fragile.
> 
> It is all rather unfortunate yes.. :/ I've tried to keep the grow stuff
> as conservative as possible while still covering all the weirdness I
> found. And while it was great fun, I do agree it would be much better to
> not have to do this.

The more I look at this patch, and the previous one, the less sure I am
about this idea.  (This isn't a comment about the code, just some
general uneasiness about the unpredictable GCC behavior we're trying to
constrain, and whether it's worth the pain.)

Covering all the corner cases is complicated, and it might get even more
complicated with future compiler optimizations.

What if we *only* use the assertions in places that really need them,
like the IBRS checks?  Would that allow us to simplify and drop these
last two (or three) patches? 

Or, maybe we should just forget the whole thing and just stick with the
dynamic IBRS checks with lfence.  Yes, it's less ideal for the kernel,
but adding these acrobatics to objtool also has a cost.

-- 
Josh