Re: [PATCH 3/3] objtool: Enable compilation of objtool for all architectures

[Julien Thierry <jthierry@redhat.com>]

On 5/19/20 9:55 PM, Matt Helsley wrote:
> objtool currently only compiles for x86 architectures. This is
> fine as it presently does not support tooling for other
> architectures. However, we would like to be able to convert other
> kernel tools to run as objtool sub commands because they too
> process ELF object files. This will allow us to convert tools
> such as recordmcount to use objtool's ELF code.
> 
> Since much of recordmcount's ELF code is copy-paste code to/from
> a variety of other kernel tools (look at modpost for example) this
> means that if we can convert recordmcount we can convert more.
> 
> We define "missing" weak definitions for subcommand entry functions
> and other weak definitions for shared functions critical to
> building existing subcommands. These return 127 when the command is
> missing which signify tools that do not exist on all architectures.
> In this case the "check" and "orc" tools do not exist on all
> architectures so we only add them for x86. Future changes adding
> support for "check", to arm64 for example, can then modify the
> SUBCMD_CHECK variable when building for arm64.
> 
> objtool is not currently wired in to KConfig to be built for other
> architectures because it's not needed for those architectures and
> there are no commands it supports other than those for x86. As more
> command support is enabled on various architectures the necessary
> KConfig changes can be made (e.g. adding "STACK_VALIDATION") to
> trigger building objtool.
> 
> Signed-off-by: Matt Helsley <mhelsley@vmware.com>
> Cc: Julien Thierry <jthierry@redhat.com>
> ---
>   tools/objtool/Build           | 13 +++++++++----
>   tools/objtool/Makefile        | 11 ++++++++++-
>   tools/objtool/arch.h          |  4 +++-
>   tools/objtool/builtin-check.c |  2 +-
>   tools/objtool/builtin-orc.c   |  3 +--
>   tools/objtool/check.c         |  4 ++--
>   tools/objtool/check.h         |  4 ----
>   tools/objtool/objtool.h       | 14 ++++++++++++++
>   tools/objtool/orc.h           | 18 ------------------
>   tools/objtool/orc_dump.c      |  3 ++-
>   tools/objtool/orc_gen.c       |  1 -
>   tools/objtool/weak.c          | 35 +++++++++++++++++++++++++++++++++++
>   12 files changed, 77 insertions(+), 35 deletions(-)
>   delete mode 100644 tools/objtool/orc.h
>   create mode 100644 tools/objtool/weak.c
> 
> diff --git a/tools/objtool/Build b/tools/objtool/Build
> index 66f44f5cd2a6..b7222d5cc7bc 100644
> --- a/tools/objtool/Build
> +++ b/tools/objtool/Build
> @@ -1,11 +1,16 @@
>   objtool-y += arch/$(SRCARCH)/
> +
> +objtool-y += weak.o
> +
> +objtool-$(SUBCMD_CHECK) += check.o
> +objtool-$(SUBCMD_CHECK) += special.o
> +objtool-$(SUBCMD_ORC) += check.o
> +objtool-$(SUBCMD_ORC) += orc_gen.o
> +objtool-$(SUBCMD_ORC) += orc_dump.o
> +
>   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
> diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
> index 6b91388aecbb..12686e2f1a56 100644
> --- a/tools/objtool/Makefile
> +++ b/tools/objtool/Makefile
> @@ -46,7 +46,16 @@ elfshdr := $(shell echo '$(pound)include <libelf.h>' | $(CC) $(CFLAGS) -x c -E -
>   CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED)
>   
>   AWK = awk
> -export srctree OUTPUT CFLAGS SRCARCH AWK
> +
> +SUBCMD_CHECK := n
> +SUBCMD_ORC := n
> +
> +ifeq ($(SRCARCH),x86)
> +	SUBCMD_CHECK := y
> +	SUBCMD_ORC := y
> +endif
> +
> +export srctree OUTPUT CFLAGS SRCARCH AWK SUBCMD_CHECK SUBCMD_ORC

Nit: I was thinking, since the list of SUBCMD_* is only going to grow
maybe it would be nicer to have a single export line for the SUBCMD_* 
variables and leave the export line of [srctree..AWK] untouched.

Just a suggestion, and only in case you respin this taking into account 
Josh's comment.

Otherwise things look good to me.

Cheers,

-- 
Julien Thierry