Here are the patches which update x86 instruction decoder build-time test.
As Stephen reported on linux-next, sometimes objdump decodes bad
instructions as normal. This will cause a false positive result on
x86 insn decoder test. This patches update the test as below;
- Show more information with V=1
- Show in which symbol the difference places.
- Just warning instead of build failure.
Thank you,
---
Masami Hiramatsu (3):
x86: insn decoder test shows build warning
x86: Show symbol name if insn decoder test failed
x86: Add verbose option to insn decoder test
arch/x86/tools/Makefile | 9 +++-
arch/x86/tools/distill.awk | 5 ++
arch/x86/tools/test_get_len.c | 99 ++++++++++++++++++++++++++++++++++-------
3 files changed, 95 insertions(+), 18 deletions(-)
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division
e-mail: [email protected]
Add verbose option to insn decoder test. This dumps decoded
instruction when building kernel with V=1.
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Stephen Rothwell <[email protected]>
Cc: Randy Dunlap <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Jim Keniston <[email protected]>
---
arch/x86/tools/Makefile | 9 ++++-
arch/x86/tools/test_get_len.c | 74 +++++++++++++++++++++++++++++++++++------
2 files changed, 71 insertions(+), 12 deletions(-)
diff --git a/arch/x86/tools/Makefile b/arch/x86/tools/Makefile
index 5e295d9..4688f90 100644
--- a/arch/x86/tools/Makefile
+++ b/arch/x86/tools/Makefile
@@ -1,6 +1,13 @@
PHONY += posttest
+
+ifeq ($(KBUILD_VERBOSE),1)
+ postest_verbose = -v
+else
+ postest_verbose =
+endif
+
quiet_cmd_posttest = TEST $@
- cmd_posttest = $(OBJDUMP) -d -j .text $(objtree)/vmlinux | awk -f $(srctree)/arch/x86/tools/distill.awk | $(obj)/test_get_len $(CONFIG_64BIT)
+ cmd_posttest = $(OBJDUMP) -d -j .text $(objtree)/vmlinux | awk -f $(srctree)/arch/x86/tools/distill.awk | $(obj)/test_get_len -$(CONFIG_64BIT) $(posttest_verbose)
posttest: $(obj)/test_get_len vmlinux
$(call cmd,posttest)
diff --git a/arch/x86/tools/test_get_len.c b/arch/x86/tools/test_get_len.c
index 376d338..5743e51 100644
--- a/arch/x86/tools/test_get_len.c
+++ b/arch/x86/tools/test_get_len.c
@@ -20,6 +20,7 @@
#include <stdio.h>
#include <string.h>
#include <assert.h>
+#include <unistd.h>
#define unlikely(cond) (cond)
@@ -36,11 +37,16 @@
*/
const char *prog;
+static int verbose;
+static int x86_64;
static void usage(void)
{
fprintf(stderr, "Usage: objdump -d a.out | awk -f distill.awk |"
- " %s [y|n](64bit flag)\n", prog);
+ " %s [-y|-n] [-v] \n", prog);
+ fprintf(stderr, "\t-y 64bit mode\n");
+ fprintf(stderr, "\t-n 32bit mode\n");
+ fprintf(stderr, "\t-v verbose mode\n");
exit(1);
}
@@ -50,6 +56,56 @@ static void malformed_line(const char *line, int line_nr)
exit(3);
}
+static void dump_field(FILE *fp, const char *name, const char *indent,
+ struct insn_field *field)
+{
+ fprintf(fp, "%s.%s = {\n", indent, name);
+ fprintf(fp, "%s\t.value = %d, bytes[] = {%x, %x, %x, %x},\n",
+ indent, field->value, field->bytes[0], field->bytes[1],
+ field->bytes[2], field->bytes[3]);
+ fprintf(fp, "%s\t.got = %d, .nbytes = %d},\n", indent,
+ field->got, field->nbytes);
+}
+
+static void dump_insn(FILE *fp, struct insn *insn)
+{
+ fprintf(fp, "Instruction = { \n");
+ dump_field(fp, "prefixes", "\t", &insn->prefixes);
+ dump_field(fp, "rex_prefix", "\t", &insn->rex_prefix);
+ dump_field(fp, "vex_prefix", "\t", &insn->vex_prefix);
+ dump_field(fp, "opcode", "\t", &insn->opcode);
+ dump_field(fp, "modrm", "\t", &insn->modrm);
+ dump_field(fp, "sib", "\t", &insn->sib);
+ dump_field(fp, "displacement", "\t", &insn->displacement);
+ dump_field(fp, "immediate1", "\t", &insn->immediate1);
+ dump_field(fp, "immediate2", "\t", &insn->immediate2);
+ fprintf(fp, "\t.attr = %x, .opnd_bytes = %d, .addr_bytes = %d,\n",
+ insn->attr, insn->opnd_bytes, insn->addr_bytes);
+ fprintf(fp, "\t.length = %d, .x86_64 = %d, .kaddr = %p}\n",
+ insn->length, insn->x86_64, insn->kaddr);
+}
+
+static void parse_args(int argc, char **argv)
+{
+ int c;
+ prog = argv[0];
+ while ((c = getopt(argc, argv, "ynv")) != -1) {
+ switch (c) {
+ case 'y':
+ x86_64 = 1;
+ break;
+ case 'n':
+ x86_64 = 0;
+ break;
+ case 'v':
+ verbose = 1;
+ break;
+ default:
+ usage();
+ }
+ }
+}
+
#define BUFSIZE 256
int main(int argc, char **argv)
@@ -57,15 +113,9 @@ int main(int argc, char **argv)
char line[BUFSIZE];
unsigned char insn_buf[16];
struct insn insn;
- int insns = 0;
- int x86_64 = 0;
-
- prog = argv[0];
- if (argc > 2)
- usage();
+ int insns = 0, c;
- if (argc == 2 && argv[1][0] == 'y')
- x86_64 = 1;
+ parse_args(argc, argv);
while (fgets(line, BUFSIZE, stdin)) {
char copy[BUFSIZE], *s, *tab1, *tab2;
@@ -97,8 +147,10 @@ int main(int argc, char **argv)
if (insn.length != nb) {
fprintf(stderr, "Error: %s", line);
fprintf(stderr, "Error: objdump says %d bytes, but "
- "insn_get_length() says %d (attr:%x)\n", nb,
- insn.length, insn.attr);
+ "insn_get_length() says %d\n", nb,
+ insn.length);
+ if (verbose)
+ dump_insn(stderr, &insn);
exit(2);
}
}
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division
e-mail: [email protected]
Show symbol name if insn decoder test find a difference.
This will help us to find out where the issue is.
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Stephen Rothwell <[email protected]>
Cc: Randy Dunlap <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Jim Keniston <[email protected]>
---
arch/x86/tools/distill.awk | 5 +++++
arch/x86/tools/test_get_len.c | 10 +++++++++-
2 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/arch/x86/tools/distill.awk b/arch/x86/tools/distill.awk
index d433619..c13c0ee 100644
--- a/arch/x86/tools/distill.awk
+++ b/arch/x86/tools/distill.awk
@@ -15,6 +15,11 @@ BEGIN {
fwait_str="9b\tfwait"
}
+/^ *[0-9a-f]+ <[^>]*>:/ {
+ # Symbol entry
+ printf("%s%s\n", $2, $1)
+}
+
/^ *[0-9a-f]+:/ {
if (split($0, field, "\t") < 3) {
# This is a continuation of the same insn.
diff --git a/arch/x86/tools/test_get_len.c b/arch/x86/tools/test_get_len.c
index 5743e51..af75e07 100644
--- a/arch/x86/tools/test_get_len.c
+++ b/arch/x86/tools/test_get_len.c
@@ -110,7 +110,7 @@ static void parse_args(int argc, char **argv)
int main(int argc, char **argv)
{
- char line[BUFSIZE];
+ char line[BUFSIZE], sym[BUFSIZE] = "<unknown>";
unsigned char insn_buf[16];
struct insn insn;
int insns = 0, c;
@@ -122,6 +122,12 @@ int main(int argc, char **argv)
int nb = 0;
unsigned int b;
+ if (line[0] == '<') {
+ /* Symbol line */
+ strcpy(sym, line);
+ continue;
+ }
+
insns++;
memset(insn_buf, 0, 16);
strcpy(copy, line);
@@ -145,6 +151,8 @@ int main(int argc, char **argv)
insn_init(&insn, insn_buf, x86_64);
insn_get_length(&insn);
if (insn.length != nb) {
+ fprintf(stderr, "Error: %s found a difference at %s\n",
+ prog, sym);
fprintf(stderr, "Error: %s", line);
fprintf(stderr, "Error: objdump says %d bytes, but "
"insn_get_length() says %d\n", nb,
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division
e-mail: [email protected]
Since some instructions are not decoded correctly by objdump, it
may cause false positive error in insn decoder posttest. This
changes build error of insn decoder test to build warning.
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Stephen Rothwell <[email protected]>
Cc: Randy Dunlap <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Jim Keniston <[email protected]>
---
arch/x86/tools/test_get_len.c | 17 +++++++++++------
1 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/arch/x86/tools/test_get_len.c b/arch/x86/tools/test_get_len.c
index af75e07..d8214dc 100644
--- a/arch/x86/tools/test_get_len.c
+++ b/arch/x86/tools/test_get_len.c
@@ -114,6 +114,7 @@ int main(int argc, char **argv)
unsigned char insn_buf[16];
struct insn insn;
int insns = 0, c;
+ int warnings = 0;
parse_args(argc, argv);
@@ -151,18 +152,22 @@ int main(int argc, char **argv)
insn_init(&insn, insn_buf, x86_64);
insn_get_length(&insn);
if (insn.length != nb) {
- fprintf(stderr, "Error: %s found a difference at %s\n",
+ warnings++;
+ fprintf(stderr, "Warning: %s found difference at %s\n",
prog, sym);
- fprintf(stderr, "Error: %s", line);
- fprintf(stderr, "Error: objdump says %d bytes, but "
+ fprintf(stderr, "Warning: %s", line);
+ fprintf(stderr, "Warning: objdump says %d bytes, but "
"insn_get_length() says %d\n", nb,
insn.length);
if (verbose)
dump_insn(stderr, &insn);
- exit(2);
}
}
- fprintf(stderr, "Succeed: decoded and checked %d instructions\n",
- insns);
+ if (warnings)
+ fprintf(stderr, "Warning: decoded and checked %d"
+ " instructions with %d warnings\n", insns, warnings);
+ else
+ fprintf(stderr, "Succeed: decoded and checked %d"
+ " instructions\n", insns);
return 0;
}
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division
e-mail: [email protected]
* Masami Hiramatsu <[email protected]> wrote:
> Here are the patches which update x86 instruction decoder build-time
> test. As Stephen reported on linux-next, sometimes objdump decodes bad
> instructions as normal. This will cause a false positive result on x86
> insn decoder test. This patches update the test as below;
>
> - Show more information with V=1
> - Show in which symbol the difference places.
> - Just warning instead of build failure.
yes, -tip testing was showing such build bugs too:
Error: ffffffff8104aae3: c5 83 3d 49 80 ee lds 0xffffffffee80493d(%rbx),%eax
Error: objdump says 6 bytes, but insn_get_length() says 3 (attr:0)
it happens with older tools, such as binutils-2.17. Modern binutils
(2.19) is fine.
We dont want to remove the build error: it helped us fix a number of
real bugs in the decoder - instead please try to create a make based
workaround based on binutils, to not run the test with binutils older
than 2.19 or so.
Thanks,
Ingo
Commit-ID: d65ff75fbe6f8ac7c17f18e4108521898468822c
Gitweb: http://git.kernel.org/tip/d65ff75fbe6f8ac7c17f18e4108521898468822c
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Mon, 16 Nov 2009 18:06:18 -0500
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 17 Nov 2009 07:16:48 +0100
x86: Add verbose option to insn decoder test
Add verbose option to insn decoder test. This dumps decoded
instruction when building kernel with V=1.
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: systemtap <[email protected]>
Cc: DLE <[email protected]>
Cc: Stephen Rothwell <[email protected]>
Cc: Randy Dunlap <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Stephen Rothwell <[email protected]>
LKML-Reference: <20091116230618.5250.18762.stgit@harusame>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/tools/Makefile | 9 ++++-
arch/x86/tools/test_get_len.c | 74 ++++++++++++++++++++++++++++++++++------
2 files changed, 71 insertions(+), 12 deletions(-)
diff --git a/arch/x86/tools/Makefile b/arch/x86/tools/Makefile
index 5e295d9..4688f90 100644
--- a/arch/x86/tools/Makefile
+++ b/arch/x86/tools/Makefile
@@ -1,6 +1,13 @@
PHONY += posttest
+
+ifeq ($(KBUILD_VERBOSE),1)
+ postest_verbose = -v
+else
+ postest_verbose =
+endif
+
quiet_cmd_posttest = TEST $@
- cmd_posttest = $(OBJDUMP) -d -j .text $(objtree)/vmlinux | awk -f $(srctree)/arch/x86/tools/distill.awk | $(obj)/test_get_len $(CONFIG_64BIT)
+ cmd_posttest = $(OBJDUMP) -d -j .text $(objtree)/vmlinux | awk -f $(srctree)/arch/x86/tools/distill.awk | $(obj)/test_get_len -$(CONFIG_64BIT) $(posttest_verbose)
posttest: $(obj)/test_get_len vmlinux
$(call cmd,posttest)
diff --git a/arch/x86/tools/test_get_len.c b/arch/x86/tools/test_get_len.c
index 376d338..5743e51 100644
--- a/arch/x86/tools/test_get_len.c
+++ b/arch/x86/tools/test_get_len.c
@@ -20,6 +20,7 @@
#include <stdio.h>
#include <string.h>
#include <assert.h>
+#include <unistd.h>
#define unlikely(cond) (cond)
@@ -36,11 +37,16 @@
*/
const char *prog;
+static int verbose;
+static int x86_64;
static void usage(void)
{
fprintf(stderr, "Usage: objdump -d a.out | awk -f distill.awk |"
- " %s [y|n](64bit flag)\n", prog);
+ " %s [-y|-n] [-v] \n", prog);
+ fprintf(stderr, "\t-y 64bit mode\n");
+ fprintf(stderr, "\t-n 32bit mode\n");
+ fprintf(stderr, "\t-v verbose mode\n");
exit(1);
}
@@ -50,6 +56,56 @@ static void malformed_line(const char *line, int line_nr)
exit(3);
}
+static void dump_field(FILE *fp, const char *name, const char *indent,
+ struct insn_field *field)
+{
+ fprintf(fp, "%s.%s = {\n", indent, name);
+ fprintf(fp, "%s\t.value = %d, bytes[] = {%x, %x, %x, %x},\n",
+ indent, field->value, field->bytes[0], field->bytes[1],
+ field->bytes[2], field->bytes[3]);
+ fprintf(fp, "%s\t.got = %d, .nbytes = %d},\n", indent,
+ field->got, field->nbytes);
+}
+
+static void dump_insn(FILE *fp, struct insn *insn)
+{
+ fprintf(fp, "Instruction = { \n");
+ dump_field(fp, "prefixes", "\t", &insn->prefixes);
+ dump_field(fp, "rex_prefix", "\t", &insn->rex_prefix);
+ dump_field(fp, "vex_prefix", "\t", &insn->vex_prefix);
+ dump_field(fp, "opcode", "\t", &insn->opcode);
+ dump_field(fp, "modrm", "\t", &insn->modrm);
+ dump_field(fp, "sib", "\t", &insn->sib);
+ dump_field(fp, "displacement", "\t", &insn->displacement);
+ dump_field(fp, "immediate1", "\t", &insn->immediate1);
+ dump_field(fp, "immediate2", "\t", &insn->immediate2);
+ fprintf(fp, "\t.attr = %x, .opnd_bytes = %d, .addr_bytes = %d,\n",
+ insn->attr, insn->opnd_bytes, insn->addr_bytes);
+ fprintf(fp, "\t.length = %d, .x86_64 = %d, .kaddr = %p}\n",
+ insn->length, insn->x86_64, insn->kaddr);
+}
+
+static void parse_args(int argc, char **argv)
+{
+ int c;
+ prog = argv[0];
+ while ((c = getopt(argc, argv, "ynv")) != -1) {
+ switch (c) {
+ case 'y':
+ x86_64 = 1;
+ break;
+ case 'n':
+ x86_64 = 0;
+ break;
+ case 'v':
+ verbose = 1;
+ break;
+ default:
+ usage();
+ }
+ }
+}
+
#define BUFSIZE 256
int main(int argc, char **argv)
@@ -57,15 +113,9 @@ int main(int argc, char **argv)
char line[BUFSIZE];
unsigned char insn_buf[16];
struct insn insn;
- int insns = 0;
- int x86_64 = 0;
-
- prog = argv[0];
- if (argc > 2)
- usage();
+ int insns = 0, c;
- if (argc == 2 && argv[1][0] == 'y')
- x86_64 = 1;
+ parse_args(argc, argv);
while (fgets(line, BUFSIZE, stdin)) {
char copy[BUFSIZE], *s, *tab1, *tab2;
@@ -97,8 +147,10 @@ int main(int argc, char **argv)
if (insn.length != nb) {
fprintf(stderr, "Error: %s", line);
fprintf(stderr, "Error: objdump says %d bytes, but "
- "insn_get_length() says %d (attr:%x)\n", nb,
- insn.length, insn.attr);
+ "insn_get_length() says %d\n", nb,
+ insn.length);
+ if (verbose)
+ dump_insn(stderr, &insn);
exit(2);
}
}
Commit-ID: 35039eb6b199749943547c8572be6604edf00229
Gitweb: http://git.kernel.org/tip/35039eb6b199749943547c8572be6604edf00229
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Mon, 16 Nov 2009 18:06:24 -0500
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 17 Nov 2009 07:16:50 +0100
x86: Show symbol name if insn decoder test failed
Show symbol name if insn decoder test find a difference.
This will help us to find out where the issue is.
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: systemtap <[email protected]>
Cc: DLE <[email protected]>
Cc: Stephen Rothwell <[email protected]>
Cc: Randy Dunlap <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Stephen Rothwell <[email protected]>
LKML-Reference: <20091116230624.5250.49813.stgit@harusame>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/tools/distill.awk | 5 +++++
arch/x86/tools/test_get_len.c | 10 +++++++++-
2 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/arch/x86/tools/distill.awk b/arch/x86/tools/distill.awk
index d433619..c13c0ee 100644
--- a/arch/x86/tools/distill.awk
+++ b/arch/x86/tools/distill.awk
@@ -15,6 +15,11 @@ BEGIN {
fwait_str="9b\tfwait"
}
+/^ *[0-9a-f]+ <[^>]*>:/ {
+ # Symbol entry
+ printf("%s%s\n", $2, $1)
+}
+
/^ *[0-9a-f]+:/ {
if (split($0, field, "\t") < 3) {
# This is a continuation of the same insn.
diff --git a/arch/x86/tools/test_get_len.c b/arch/x86/tools/test_get_len.c
index 5743e51..af75e07 100644
--- a/arch/x86/tools/test_get_len.c
+++ b/arch/x86/tools/test_get_len.c
@@ -110,7 +110,7 @@ static void parse_args(int argc, char **argv)
int main(int argc, char **argv)
{
- char line[BUFSIZE];
+ char line[BUFSIZE], sym[BUFSIZE] = "<unknown>";
unsigned char insn_buf[16];
struct insn insn;
int insns = 0, c;
@@ -122,6 +122,12 @@ int main(int argc, char **argv)
int nb = 0;
unsigned int b;
+ if (line[0] == '<') {
+ /* Symbol line */
+ strcpy(sym, line);
+ continue;
+ }
+
insns++;
memset(insn_buf, 0, 16);
strcpy(copy, line);
@@ -145,6 +151,8 @@ int main(int argc, char **argv)
insn_init(&insn, insn_buf, x86_64);
insn_get_length(&insn);
if (insn.length != nb) {
+ fprintf(stderr, "Error: %s found a difference at %s\n",
+ prog, sym);
fprintf(stderr, "Error: %s", line);
fprintf(stderr, "Error: objdump says %d bytes, but "
"insn_get_length() says %d\n", nb,
Ingo Molnar wrote:
>
> * Masami Hiramatsu <[email protected]> wrote:
>
>> Here are the patches which update x86 instruction decoder build-time
>> test. As Stephen reported on linux-next, sometimes objdump decodes bad
>> instructions as normal. This will cause a false positive result on x86
>> insn decoder test. This patches update the test as below;
>>
>> - Show more information with V=1
>> - Show in which symbol the difference places.
>> - Just warning instead of build failure.
>
> yes, -tip testing was showing such build bugs too:
>
> Error: ffffffff8104aae3: c5 83 3d 49 80 ee lds 0xffffffffee80493d(%rbx),%eax
> Error: objdump says 6 bytes, but insn_get_length() says 3 (attr:0)
>
> it happens with older tools, such as binutils-2.17. Modern binutils
> (2.19) is fine.
Thank you for telling me.
> We dont want to remove the build error: it helped us fix a number of
> real bugs in the decoder - instead please try to create a make based
> workaround based on binutils, to not run the test with binutils older
> than 2.19 or so.
OK, that's fine for me.
Thank you,
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division
e-mail: [email protected]
On 11/16/2009 10:13 PM, Ingo Molnar wrote:
>
> * Masami Hiramatsu <[email protected]> wrote:
>
>> Here are the patches which update x86 instruction decoder build-time
>> test. As Stephen reported on linux-next, sometimes objdump decodes bad
>> instructions as normal. This will cause a false positive result on x86
>> insn decoder test. This patches update the test as below;
>>
>> - Show more information with V=1
>> - Show in which symbol the difference places.
>> - Just warning instead of build failure.
>
> yes, -tip testing was showing such build bugs too:
>
> Error: ffffffff8104aae3: c5 83 3d 49 80 ee lds 0xffffffffee80493d(%rbx),%eax
> Error: objdump says 6 bytes, but insn_get_length() says 3 (attr:0)
>
> it happens with older tools, such as binutils-2.17. Modern binutils
> (2.19) is fine.
>
> We dont want to remove the build error: it helped us fix a number of
> real bugs in the decoder - instead please try to create a make based
> workaround based on binutils, to not run the test with binutils older
> than 2.19 or so.
>
One idea might be to instead of binutils to use NASM. The entire NASM
disassembler is small enough (about 10,000 lines including build tools
and instruction database) that we could fit it in the tree in a pinch.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
Commit-ID: ce64c62074d945fe5f8a7f01bdc30125f994ea67
Gitweb: http://git.kernel.org/tip/ce64c62074d945fe5f8a7f01bdc30125f994ea67
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Mon, 16 Nov 2009 18:06:31 -0500
Committer: Ingo Molnar <[email protected]>
CommitDate: Thu, 19 Nov 2009 21:40:13 +0100
x86: Instruction decoder test should generate build warning
Since some instructions are not decoded correctly by older
versions of objdump, it may cause false positive error in insn
decoder posttest.
This changes build error of insn decoder test to build warning.
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: systemtap <[email protected]>
Cc: DLE <[email protected]>
Cc: Stephen Rothwell <[email protected]>
Cc: Randy Dunlap <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Stephen Rothwell <[email protected]>
LKML-Reference: <20091116230631.5250.41579.stgit@harusame>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/tools/test_get_len.c | 17 +++++++++++------
1 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/arch/x86/tools/test_get_len.c b/arch/x86/tools/test_get_len.c
index af75e07..d8214dc 100644
--- a/arch/x86/tools/test_get_len.c
+++ b/arch/x86/tools/test_get_len.c
@@ -114,6 +114,7 @@ int main(int argc, char **argv)
unsigned char insn_buf[16];
struct insn insn;
int insns = 0, c;
+ int warnings = 0;
parse_args(argc, argv);
@@ -151,18 +152,22 @@ int main(int argc, char **argv)
insn_init(&insn, insn_buf, x86_64);
insn_get_length(&insn);
if (insn.length != nb) {
- fprintf(stderr, "Error: %s found a difference at %s\n",
+ warnings++;
+ fprintf(stderr, "Warning: %s found difference at %s\n",
prog, sym);
- fprintf(stderr, "Error: %s", line);
- fprintf(stderr, "Error: objdump says %d bytes, but "
+ fprintf(stderr, "Warning: %s", line);
+ fprintf(stderr, "Warning: objdump says %d bytes, but "
"insn_get_length() says %d\n", nb,
insn.length);
if (verbose)
dump_insn(stderr, &insn);
- exit(2);
}
}
- fprintf(stderr, "Succeed: decoded and checked %d instructions\n",
- insns);
+ if (warnings)
+ fprintf(stderr, "Warning: decoded and checked %d"
+ " instructions with %d warnings\n", insns, warnings);
+ else
+ fprintf(stderr, "Succeed: decoded and checked %d"
+ " instructions\n", insns);
return 0;
}
Hi Ingo,
Ingo Molnar wrote:
> We dont want to remove the build error: it helped us fix a number of
> real bugs in the decoder - instead please try to create a make based
> workaround based on binutils, to not run the test with binutils older
> than 2.19 or so.
Agreed, this patch checks objdump version and skip the test if it is older
than 2.19.
Thank you,
---
Masami Hiramatsu (2):
x86: insn decoder test checks objdump version
[BUGFIX] x86: Fix insn decoder test typos
arch/x86/tools/Makefile | 15 ++++++++++++---
arch/x86/tools/chkobjdump.awk | 23 +++++++++++++++++++++++
2 files changed, 35 insertions(+), 3 deletions(-)
create mode 100644 arch/x86/tools/chkobjdump.awk
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division
e-mail: [email protected]
Fix postest_verbose to posttest_verbose, and add posttest_64bit option
for CONFIG_64BIT != y, since old command just passed '-' instead
of '-n' when CONFIG_64BIT is not set.
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Stephen Rothwell <[email protected]>
Cc: Randy Dunlap <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Jim Keniston <[email protected]>
---
arch/x86/tools/Makefile | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/arch/x86/tools/Makefile b/arch/x86/tools/Makefile
index 4688f90..c80b079 100644
--- a/arch/x86/tools/Makefile
+++ b/arch/x86/tools/Makefile
@@ -1,13 +1,19 @@
PHONY += posttest
ifeq ($(KBUILD_VERBOSE),1)
- postest_verbose = -v
+ posttest_verbose = -v
else
- postest_verbose =
+ posttest_verbose =
+endif
+
+ifeq ($(CONFIG_64BIT),y)
+ posttest_64bit = -y
+else
+ posttest_64bit = -n
endif
quiet_cmd_posttest = TEST $@
- cmd_posttest = $(OBJDUMP) -d -j .text $(objtree)/vmlinux | awk -f $(srctree)/arch/x86/tools/distill.awk | $(obj)/test_get_len -$(CONFIG_64BIT) $(posttest_verbose)
+ cmd_posttest = $(OBJDUMP) -d -j .text $(objtree)/vmlinux | $(AWK) -f $(srctree)/arch/x86/tools/distill.awk | $(obj)/test_get_len $(posttest_64bit) $(posttest_verbose)
posttest: $(obj)/test_get_len vmlinux
$(call cmd,posttest)
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division
e-mail: [email protected]
Check objdump version before using it for insn decoder build test,
because some older objdump can't decode AVX code correctly.
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Stephen Rothwell <[email protected]>
Cc: Randy Dunlap <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Jim Keniston <[email protected]>
---
arch/x86/tools/Makefile | 5 ++++-
arch/x86/tools/chkobjdump.awk | 23 +++++++++++++++++++++++
2 files changed, 27 insertions(+), 1 deletions(-)
create mode 100644 arch/x86/tools/chkobjdump.awk
diff --git a/arch/x86/tools/Makefile b/arch/x86/tools/Makefile
index c80b079..f820826 100644
--- a/arch/x86/tools/Makefile
+++ b/arch/x86/tools/Makefile
@@ -12,8 +12,11 @@ else
posttest_64bit = -n
endif
+distill_awk = $(srctree)/arch/x86/tools/distill.awk
+chkobjdump = $(srctree)/arch/x86/tools/chkobjdump.awk
+
quiet_cmd_posttest = TEST $@
- cmd_posttest = $(OBJDUMP) -d -j .text $(objtree)/vmlinux | $(AWK) -f $(srctree)/arch/x86/tools/distill.awk | $(obj)/test_get_len $(posttest_64bit) $(posttest_verbose)
+ cmd_posttest = ($(OBJDUMP) -v | $(AWK) -f $(chkobjdump)) || $(OBJDUMP) -d -j .text $(objtree)/vmlinux | $(AWK) -f $(distill_awk) | $(obj)/test_get_len $(posttest_64bit) $(posttest_verbose)
posttest: $(obj)/test_get_len vmlinux
$(call cmd,posttest)
diff --git a/arch/x86/tools/chkobjdump.awk b/arch/x86/tools/chkobjdump.awk
new file mode 100644
index 0000000..0d13cd9
--- /dev/null
+++ b/arch/x86/tools/chkobjdump.awk
@@ -0,0 +1,23 @@
+# GNU objdump version checker
+#
+# Usage:
+# objdump -v | awk -f chkobjdump.awk
+BEGIN {
+ # objdump version 2.19 or later is OK for the test.
+ od_ver = 2;
+ od_sver = 19;
+}
+
+/^GNU/ {
+ split($4, ver, ".");
+ if (ver[1] > od_ver ||
+ (ver[1] == od_ver && ver[2] >= od_sver)) {
+ exit 1;
+ } else {
+ printf("Warning: objdump version %s is older than %d.%d\n",
+ $4, od_ver, od_sver);
+ print("Warning: Skipping posttest.");
+ # Logic is inverted, because we just skip test without error.
+ exit 0;
+ }
+}
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division
e-mail: [email protected]
Commit-ID: 80509e27e40d7554e576405ed9f5b7966c567112
Gitweb: http://git.kernel.org/tip/80509e27e40d7554e576405ed9f5b7966c567112
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Fri, 20 Nov 2009 12:13:08 -0500
Committer: H. Peter Anvin <[email protected]>
CommitDate: Fri, 20 Nov 2009 22:59:36 -0800
x86: Fix insn decoder test typos
Fix postest_verbose to posttest_verbose, and add posttest_64bit option
for CONFIG_64BIT != y, since old command just passed '-' instead
of '-n' when CONFIG_64BIT is not set.
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Stephen Rothwell <[email protected]>
Cc: Randy Dunlap <[email protected]>
Cc: Jim Keniston <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: H. Peter Anvin <[email protected]>
---
arch/x86/tools/Makefile | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/arch/x86/tools/Makefile b/arch/x86/tools/Makefile
index 4688f90..c80b079 100644
--- a/arch/x86/tools/Makefile
+++ b/arch/x86/tools/Makefile
@@ -1,13 +1,19 @@
PHONY += posttest
ifeq ($(KBUILD_VERBOSE),1)
- postest_verbose = -v
+ posttest_verbose = -v
else
- postest_verbose =
+ posttest_verbose =
+endif
+
+ifeq ($(CONFIG_64BIT),y)
+ posttest_64bit = -y
+else
+ posttest_64bit = -n
endif
quiet_cmd_posttest = TEST $@
- cmd_posttest = $(OBJDUMP) -d -j .text $(objtree)/vmlinux | awk -f $(srctree)/arch/x86/tools/distill.awk | $(obj)/test_get_len -$(CONFIG_64BIT) $(posttest_verbose)
+ cmd_posttest = $(OBJDUMP) -d -j .text $(objtree)/vmlinux | $(AWK) -f $(srctree)/arch/x86/tools/distill.awk | $(obj)/test_get_len $(posttest_64bit) $(posttest_verbose)
posttest: $(obj)/test_get_len vmlinux
$(call cmd,posttest)
Commit-ID: 6f5f67267dc4faecd9cba63894de92ca92a608b8
Gitweb: http://git.kernel.org/tip/6f5f67267dc4faecd9cba63894de92ca92a608b8
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Fri, 20 Nov 2009 12:13:14 -0500
Committer: H. Peter Anvin <[email protected]>
CommitDate: Fri, 20 Nov 2009 23:01:04 -0800
x86: insn decoder test checks objdump version
Check objdump version before using it for insn decoder build test,
because some older objdump can't decode AVX code correctly.
Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Stephen Rothwell <[email protected]>
Cc: Randy Dunlap <[email protected]>
Cc: Jim Keniston <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: H. Peter Anvin <[email protected]>
---
arch/x86/tools/Makefile | 5 ++++-
arch/x86/tools/chkobjdump.awk | 23 +++++++++++++++++++++++
2 files changed, 27 insertions(+), 1 deletions(-)
diff --git a/arch/x86/tools/Makefile b/arch/x86/tools/Makefile
index c80b079..f820826 100644
--- a/arch/x86/tools/Makefile
+++ b/arch/x86/tools/Makefile
@@ -12,8 +12,11 @@ else
posttest_64bit = -n
endif
+distill_awk = $(srctree)/arch/x86/tools/distill.awk
+chkobjdump = $(srctree)/arch/x86/tools/chkobjdump.awk
+
quiet_cmd_posttest = TEST $@
- cmd_posttest = $(OBJDUMP) -d -j .text $(objtree)/vmlinux | $(AWK) -f $(srctree)/arch/x86/tools/distill.awk | $(obj)/test_get_len $(posttest_64bit) $(posttest_verbose)
+ cmd_posttest = ($(OBJDUMP) -v | $(AWK) -f $(chkobjdump)) || $(OBJDUMP) -d -j .text $(objtree)/vmlinux | $(AWK) -f $(distill_awk) | $(obj)/test_get_len $(posttest_64bit) $(posttest_verbose)
posttest: $(obj)/test_get_len vmlinux
$(call cmd,posttest)
diff --git a/arch/x86/tools/chkobjdump.awk b/arch/x86/tools/chkobjdump.awk
new file mode 100644
index 0000000..0d13cd9
--- /dev/null
+++ b/arch/x86/tools/chkobjdump.awk
@@ -0,0 +1,23 @@
+# GNU objdump version checker
+#
+# Usage:
+# objdump -v | awk -f chkobjdump.awk
+BEGIN {
+ # objdump version 2.19 or later is OK for the test.
+ od_ver = 2;
+ od_sver = 19;
+}
+
+/^GNU/ {
+ split($4, ver, ".");
+ if (ver[1] > od_ver ||
+ (ver[1] == od_ver && ver[2] >= od_sver)) {
+ exit 1;
+ } else {
+ printf("Warning: objdump version %s is older than %d.%d\n",
+ $4, od_ver, od_sver);
+ print("Warning: Skipping posttest.");
+ # Logic is inverted, because we just skip test without error.
+ exit 0;
+ }
+}