Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 17 Apr 2020 20:23:39 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     Alexandre Chartre <alexandre.chartre@oracle.com>
Cc:     tglx@linutronix.de, jpoimboe@redhat.com,
        linux-kernel@vger.kernel.org, x86@kernel.org, mhiramat@kernel.org,
        mbenes@suse.cz, jthierry@redhat.com
Subject: Re: [PATCH v5 02/17] objtool: Better handle IRET
Message-ID: <20200417182339.GJ20730@hirez.programming.kicks-ass.net>
References: <20200416114706.625340212@infradead.org>
 <20200416115118.631224674@infradead.org>
 <8692ee18-e553-6f90-5291-62d6d57407cd@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <8692ee18-e553-6f90-5291-62d6d57407cd@oracle.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Apr 17, 2020 at 07:37:32PM +0200, Alexandre Chartre wrote:
> > @@ -2243,6 +2232,20 @@ static int validate_branch(struct objtoo
> >   			break;
> > +		case INSN_EXCEPTION_RETURN:
> > +			if (handle_insn_ops(insn, &state))
> > +				return 1;
> 
> Do we need to update the stack state for normal IRET? This wasn't done before.
> So shouldn't this better be:
> 
>                case INSN_EXCEPTION_RETURN:
>                         if (!func)
>                                 return 0;
> 
>                         if (handle_insn_ops(insn, &state))
>                                 return 1;
> 
>                         break;

Well, I was going to do the unconditional handle_insn_ops(), like
mentioned, but then that intra_function_call thing spoiled it.

It doesn't matter though; STT_NOTYPE doesn't care.

> > +
> > +			/*
> > +			 * This handles x86's sync_core() case, where we use an
> > +			 * IRET to self. All 'normal' IRET instructions are in
> > +			 * STT_NOTYPE entry symbols.
> > +			 */
> > +			if (func)
> > +				break;
> 
> Is it worth checking that func->name is effectively "sync_core"?

It's an inline..