Re: [RFC][PATCH 4/5] objtool: Enable compilation of objtool for all architectures

[Julien Thierry]

On 5/13/20 4:59 PM, Matt Helsley wrote:
> On Tue, May 12, 2020 at 06:04:56PM +0100, Julien Thierry wrote:
> > Hi Matt,
> >
> > On 5/11/20 6:35 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 a "missing" architecture which contains weak definitions
> > > for tools that do not exist on all architectures. In this case the
> > > "check" and "orc" tools do not exist on all architectures.
> > >
> > > To test building for other architectures ($arch below):
> > >
> > > 	cd tools/objtool/arch
> > > 	ln -s missing $arch
> > > 	make O=build-$arch ARCH=$arch tools/objtool
> >
> > Since the stuff under arch/missing is only weak symbols to make up for
> > missing subcmd implementations, can we put everything in a file
> > subcmd_defaults.c (name up for debate!) that would be always be compiled an
> > linked. And some SUBCMD_XXX is set to "y", the corresponding object file
> > gets compiled and overrides the weak symbols from subcmd_defaults.c .
>
> Hmm, I like keeping them separated along similar lines to the other
> code because it makes it easier to see the intended correspondence and
> likely will keep the files more readable / smaller. I could
> just move them out of arch/missing and into missing_check.c and so forth.
>
> What do you think of that?

I do prefer that to the introduction of an arch/missing.

Still, I'm not sure I see much benefit in splitting those small 
implementations in separate files, but it's not a problem either. This 
seems more a matter of taste rather than one approach working better 
than the other. So it's more up to what the maintainer prefer! :)

> > > diff --git a/tools/objtool/Build b/tools/objtool/Build
> > > index 66f44f5cd2a6..fb6e6faf6f10 100644
> > > --- a/tools/objtool/Build
> > > +++ b/tools/objtool/Build
> > > @@ -1,11 +1,12 @@
> > >    objtool-y += arch/$(SRCARCH)/
> > > +
> > > +objtool-$(SUBCMD_CHECK) += 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
> >
> > I'm not convinced by the moving of special under arch/x86 and I didn't
> > understand it at first.
> >
> > I guess you did it because it is only used by the check subcmd, which is
> > currently only implemented by x86. Is that the reason?
>
> Yeah, that was the original reasoning and this is an artifact of the
> previous patch set.
>   
> > I feel that the proper way to do it would be to leave special.c/h where they
> > are and have "objtool-$(SUBCMD_CHECK) += special.o". Unless there was some
> > other motivation for it.
>
> This makes sense. I'll incorporate that in the next posting.

Great, thanks!

--
Julien Thierry