linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] tools: move initial declarations out of 'for' loop
@ 2019-03-05  5:48 Masahiro Yamada
  2019-03-05  5:48 ` [PATCH 2/3] objtool: move stack-validation.txt to Documentation/ Masahiro Yamada
  2019-03-05  5:48 ` [PATCH 3/3] objtool: move tools/objtool/ to scripts/objtool/ Masahiro Yamada
  0 siblings, 2 replies; 8+ messages in thread
From: Masahiro Yamada @ 2019-03-05  5:48 UTC (permalink / raw)
  To: Ingo Molnar, Josh Poimboeuf, Peter Zijlstra
  Cc: Thomas Gleixner, Douglas Anderson, Robin Meijboom,
	Borislav Petkov, H . Peter Anvin, x86, Linus Torvalds,
	Sam Ravnborg, linux-kbuild, Masahiro Yamada, linux-kernel

When I was trying to compile this code for hostprogs-y notation of
Kbuild, I was hit by the following error.

  error: ‘for’ loop initial declarations are only allowed in C99 or C11 mode

This is because KBUILD_HOSTCFLAGS specifies -std=gnu89 whereas the tools
Makefile compiles it with -std=gnu99.

Of course, it would be possible to pass -std=gnu99 per file, but it
shouldn't hurt to fix the C code.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 tools/lib/subcmd/parse-options.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/lib/subcmd/parse-options.c b/tools/lib/subcmd/parse-options.c
index dbb9efb..1bd858a2 100644
--- a/tools/lib/subcmd/parse-options.c
+++ b/tools/lib/subcmd/parse-options.c
@@ -630,6 +630,7 @@ int parse_options_subcommand(int argc, const char **argv, const struct option *o
 			const char *const subcommands[], const char *usagestr[], int flags)
 {
 	struct parse_opt_ctx_t ctx;
+	int i;
 
 	/* build usage string if it's not provided */
 	if (subcommands && !usagestr[0]) {
@@ -637,7 +638,7 @@ int parse_options_subcommand(int argc, const char **argv, const struct option *o
 
 		astrcatf(&buf, "%s %s [<options>] {", subcmd_config.exec_name, argv[0]);
 
-		for (int i = 0; subcommands[i]; i++) {
+		for (i = 0; subcommands[i]; i++) {
 			if (i)
 				astrcat(&buf, "|");
 			astrcat(&buf, subcommands[i]);
@@ -663,7 +664,7 @@ int parse_options_subcommand(int argc, const char **argv, const struct option *o
 		exit(130);
 	case PARSE_OPT_LIST_SUBCMDS:
 		if (subcommands) {
-			for (int i = 0; subcommands[i]; i++)
+			for (i = 0; subcommands[i]; i++)
 				printf("%s ", subcommands[i]);
 		}
 		putchar('\n');
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] objtool: move stack-validation.txt to Documentation/
  2019-03-05  5:48 [PATCH 1/3] tools: move initial declarations out of 'for' loop Masahiro Yamada
@ 2019-03-05  5:48 ` Masahiro Yamada
  2019-03-05 14:44   ` Jonathan Corbet
  2019-03-05  5:48 ` [PATCH 3/3] objtool: move tools/objtool/ to scripts/objtool/ Masahiro Yamada
  1 sibling, 1 reply; 8+ messages in thread
From: Masahiro Yamada @ 2019-03-05  5:48 UTC (permalink / raw)
  To: Ingo Molnar, Josh Poimboeuf, Peter Zijlstra
  Cc: Thomas Gleixner, Douglas Anderson, Robin Meijboom,
	Borislav Petkov, H . Peter Anvin, x86, Linus Torvalds,
	Sam Ravnborg, linux-kbuild, Masahiro Yamada, linux-doc,
	Jonathan Corbet, linux-kernel

Move the document to the standard Documentation/ directory instead of
creating the same directory structure under objtool/.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 {tools/objtool/Documentation => Documentation}/stack-validation.txt | 0
 Documentation/x86/orc-unwinder.txt                                  | 2 +-
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename {tools/objtool/Documentation => Documentation}/stack-validation.txt (100%)

diff --git a/tools/objtool/Documentation/stack-validation.txt b/Documentation/stack-validation.txt
similarity index 100%
rename from tools/objtool/Documentation/stack-validation.txt
rename to Documentation/stack-validation.txt
diff --git a/Documentation/x86/orc-unwinder.txt b/Documentation/x86/orc-unwinder.txt
index cd4b29b..3aeff80 100644
--- a/Documentation/x86/orc-unwinder.txt
+++ b/Documentation/x86/orc-unwinder.txt
@@ -137,7 +137,7 @@ Unwinder implementation details
 
 Objtool generates the ORC data by integrating with the compile-time
 stack metadata validation feature, which is described in detail in
-tools/objtool/Documentation/stack-validation.txt.  After analyzing all
+Documentation/stack-validation.txt.  After analyzing all
 the code paths of a .o file, it creates an array of orc_entry structs,
 and a parallel array of instruction addresses associated with those
 structs, and writes them to the .orc_unwind and .orc_unwind_ip sections
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] objtool: move tools/objtool/ to scripts/objtool/
  2019-03-05  5:48 [PATCH 1/3] tools: move initial declarations out of 'for' loop Masahiro Yamada
  2019-03-05  5:48 ` [PATCH 2/3] objtool: move stack-validation.txt to Documentation/ Masahiro Yamada
@ 2019-03-05  5:48 ` Masahiro Yamada
  2019-03-05 14:46   ` Josh Poimboeuf
  1 sibling, 1 reply; 8+ messages in thread
From: Masahiro Yamada @ 2019-03-05  5:48 UTC (permalink / raw)
  To: Ingo Molnar, Josh Poimboeuf, Peter Zijlstra
  Cc: Thomas Gleixner, Douglas Anderson, Robin Meijboom,
	Borislav Petkov, H . Peter Anvin, x86, Linus Torvalds,
	Sam Ravnborg, linux-kbuild, Masahiro Yamada, linux-kernel,
	Michal Marek

Tools that are globally needed for building the kernel are supposed to
be put in the scripts/ directory, but objtool is the exceptional one.

People were confused by objtool's Makefile since most of tools run on
the target whereas objtool runs on the host machine. Some time ago,
Douglas Anderson suggested to move tools/objtool/ to scripts/tools/
(https://patchwork.kernel.org/patch/9983535/#20996057), but we did
not go as far as submitting a patch at that time.

Recently, Robin Meijboom reported that the generated files for objtool
were never cleaned up. (https://patchwork.kernel.org/patch/10813797/)

So, I looked into this to fix the distortion. Move tools/objtool/ to
scripts/objtool/, and rewrite Makefiles in the Kbuild-ish manner.

I also cleaned up the top-level Makefile by moving the libelf check
scripts/objtool/Makefile.

I see some benefits in this.

[1] generated files under scripts/ are cleaned up by 'make mrproper'

Currently, the generated files under tools/objtool/ are not cleaned up
since the build system under tools/ is a complete different world.

If we want to clean them up, it should be done by 'make mrproper'
instead of 'make clean' since objtool is needed for building external
modules.

This commit naturally solves the issue since Kbuild cleans under
scripts/ by 'make mrproper'.

[2] build log looks nicer

Currently, the build log under tools/objtool/ shows absolute paths, and
displays 'DESCEND  objtool' every time for incremental building.

Switching over to the standard Kbuild scheme will improve the log.

  Before:

  DESCEND  objtool
  HOSTCC   /home/masahiro/workspace/linux/tools/objtool/fixdep.o
  HOSTLD   /home/masahiro/workspace/linux/tools/objtool/fixdep-in.o
  LINK     /home/masahiro/workspace/linux/tools/objtool/fixdep
  CC       /home/masahiro/workspace/linux/tools/objtool/exec-cmd.o
  CC       /home/masahiro/workspace/linux/tools/objtool/help.o
  CC       /home/masahiro/workspace/linux/tools/objtool/pager.o
  CC       /home/masahiro/workspace/linux/tools/objtool/parse-options.o
  CC       /home/masahiro/workspace/linux/tools/objtool/run-command.o
  CC       /home/masahiro/workspace/linux/tools/objtool/sigchain.o
  CC       /home/masahiro/workspace/linux/tools/objtool/subcmd-config.o
  LD       /home/masahiro/workspace/linux/tools/objtool/libsubcmd-in.o
  AR       /home/masahiro/workspace/linux/tools/objtool/libsubcmd.a
  GEN      /home/masahiro/workspace/linux/tools/objtool/arch/x86/lib/inat-tables.c
  CC       /home/masahiro/workspace/linux/tools/objtool/arch/x86/decode.o
  LD       /home/masahiro/workspace/linux/tools/objtool/arch/x86/objtool-in.o
  CC       /home/masahiro/workspace/linux/tools/objtool/builtin-check.o
  CC       /home/masahiro/workspace/linux/tools/objtool/builtin-orc.o
  CC       /home/masahiro/workspace/linux/tools/objtool/check.o
  CC       /home/masahiro/workspace/linux/tools/objtool/orc_gen.o
  CC       /home/masahiro/workspace/linux/tools/objtool/orc_dump.o
  CC       /home/masahiro/workspace/linux/tools/objtool/elf.o
  CC       /home/masahiro/workspace/linux/tools/objtool/special.o
  CC       /home/masahiro/workspace/linux/tools/objtool/objtool.o
  CC       /home/masahiro/workspace/linux/tools/objtool/libstring.o
  CC       /home/masahiro/workspace/linux/tools/objtool/str_error_r.o
  LD       /home/masahiro/workspace/linux/tools/objtool/objtool-in.o
  LINK     /home/masahiro/workspace/linux/tools/objtool/objtool

  After:

  HOSTCC  scripts/objtool/builtin-check.o
  HOSTCC  scripts/objtool/builtin-orc.o
  HOSTCC  scripts/objtool/check.o
  HOSTCC  scripts/objtool/orc_gen.o
  HOSTCC  scripts/objtool/orc_dump.o
  HOSTCC  scripts/objtool/elf.o
  HOSTCC  scripts/objtool/special.o
  HOSTCC  scripts/objtool/objtool.o
  HOSTCC  scripts/objtool/libstring.o
  HOSTCC  scripts/objtool/str_error_r.o
  HOSTCC  scripts/objtool/exec-cmd.o
  HOSTCC  scripts/objtool/pager.o
  HOSTCC  scripts/objtool/parse-options.o
  HOSTCC  scripts/objtool/run-command.o
  HOSTCC  scripts/objtool/sigchain.o
  HOSTCC  scripts/objtool/subcmd-config.o
  AWK     scripts/objtool/arch/x86/lib/inat-tables.c
  HOSTCC  scripts/objtool/arch/x86/decode.o
  HOSTLD  scripts/objtool/objtool

[3] simplify distro package script

The special handling in scripts/package/buildeb is no longer needed
since all host programs are copied to linux-headers packages.

Suggested-by: Douglas Anderson <dianders@chromium.org>
Reported-by: Robin Meijboom <robin@meijboom.info>
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 MAINTAINERS                                        |  2 +-
 Makefile                                           | 24 +--------
 scripts/Makefile                                   |  1 +
 scripts/Makefile.build                             |  4 +-
 {tools => scripts}/objtool/.gitignore              |  1 -
 scripts/objtool/Makefile                           | 43 +++++++++++++++
 {tools => scripts}/objtool/arch.h                  |  0
 scripts/objtool/arch/x86/Makefile                  | 20 +++++++
 {tools => scripts}/objtool/arch/x86/decode.c       |  0
 .../objtool/arch/x86/include/asm/inat.h            |  0
 .../objtool/arch/x86/include/asm/inat_types.h      |  0
 .../objtool/arch/x86/include/asm/insn.h            |  0
 .../objtool/arch/x86/include/asm/orc_types.h       |  0
 {tools => scripts}/objtool/arch/x86/lib/inat.c     |  0
 {tools => scripts}/objtool/arch/x86/lib/insn.c     |  0
 .../objtool/arch/x86/lib/x86-opcode-map.txt        |  0
 .../objtool/arch/x86/tools/gen-insn-attr-x86.awk   |  0
 {tools => scripts}/objtool/builtin-check.c         |  0
 {tools => scripts}/objtool/builtin-orc.c           |  0
 {tools => scripts}/objtool/builtin.h               |  0
 {tools => scripts}/objtool/cfi.h                   |  0
 {tools => scripts}/objtool/check.c                 |  0
 {tools => scripts}/objtool/check.h                 |  0
 {tools => scripts}/objtool/elf.c                   |  0
 {tools => scripts}/objtool/elf.h                   |  0
 scripts/objtool/exec-cmd.c                         |  2 +
 scripts/objtool/libstring.c                        |  1 +
 {tools => scripts}/objtool/objtool.c               |  0
 {tools => scripts}/objtool/orc.h                   |  0
 {tools => scripts}/objtool/orc_dump.c              |  0
 {tools => scripts}/objtool/orc_gen.c               |  0
 scripts/objtool/pager.c                            |  1 +
 scripts/objtool/parse-options.c                    |  2 +
 scripts/objtool/run-command.c                      |  2 +
 scripts/objtool/sigchain.c                         |  2 +
 {tools => scripts}/objtool/special.c               |  0
 {tools => scripts}/objtool/special.h               |  0
 scripts/objtool/str_error_r.c                      |  1 +
 scripts/objtool/subcmd-config.c                    |  1 +
 {tools => scripts}/objtool/sync-check.sh           |  8 +--
 {tools => scripts}/objtool/warn.h                  |  0
 scripts/package/builddeb                           |  3 --
 tools/objtool/Build                                | 22 --------
 tools/objtool/Makefile                             | 63 ----------------------
 tools/objtool/arch/x86/Build                       | 12 -----
 45 files changed, 84 insertions(+), 131 deletions(-)
 rename {tools => scripts}/objtool/.gitignore (83%)
 create mode 100644 scripts/objtool/Makefile
 rename {tools => scripts}/objtool/arch.h (100%)
 create mode 100644 scripts/objtool/arch/x86/Makefile
 rename {tools => scripts}/objtool/arch/x86/decode.c (100%)
 rename {tools => scripts}/objtool/arch/x86/include/asm/inat.h (100%)
 rename {tools => scripts}/objtool/arch/x86/include/asm/inat_types.h (100%)
 rename {tools => scripts}/objtool/arch/x86/include/asm/insn.h (100%)
 rename {tools => scripts}/objtool/arch/x86/include/asm/orc_types.h (100%)
 rename {tools => scripts}/objtool/arch/x86/lib/inat.c (100%)
 rename {tools => scripts}/objtool/arch/x86/lib/insn.c (100%)
 rename {tools => scripts}/objtool/arch/x86/lib/x86-opcode-map.txt (100%)
 rename {tools => scripts}/objtool/arch/x86/tools/gen-insn-attr-x86.awk (100%)
 rename {tools => scripts}/objtool/builtin-check.c (100%)
 rename {tools => scripts}/objtool/builtin-orc.c (100%)
 rename {tools => scripts}/objtool/builtin.h (100%)
 rename {tools => scripts}/objtool/cfi.h (100%)
 rename {tools => scripts}/objtool/check.c (100%)
 rename {tools => scripts}/objtool/check.h (100%)
 rename {tools => scripts}/objtool/elf.c (100%)
 rename {tools => scripts}/objtool/elf.h (100%)
 create mode 100644 scripts/objtool/exec-cmd.c
 create mode 100644 scripts/objtool/libstring.c
 rename {tools => scripts}/objtool/objtool.c (100%)
 rename {tools => scripts}/objtool/orc.h (100%)
 rename {tools => scripts}/objtool/orc_dump.c (100%)
 rename {tools => scripts}/objtool/orc_gen.c (100%)
 create mode 100644 scripts/objtool/pager.c
 create mode 100644 scripts/objtool/parse-options.c
 create mode 100644 scripts/objtool/run-command.c
 create mode 100644 scripts/objtool/sigchain.c
 rename {tools => scripts}/objtool/special.c (100%)
 rename {tools => scripts}/objtool/special.h (100%)
 create mode 100644 scripts/objtool/str_error_r.c
 create mode 100644 scripts/objtool/subcmd-config.c
 rename {tools => scripts}/objtool/sync-check.sh (62%)
 rename {tools => scripts}/objtool/warn.h (100%)
 delete mode 100644 tools/objtool/Build
 delete mode 100644 tools/objtool/Makefile
 delete mode 100644 tools/objtool/arch/x86/Build

diff --git a/MAINTAINERS b/MAINTAINERS
index dce5c09..2fd8b9d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10948,7 +10948,7 @@ OBJTOOL
 M:	Josh Poimboeuf <jpoimboe@redhat.com>
 M:	Peter Zijlstra <peterz@infradead.org>
 S:	Supported
-F:	tools/objtool/
+F:	scripts/objtool/
 
 OCXL (Open Coherent Accelerator Processor Interface OpenCAPI) DRIVER
 M:	Frederic Barrat <fbarrat@linux.ibm.com>
diff --git a/Makefile b/Makefile
index d5713e7..4251aca 100644
--- a/Makefile
+++ b/Makefile
@@ -944,17 +944,6 @@ mod_sign_cmd = true
 endif
 export mod_sign_cmd
 
-ifdef CONFIG_STACK_VALIDATION
-  has_libelf := $(call try-run,\
-		echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -,1,0)
-  ifeq ($(has_libelf),1)
-    objtool_target := tools/objtool FORCE
-  else
-    SKIP_STACK_VALIDATION := 1
-    export SKIP_STACK_VALIDATION
-  endif
-endif
-
 PHONY += prepare0
 
 ifeq ($(KBUILD_EXTMOD),)
@@ -1092,7 +1081,7 @@ prepare0: archprepare
 	$(Q)$(MAKE) $(build)=.
 
 # All the preparing..
-prepare: prepare0 prepare-objtool
+prepare: prepare0
 
 # Support for using generic headers in asm-generic
 asm-generic := -f $(srctree)/scripts/Makefile.asm-generic obj
@@ -1103,17 +1092,6 @@ asm-generic: uapi-asm-generic
 uapi-asm-generic:
 	$(Q)$(MAKE) $(asm-generic)=arch/$(SRCARCH)/include/generated/uapi/asm
 
-PHONY += prepare-objtool
-prepare-objtool: $(objtool_target)
-ifeq ($(SKIP_STACK_VALIDATION),1)
-ifdef CONFIG_UNWINDER_ORC
-	@echo "error: Cannot generate ORC metadata for CONFIG_UNWINDER_ORC=y, please install libelf-dev, libelf-devel or elfutils-libelf-devel" >&2
-	@false
-else
-	@echo "warning: Cannot use CONFIG_STACK_VALIDATION=y, please install libelf-dev, libelf-devel or elfutils-libelf-devel" >&2
-endif
-endif
-
 # Generate some files
 # ---------------------------------------------------------------------------
 
diff --git a/scripts/Makefile b/scripts/Makefile
index feb1f71..edb780f 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -36,6 +36,7 @@ PHONY += build_unifdef
 build_unifdef: $(obj)/unifdef
 	@:
 
+subdir-$(CONFIG_STACK_VALIDATION) += objtool
 subdir-$(CONFIG_GCC_PLUGINS) += gcc-plugins
 subdir-$(CONFIG_MODVERSIONS) += genksyms
 subdir-$(CONFIG_SECURITY_SELINUX) += selinux
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index fd03d60..ccf8310 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -206,9 +206,8 @@ endif # CC_USING_RECORD_MCOUNT
 endif # CONFIG_FTRACE_MCOUNT_RECORD
 
 ifdef CONFIG_STACK_VALIDATION
-ifneq ($(SKIP_STACK_VALIDATION),1)
 
-__objtool_obj := $(objtree)/tools/objtool/objtool
+__objtool_obj := $(objtree)/scripts/objtool/objtool
 
 objtool_args = $(if $(CONFIG_UNWINDER_ORC),orc generate,check)
 
@@ -234,7 +233,6 @@ objtool_obj = $(if $(patsubst y%,, \
 	$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
 	$(__objtool_obj))
 
-endif # SKIP_STACK_VALIDATION
 endif # CONFIG_STACK_VALIDATION
 
 # Rebuild all objects when objtool changes, or is enabled/disabled.
diff --git a/tools/objtool/.gitignore b/scripts/objtool/.gitignore
similarity index 83%
rename from tools/objtool/.gitignore
rename to scripts/objtool/.gitignore
index 914cff1..103ee54 100644
--- a/tools/objtool/.gitignore
+++ b/scripts/objtool/.gitignore
@@ -1,3 +1,2 @@
 arch/x86/lib/inat-tables.c
 objtool
-fixdep
diff --git a/scripts/objtool/Makefile b/scripts/objtool/Makefile
new file mode 100644
index 0000000..34aa625
--- /dev/null
+++ b/scripts/objtool/Makefile
@@ -0,0 +1,43 @@
+# SPDX-License-Identifier: GPL-2.0
+
+libelf-missing := $(call try-run,\
+	echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -,,1)
+
+$(if $(libelf-missing), \
+$(error "Cannot use CONFIG_STACK_VALIDATION=y, please install libelf-dev, libelf-devel or elfutils-libelf-devel"))
+
+hostprogs-y := objtool
+always := objtool sync-check
+
+HOSTLDLIBS_objtool := -lelf
+
+objtool-objs := \
+	builtin-check.o \
+	builtin-orc.o \
+	check.o \
+	orc_gen.o \
+	orc_dump.o \
+	elf.o \
+	special.o \
+	objtool.o \
+	libstring.o \
+	str_error_r.o \
+	exec-cmd.o \
+	pager.o \
+	parse-options.o \
+	run-command.o \
+	sigchain.o \
+	subcmd-config.o
+
+HOST_EXTRACFLAGS := \
+	-I $(srctree)/tools/include \
+	-I $(srctree)/tools/lib \
+	-I $(srctree)/$(src)/arch/$(SRCARCH)/include \
+	-Werror -Wno-switch-default -Wno-switch-enum -Wno-packed -g
+
+include $(srctree)/$(src)/arch/$(SRCARCH)/Makefile
+
+cmd_sync_check = $(CONFIG_SHELL) $(srctree)/$(src)/sync-check.sh
+
+$(obj)/sync-check: FORCE
+	$(call cmd,sync_check)
diff --git a/tools/objtool/arch.h b/scripts/objtool/arch.h
similarity index 100%
rename from tools/objtool/arch.h
rename to scripts/objtool/arch.h
diff --git a/scripts/objtool/arch/x86/Makefile b/scripts/objtool/arch/x86/Makefile
new file mode 100644
index 0000000..cb1515e
--- /dev/null
+++ b/scripts/objtool/arch/x86/Makefile
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0
+
+$(shell mkdir -p $(obj)/arch/$(SRCARCH)/lib)
+
+objtool-objs += arch/$(SRCARCH)/decode.o
+
+HOSTCFLAGS_decode.o := -I $(objtree)/$(obj)/arch/$(SRCARCH)/lib
+
+$(obj)/arch/$(SRCARCH)/decode.o: $(obj)/arch/$(SRCARCH)/lib/inat-tables.c
+
+inat_tables_script = $(srctree)/$(src)/arch/$(SRCARCH)/tools/gen-insn-attr-x86.awk
+inat_tables_maps = $(srctree)/$(src)/arch/$(SRCARCH)/lib/x86-opcode-map.txt
+
+quiet_cmd_inat_table = AWK     $@
+      cmd_inat_table = $(AWK) -f $(inat_tables_script) $(inat_tables_maps) > $@
+
+$(obj)/arch/$(SRCARCH)/lib/inat-tables.c: $(inat_tables_script) $(inat_tables_maps)
+	$(call cmd,inat_table)
+
+clean-files += arch/$(SRCARCH)/lib/inat-tables.c
diff --git a/tools/objtool/arch/x86/decode.c b/scripts/objtool/arch/x86/decode.c
similarity index 100%
rename from tools/objtool/arch/x86/decode.c
rename to scripts/objtool/arch/x86/decode.c
diff --git a/tools/objtool/arch/x86/include/asm/inat.h b/scripts/objtool/arch/x86/include/asm/inat.h
similarity index 100%
rename from tools/objtool/arch/x86/include/asm/inat.h
rename to scripts/objtool/arch/x86/include/asm/inat.h
diff --git a/tools/objtool/arch/x86/include/asm/inat_types.h b/scripts/objtool/arch/x86/include/asm/inat_types.h
similarity index 100%
rename from tools/objtool/arch/x86/include/asm/inat_types.h
rename to scripts/objtool/arch/x86/include/asm/inat_types.h
diff --git a/tools/objtool/arch/x86/include/asm/insn.h b/scripts/objtool/arch/x86/include/asm/insn.h
similarity index 100%
rename from tools/objtool/arch/x86/include/asm/insn.h
rename to scripts/objtool/arch/x86/include/asm/insn.h
diff --git a/tools/objtool/arch/x86/include/asm/orc_types.h b/scripts/objtool/arch/x86/include/asm/orc_types.h
similarity index 100%
rename from tools/objtool/arch/x86/include/asm/orc_types.h
rename to scripts/objtool/arch/x86/include/asm/orc_types.h
diff --git a/tools/objtool/arch/x86/lib/inat.c b/scripts/objtool/arch/x86/lib/inat.c
similarity index 100%
rename from tools/objtool/arch/x86/lib/inat.c
rename to scripts/objtool/arch/x86/lib/inat.c
diff --git a/tools/objtool/arch/x86/lib/insn.c b/scripts/objtool/arch/x86/lib/insn.c
similarity index 100%
rename from tools/objtool/arch/x86/lib/insn.c
rename to scripts/objtool/arch/x86/lib/insn.c
diff --git a/tools/objtool/arch/x86/lib/x86-opcode-map.txt b/scripts/objtool/arch/x86/lib/x86-opcode-map.txt
similarity index 100%
rename from tools/objtool/arch/x86/lib/x86-opcode-map.txt
rename to scripts/objtool/arch/x86/lib/x86-opcode-map.txt
diff --git a/tools/objtool/arch/x86/tools/gen-insn-attr-x86.awk b/scripts/objtool/arch/x86/tools/gen-insn-attr-x86.awk
similarity index 100%
rename from tools/objtool/arch/x86/tools/gen-insn-attr-x86.awk
rename to scripts/objtool/arch/x86/tools/gen-insn-attr-x86.awk
diff --git a/tools/objtool/builtin-check.c b/scripts/objtool/builtin-check.c
similarity index 100%
rename from tools/objtool/builtin-check.c
rename to scripts/objtool/builtin-check.c
diff --git a/tools/objtool/builtin-orc.c b/scripts/objtool/builtin-orc.c
similarity index 100%
rename from tools/objtool/builtin-orc.c
rename to scripts/objtool/builtin-orc.c
diff --git a/tools/objtool/builtin.h b/scripts/objtool/builtin.h
similarity index 100%
rename from tools/objtool/builtin.h
rename to scripts/objtool/builtin.h
diff --git a/tools/objtool/cfi.h b/scripts/objtool/cfi.h
similarity index 100%
rename from tools/objtool/cfi.h
rename to scripts/objtool/cfi.h
diff --git a/tools/objtool/check.c b/scripts/objtool/check.c
similarity index 100%
rename from tools/objtool/check.c
rename to scripts/objtool/check.c
diff --git a/tools/objtool/check.h b/scripts/objtool/check.h
similarity index 100%
rename from tools/objtool/check.h
rename to scripts/objtool/check.h
diff --git a/tools/objtool/elf.c b/scripts/objtool/elf.c
similarity index 100%
rename from tools/objtool/elf.c
rename to scripts/objtool/elf.c
diff --git a/tools/objtool/elf.h b/scripts/objtool/elf.h
similarity index 100%
rename from tools/objtool/elf.h
rename to scripts/objtool/elf.h
diff --git a/scripts/objtool/exec-cmd.c b/scripts/objtool/exec-cmd.c
new file mode 100644
index 0000000..4a153ea
--- /dev/null
+++ b/scripts/objtool/exec-cmd.c
@@ -0,0 +1,2 @@
+#define _GNU_SOURCE
+#include "../../tools/lib/subcmd/exec-cmd.c"
diff --git a/scripts/objtool/libstring.c b/scripts/objtool/libstring.c
new file mode 100644
index 0000000..840c910c
--- /dev/null
+++ b/scripts/objtool/libstring.c
@@ -0,0 +1 @@
+#include "../../tools/lib/string.c"
diff --git a/tools/objtool/objtool.c b/scripts/objtool/objtool.c
similarity index 100%
rename from tools/objtool/objtool.c
rename to scripts/objtool/objtool.c
diff --git a/tools/objtool/orc.h b/scripts/objtool/orc.h
similarity index 100%
rename from tools/objtool/orc.h
rename to scripts/objtool/orc.h
diff --git a/tools/objtool/orc_dump.c b/scripts/objtool/orc_dump.c
similarity index 100%
rename from tools/objtool/orc_dump.c
rename to scripts/objtool/orc_dump.c
diff --git a/tools/objtool/orc_gen.c b/scripts/objtool/orc_gen.c
similarity index 100%
rename from tools/objtool/orc_gen.c
rename to scripts/objtool/orc_gen.c
diff --git a/scripts/objtool/pager.c b/scripts/objtool/pager.c
new file mode 100644
index 0000000..88652da
--- /dev/null
+++ b/scripts/objtool/pager.c
@@ -0,0 +1 @@
+#include "../../tools/lib/subcmd/pager.c"
diff --git a/scripts/objtool/parse-options.c b/scripts/objtool/parse-options.c
new file mode 100644
index 0000000..a9d29b4
--- /dev/null
+++ b/scripts/objtool/parse-options.c
@@ -0,0 +1,2 @@
+#define _GNU_SOURCE
+#include "../../tools/lib/subcmd/parse-options.c"
diff --git a/scripts/objtool/run-command.c b/scripts/objtool/run-command.c
new file mode 100644
index 0000000..f37ca5e
--- /dev/null
+++ b/scripts/objtool/run-command.c
@@ -0,0 +1,2 @@
+#define _GNU_SOURCE
+#include "../../tools/lib/subcmd/run-command.c"
diff --git a/scripts/objtool/sigchain.c b/scripts/objtool/sigchain.c
new file mode 100644
index 0000000..a5ad9bf
--- /dev/null
+++ b/scripts/objtool/sigchain.c
@@ -0,0 +1,2 @@
+#define _GNU_SOURCE
+#include "../../tools/lib/subcmd/sigchain.c"
diff --git a/tools/objtool/special.c b/scripts/objtool/special.c
similarity index 100%
rename from tools/objtool/special.c
rename to scripts/objtool/special.c
diff --git a/tools/objtool/special.h b/scripts/objtool/special.h
similarity index 100%
rename from tools/objtool/special.h
rename to scripts/objtool/special.h
diff --git a/scripts/objtool/str_error_r.c b/scripts/objtool/str_error_r.c
new file mode 100644
index 0000000..a32f7ae
--- /dev/null
+++ b/scripts/objtool/str_error_r.c
@@ -0,0 +1 @@
+#include "../../tools/lib/str_error_r.c"
diff --git a/scripts/objtool/subcmd-config.c b/scripts/objtool/subcmd-config.c
new file mode 100644
index 0000000..6834fd6
--- /dev/null
+++ b/scripts/objtool/subcmd-config.c
@@ -0,0 +1 @@
+#include "../../tools/lib/subcmd/subcmd-config.c"
diff --git a/tools/objtool/sync-check.sh b/scripts/objtool/sync-check.sh
similarity index 62%
rename from tools/objtool/sync-check.sh
rename to scripts/objtool/sync-check.sh
index 1470e74..98d2c97 100755
--- a/tools/objtool/sync-check.sh
+++ b/scripts/objtool/sync-check.sh
@@ -16,11 +16,13 @@ check()
 {
 	local file=$1
 
-	diff $file ../../$file > /dev/null ||
-		echo "Warning: synced file at 'tools/objtool/$file' differs from latest kernel version at '$file'"
+	diff $file scripts/objtool/$file > /dev/null ||
+		echo "Warning: synced file at 'scripts/objtool/$file' differs from latest kernel version at '$file'"
 }
 
-if [ ! -d ../../kernel ] || [ ! -d ../../tools ] || [ ! -d ../objtool ]; then
+cd $srctree
+
+if [ ! -d kernel ] || [ ! -d scripts/objtool ]; then
 	exit 0
 fi
 
diff --git a/tools/objtool/warn.h b/scripts/objtool/warn.h
similarity index 100%
rename from tools/objtool/warn.h
rename to scripts/objtool/warn.h
diff --git a/scripts/package/builddeb b/scripts/package/builddeb
index f43a274..abed281 100755
--- a/scripts/package/builddeb
+++ b/scripts/package/builddeb
@@ -158,9 +158,6 @@ done
 (cd $srctree; find arch/*/include include scripts -type f -o -type l) >> "$objtree/debian/hdrsrcfiles"
 (cd $srctree; find arch/$SRCARCH -name module.lds -o -name Kbuild.platforms -o -name Platform) >> "$objtree/debian/hdrsrcfiles"
 (cd $srctree; find $(find arch/$SRCARCH -name include -o -name scripts -type d) -type f) >> "$objtree/debian/hdrsrcfiles"
-if grep -q '^CONFIG_STACK_VALIDATION=y' $KCONFIG_CONFIG ; then
-	(cd $objtree; find tools/objtool -type f -executable) >> "$objtree/debian/hdrobjfiles"
-fi
 (cd $objtree; find arch/$SRCARCH/include Module.symvers include scripts -type f) >> "$objtree/debian/hdrobjfiles"
 if grep -q '^CONFIG_GCC_PLUGINS=y' $KCONFIG_CONFIG ; then
 	(cd $objtree; find scripts/gcc-plugins -name \*.so -o -name gcc-common.h) >> "$objtree/debian/hdrobjfiles"
diff --git a/tools/objtool/Build b/tools/objtool/Build
deleted file mode 100644
index 749becd..0000000
--- a/tools/objtool/Build
+++ /dev/null
@@ -1,22 +0,0 @@
-objtool-y += arch/$(SRCARCH)/
-objtool-y += builtin-check.o
-objtool-y += builtin-orc.o
-objtool-y += check.o
-objtool-y += orc_gen.o
-objtool-y += orc_dump.o
-objtool-y += elf.o
-objtool-y += special.o
-objtool-y += objtool.o
-
-objtool-y += libstring.o
-objtool-y += str_error_r.o
-
-CFLAGS += -I$(srctree)/tools/lib
-
-$(OUTPUT)libstring.o: ../lib/string.c FORCE
-	$(call rule_mkdir)
-	$(call if_changed_dep,cc_o_c)
-
-$(OUTPUT)str_error_r.o: ../lib/str_error_r.c FORCE
-	$(call rule_mkdir)
-	$(call if_changed_dep,cc_o_c)
diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
deleted file mode 100644
index c9d038f..0000000
--- a/tools/objtool/Makefile
+++ /dev/null
@@ -1,63 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-include ../scripts/Makefile.include
-include ../scripts/Makefile.arch
-
-ifeq ($(ARCH),x86_64)
-ARCH := x86
-endif
-
-# always use the host compiler
-HOSTCC	?= gcc
-HOSTLD	?= ld
-CC	 = $(HOSTCC)
-LD	 = $(HOSTLD)
-AR	 = ar
-
-ifeq ($(srctree),)
-srctree := $(patsubst %/,%,$(dir $(CURDIR)))
-srctree := $(patsubst %/,%,$(dir $(srctree)))
-endif
-
-SUBCMD_SRCDIR		= $(srctree)/tools/lib/subcmd/
-LIBSUBCMD_OUTPUT	= $(if $(OUTPUT),$(OUTPUT),$(CURDIR)/)
-LIBSUBCMD		= $(LIBSUBCMD_OUTPUT)libsubcmd.a
-
-OBJTOOL    := $(OUTPUT)objtool
-OBJTOOL_IN := $(OBJTOOL)-in.o
-
-all: $(OBJTOOL)
-
-INCLUDES := -I$(srctree)/tools/include \
-	    -I$(srctree)/tools/arch/$(HOSTARCH)/include/uapi \
-	    -I$(srctree)/tools/objtool/arch/$(ARCH)/include
-WARNINGS := $(EXTRA_WARNINGS) -Wno-switch-default -Wno-switch-enum -Wno-packed
-CFLAGS   += -Werror $(WARNINGS) $(KBUILD_HOSTCFLAGS) -g $(INCLUDES)
-LDFLAGS  += -lelf $(LIBSUBCMD) $(KBUILD_HOSTLDFLAGS)
-
-# Allow old libelf to be used:
-elfshdr := $(shell echo '$(pound)include <libelf.h>' | $(CC) $(CFLAGS) -x c -E - | grep elf_getshdr)
-CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED)
-
-AWK = awk
-export srctree OUTPUT CFLAGS SRCARCH AWK
-include $(srctree)/tools/build/Makefile.include
-
-$(OBJTOOL_IN): fixdep FORCE
-	@$(MAKE) $(build)=objtool
-
-$(OBJTOOL): $(LIBSUBCMD) $(OBJTOOL_IN)
-	@$(CONFIG_SHELL) ./sync-check.sh
-	$(QUIET_LINK)$(CC) $(OBJTOOL_IN) $(LDFLAGS) -o $@
-
-
-$(LIBSUBCMD): fixdep FORCE
-	$(Q)$(MAKE) -C $(SUBCMD_SRCDIR) OUTPUT=$(LIBSUBCMD_OUTPUT)
-
-clean:
-	$(call QUIET_CLEAN, objtool) $(RM) $(OBJTOOL)
-	$(Q)find $(OUTPUT) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
-	$(Q)$(RM) $(OUTPUT)arch/x86/lib/inat-tables.c $(OUTPUT)fixdep
-
-FORCE:
-
-.PHONY: clean FORCE
diff --git a/tools/objtool/arch/x86/Build b/tools/objtool/arch/x86/Build
deleted file mode 100644
index b998412..0000000
--- a/tools/objtool/arch/x86/Build
+++ /dev/null
@@ -1,12 +0,0 @@
-objtool-y += decode.o
-
-inat_tables_script = arch/x86/tools/gen-insn-attr-x86.awk
-inat_tables_maps = arch/x86/lib/x86-opcode-map.txt
-
-$(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/lib/inat-tables.c
-
-CFLAGS_decode.o += -I$(OUTPUT)arch/x86/lib
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] objtool: move stack-validation.txt to Documentation/
  2019-03-05  5:48 ` [PATCH 2/3] objtool: move stack-validation.txt to Documentation/ Masahiro Yamada
@ 2019-03-05 14:44   ` Jonathan Corbet
  0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Corbet @ 2019-03-05 14:44 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Ingo Molnar, Josh Poimboeuf, Peter Zijlstra, Thomas Gleixner,
	Douglas Anderson, Robin Meijboom, Borislav Petkov,
	H . Peter Anvin, x86, Linus Torvalds, Sam Ravnborg, linux-kbuild,
	linux-doc, linux-kernel

On Tue,  5 Mar 2019 14:48:15 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> Move the document to the standard Documentation/ directory instead of
> creating the same directory structure under objtool/.

This seems like a good idea to me, but can I make a couple of requests?

 - Convert it to RST while you're at it?  It's 99% there already.

 - Let's not add it to the top-level Documentation/ mess; I'm slowly
   trying to bring some order there.  Perhaps a better home would be
   Documentation/kbuild/ ?

Thanks,

jon

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] objtool: move tools/objtool/ to scripts/objtool/
  2019-03-05  5:48 ` [PATCH 3/3] objtool: move tools/objtool/ to scripts/objtool/ Masahiro Yamada
@ 2019-03-05 14:46   ` Josh Poimboeuf
  2019-03-05 16:08     ` Masahiro Yamada
  0 siblings, 1 reply; 8+ messages in thread
From: Josh Poimboeuf @ 2019-03-05 14:46 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Ingo Molnar, Peter Zijlstra, Thomas Gleixner, Douglas Anderson,
	Robin Meijboom, Borislav Petkov, H . Peter Anvin, x86,
	Linus Torvalds, Sam Ravnborg, linux-kbuild, linux-kernel,
	Michal Marek

On Tue, Mar 05, 2019 at 02:48:16PM +0900, Masahiro Yamada wrote:
> Tools that are globally needed for building the kernel are supposed to
> be put in the scripts/ directory, but objtool is the exceptional one.
> 
> People were confused by objtool's Makefile since most of tools run on
> the target whereas objtool runs on the host machine. Some time ago,
> Douglas Anderson suggested to move tools/objtool/ to scripts/tools/
> (https://patchwork.kernel.org/patch/9983535/#20996057), but we did
> not go as far as submitting a patch at that time.
> 
> Recently, Robin Meijboom reported that the generated files for objtool
> were never cleaned up. (https://patchwork.kernel.org/patch/10813797/)
> 
> So, I looked into this to fix the distortion. Move tools/objtool/ to
> scripts/objtool/, and rewrite Makefiles in the Kbuild-ish manner.
> 
> I also cleaned up the top-level Makefile by moving the libelf check
> scripts/objtool/Makefile.
>
> I see some benefits in this.
> 
> [1] generated files under scripts/ are cleaned up by 'make mrproper'
> 
> Currently, the generated files under tools/objtool/ are not cleaned up
> since the build system under tools/ is a complete different world.
> 
> If we want to clean them up, it should be done by 'make mrproper'
> instead of 'make clean' since objtool is needed for building external
> modules.
> 
> This commit naturally solves the issue since Kbuild cleans under
> scripts/ by 'make mrproper'.

Yes, but this can be solved instead with a one-line patch.

> [2] build log looks nicer
> 
> Currently, the build log under tools/objtool/ shows absolute paths, and
> displays 'DESCEND  objtool' every time for incremental building.
> 
> Switching over to the standard Kbuild scheme will improve the log.
> 
>   Before:
> 
>   DESCEND  objtool
>   HOSTCC   /home/masahiro/workspace/linux/tools/objtool/fixdep.o
>   HOSTLD   /home/masahiro/workspace/linux/tools/objtool/fixdep-in.o
>   LINK     /home/masahiro/workspace/linux/tools/objtool/fixdep
>   CC       /home/masahiro/workspace/linux/tools/objtool/exec-cmd.o
>   CC       /home/masahiro/workspace/linux/tools/objtool/help.o
>   CC       /home/masahiro/workspace/linux/tools/objtool/pager.o
>   CC       /home/masahiro/workspace/linux/tools/objtool/parse-options.o
>   CC       /home/masahiro/workspace/linux/tools/objtool/run-command.o
>   CC       /home/masahiro/workspace/linux/tools/objtool/sigchain.o
>   CC       /home/masahiro/workspace/linux/tools/objtool/subcmd-config.o
>   LD       /home/masahiro/workspace/linux/tools/objtool/libsubcmd-in.o
>   AR       /home/masahiro/workspace/linux/tools/objtool/libsubcmd.a
>   GEN      /home/masahiro/workspace/linux/tools/objtool/arch/x86/lib/inat-tables.c
>   CC       /home/masahiro/workspace/linux/tools/objtool/arch/x86/decode.o
>   LD       /home/masahiro/workspace/linux/tools/objtool/arch/x86/objtool-in.o
>   CC       /home/masahiro/workspace/linux/tools/objtool/builtin-check.o
>   CC       /home/masahiro/workspace/linux/tools/objtool/builtin-orc.o
>   CC       /home/masahiro/workspace/linux/tools/objtool/check.o
>   CC       /home/masahiro/workspace/linux/tools/objtool/orc_gen.o
>   CC       /home/masahiro/workspace/linux/tools/objtool/orc_dump.o
>   CC       /home/masahiro/workspace/linux/tools/objtool/elf.o
>   CC       /home/masahiro/workspace/linux/tools/objtool/special.o
>   CC       /home/masahiro/workspace/linux/tools/objtool/objtool.o
>   CC       /home/masahiro/workspace/linux/tools/objtool/libstring.o
>   CC       /home/masahiro/workspace/linux/tools/objtool/str_error_r.o
>   LD       /home/masahiro/workspace/linux/tools/objtool/objtool-in.o
>   LINK     /home/masahiro/workspace/linux/tools/objtool/objtool
> 
>   After:
> 
>   HOSTCC  scripts/objtool/builtin-check.o
>   HOSTCC  scripts/objtool/builtin-orc.o
>   HOSTCC  scripts/objtool/check.o
>   HOSTCC  scripts/objtool/orc_gen.o
>   HOSTCC  scripts/objtool/orc_dump.o
>   HOSTCC  scripts/objtool/elf.o
>   HOSTCC  scripts/objtool/special.o
>   HOSTCC  scripts/objtool/objtool.o
>   HOSTCC  scripts/objtool/libstring.o
>   HOSTCC  scripts/objtool/str_error_r.o
>   HOSTCC  scripts/objtool/exec-cmd.o
>   HOSTCC  scripts/objtool/pager.o
>   HOSTCC  scripts/objtool/parse-options.o
>   HOSTCC  scripts/objtool/run-command.o
>   HOSTCC  scripts/objtool/sigchain.o
>   HOSTCC  scripts/objtool/subcmd-config.o
>   AWK     scripts/objtool/arch/x86/lib/inat-tables.c
>   HOSTCC  scripts/objtool/arch/x86/decode.o
>   HOSTLD  scripts/objtool/objtool

That does look better.

> [3] simplify distro package script
> 
> The special handling in scripts/package/buildeb is no longer needed
> since all host programs are copied to linux-headers packages.

I do see the benefits.  In many ways objtool is a more natural fit in
the scripts dir.  But there are also some downsides to this change:

- This will make it harder to package objtool as a standalone tool.  It
  has a lot of functionality that could be useful to other non-kernel
  projects.

- It shares libsubcmd with perf.  Including the subcmd .c files from
  tools is hacky.

- It's disruptive: it will break all the out-of-tree distro kernel
  packaging scripts which now look for objtool in tools.

You're right that objtool isn't a natural fit in tools, because it's
also used as part of the build.  But it's not a natural fit in scripts
either.  I think we've resolved most of those issues and it seems to be
working well these days.

So instead of disrupting everything because "make mrproper" doesn't
work, I think I'd rather just do the following:

diff --git a/Makefile b/Makefile
index ac5ac28a24e9..7e6696c9b862 100644
--- a/Makefile
+++ b/Makefile
@@ -1364,7 +1364,7 @@ PHONY += $(mrproper-dirs) mrproper
 $(mrproper-dirs):
 	$(Q)$(MAKE) $(clean)=$(patsubst _mrproper_%,%,$@)
 
-mrproper: clean $(mrproper-dirs)
+mrproper: clean $(mrproper-dirs) tools/objtool_clean
 	$(call cmd,rmdirs)
 	$(call cmd,rmfiles)
 

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] objtool: move tools/objtool/ to scripts/objtool/
  2019-03-05 14:46   ` Josh Poimboeuf
@ 2019-03-05 16:08     ` Masahiro Yamada
  2019-03-05 16:36       ` Josh Poimboeuf
  0 siblings, 1 reply; 8+ messages in thread
From: Masahiro Yamada @ 2019-03-05 16:08 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: Ingo Molnar, Peter Zijlstra, Thomas Gleixner, Douglas Anderson,
	Robin Meijboom, Borislav Petkov, H . Peter Anvin, X86 ML,
	Linus Torvalds, Sam Ravnborg, Linux Kbuild mailing list,
	Linux Kernel Mailing List, Michal Marek

On Tue, Mar 5, 2019 at 11:46 PM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> On Tue, Mar 05, 2019 at 02:48:16PM +0900, Masahiro Yamada wrote:
> > Tools that are globally needed for building the kernel are supposed to
> > be put in the scripts/ directory, but objtool is the exceptional one.
> >
> > People were confused by objtool's Makefile since most of tools run on
> > the target whereas objtool runs on the host machine. Some time ago,
> > Douglas Anderson suggested to move tools/objtool/ to scripts/tools/
> > (https://patchwork.kernel.org/patch/9983535/#20996057), but we did
> > not go as far as submitting a patch at that time.
> >
> > Recently, Robin Meijboom reported that the generated files for objtool
> > were never cleaned up. (https://patchwork.kernel.org/patch/10813797/)
> >
> > So, I looked into this to fix the distortion. Move tools/objtool/ to
> > scripts/objtool/, and rewrite Makefiles in the Kbuild-ish manner.
> >
> > I also cleaned up the top-level Makefile by moving the libelf check
> > scripts/objtool/Makefile.
> >
> > I see some benefits in this.
> >
> > [1] generated files under scripts/ are cleaned up by 'make mrproper'
> >
> > Currently, the generated files under tools/objtool/ are not cleaned up
> > since the build system under tools/ is a complete different world.
> >
> > If we want to clean them up, it should be done by 'make mrproper'
> > instead of 'make clean' since objtool is needed for building external
> > modules.
> >
> > This commit naturally solves the issue since Kbuild cleans under
> > scripts/ by 'make mrproper'.
>
> Yes, but this can be solved instead with a one-line patch.
>
> > [2] build log looks nicer
> >
> > Currently, the build log under tools/objtool/ shows absolute paths, and
> > displays 'DESCEND  objtool' every time for incremental building.
> >
> > Switching over to the standard Kbuild scheme will improve the log.
> >
> >   Before:
> >
> >   DESCEND  objtool
> >   HOSTCC   /home/masahiro/workspace/linux/tools/objtool/fixdep.o
> >   HOSTLD   /home/masahiro/workspace/linux/tools/objtool/fixdep-in.o
> >   LINK     /home/masahiro/workspace/linux/tools/objtool/fixdep
> >   CC       /home/masahiro/workspace/linux/tools/objtool/exec-cmd.o
> >   CC       /home/masahiro/workspace/linux/tools/objtool/help.o
> >   CC       /home/masahiro/workspace/linux/tools/objtool/pager.o
> >   CC       /home/masahiro/workspace/linux/tools/objtool/parse-options.o
> >   CC       /home/masahiro/workspace/linux/tools/objtool/run-command.o
> >   CC       /home/masahiro/workspace/linux/tools/objtool/sigchain.o
> >   CC       /home/masahiro/workspace/linux/tools/objtool/subcmd-config.o
> >   LD       /home/masahiro/workspace/linux/tools/objtool/libsubcmd-in.o
> >   AR       /home/masahiro/workspace/linux/tools/objtool/libsubcmd.a
> >   GEN      /home/masahiro/workspace/linux/tools/objtool/arch/x86/lib/inat-tables.c
> >   CC       /home/masahiro/workspace/linux/tools/objtool/arch/x86/decode.o
> >   LD       /home/masahiro/workspace/linux/tools/objtool/arch/x86/objtool-in.o
> >   CC       /home/masahiro/workspace/linux/tools/objtool/builtin-check.o
> >   CC       /home/masahiro/workspace/linux/tools/objtool/builtin-orc.o
> >   CC       /home/masahiro/workspace/linux/tools/objtool/check.o
> >   CC       /home/masahiro/workspace/linux/tools/objtool/orc_gen.o
> >   CC       /home/masahiro/workspace/linux/tools/objtool/orc_dump.o
> >   CC       /home/masahiro/workspace/linux/tools/objtool/elf.o
> >   CC       /home/masahiro/workspace/linux/tools/objtool/special.o
> >   CC       /home/masahiro/workspace/linux/tools/objtool/objtool.o
> >   CC       /home/masahiro/workspace/linux/tools/objtool/libstring.o
> >   CC       /home/masahiro/workspace/linux/tools/objtool/str_error_r.o
> >   LD       /home/masahiro/workspace/linux/tools/objtool/objtool-in.o
> >   LINK     /home/masahiro/workspace/linux/tools/objtool/objtool
> >
> >   After:
> >
> >   HOSTCC  scripts/objtool/builtin-check.o
> >   HOSTCC  scripts/objtool/builtin-orc.o
> >   HOSTCC  scripts/objtool/check.o
> >   HOSTCC  scripts/objtool/orc_gen.o
> >   HOSTCC  scripts/objtool/orc_dump.o
> >   HOSTCC  scripts/objtool/elf.o
> >   HOSTCC  scripts/objtool/special.o
> >   HOSTCC  scripts/objtool/objtool.o
> >   HOSTCC  scripts/objtool/libstring.o
> >   HOSTCC  scripts/objtool/str_error_r.o
> >   HOSTCC  scripts/objtool/exec-cmd.o
> >   HOSTCC  scripts/objtool/pager.o
> >   HOSTCC  scripts/objtool/parse-options.o
> >   HOSTCC  scripts/objtool/run-command.o
> >   HOSTCC  scripts/objtool/sigchain.o
> >   HOSTCC  scripts/objtool/subcmd-config.o
> >   AWK     scripts/objtool/arch/x86/lib/inat-tables.c
> >   HOSTCC  scripts/objtool/arch/x86/decode.o
> >   HOSTLD  scripts/objtool/objtool
>
> That does look better.
>
> > [3] simplify distro package script
> >
> > The special handling in scripts/package/buildeb is no longer needed
> > since all host programs are copied to linux-headers packages.
>
> I do see the benefits.  In many ways objtool is a more natural fit in
> the scripts dir.  But there are also some downsides to this change:
>
> - This will make it harder to package objtool as a standalone tool.  It
>   has a lot of functionality that could be useful to other non-kernel
>   projects.


If it is really useful for other projects,
I'd like to see it as a real standalone tool,
i.e. split as a separate project.


> - It shares libsubcmd with perf.  Including the subcmd .c files from
>   tools is hacky.

Yes, hacky. We are shifting the ugliness between C and Build system.

But, this hack can be solved; if subcmd library is useful, you can copy
only necessary code into the objtool directory. You do not need all
helpers from libsubcmd.


> - It's disruptive: it will break all the out-of-tree distro kernel
>   packaging scripts which now look for objtool in tools.

All artifacts under scripts/ should be contained in the package.
So, it should work.


> You're right that objtool isn't a natural fit in tools, because it's
> also used as part of the build.  But it's not a natural fit in scripts
> either.  I think we've resolved most of those issues and it seems to be
> working well these days.
>
> So instead of disrupting everything because "make mrproper" doesn't
> work, I think I'd rather just do the following:
>
> diff --git a/Makefile b/Makefile
> index ac5ac28a24e9..7e6696c9b862 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1364,7 +1364,7 @@ PHONY += $(mrproper-dirs) mrproper
>  $(mrproper-dirs):
>         $(Q)$(MAKE) $(clean)=$(patsubst _mrproper_%,%,$@)
>
> -mrproper: clean $(mrproper-dirs)
> +mrproper: clean $(mrproper-dirs) tools/objtool_clean
>         $(call cmd,rmdirs)
>         $(call cmd,rmfiles)
>

Works, and the clean log is also brilliant.

I see the same log even when
there is nothing to clean.


masahiro@grover:~/ref/linux$ make mrproper
  CLEAN   .
  CLEAN   arch/x86/entry/vdso
  CLEAN   usr
  CLEAN   arch/x86/tools
  CLEAN   .tmp_versions
  CLEAN   scripts/basic
  CLEAN   scripts/kconfig
  CLEAN   scripts/mod
  CLEAN   scripts/selinux/genheaders
  CLEAN   scripts/selinux/mdp
  CLEAN   scripts
  DESCEND  objtool
  CLEAN    objtool
  CLEAN   include/config include/generated arch/x86/include/generated
  CLEAN   .config
masahiro@grover:~/ref/linux$ make mrproper
  DESCEND  objtool
  CLEAN    objtool
masahiro@grover:~/ref/linux$ make mrproper
  DESCEND  objtool
  CLEAN    objtool



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] objtool: move tools/objtool/ to scripts/objtool/
  2019-03-05 16:08     ` Masahiro Yamada
@ 2019-03-05 16:36       ` Josh Poimboeuf
  2019-03-05 16:59         ` Peter Zijlstra
  0 siblings, 1 reply; 8+ messages in thread
From: Josh Poimboeuf @ 2019-03-05 16:36 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Ingo Molnar, Peter Zijlstra, Thomas Gleixner, Douglas Anderson,
	Robin Meijboom, Borislav Petkov, H . Peter Anvin, X86 ML,
	Linus Torvalds, Sam Ravnborg, Linux Kbuild mailing list,
	Linux Kernel Mailing List, Michal Marek

On Wed, Mar 06, 2019 at 01:08:27AM +0900, Masahiro Yamada wrote:
> > I do see the benefits.  In many ways objtool is a more natural fit in
> > the scripts dir.  But there are also some downsides to this change:
> >
> > - This will make it harder to package objtool as a standalone tool.  It
> >   has a lot of functionality that could be useful to other non-kernel
> >   projects.
> 
> 
> If it is really useful for other projects,
> I'd like to see it as a real standalone tool,
> i.e. split as a separate project.

Fine, but who's going to do it?  I don't have time for creating such a
project anytime soon.  And anyway, moving the code to scripts in the
meantime would just make that harder when we do it eventually.

> > - It shares libsubcmd with perf.  Including the subcmd .c files from
> >   tools is hacky.
> 
> Yes, hacky. We are shifting the ugliness between C and Build system.
> 
> But, this hack can be solved; if subcmd library is useful, you can copy
> only necessary code into the objtool directory. You do not need all
> helpers from libsubcmd.

But then we are duplicating code...

> > - It's disruptive: it will break all the out-of-tree distro kernel
> >   packaging scripts which now look for objtool in tools.
> 
> All artifacts under scripts/ should be contained in the package.
> So, it should work.

It *will* break distro scripts which already expect objtool to be
located at tools/objtool/objtool.

> > You're right that objtool isn't a natural fit in tools, because it's
> > also used as part of the build.  But it's not a natural fit in scripts
> > either.  I think we've resolved most of those issues and it seems to be
> > working well these days.
> >
> > So instead of disrupting everything because "make mrproper" doesn't
> > work, I think I'd rather just do the following:
> >
> > diff --git a/Makefile b/Makefile
> > index ac5ac28a24e9..7e6696c9b862 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1364,7 +1364,7 @@ PHONY += $(mrproper-dirs) mrproper
> >  $(mrproper-dirs):
> >         $(Q)$(MAKE) $(clean)=$(patsubst _mrproper_%,%,$@)
> >
> > -mrproper: clean $(mrproper-dirs)
> > +mrproper: clean $(mrproper-dirs) tools/objtool_clean
> >         $(call cmd,rmdirs)
> >         $(call cmd,rmfiles)
> >
> 
> Works, and the clean log is also brilliant.
> 
> I see the same log even when
> there is nothing to clean.
> 
> 
> masahiro@grover:~/ref/linux$ make mrproper
>   CLEAN   .
>   CLEAN   arch/x86/entry/vdso
>   CLEAN   usr
>   CLEAN   arch/x86/tools
>   CLEAN   .tmp_versions
>   CLEAN   scripts/basic
>   CLEAN   scripts/kconfig
>   CLEAN   scripts/mod
>   CLEAN   scripts/selinux/genheaders
>   CLEAN   scripts/selinux/mdp
>   CLEAN   scripts
>   DESCEND  objtool
>   CLEAN    objtool
>   CLEAN   include/config include/generated arch/x86/include/generated
>   CLEAN   .config
> masahiro@grover:~/ref/linux$ make mrproper
>   DESCEND  objtool
>   CLEAN    objtool
> masahiro@grover:~/ref/linux$ make mrproper
>   DESCEND  objtool
>   CLEAN    objtool

Ok, but I'm sure that could be fixed.  That's hardly a justification to
move objtool.

Anyway I would like to hear what Ingo and Peter think about moving it.

-- 
Josh

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] objtool: move tools/objtool/ to scripts/objtool/
  2019-03-05 16:36       ` Josh Poimboeuf
@ 2019-03-05 16:59         ` Peter Zijlstra
  0 siblings, 0 replies; 8+ messages in thread
From: Peter Zijlstra @ 2019-03-05 16:59 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: Masahiro Yamada, Ingo Molnar, Thomas Gleixner, Douglas Anderson,
	Robin Meijboom, Borislav Petkov, H . Peter Anvin, X86 ML,
	Linus Torvalds, Sam Ravnborg, Linux Kbuild mailing list,
	Linux Kernel Mailing List, Michal Marek

On Tue, Mar 05, 2019 at 10:36:09AM -0600, Josh Poimboeuf wrote:
> Anyway I would like to hear what Ingo and Peter think about moving it.

Pain :-)

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-03-05 17:00 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-05  5:48 [PATCH 1/3] tools: move initial declarations out of 'for' loop Masahiro Yamada
2019-03-05  5:48 ` [PATCH 2/3] objtool: move stack-validation.txt to Documentation/ Masahiro Yamada
2019-03-05 14:44   ` Jonathan Corbet
2019-03-05  5:48 ` [PATCH 3/3] objtool: move tools/objtool/ to scripts/objtool/ Masahiro Yamada
2019-03-05 14:46   ` Josh Poimboeuf
2019-03-05 16:08     ` Masahiro Yamada
2019-03-05 16:36       ` Josh Poimboeuf
2019-03-05 16:59         ` Peter Zijlstra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).