objtool warning fixes
cover objtool warning fixes
Fixes for some objtool-related warnings reported by Linus.
Josh Poimboeuf (3):
objtool: support new gcc 6 switch jump table pattern
objtool: resync x86 instruction decoder with the kernel's
objtool: un-capitalize "Warning" for out-of-sync instruction decoder
tools/objtool/Makefile | 2 +- tools/objtool/arch/x86/insn/gen-insn-attr-x86.awk | 11 +- tools/objtool/arch/x86/insn/inat.h | 17 +- tools/objtool/arch/x86/insn/insn.c | 18 +- tools/objtool/arch/x86/insn/insn.h | 12 +- tools/objtool/arch/x86/insn/x86-opcode-map.txt | 263 ++++++++++++++-------- tools/objtool/builtin-check.c | 140 +++++++----- 7 files changed, 309 insertions(+), 154 deletions(-)
Signature · 1 line
2.7.4
PATCH 1/3 objtool: support new gcc 6 switch jump table pattern
This fixes some false positive objtool warnings seen with gcc 6.1.1:
kernel/trace/ring_buffer.o: warning: objtool: ring_buffer_read_page()+0x36c: sibling call from callable instruction with changed frame pointer arch/x86/kernel/reboot.o: warning: objtool: native_machine_emergency_restart()+0x139: sibling call from callable instruction with changed frame pointer lib/xz/xz_dec_stream.o: warning: objtool: xz_dec_run()+0xc2: sibling call from callable instruction with changed frame pointer
With gcc 6, a new code pattern is sometimes used to access a switch statement jump table in .rodata, which objtool doesn't yet recognize:
mov [rodata addr],%reg1
... some instructions ...
jmpq *(%reg1,%reg2,8)
Add support for detecting that pattern. The detection code is rather crude, but it's still effective at weeding out false positives and catching real warnings. It can be refined later once objtool starts reading DWARF CFI.
--- tools/objtool/builtin-check.c | 140 ++++++++++++++++++++++++++---------------- 1 file changed, 88 insertions(+), 52 deletions(-)
tools/objtool/builtin-check.c +88 -52
@@ -107,6 +107,12 @@ static struct instruction *next_insn_same_sec(struct objtool_file *file, insn->offset < func->offset + func->len; \ insn = list_next_entry(insn, list)) +#define func_for_each_insn_continue_reverse (file, func, insn) \ + for (insn = list_prev_entry(insn, list); \ + &insn->list != &file->insn_list && \ + insn->sec == func->sec && insn->offset >= func->offset; \ + insn = list_prev_entry(insn, list)) +#define sec_for_each_insn_from (file, insn) \ for (; insn; insn = next_insn_same_sec(file, insn))
@@ -664,65 +670,95 @@ static int add_switch_table(struct objtool_file *file, struct symbol *func,return 0 ; } -static int add_func_switch_tables (struct objtool_file *file, -struct symbol *func) +/* +* find_switch_table() - Given a dynamic jump, find the switch jump table in +* .rodata associated with it. +* +* There are 3 basic patterns: +* +* 1. jmpq *[rodata addr](,%reg,8) +* +* This is the most common case by far. It jumps to an address in a simple +* jump table which is stored in .rodata. +* +* 2. jmpq *[rodata addr](%rip) +* +* This is caused by a rare gcc quirk, currently only seen in three driver +* functions in the kernel, only with certain obscure non-distro configs. +* +* As part of an optimization, gcc makes a copy of an existing switch jump +* table, modifies it, and then hard-codes the jump (albeit with an indirect +* jump) to use a single entry in the table. The rest of the jump table and +* some of its jump targets remain as dead code. +* +* In such a case we can just crudely ignore all unreachable instruction +* warnings for the entire object file. Ideally we would just ignore them +* for the function, but that would require redesigning the code quite a +* bit. And honestly that's just not worth doing: unreachable instruction +* warnings are of questionable value anyway, and this is such a rare issue. +* +* 3. mov [rodata addr],%reg1 +* ... some instructions ... +* jmpq *(%reg1,%reg2,8) +* +* This is a fairly uncommon pattern which is new for gcc 6. As of this +* writing, there are 11 occurrences of it in the allmodconfig kernel. +* +* TODO: Once we have DWARF CFI and smarter instruction decoding logic, +* ensure the same register is used in the mov and jump instructions. +*/ +static struct rela *find_switch_table (struct objtool_file *file, +struct symbol *func, +struct instruction *insn) { -struct instruction *insn, *prev_jump; -struct rela *text_rela, *rodata_rela, *prev_rela =NULL ; -int ret; +struct rela *text_rela, *rodata_rela; - prev_jump =NULL ; + text_rela =find_rela_by_dest_range (insn->sec, insn->offset, insn->len); +if (text_rela && text_rela->sym == file->rodata->sym) { +/* case 1 */ + rodata_rela =find_rela_by_dest (file->rodata, + text_rela->addend); +if (rodata_rela) +return rodata_rela; -func_for_each_insn (file, func, insn) { -if (insn->type !=INSN_JUMP_DYNAMIC ) -continue ; +/* case 2 */ + rodata_rela =find_rela_by_dest (file->rodata, + text_rela->addend +4 ); +if (!rodata_rela) +return NULL ; + file->ignore_unreachables = true; +return rodata_rela; + } + +/* case 3 */ +func_for_each_insn_continue_reverse (file, func, insn) { +if (insn->type ==INSN_JUMP_UNCONDITIONAL || + insn->type ==INSN_JUMP_DYNAMIC ) +break ; text_rela =find_rela_by_dest_range (insn->sec, insn->offset, insn->len); -if (!text_rela || text_rela->sym != file->rodata->sym) -continue ; +if (text_rela && text_rela->sym == file->rodata->sym) +return find_rela_by_dest (file->rodata, + text_rela->addend); + } -/* common case: jmpq *[addr](,%rax,8) */ - rodata_rela =find_rela_by_dest (file->rodata, - text_rela->addend); +return NULL ; +} -/* -* rare case: jmpq *[addr](%rip) -* -* This check is for a rare gcc quirk, currently only seen in -* three driver functions in the kernel, only with certain -* obscure non-distro configs. -* -* As part of an optimization, gcc makes a copy of an existing -* switch jump table, modifies it, and then hard-codes the jump -* (albeit with an indirect jump) to use a single entry in the -* table. The rest of the jump table and some of its jump -* targets remain as dead code. -* -* In such a case we can just crudely ignore all unreachable -* instruction warnings for the entire object file. Ideally we -* would just ignore them for the function, but that would -* require redesigning the code quite a bit. And honestly -* that's just not worth doing: unreachable instruction -* warnings are of questionable value anyway, and this is such -* a rare issue. -* -* kbuild reports: -* - https://lkml.kernel.org/r/201603231906.LWcVUpxm%[email protected] -* - https://lkml.kernel.org/r/201603271114.K9i45biy%[email protected] -* - https://lkml.kernel.org/r/201603291058.zuJ6ben1%[email protected] -* -* gcc bug: -* - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70604 -*/ -if (!rodata_rela) { - rodata_rela =find_rela_by_dest (file->rodata, - text_rela->addend +4 ); -if (rodata_rela) - file->ignore_unreachables = true; - } +static int add_func_switch_tables (struct objtool_file *file, +struct symbol *func) +{ +struct instruction *insn, *prev_jump =NULL ; +struct rela *rela, *prev_rela =NULL ; +int ret; -if (!rodata_rela) +func_for_each_insn (file, func, insn) { +if (insn->type !=INSN_JUMP_DYNAMIC ) +continue ; + + rela =find_switch_table (file, func, insn); +if (!rela)continue ;/*
@@ -732,13 +768,13 @@ static int add_func_switch_tables(struct objtool_file *file,*/ if (prev_jump) { ret =add_switch_table (file, func, prev_jump, prev_rela, - rodata_rela); + rela);if (ret)return ret; } prev_jump = insn; - prev_rela = rodata_rela; + prev_rela = rela; }if (prev_jump) {
Signature · 1 line
2.7.4
tip-bot for Josh Poimboeuf
merge bot[tip:core/urgent] objtool: Support new GCC 6 switch jump table pattern
objtool: Support new GCC 6 switch jump table pattern
Commit-ID: 6d01f28bc08814abc7b8f8973cc71195537f4c80 Gitweb: http://git.kernel.org/tip/6d01f28bc08814abc7b8f8973cc71195537f4c80 Author: Josh Poimboeuf <[email protected]> AuthorDate: Thu, 28 Jul 2016 19:14:58 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 29 Jul 2016 16:56:21 +0200
objtool: Support new GCC 6 switch jump table pattern
This fixes some false positive objtool warnings seen with gcc 6.1.1:
kernel/trace/ring_buffer.o: warning: objtool: ring_buffer_read_page()+0x36c: sibling call from callable instruction with changed frame pointer arch/x86/kernel/reboot.o: warning: objtool: native_machine_emergency_restart()+0x139: sibling call from callable instruction with changed frame pointer lib/xz/xz_dec_stream.o: warning: objtool: xz_dec_run()+0xc2: sibling call from callable instruction with changed frame pointer
With GCC 6, a new code pattern is sometimes used to access a switch statement jump table in .rodata, which objtool doesn't yet recognize:
mov [rodata addr],%reg1
... some instructions ...
jmpq *(%reg1,%reg2,8)
Add support for detecting that pattern. The detection code is rather crude, but it's still effective at weeding out false positives and catching real warnings. It can be refined later once objtool starts reading DWARF CFI.
--- tools/objtool/builtin-check.c | 140 ++++++++++++++++++++++++++---------------- 1 file changed, 88 insertions(+), 52 deletions(-)
tools/objtool/builtin-check.c +88 -52
@@ -107,6 +107,12 @@ static struct instruction *next_insn_same_sec(struct objtool_file *file, insn->offset < func->offset + func->len; \ insn = list_next_entry(insn, list)) +#define func_for_each_insn_continue_reverse (file, func, insn) \ + for (insn = list_prev_entry(insn, list); \ + &insn->list != &file->insn_list && \ + insn->sec == func->sec && insn->offset >= func->offset; \ + insn = list_prev_entry(insn, list)) +#define sec_for_each_insn_from (file, insn) \ for (; insn; insn = next_insn_same_sec(file, insn))
@@ -664,65 +670,95 @@ static int add_switch_table(struct objtool_file *file, struct symbol *func,return 0 ; } -static int add_func_switch_tables (struct objtool_file *file, -struct symbol *func) +/* +* find_switch_table() - Given a dynamic jump, find the switch jump table in +* .rodata associated with it. +* +* There are 3 basic patterns: +* +* 1. jmpq *[rodata addr](,%reg,8) +* +* This is the most common case by far. It jumps to an address in a simple +* jump table which is stored in .rodata. +* +* 2. jmpq *[rodata addr](%rip) +* +* This is caused by a rare GCC quirk, currently only seen in three driver +* functions in the kernel, only with certain obscure non-distro configs. +* +* As part of an optimization, GCC makes a copy of an existing switch jump +* table, modifies it, and then hard-codes the jump (albeit with an indirect +* jump) to use a single entry in the table. The rest of the jump table and +* some of its jump targets remain as dead code. +* +* In such a case we can just crudely ignore all unreachable instruction +* warnings for the entire object file. Ideally we would just ignore them +* for the function, but that would require redesigning the code quite a +* bit. And honestly that's just not worth doing: unreachable instruction +* warnings are of questionable value anyway, and this is such a rare issue. +* +* 3. mov [rodata addr],%reg1 +* ... some instructions ... +* jmpq *(%reg1,%reg2,8) +* +* This is a fairly uncommon pattern which is new for GCC 6. As of this +* writing, there are 11 occurrences of it in the allmodconfig kernel. +* +* TODO: Once we have DWARF CFI and smarter instruction decoding logic, +* ensure the same register is used in the mov and jump instructions. +*/ +static struct rela *find_switch_table (struct objtool_file *file, +struct symbol *func, +struct instruction *insn) { -struct instruction *insn, *prev_jump; -struct rela *text_rela, *rodata_rela, *prev_rela =NULL ; -int ret; +struct rela *text_rela, *rodata_rela; - prev_jump =NULL ; + text_rela =find_rela_by_dest_range (insn->sec, insn->offset, insn->len); +if (text_rela && text_rela->sym == file->rodata->sym) { +/* case 1 */ + rodata_rela =find_rela_by_dest (file->rodata, + text_rela->addend); +if (rodata_rela) +return rodata_rela; -func_for_each_insn (file, func, insn) { -if (insn->type !=INSN_JUMP_DYNAMIC ) -continue ; +/* case 2 */ + rodata_rela =find_rela_by_dest (file->rodata, + text_rela->addend +4 ); +if (!rodata_rela) +return NULL ; + file->ignore_unreachables = true; +return rodata_rela; + } + +/* case 3 */ +func_for_each_insn_continue_reverse (file, func, insn) { +if (insn->type ==INSN_JUMP_UNCONDITIONAL || + insn->type ==INSN_JUMP_DYNAMIC ) +break ; text_rela =find_rela_by_dest_range (insn->sec, insn->offset, insn->len); -if (!text_rela || text_rela->sym != file->rodata->sym) -continue ; +if (text_rela && text_rela->sym == file->rodata->sym) +return find_rela_by_dest (file->rodata, + text_rela->addend); + } -/* common case: jmpq *[addr](,%rax,8) */ - rodata_rela =find_rela_by_dest (file->rodata, - text_rela->addend); +return NULL ; +} -/* -* rare case: jmpq *[addr](%rip) -* -* This check is for a rare gcc quirk, currently only seen in -* three driver functions in the kernel, only with certain -* obscure non-distro configs. -* -* As part of an optimization, gcc makes a copy of an existing -* switch jump table, modifies it, and then hard-codes the jump -* (albeit with an indirect jump) to use a single entry in the -* table. The rest of the jump table and some of its jump -* targets remain as dead code. -* -* In such a case we can just crudely ignore all unreachable -* instruction warnings for the entire object file. Ideally we -* would just ignore them for the function, but that would -* require redesigning the code quite a bit. And honestly -* that's just not worth doing: unreachable instruction -* warnings are of questionable value anyway, and this is such -* a rare issue. -* -* kbuild reports: -* - https://lkml.kernel.org/r/201603231906.LWcVUpxm%[email protected] -* - https://lkml.kernel.org/r/201603271114.K9i45biy%[email protected] -* - https://lkml.kernel.org/r/201603291058.zuJ6ben1%[email protected] -* -* gcc bug: -* - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70604 -*/ -if (!rodata_rela) { - rodata_rela =find_rela_by_dest (file->rodata, - text_rela->addend +4 ); -if (rodata_rela) - file->ignore_unreachables = true; - } +static int add_func_switch_tables (struct objtool_file *file, +struct symbol *func) +{ +struct instruction *insn, *prev_jump =NULL ; +struct rela *rela, *prev_rela =NULL ; +int ret; -if (!rodata_rela) +func_for_each_insn (file, func, insn) { +if (insn->type !=INSN_JUMP_DYNAMIC ) +continue ; + + rela =find_switch_table (file, func, insn); +if (!rela)continue ;/*
@@ -732,13 +768,13 @@ static int add_func_switch_tables(struct objtool_file *file,*/ if (prev_jump) { ret =add_switch_table (file, func, prev_jump, prev_rela, - rodata_rela); + rela);if (ret)return ret; } prev_jump = insn; - prev_rela = rodata_rela; + prev_rela = rela; }if (prev_jump) {
PATCH 2/3 objtool: resync x86 instruction decoder with the kernel's
In reply to Josh PoimboeufThis fixes the following warning:
Warning: objtool: x86 instruction decoder differs from kernel
Unfortunately we have three identical copies of the x86 instruction decoder in the kernel tree that have to be manually kept in sync.
It's on my TODO list to at least library-ize the ones in the tools subdir so we'd only have two of them instead of three. In the meantime, here's another manual sync.
--- tools/objtool/arch/x86/insn/gen-insn-attr-x86.awk | 11 +- tools/objtool/arch/x86/insn/inat.h | 17 +- tools/objtool/arch/x86/insn/insn.c | 18 +- tools/objtool/arch/x86/insn/insn.h | 12 +- tools/objtool/arch/x86/insn/x86-opcode-map.txt | 263 ++++++++++++++-------- 5 files changed, 220 insertions(+), 101 deletions(-)
tools/objtool/arch/x86/insn/gen-insn-attr-x86.awk +8 -3
@@ -72,12 +72,14 @@ BEGIN { lprefix_expr = "\\((66|F2|F3)\\)" max_lprefix = 4 - # All opcodes starting with lower-case 'v' or with (v1) superscript + # All opcodes starting with lower-case 'v', 'k' or with (v1) superscript # accepts VEX prefix - vexok_opcode_expr = "^v.*" + vexok_opcode_expr = "^[vk].*" vexok_expr = "\\(v1\\)" # All opcodes with (v) superscript supports *only* VEX prefix vexonly_expr = "\\(v\\)" + # All opcodes with (ev) superscript supports *only* EVEX prefix + evexonly_expr = "\\(ev\\)" prefix_expr = "\\(Prefix\\)" prefix_num["Operand-Size"] = "INAT_PFX_OPNDSZ"
@@ -95,6 +97,7 @@ BEGIN { prefix_num["Address-Size"] = "INAT_PFX_ADDRSZ" prefix_num["VEX+1byte"] = "INAT_PFX_VEX2" prefix_num["VEX+2byte"] = "INAT_PFX_VEX3" + prefix_num["EVEX"] = "INAT_PFX_EVEX" clear_vars() }
@@ -319,7 +322,9 @@ function convert_operands(count,opnd, i,j,imm,mod) flags = add_flags(flags, "INAT_MODRM") # check VEX codes - if (match(ext, vexonly_expr)) + if (match(ext, evexonly_expr)) + flags = add_flags(flags, "INAT_VEXOK | INAT_EVEXONLY") + else if (match(ext, vexonly_expr)) flags = add_flags(flags, "INAT_VEXOK | INAT_VEXONLY") else if (match(ext, vexok_expr) || match(opcode, vexok_opcode_expr)) flags = add_flags(flags, "INAT_VEXOK")
tools/objtool/arch/x86/insn/inat.h +15 -2
@@ -48,6 +48,7 @@/* AVX VEX prefixes */ #define INAT_PFX_VEX2 13/* 2-bytes VEX prefix */ #define INAT_PFX_VEX3 14/* 3-bytes VEX prefix */ +#define INAT_PFX_EVEX 15/* EVEX prefix */ #define INAT_LSTPFX_MAX 3#define INAT_LGCPFX_MAX 11
@@ -89,6 +90,7 @@#define INAT_VARIANT (1 << (INAT_FLAG_OFFS + 4))#define INAT_VEXOK (1 << (INAT_FLAG_OFFS + 5))#define INAT_VEXONLY (1 << (INAT_FLAG_OFFS + 6)) +#define INAT_EVEXONLY (1 << (INAT_FLAG_OFFS + 7))/* Attribute making macros for attribute tables */ #define INAT_MAKE_PREFIX (pfx) (pfx << INAT_PFX_OFFS)#define INAT_MAKE_ESCAPE (esc) (esc << INAT_ESC_OFFS)
@@ -141,7 +143,13 @@ static inline int inat_last_prefix_id(insn_attr_t attr)static inline int inat_is_vex_prefix (insn_attr_t attr) { attr &=INAT_PFX_MASK ; -return attr ==INAT_PFX_VEX2 || attr ==INAT_PFX_VEX3 ; +return attr ==INAT_PFX_VEX2 || attr ==INAT_PFX_VEX3 || + attr ==INAT_PFX_EVEX ; +} + +static inline int inat_is_evex_prefix (insn_attr_t attr) +{ +return (attr &INAT_PFX_MASK ) ==INAT_PFX_EVEX ; }static inline int inat_is_vex3_prefix (insn_attr_t attr)
@@ -216,6 +224,11 @@ static inline int inat_accept_vex(insn_attr_t attr)static inline int inat_must_vex (insn_attr_t attr) { -return attr &INAT_VEXONLY ; +return attr & (INAT_VEXONLY |INAT_EVEXONLY ); +} + +static inline int inat_must_evex (insn_attr_t attr) +{ +return attr &INAT_EVEXONLY ; }#endif
tools/objtool/arch/x86/insn/insn.c +15 -3
@@ -155,14 +155,24 @@ found:/* * In 32-bits mode, if the [7:6] bits (mod bits of * ModRM) on the second byte are not 11b, it is -* LDS or LES. +* LDS or LES or BOUND. */ if (X86_MODRM_MOD (b2) !=3 ) goto vex_end; } insn->vex_prefix.bytes[0 ] = b; insn->vex_prefix.bytes[1 ] = b2; -if (inat_is_vex3_prefix (attr)) { +if (inat_is_evex_prefix (attr)) { + b2 =peek_nbyte_next (insn_byte_t, insn,2 ); + insn->vex_prefix.bytes[2 ] = b2; + b2 =peek_nbyte_next (insn_byte_t, insn,3 ); + insn->vex_prefix.bytes[3 ] = b2; + insn->vex_prefix.nbytes =4 ; + insn->next_byte +=4 ; +if (insn->x86_64 &&X86_VEX_W (b2)) +/* VEX.W overrides opnd_size */ + insn->opnd_bytes =8 ; + }else if (inat_is_vex3_prefix (attr)) { b2 =peek_nbyte_next (insn_byte_t, insn,2 ); insn->vex_prefix.bytes[2 ] = b2; insn->vex_prefix.nbytes =3 ;
@@ -221,7 +231,9 @@ void insn_get_opcode(struct insn *insn) m =insn_vex_m_bits (insn); p =insn_vex_p_bits (insn); insn->attr =inat_get_avx_attribute (op, m, p); -if (!inat_accept_vex (insn->attr) && !inat_is_group (insn->attr)) +if ((inat_must_evex (insn->attr) && !insn_is_evex (insn)) || + (!inat_accept_vex (insn->attr) && + !inat_is_group (insn->attr))) insn->attr =0 ;/* This instruction is bad */ goto end;/* VEX has only 1 byte for opcode */ }
tools/objtool/arch/x86/insn/insn.h +11 -1
@@ -91,6 +91,7 @@ struct insn {#define X86_VEX_B (vex) ((vex) & 0x20)/* VEX3 Byte1 */ #define X86_VEX_L (vex) ((vex) & 0x04)/* VEX3 Byte2, VEX2 Byte1 */ /* VEX bit fields */ +#define X86_EVEX_M (vex) ((vex) & 0x03)/* EVEX Byte1 */ #define X86_VEX3_M (vex) ((vex) & 0x1f)/* VEX3 Byte1 */ #define X86_VEX2_M 1/* VEX2.M always 1 */ #define X86_VEX_V (vex) (((vex) & 0x78) >> 3)/* VEX3 Byte2, VEX2 Byte1 */
@@ -133,6 +134,13 @@ static inline int insn_is_avx(struct insn *insn)return (insn->vex_prefix.value !=0 ); } +static inline int insn_is_evex (struct insn *insn) +{ +if (!insn->prefixes.got) +insn_get_prefixes (insn); +return (insn->vex_prefix.nbytes ==4 ); +} +/* Ensure this instruction is decoded completely */ static inline int insn_complete (struct insn *insn) {
@@ -144,8 +152,10 @@ static inline insn_byte_t insn_vex_m_bits(struct insn *insn) {if (insn->vex_prefix.nbytes ==2 )/* 2 bytes VEX */ return X86_VEX2_M ; -else +else if (insn->vex_prefix.nbytes ==3 )/* 3 bytes VEX */ return X86_VEX3_M (insn->vex_prefix.bytes[1 ]); +else /* EVEX */ +return X86_EVEX_M (insn->vex_prefix.bytes[1 ]); }static inline insn_byte_t insn_vex_p_bits (struct insn *insn)
tools/objtool/arch/x86/insn/x86-opcode-map.txt +171 -92
438 lines not on this page · raw message
Signature · 1 line
2.7.4
tip-bot for Josh Poimboeuf
merge bot[tip:core/urgent] objtool: Resync x86 instruction decoder with the kernel's
objtool: Resync x86 instruction decoder with the kernel's
Commit-ID: 8d94c2f9198301bd78feb1d9ad6c08a8ee7b5d1c Gitweb: http://git.kernel.org/tip/8d94c2f9198301bd78feb1d9ad6c08a8ee7b5d1c Author: Josh Poimboeuf <[email protected]> AuthorDate: Thu, 28 Jul 2016 19:14:59 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 29 Jul 2016 16:57:16 +0200
objtool: Resync x86 instruction decoder with the kernel's
This fixes the following warning:
Warning: objtool: x86 instruction decoder differs from kernel
Unfortunately we have three identical copies of the x86 instruction decoder in the kernel tree that have to be manually kept in sync.
It's on my TODO list to at least library-ize the ones in the tools subdir so we'd only have two of them instead of three. In the meantime, here's another manual sync.
--- tools/objtool/arch/x86/insn/gen-insn-attr-x86.awk | 11 +- tools/objtool/arch/x86/insn/inat.h | 17 +- tools/objtool/arch/x86/insn/insn.c | 18 +- tools/objtool/arch/x86/insn/insn.h | 12 +- tools/objtool/arch/x86/insn/x86-opcode-map.txt | 263 ++++++++++++++-------- 5 files changed, 220 insertions(+), 101 deletions(-)
tools/objtool/arch/x86/insn/gen-insn-attr-x86.awk +8 -3
@@ -72,12 +72,14 @@ BEGIN { lprefix_expr = "\\((66|F2|F3)\\)" max_lprefix = 4 - # All opcodes starting with lower-case 'v' or with (v1) superscript + # All opcodes starting with lower-case 'v', 'k' or with (v1) superscript # accepts VEX prefix - vexok_opcode_expr = "^v.*" + vexok_opcode_expr = "^[vk].*" vexok_expr = "\\(v1\\)" # All opcodes with (v) superscript supports *only* VEX prefix vexonly_expr = "\\(v\\)" + # All opcodes with (ev) superscript supports *only* EVEX prefix + evexonly_expr = "\\(ev\\)" prefix_expr = "\\(Prefix\\)" prefix_num["Operand-Size"] = "INAT_PFX_OPNDSZ"
@@ -95,6 +97,7 @@ BEGIN { prefix_num["Address-Size"] = "INAT_PFX_ADDRSZ" prefix_num["VEX+1byte"] = "INAT_PFX_VEX2" prefix_num["VEX+2byte"] = "INAT_PFX_VEX3" + prefix_num["EVEX"] = "INAT_PFX_EVEX" clear_vars() }
@@ -319,7 +322,9 @@ function convert_operands(count,opnd, i,j,imm,mod) flags = add_flags(flags, "INAT_MODRM") # check VEX codes - if (match(ext, vexonly_expr)) + if (match(ext, evexonly_expr)) + flags = add_flags(flags, "INAT_VEXOK | INAT_EVEXONLY") + else if (match(ext, vexonly_expr)) flags = add_flags(flags, "INAT_VEXOK | INAT_VEXONLY") else if (match(ext, vexok_expr) || match(opcode, vexok_opcode_expr)) flags = add_flags(flags, "INAT_VEXOK")
tools/objtool/arch/x86/insn/inat.h +15 -2
@@ -48,6 +48,7 @@/* AVX VEX prefixes */ #define INAT_PFX_VEX2 13/* 2-bytes VEX prefix */ #define INAT_PFX_VEX3 14/* 3-bytes VEX prefix */ +#define INAT_PFX_EVEX 15/* EVEX prefix */ #define INAT_LSTPFX_MAX 3#define INAT_LGCPFX_MAX 11
@@ -89,6 +90,7 @@#define INAT_VARIANT (1 << (INAT_FLAG_OFFS + 4))#define INAT_VEXOK (1 << (INAT_FLAG_OFFS + 5))#define INAT_VEXONLY (1 << (INAT_FLAG_OFFS + 6)) +#define INAT_EVEXONLY (1 << (INAT_FLAG_OFFS + 7))/* Attribute making macros for attribute tables */ #define INAT_MAKE_PREFIX (pfx) (pfx << INAT_PFX_OFFS)#define INAT_MAKE_ESCAPE (esc) (esc << INAT_ESC_OFFS)
@@ -141,7 +143,13 @@ static inline int inat_last_prefix_id(insn_attr_t attr)static inline int inat_is_vex_prefix (insn_attr_t attr) { attr &=INAT_PFX_MASK ; -return attr ==INAT_PFX_VEX2 || attr ==INAT_PFX_VEX3 ; +return attr ==INAT_PFX_VEX2 || attr ==INAT_PFX_VEX3 || + attr ==INAT_PFX_EVEX ; +} + +static inline int inat_is_evex_prefix (insn_attr_t attr) +{ +return (attr &INAT_PFX_MASK ) ==INAT_PFX_EVEX ; }static inline int inat_is_vex3_prefix (insn_attr_t attr)
@@ -216,6 +224,11 @@ static inline int inat_accept_vex(insn_attr_t attr)static inline int inat_must_vex (insn_attr_t attr) { -return attr &INAT_VEXONLY ; +return attr & (INAT_VEXONLY |INAT_EVEXONLY ); +} + +static inline int inat_must_evex (insn_attr_t attr) +{ +return attr &INAT_EVEXONLY ; }#endif
tools/objtool/arch/x86/insn/insn.c +15 -3
@@ -155,14 +155,24 @@ found:/* * In 32-bits mode, if the [7:6] bits (mod bits of * ModRM) on the second byte are not 11b, it is -* LDS or LES. +* LDS or LES or BOUND. */ if (X86_MODRM_MOD (b2) !=3 ) goto vex_end; } insn->vex_prefix.bytes[0 ] = b; insn->vex_prefix.bytes[1 ] = b2; -if (inat_is_vex3_prefix (attr)) { +if (inat_is_evex_prefix (attr)) { + b2 =peek_nbyte_next (insn_byte_t, insn,2 ); + insn->vex_prefix.bytes[2 ] = b2; + b2 =peek_nbyte_next (insn_byte_t, insn,3 ); + insn->vex_prefix.bytes[3 ] = b2; + insn->vex_prefix.nbytes =4 ; + insn->next_byte +=4 ; +if (insn->x86_64 &&X86_VEX_W (b2)) +/* VEX.W overrides opnd_size */ + insn->opnd_bytes =8 ; + }else if (inat_is_vex3_prefix (attr)) { b2 =peek_nbyte_next (insn_byte_t, insn,2 ); insn->vex_prefix.bytes[2 ] = b2; insn->vex_prefix.nbytes =3 ;
@@ -221,7 +231,9 @@ void insn_get_opcode(struct insn *insn) m =insn_vex_m_bits (insn); p =insn_vex_p_bits (insn); insn->attr =inat_get_avx_attribute (op, m, p); -if (!inat_accept_vex (insn->attr) && !inat_is_group (insn->attr)) +if ((inat_must_evex (insn->attr) && !insn_is_evex (insn)) || + (!inat_accept_vex (insn->attr) && + !inat_is_group (insn->attr))) insn->attr =0 ;/* This instruction is bad */ goto end;/* VEX has only 1 byte for opcode */ }
tools/objtool/arch/x86/insn/insn.h +11 -1
@@ -91,6 +91,7 @@ struct insn {#define X86_VEX_B (vex) ((vex) & 0x20)/* VEX3 Byte1 */ #define X86_VEX_L (vex) ((vex) & 0x04)/* VEX3 Byte2, VEX2 Byte1 */ /* VEX bit fields */ +#define X86_EVEX_M (vex) ((vex) & 0x03)/* EVEX Byte1 */ #define X86_VEX3_M (vex) ((vex) & 0x1f)/* VEX3 Byte1 */ #define X86_VEX2_M 1/* VEX2.M always 1 */ #define X86_VEX_V (vex) (((vex) & 0x78) >> 3)/* VEX3 Byte2, VEX2 Byte1 */
@@ -133,6 +134,13 @@ static inline int insn_is_avx(struct insn *insn)return (insn->vex_prefix.value !=0 ); } +static inline int insn_is_evex (struct insn *insn) +{ +if (!insn->prefixes.got) +insn_get_prefixes (insn); +return (insn->vex_prefix.nbytes ==4 ); +} +/* Ensure this instruction is decoded completely */ static inline int insn_complete (struct insn *insn) {
@@ -144,8 +152,10 @@ static inline insn_byte_t insn_vex_m_bits(struct insn *insn) {if (insn->vex_prefix.nbytes ==2 )/* 2 bytes VEX */ return X86_VEX2_M ; -else +else if (insn->vex_prefix.nbytes ==3 )/* 3 bytes VEX */ return X86_VEX3_M (insn->vex_prefix.bytes[1 ]); +else /* EVEX */ +return X86_EVEX_M (insn->vex_prefix.bytes[1 ]); }static inline insn_byte_t insn_vex_p_bits (struct insn *insn)
tools/objtool/arch/x86/insn/x86-opcode-map.txt +171 -92
438 lines not on this page · raw message
PATCH 3/3 objtool: un-capitalize "Warning" for out-of-sync instruction decoder
In reply to Josh PoimboeufChange "Warning" to "warning" to make it look more like a gcc warning. Hopefully that will be enough to help the 0-day bot or other automated tools catch this warning earlier before it ends up in Linus's tree.
--- tools/objtool/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/objtool/Makefile +1 -1
@@ -51,7 +51,7 @@ $(OBJTOOL): $(LIBSUBCMD) $(OBJTOOL_IN) diff -I'^#include' arch/x86/insn/insn.h ../../arch/x86/include/asm/insn.h >/dev/null && \ diff -I'^#include' arch/x86/insn/inat.h ../../arch/x86/include/asm/inat.h >/dev/null && \ diff -I'^#include' arch/x86/insn/inat_types.h ../../arch/x86/include/asm/inat_types.h >/dev/null) \ - || echo "Warning: objtool: x86 instruction decoder differs from kernel" >&2 )) || true + || echo "warning: objtool: x86 instruction decoder differs from kernel" >&2 )) || true $(QUIET_LINK)$(CC) $(OBJTOOL_IN) $(LDFLAGS) -o $@
Signature · 1 line
2.7.4
tip-bot for Josh Poimboeuf
merge bot[tip:core/urgent] objtool: Un-capitalize "Warning" for out-of-sync instruction decoder
objtool: Un-capitalize "Warning" for out-of-sync instruction decoder
Commit-ID: 2af04ea545f49bc4ac59b6aaa644312ecd09a78e Gitweb: http://git.kernel.org/tip/2af04ea545f49bc4ac59b6aaa644312ecd09a78e Author: Josh Poimboeuf <[email protected]> AuthorDate: Thu, 28 Jul 2016 19:15:00 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 29 Jul 2016 16:57:16 +0200
objtool: Un-capitalize "Warning" for out-of-sync instruction decoder
Change "Warning" to "warning" to make it look more like a GCC warning. Hopefully that will be enough to help the 0-day bot or other automated tools catch this warning earlier before it ends up in Linus's tree.
--- tools/objtool/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/objtool/Makefile +1 -1
@@ -51,7 +51,7 @@ $(OBJTOOL): $(LIBSUBCMD) $(OBJTOOL_IN) diff -I'^#include' arch/x86/insn/insn.h ../../arch/x86/include/asm/insn.h >/dev/null && \ diff -I'^#include' arch/x86/insn/inat.h ../../arch/x86/include/asm/inat.h >/dev/null && \ diff -I'^#include' arch/x86/insn/inat_types.h ../../arch/x86/include/asm/inat_types.h >/dev/null) \ - || echo "Warning: objtool: x86 instruction decoder differs from kernel" >&2 )) || true + || echo "warning: objtool: x86 instruction decoder differs from kernel" >&2 )) || true $(QUIET_LINK)$(CC) $(OBJTOOL_IN) $(LDFLAGS) -o $@