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

[Julien Thierry <jthierry@redhat.com>]

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