objtool: sync warning fix and cleanups

PATCH In review Josh Poimboeuf 3 participants 7 messages Nov 3, 2017 – Nov 4, 2017 objtool

lore

Share & tools

lore

Josh Poimboeuf

author

2017-11-03 14:31 UTC #0 lore raw

cover objtool: sync warning fix and cleanups

Patch 1 fixes the "x86 instruction decoder differs from kernel" warning.

Patches 2-3 improve the sync checking so that future warnings will be more useful.

Josh Poimboeuf (3):
objtool: Resync objtool's instruction decoder with the kernel's
objtool: Move synced files to their original relative locations
objtool: Move sync check to a script

 tools/objtool/.gitignore                           |  2 +-
 tools/objtool/Makefile                             | 20 +++------------
 tools/objtool/arch/x86/Build                       | 10 +++-----
 tools/objtool/arch/x86/decode.c                    |  6 ++---
 .../objtool/arch/x86/{insn => include/asm}/inat.h  |  2 +-
 .../arch/x86/{insn => include/asm}/inat_types.h    |  0
 .../objtool/arch/x86/{insn => include/asm}/insn.h  |  2 +-
 .../objtool/{ => arch/x86/include/asm}/orc_types.h |  0
 tools/objtool/arch/x86/{insn => lib}/inat.c        |  2 +-
 tools/objtool/arch/x86/{insn => lib}/insn.c        |  4 +--
 .../arch/x86/{insn => lib}/x86-opcode-map.txt      |  0
 .../arch/x86/{insn => tools}/gen-insn-attr-x86.awk |  1 +
 tools/objtool/orc.h                                |  2 +-
 tools/objtool/sync-check.sh                        | 29 ++++++++++++++++++++++
 14 files changed, 48 insertions(+), 32 deletions(-)
 rename tools/objtool/arch/x86/{insn => include/asm}/inat.h (99%)
 rename tools/objtool/arch/x86/{insn => include/asm}/inat_types.h (100%)
 rename tools/objtool/arch/x86/{insn => include/asm}/insn.h (99%)
 rename tools/objtool/{ => arch/x86/include/asm}/orc_types.h (100%)
 rename tools/objtool/arch/x86/{insn => lib}/inat.c (99%)
 rename tools/objtool/arch/x86/{insn => lib}/insn.c (99%)
 rename tools/objtool/arch/x86/{insn => lib}/x86-opcode-map.txt (100%)
 rename tools/objtool/arch/x86/{insn => tools}/gen-insn-attr-x86.awk (99%)
 create mode 100755 tools/objtool/sync-check.sh

Signature · 1 line

2.13.6

Josh Poimboeuf

author

2017-11-03 14:31 UTC #1 lore raw

PATCH 1/3 objtool: Resync objtool's instruction decoder with the kernel's

This fixes the following warning:

warning: objtool: x86 instruction decoder differs from kernel

Reported-by Stephen Rothwell <[email protected]>

Signed-off-by Josh Poimboeuf <[email protected]>

---
 tools/objtool/arch/x86/insn/gen-insn-attr-x86.awk | 1 +
 1 file changed, 1 insertion(+)

tools/objtool/arch/x86/insn/gen-insn-attr-x86.awk +1 -0

index a3d2c62fd805..b02a36b2c14f 100644
@@ -1,4 +1,5 @@
 #!/bin/awk -f
+# SPDX-License-Identifier: GPL-2.0
 # gen-insn-attr-x86.awk: Instruction attribute table generator
 # Written by Masami Hiramatsu <[email protected]>
 #

Signature · 1 line

2.13.6

tip-bot for Josh Poimboeuf

merge bot

2017-11-04 08:36 UTC #2 lore raw

[tip:core/urgent] objtool: Resync objtool's instruction decoder source code copy with the kernel's latest version

objtool: Resync objtool's instruction decoder source code copy with the kernel's latest version

Commit-ID:  da0db32bbe816085d7f7f9cebfc825e55eff811f
Gitweb:     https://git.kernel.org/tip/da0db32bbe816085d7f7f9cebfc825e55eff811f
Author:     Josh Poimboeuf <[email protected]>
AuthorDate: Fri, 3 Nov 2017 09:31:43 -0500
Committer:  Ingo Molnar <[email protected]>
CommitDate: Sat, 4 Nov 2017 08:54:06 +0100

