x86: decoder bugfixes
cover x86: decoder bugfixes
Hi Ingo,
Here, I fixed bugs in x86 instruction decoder which fixes instruction decoder warnings. This series also fixes below bugs;
<Important>
- Doesn't build decoder selftest tool (test_get_len).
- Fails to decode some grouped AVX instructions.
<Minor>
- insn_sanity may dump message twice on same instruction
- insn_sanity outputs information message on stderr.
Now I'm also trying to update x86 opcode map according to Intel SDM, which allow us to decode AVX2 instructions.
Thank you,
---
Masami Hiramatsu (4):
[BUGFIX] x86/tools: Fix Makefile to build all test tools [BUGFIX] x86: Fix instruction decoder to handle grouped AVX instructions [BUGFIX] x86/tools: Fix instruction decoder message output [BUGFIX] x86/tools: Fix insn_sanity message outputs
arch/x86/lib/inat.c | 9 ++++++++- arch/x86/lib/insn.c | 4 +++- arch/x86/tools/Makefile | 3 +-- arch/x86/tools/insn_sanity.c | 11 +++++------ 4 files changed, 17 insertions(+), 10 deletions(-)
Signature · 4 lines
Masami HIRAMATSU Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: [email protected]
PATCH 1/4 x86/tools: Fix Makefile to build all test tools
Fix arch/x86/tools/Makefile to compile both test tools correctly. This bug leads build error.
--- arch/x86/tools/Makefile | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
arch/x86/tools/Makefile +1 -2
@@ -25,8 +25,7 @@ posttest: $(obj)/test_get_len vmlinux $(obj)/insn_sanity $(call cmd,posttest) $(call cmd,sanitytest) -hostprogs-y := test_get_len -hostprogs-y := insn_sanity +hostprogs-y += test_get_len insn_sanity # -I needed for generated C source and C source which in the kernel tree. HOSTCFLAGS_test_get_len.o := -Wall -I$(objtree)/arch/x86/lib/ -I$(srctree)/arch/x86/include/ -I$(srctree)/arch/x86/lib/ -I$(srctree)/include/
PATCH 2/4 x86: Fix instruction decoder to handle grouped AVX instructions
In reply to Masami HiramatsuFor reducing memory usage of attribute table, x86 instruction decoder puts "Group" attribute only on "no-last-prefix" attribute table (same as vex_p == 0 case).
Thus, the decoder should look no-last-prefix table first, and then only if it is not a group, move on to "with-last-prefix" table (vex_p != 0).
However, current implementation, inat_get_avx_attribute() looks with-last-prefix directly. So, when decoding
a grouped AVX instruction, the decoder fails to find correct group because there is no "Group" attribute on the table. This ends up with the mis-decoding of instructions, as Ingo reported in http://thread.gmane.org/gmane.linux.kernel/1214103
This patch fixes it to check no-last-prefix table first even if that is an AVX instruction, and get an attribute from "with last-prefix" table only if that is not a group.
--- arch/x86/lib/inat.c | 9 ++++++++- arch/x86/lib/insn.c | 4 +++- 2 files changed, 11 insertions(+), 2 deletions(-)
arch/x86/lib/inat.c +8 -1
@@ -82,9 +82,16 @@ insn_attr_t inat_get_avx_attribute(insn_byte_t opcode, insn_byte_t vex_m,const insn_attr_t *table;if (vex_m >X86_VEX_M_MAX || vex_p >INAT_LSTPFX_MAX )return 0 ; - table = inat_avx_tables[vex_m][vex_p]; +/* At first, this checks the master table */ + table = inat_avx_tables[vex_m][0 ];if (!table)return 0 ; +if (!inat_is_group (table[opcode]) && vex_p) { +/* If this is not a group, get attribute directly */ + table = inat_avx_tables[vex_m][vex_p]; +if (!table) +return 0 ; + }return table[opcode]; }
arch/x86/lib/insn.c +3 -1
@@ -202,7 +202,7 @@ 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)) +if (!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 */ }
@@ -249,6 +249,8 @@ void insn_get_modrm(struct insn *insn) pfx =insn_last_prefix (insn); insn->attr =inat_get_group_attribute (mod, pfx, insn->attr); +if (insn_is_avx (insn) && !inat_accept_vex (insn->attr)) + insn->attr =0 ;/* This is bad */ } }
PATCH 3/4 x86/tools: Fix instruction decoder message output
In reply to Masami HiramatsuFix instruction decoder test (insn_sanity), so that it doesn't show both info and error messages twice on same instruction. (In that case, show only error message)
--- arch/x86/tools/insn_sanity.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-)
arch/x86/tools/insn_sanity.c +3 -4
@@ -257,15 +257,14 @@ int main(int argc, char **argv)insn_init (&insn, insn_buf, x86_64);insn_get_length (&insn); -if (verbose && !insn_complete (&insn)) -dump_stream (stdout,"Info: Found an undecodable input" , i, insn_buf, &insn); -if (insn.next_byte <= insn.kaddr || insn.kaddr +MAX_INSN_SIZE < insn.next_byte) {/* Access out-of-range memory */ dump_stream (stdout,"Error: Found an access violation" , i, insn_buf, &insn); errors++; - } + }else if (verbose && !insn_complete (&insn)) +dump_stream (stdout,"Info: Found an undecodable input" , i, insn_buf, &insn); + insns++; }
PATCH 4/4 x86/tools: Fix insn_sanity message outputs
In reply to Masami HiramatsuFix x86 instruction decoder test to dump all error messages to stderr and others to stdout.
--- arch/x86/tools/insn_sanity.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
arch/x86/tools/insn_sanity.c +2 -2
@@ -102,7 +102,7 @@ static void dump_stream(FILE *fp, const char *msg, unsigned long nr_iter,fprintf (fp,"%s:\n" , msg); -dump_insn (stderr, insn); +dump_insn (fp, insn);fprintf (fp,"You can reproduce this with below command(s);\n" );
@@ -260,7 +260,7 @@ int main(int argc, char **argv)if (insn.next_byte <= insn.kaddr || insn.kaddr +MAX_INSN_SIZE < insn.next_byte) {/* Access out-of-range memory */ -dump_stream (stdout,"Error: Found an access violation" , i, insn_buf, &insn); +dump_stream (stderr,"Error: Found an access violation" , i, insn_buf, &insn); errors++; }else if (verbose && !insn_complete (&insn))dump_stream (stdout,"Info: Found an undecodable input" , i, insn_buf, &insn);