From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Google-Smtp-Source: ACJfBosTSUZkKrBzwCG8D0X3CBdL8y7nfXV5uiUDI/XDuD92lOJiskd3jPnjxs+mwXOfnwqrJi13 ARC-Seal: i=1; a=rsa-sha256; t=1516380176; cv=none; d=google.com; s=arc-20160816; b=B2WWFcpdtYuoBwpPBA6fMgDPxuHvDbzLG3oSGOI4IMt0hmw5as+8MPQIjkVrAhe0K5 OYBJrPVjtfH09+DMF5vvImu+HaPn9F2rNjF07T1DXABWa6hU5ZhcFHJC2ytMZwF6yX74 q3mBKf27wuNhXmk2XDHqt09Ek0pUHjkz+PP8FfNa+vxeWf/i+LaFIYWpLz5FL8WI/sQV zRY2VrcZAt9tB7M7AWVzqQVogP7m6sLLYSzOVlTUDVCEucNF5FX3YF320nPdzwVopI7e rL5KEgdi6n9YHj+VkXAk4h/ekhsk9coKoCN31v0au+ur0mgkH+SnSQkug7JGq1sBNu4L oyoA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:dkim-signature :arc-authentication-results; bh=2nvTYhq5rOICM18PqPQIqHxWMdzvM9i+Xjw8pisB6vo=; b=dIA/oEwivYCdAQbhY/QF7yYCgDVOjFSlMyAuu0eiVbxSlro0Gr3f2kwl70qTlgzWE8 Qz1t6N9gepGy4+5bvA41fQsHuavu++k3zW1a83Gi61DgT+Qv5NahdUPWzAXtzqgkqLaL Znru5DaiAQ3N0o6WcbDdfBfD5jK3cHcnCaQcQa4e1FoyaxydU2E8AhO3BkbH7GEeuyq4 79QSyarwOjDDF5Bc3DFN/V3W7x4mNPcHk5ieheUyrs5MtkMs+OwW+O2n5UzmQEwm10IQ vKgn7747Nf9fMPd7i0xR/1oQGXWvez8RiLPOPiyJx4wrKRRIZ4Lv6SNMY6rE71NJADja nfeQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@infradead.org header.s=merlin.20170209 header.b=bEi0Qxgi; spf=pass (google.com: best guess record for domain of peterz@infradead.org designates 205.233.59.134 as permitted sender) smtp.mailfrom=peterz@infradead.org Authentication-Results: mx.google.com; dkim=pass header.i=@infradead.org header.s=merlin.20170209 header.b=bEi0Qxgi; spf=pass (google.com: best guess record for domain of peterz@infradead.org designates 205.233.59.134 as permitted sender) smtp.mailfrom=peterz@infradead.org Date: Fri, 19 Jan 2018 17:42:29 +0100 From: Peter Zijlstra To: David Woodhouse , Thomas Gleixner , Josh Poimboeuf Cc: linux-kernel@vger.kernel.org, Dave Hansen , Ashok Raj , Tim Chen , Andy Lutomirski , Linus Torvalds , Greg KH , Andrea Arcangeli , Andi Kleen , Arjan Van De Ven , Dan Williams , Paolo Bonzini , Jun Nakajima , Asit Mallick , Jason Baron Subject: Re: [PATCH 18/35] objtool: Another static block fail Message-ID: <20180119164229.GR2228@hirez.programming.kicks-ass.net> References: <20180118134800.711245485@infradead.org> <20180118140152.536715831@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180118140152.536715831@infradead.org> User-Agent: Mutt/1.9.2 (2017-12-15) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-LABELS: =?utf-8?b?IlxcSW1wb3J0YW50Ig==?= X-GMAIL-THRID: =?utf-8?q?1590039860183184435?= X-GMAIL-MSGID: =?utf-8?q?1590039860183184435?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: On Thu, Jan 18, 2018 at 02:48:18PM +0100, Peter Zijlstra wrote: > @@ -1216,7 +1218,8 @@ static bool __grow_static_block(struct o > > switch (insn->type) { > case INSN_JUMP_UNCONDITIONAL: > - /* mark this instruction, terminate this section */ > + /* follow the jump, mark this instruction, terminate this section */ > + jmp_grow_static_block(file, insn->jump_dest); Hmm we should only do this when the jump is backwards, if its forwards we should just mark the destination instruction and let the main iteration loop take it. > insn->static_jump_dest = true; > return false; > something like the below merged in maybe; I'm going to look at this again later, my brain is completely useless today :/ Josh, how would you feel about adding the bits required to make objtool a full disassembler? That would make it far easier to visualse the state and I don't think its _that_ much more, all the difficult bits are already done afaict, all we need is infrastructure to print the already fully decoded instruction. --- --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1210,16 +1210,16 @@ static int read_retpoline_hints(struct o static void jmp_grow_static_block(struct objtool_file *file, struct instruction *insn); static bool __grow_static_block(struct objtool_file *file, - struct instruction *insn, bool ign_bt) + struct instruction *insn) { - /* if a jump can come in here, terminate */ - if (insn->branch_target && !ign_bt) + /* if a !static jump can come in here, terminate */ + if (insn->branch_target && !insn->static_jump_dest) return false; switch (insn->type) { case INSN_JUMP_UNCONDITIONAL: /* follow the jump, mark this instruction, terminate this section */ - jmp_grow_static_block(file, insn->jump_dest); + jmp_grow_static_block(file, insn); insn->static_jump_dest = true; return false; @@ -1243,27 +1243,43 @@ static bool __grow_static_block(struct o static void jmp_grow_static_block(struct objtool_file *file, struct instruction *insn) { - bool ignore = true; + struct instruction *dest = insn->jump_dest; + bool static_block = true; - /* !jump_dest */ - if (!insn) + if (!dest) return; /* more than a single site jumps here, can't be certain */ - if (insn->branch_target > 1) + if (dest->branch_target > 1) return; - for (; &insn->list != &file->insn_list; - insn = list_next_entry(insn, list)) { - + if (dest->offset > insn->offset) { /* - * Per definition the first insn of a jump is a branch target, - * don't terminate because of that. + * fwd jump, the main iteration will still get there. + * make sure it continues the section there. */ - if (!__grow_static_block(file, insn, ignore)) - break; + dest->static_jump_dest = true; + return; + } - ignore = false; + /* backwards jump, we need to revisit the instructions */ + for (; static_block && dest != insn; dest = list_next_entry(dest, list)) { + + static_block = __grow_static_block(file, dest); + + if (insn->type == INSN_CALL) { + struct symbol *func = insn->call_dest; + if (!func) + continue; + + /* + * we flipped the call to static, update the stats. + */ + if (insn->static_jump_dest) { + func->non_static_call--; + func->static_call++; + } + } } } @@ -1276,7 +1292,7 @@ static int grow_static_blocks(struct obj for_each_insn(file, insn) { if (static_block || insn->static_jump_dest) - static_block = __grow_static_block(file, insn, !static_block); + static_block = __grow_static_block(file, insn); if (insn->type == INSN_CALL) { func = insn->call_dest; @@ -1284,9 +1300,9 @@ static int grow_static_blocks(struct obj continue; if (static_block) - func->static_call = true; + func->static_call++; else - func->non_static_call = true; + func->non_static_call++; } } @@ -1301,7 +1317,7 @@ static int grow_static_blocks(struct obj /* static && !non_static -- only static callers */ func_for_each_insn(file, func, insn) { - if (!__grow_static_block(file, insn, false)) + if (!__grow_static_block(file, insn)) break; } } --- a/tools/objtool/elf.h +++ b/tools/objtool/elf.h @@ -61,7 +61,7 @@ struct symbol { unsigned char bind, type; unsigned long offset; unsigned int len; - bool static_call, non_static_call; + unsigned int static_call, non_static_call; }; struct rela {