objtool: Resync objtool's instruction decoder source code copy with the kernel's latest version

This fixes the following warning:

warning: objtool: x86 instruction decoder differs from kernel

Reported-by Stephen Rothwell <[email protected]>

Signed-off-by Josh Poimboeuf <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/013315a808ccf5580abc293808827c8e2b5e1354.1509719152.git.jpoimboe@redhat.com open here

Signed-off-by Ingo Molnar <[email protected]>

---
 tools/objtool/arch/x86/insn/gen-insn-attr-x86.awk | 1 +
 1 file changed, 1 insertion(+)

tools/objtool/arch/x86/insn/gen-insn-attr-x86.awk +1 -0

index a3d2c62..b02a36b 100644
@@ -1,4 +1,5 @@
 #!/bin/awk -f
+# SPDX-License-Identifier: GPL-2.0
 # gen-insn-attr-x86.awk: Instruction attribute table generator
 # Written by Masami Hiramatsu <[email protected]>
 #

Josh Poimboeuf

author

2017-11-03 14:33 UTC #3 lore raw

PATCH 2/3 objtool: Move synced files to their original relative locations

In reply to Josh Poimboeuf

This will enable more straightforward comparisons, and it also allows the files to be 100% identical.

Signed-off-by Josh Poimboeuf <[email protected]>

---
 tools/objtool/.gitignore                             |  2 +-
 tools/objtool/Makefile                               | 20 +++++++++++---------
 tools/objtool/arch/x86/Build                         | 10 ++++------
 tools/objtool/arch/x86/decode.c                      |  6 +++---
 tools/objtool/arch/x86/{insn => include/asm}/inat.h  |  2 +-
 .../arch/x86/{insn => include/asm}/inat_types.h      |  0
 tools/objtool/arch/x86/{insn => include/asm}/insn.h  |  2 +-
 tools/objtool/{ => arch/x86/include/asm}/orc_types.h |  0
 tools/objtool/arch/x86/{insn => lib}/inat.c          |  2 +-
 tools/objtool/arch/x86/{insn => lib}/insn.c          |  4 ++--
 .../arch/x86/{insn => lib}/x86-opcode-map.txt        |  0
 .../arch/x86/{insn => tools}/gen-insn-attr-x86.awk   |  0
 tools/objtool/orc.h                                  |  2 +-
 13 files changed, 25 insertions(+), 25 deletions(-)
 rename tools/objtool/arch/x86/{insn => include/asm}/inat.h (99%)
 rename tools/objtool/arch/x86/{insn => include/asm}/inat_types.h (100%)
 rename tools/objtool/arch/x86/{insn => include/asm}/insn.h (99%)
 rename tools/objtool/{ => arch/x86/include/asm}/orc_types.h (100%)
 rename tools/objtool/arch/x86/{insn => lib}/inat.c (99%)
 rename tools/objtool/arch/x86/{insn => lib}/insn.c (99%)
 rename tools/objtool/arch/x86/{insn => lib}/x86-opcode-map.txt (100%)
 rename tools/objtool/arch/x86/{insn => tools}/gen-insn-attr-x86.awk (100%)

tools/objtool/.gitignore +1 -1

index d3102c865a95..914cff12899b 100644
@@ -1,3 +1,3 @@
-arch/x86/insn/inat-tables.c
+arch/x86/lib/inat-tables.c
 objtool
 fixdep

tools/objtool/Makefile +11 -9

index 424b1965d06f..7f4771ed0666 100644
@@ -25,7 +25,9 @@ OBJTOOL_IN := $(OBJTOOL)-in.o
 
 all: $(OBJTOOL)
 
-INCLUDES := -I$(srctree)/tools/include -I$(srctree)/tools/arch/$(HOSTARCH)/include/uapi
+INCLUDES := -I$(srctree)/tools/include \
+	    -I$(srctree)/tools/arch/$(HOSTARCH)/include/uapi \
+	    -I$(srctree)/tools/objtool/arch/$(HOSTARCH)/include
 WARNINGS := $(EXTRA_WARNINGS) -Wno-switch-default -Wno-switch-enum -Wno-packed
 CFLAGS   += -Wall -Werror $(WARNINGS) -fomit-frame-pointer -O2 -g $(INCLUDES)
 LDFLAGS  += -lelf $(LIBSUBCMD)

