Re: [PATCH 4/4] kbuild: factor out the common objtool arguments

From: Masahiro Yamada <masahiroy@kernel.org>
To: Sedat Dilek <sedat.dilek@gmail.com>
Cc: Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Michal Marek <michal.lkml@markovi.net>
Subject: Re: [PATCH 4/4] kbuild: factor out the common objtool arguments
Date: Mon, 30 May 2022 02:34:20 +0900	[thread overview]
Message-ID: <CAK7LNATwgT-FQ99Ex6MOc+rYEe7tc8ffoF-5e12jfbeaEM1vhw@mail.gmail.com> (raw)
In-Reply-To: <CA+icZUWkOrWYSY3ixxfF=vsuq1xw3mU+p3NMqBpY0OpM02916g@mail.gmail.com>

On Sun, May 29, 2022 at 11:47 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Sat, May 28, 2022 at 9:45 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > scripts/Makefile.build and scripts/link-vmlinux.sh have similar setups
> > for the objtool arguments.
> >
> > It is difficult to factor out them because the vmlinux build rule is
> > written in a shell script. It is somewhat tedious to touch the two
> > files every time a new objtool option is supported.
> >
> > To reduce the code duplication, implement everything about objtool in
> > Makefile.
> >
> > Move the objtool for vmlinux.o into scripts/Makefile.vmlinux_o.
> >
> > Move the common macros to Makefile.lib so they are shared by
> > Makefile.build and Makefile.vmlinux_o.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
>
> With some comments (see below).
>
> Tested-by: Sedat Dilek <sedat.dilek@gmail.com> # LLVM-14 (x86-64)
>
> -Sedat-
>
> > ---
> >
> >  scripts/Makefile.build     | 26 --------------
> >  scripts/Makefile.lib       | 26 ++++++++++++++
> >  scripts/Makefile.vmlinux_o | 26 ++++++++++++++
> >  scripts/link-vmlinux.sh    | 71 --------------------------------------
> >  4 files changed, 52 insertions(+), 97 deletions(-)
> >
> > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > index 4cb7145071b9..1f01ac65c0cd 100644
> > --- a/scripts/Makefile.build
> > +++ b/scripts/Makefile.build
> > @@ -210,38 +210,12 @@ cmd_record_mcount = $(if $(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags)),
> >         $(sub_cmd_record_mcount))
> >  endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
> >
> > -ifdef CONFIG_OBJTOOL
> > -
> > -objtool := $(objtree)/tools/objtool/objtool
> > -
> > -objtool_args =                                                         \
> > -       $(if $(CONFIG_HAVE_JUMP_LABEL_HACK), --hacks=jump_label)        \
> > -       $(if $(CONFIG_HAVE_NOINSTR_HACK), --hacks=noinstr)              \
> > -       $(if $(CONFIG_X86_KERNEL_IBT), --ibt)                           \
> > -       $(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount)             \
> > -       $(if $(CONFIG_UNWINDER_ORC), --orc)                             \
> > -       $(if $(CONFIG_RETPOLINE), --retpoline)                          \
> > -       $(if $(CONFIG_SLS), --sls)                                      \
> > -       $(if $(CONFIG_STACK_VALIDATION), --stackval)                    \
> > -       $(if $(CONFIG_HAVE_STATIC_CALL_INLINE), --static-call)          \
> > -       --uaccess                                                       \
> > -       $(if $(delay-objtool), --link)                                  \
> > -       $(if $(part-of-module), --module)                               \
> > -       $(if $(CONFIG_GCOV_KERNEL), --no-unreachable)
> > -
> > -cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
> > -cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
> > -
> > -endif # CONFIG_OBJTOOL
> > -
> >  # 'OBJECT_FILES_NON_STANDARD := y': skip objtool checking for a directory
> >  # 'OBJECT_FILES_NON_STANDARD_foo.o := 'y': skip objtool checking for a file
> >  # 'OBJECT_FILES_NON_STANDARD_foo.o := 'n': override directory skip for a file
> >
>
> ^^ What is with this block?
> If this belongs together with objtool - shall this be moved, too?

No.
These are unneeded for vmlinux.o

> > -       fi
> > -}
> > -
> >  # Link of vmlinux
> >  # ${1} - output file
> >  # ${2}, ${3}, ... - optional extra .o files
> > @@ -298,7 +228,6 @@ ${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init need-builtin=1
> >
> >  #link vmlinux.o
>
> ^^ While you are at it, change the comment to "# Link of vmlinux.o".

In my plan, this code will be gone sooner or later.

It would be a noise.

-- 
Best Regards
Masahiro Yamada