@@ -46,16 +48,16 @@ $(OBJTOOL_IN): fixdep FORCE
 $(OBJTOOL): $(LIBSUBCMD) $(OBJTOOL_IN)
 	@(diff -I 2>&1 | grep -q 'option requires an argument' && \
 	test -d ../../kernel -a -d ../../tools -a -d ../objtool && (( \
-	diff -I'^#include' arch/x86/insn/insn.c ../../arch/x86/lib/insn.c >/dev/null && \
-	diff -I'^#include' arch/x86/insn/inat.c ../../arch/x86/lib/inat.c >/dev/null && \
-	diff arch/x86/insn/x86-opcode-map.txt ../../arch/x86/lib/x86-opcode-map.txt >/dev/null && \
-	diff arch/x86/insn/gen-insn-attr-x86.awk ../../arch/x86/tools/gen-insn-attr-x86.awk >/dev/null && \
-	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) \
+	diff arch/x86/lib/insn.c ../../arch/x86/lib/insn.c >/dev/null && \
+	diff arch/x86/lib/inat.c ../../arch/x86/lib/inat.c >/dev/null && \
+	diff arch/x86/lib/x86-opcode-map.txt ../../arch/x86/lib/x86-opcode-map.txt >/dev/null && \
+	diff arch/x86/tools/gen-insn-attr-x86.awk ../../arch/x86/tools/gen-insn-attr-x86.awk >/dev/null && \
+	diff arch/x86/include/asm/insn.h ../../arch/x86/include/asm/insn.h >/dev/null && \
+	diff arch/x86/include/asm/inat.h ../../arch/x86/include/asm/inat.h >/dev/null && \
+	diff arch/x86/include/asm/inat_types.h ../../arch/x86/include/asm/inat_types.h >/dev/null) \
 	|| echo "warning: objtool: x86 instruction decoder differs from kernel" >&2 )) || true
 	@(test -d ../../kernel -a -d ../../tools -a -d ../objtool && (( \
-	diff ../../arch/x86/include/asm/orc_types.h orc_types.h >/dev/null) \
+	diff ../../arch/x86/include/asm/orc_types.h arch/x86/include/asm/orc_types.h >/dev/null) \
 	|| echo "warning: objtool: orc_types.h differs from kernel" >&2 )) || true
 	$(QUIET_LINK)$(CC) $(OBJTOOL_IN) $(LDFLAGS) -o $@

tools/objtool/arch/x86/Build +4 -6

index debbdb0b5c43..dddadc7948f7 100644
@@ -1,12 +1,10 @@
 objtool-y += decode.o
 
-inat_tables_script = arch/x86/insn/gen-insn-attr-x86.awk
-inat_tables_maps = arch/x86/insn/x86-opcode-map.txt
+inat_tables_script = arch/x86/tools/gen-insn-attr-x86.awk
+inat_tables_maps = arch/x86/lib/x86-opcode-map.txt
 
-$(OUTPUT)arch/x86/insn/inat-tables.c: $(inat_tables_script) $(inat_tables_maps)
+$(OUTPUT)arch/x86/lib/inat-tables.c: $(inat_tables_script) $(inat_tables_maps)
 	$(call rule_mkdir)
 	$(Q)$(call echo-cmd,gen)$(AWK) -f $(inat_tables_script) $(inat_tables_maps) > $@
 
-$(OUTPUT)arch/x86/decode.o: $(OUTPUT)arch/x86/insn/inat-tables.c
-
-CFLAGS_decode.o += -I$(OUTPUT)arch/x86/insn
+$(OUTPUT)arch/x86/decode.o: $(OUTPUT)arch/x86/lib/inat-tables.c

tools/objtool/arch/x86/decode.c +3 -3

index 34a579f806e3..8acfc47af70e 100644
@@ -19,9 +19,9 @@
 #include <stdlib.h>
 
 #define unlikely(cond) (cond)
-#include "insn/insn.h"
-#include "insn/inat.c"
-#include "insn/insn.c"
+#include <asm/insn.h>
+#include "lib/inat.c"
+#include "lib/insn.c"
 
 #include "../../elf.h"
 #include "../../arch.h"

tools/objtool/arch/x86/include/asm/inat.h renamed from tools/objtool/arch/x86/insn/inat.h +1 -1

similarity index 99%
rename from tools/objtool/arch/x86/insn/inat.h
rename to tools/objtool/arch/x86/include/asm/inat.h
index 125ecd2a300d..02aff0867211 100644
@@ -20,7 +20,7 @@
  * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
  *
  */
-#include "inat_types.h"
+#include <asm/inat_types.h>
 
 /*
  * Internal bits. Don't use bitmasks directly, because these bits are

tools/objtool/arch/x86/include/asm/inat_types.h renamed from tools/objtool/arch/x86/insn/inat_types.h +0 -0

similarity index 100%
rename from tools/objtool/arch/x86/insn/inat_types.h
rename to tools/objtool/arch/x86/include/asm/inat_types.h

tools/objtool/arch/x86/include/asm/insn.h renamed from tools/objtool/arch/x86/insn/insn.h +1 -1

similarity index 99%
rename from tools/objtool/arch/x86/insn/insn.h
rename to tools/objtool/arch/x86/include/asm/insn.h
index e23578c7b1be..b3e32b010ab1 100644
@@ -21,7 +21,7 @@
  */
 
 /* insn_attr_t is defined in inat.h */
-#include "inat.h"
+#include <asm/inat.h>
 
 struct insn_field {
 	union {

tools/objtool/arch/x86/include/asm/orc_types.h renamed from tools/objtool/orc_types.h +0 -0

similarity index 100%
rename from tools/objtool/orc_types.h
rename to tools/objtool/arch/x86/include/asm/orc_types.h

tools/objtool/arch/x86/lib/inat.c renamed from tools/objtool/arch/x86/insn/inat.c +1 -1

similarity index 99%
rename from tools/objtool/arch/x86/insn/inat.c
rename to tools/objtool/arch/x86/lib/inat.c
index e4bf28e6f4c7..c1f01a8e9f65 100644
@@ -18,7 +18,7 @@
  * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
  *
  */
-#include "insn.h"
+#include <asm/insn.h>
 
 /* Attribute tables are generated from opcode map */
 #include "inat-tables.c"

tools/objtool/arch/x86/lib/insn.c renamed from tools/objtool/arch/x86/insn/insn.c +2 -2

similarity index 99%
rename from tools/objtool/arch/x86/insn/insn.c
rename to tools/objtool/arch/x86/lib/insn.c
index ca983e2bea8b..1088eb8f3a5f 100644
@@ -23,8 +23,8 @@
 #else
 #include <string.h>
 #endif
-#include "inat.h"
-#include "insn.h"
+#include <asm/inat.h>
+#include <asm/insn.h>
 
 /* Verify next sizeof(t) bytes can be on the same instruction */
 #define validate_next(t, insn, n)	\

tools/objtool/arch/x86/lib/x86-opcode-map.txt renamed from tools/objtool/arch/x86/insn/x86-opcode-map.txt +0 -0

similarity index 100%
rename from tools/objtool/arch/x86/insn/x86-opcode-map.txt
rename to tools/objtool/arch/x86/lib/x86-opcode-map.txt

tools/objtool/arch/x86/tools/gen-insn-attr-x86.awk renamed from tools/objtool/arch/x86/insn/gen-insn-attr-x86.awk +0 -0

similarity index 100%
rename from tools/objtool/arch/x86/insn/gen-insn-attr-x86.awk
rename to tools/objtool/arch/x86/tools/gen-insn-attr-x86.awk

tools/objtool/orc.h +1 -1

index a4139e386ef3..b0e92a6d0903 100644
@@ -18,7 +18,7 @@
 #ifndef _ORC_H
 #define _ORC_H
 
-#include "orc_types.h"
+#include <asm/orc_types.h>
 
 struct objtool_file;

Signature · 1 line

2.13.6

Josh Poimboeuf

author

2017-11-03 14:31 UTC #4 lore raw

PATCH 3/3 objtool: Move sync check to a script

In reply to Josh Poimboeuf

Replace the nasty diff checks in the objtool Makefile with a clean bash script, and make the warnings more specific.

Heavily inspired by tools/perf/check-headers.sh.

Signed-off-by Josh Poimboeuf <[email protected]>

---
 tools/objtool/Makefile      | 16 +---------------
 tools/objtool/sync-check.sh | 29 +++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 15 deletions(-)
 create mode 100755 tools/objtool/sync-check.sh

tools/objtool/Makefile +1 ~~-15~~

index 7f4771ed0666..1cde71c231d5 100644
@@ -43,22 +43,8 @@ include $(srctree)/tools/build/Makefile.include
 $(OBJTOOL_IN): fixdep FORCE
 	@$(MAKE) $(build)=objtool
 
-# Busybox's diff doesn't have -I, avoid warning in that case
-#
 $(OBJTOOL): $(LIBSUBCMD) $(OBJTOOL_IN)
-	@(diff -I 2>&1 | grep -q 'option requires an argument' && \
-	test -d ../../kernel -a -d ../../tools -a -d ../objtool && (( \
-	diff arch/x86/lib/insn.c ../../arch/x86/lib/insn.c >/dev/null && \
-	diff arch/x86/lib/inat.c ../../arch/x86/lib/inat.c >/dev/null && \
-	diff arch/x86/lib/x86-opcode-map.txt ../../arch/x86/lib/x86-opcode-map.txt >/dev/null && \
-	diff arch/x86/tools/gen-insn-attr-x86.awk ../../arch/x86/tools/gen-insn-attr-x86.awk >/dev/null && \
-	diff arch/x86/include/asm/insn.h ../../arch/x86/include/asm/insn.h >/dev/null && \
-	diff arch/x86/include/asm/inat.h ../../arch/x86/include/asm/inat.h >/dev/null && \
-	diff arch/x86/include/asm/inat_types.h ../../arch/x86/include/asm/inat_types.h >/dev/null) \
-	|| echo "warning: objtool: x86 instruction decoder differs from kernel" >&2 )) || true
-	@(test -d ../../kernel -a -d ../../tools -a -d ../objtool && (( \
-	diff ../../arch/x86/include/asm/orc_types.h arch/x86/include/asm/orc_types.h >/dev/null) \
-	|| echo "warning: objtool: orc_types.h differs from kernel" >&2 )) || true
+	@./sync-check.sh
 	$(QUIET_LINK)$(CC) $(OBJTOOL_IN) $(LDFLAGS) -o $@

tools/objtool/sync-check.sh new file +29 -0

new file mode 100755
index 000000000000..f2e5f8b0460c
@@ -0,0 +1,29 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+FILES='
+arch/x86/lib/insn.c
+arch/x86/lib/inat.c
+arch/x86/lib/x86-opcode-map.txt
+arch/x86/tools/gen-insn-attr-x86.awk
+arch/x86/include/asm/insn.h
+arch/x86/include/asm/inat.h
+arch/x86/include/asm/inat_types.h
+arch/x86/include/asm/orc_types.h
+'
+
+check()
+{
+	local file=$1
+
+	diff $file ../../$file &> /dev/null ||
+		echo "Warning: synced file at 'tools/objtool/$file' differs from latest kernel version at '$file'"
+}
+
+if [ ! -d ../../kernel ] || [ ! -d ../../tools ] || [ ! -d ../objtool ]; then
+	exit 0
+fi
+
+for i in $FILES; do
+  check $i
+done

Signature · 1 line

2.13.6

Josh Poimboeuf

author

2017-11-03 18:54 UTC #5 lore raw

In reply to Josh Poimboeuf

Quoted 4 lines · Josh Poimboeuf

Patch 1 fixes the "x86 instruction decoder differs from kernel" warning.
Patches 2-3 improve the sync checking so that future warnings will be more useful.

Hm, patch 2 seems to be causing a 0-day bot error which I need to look into.

However the real fix is in patch 1, it can be taken immediately.

Signature · 1 line

Josh

Ingo Molnar

2017-11-04 07:55 UTC #6 lore raw

Quoted 10 lines · Josh Poimboeuf

Quoted 4 lines · Josh Poimboeuf
Patch 1 fixes the "x86 instruction decoder differs from kernel" warning.
Patches 2-3 improve the sync checking so that future warnings will be more useful.
Hm, patch 2 seems to be causing a 0-day bot error which I need to look into.
However the real fix is in patch 1, it can be taken immediately.

Ok, picked up #1 and will wait for v2 of the other two patches.

Thanks,

Ingo