Re: [PATCH v6 03/30] objtool: Disassemble code with libopcodes instead of running objdump

[Alexandre Chartre <alexandre.chartre@oracle.com>]

On 11/26/25 11:07, David Laight wrote:
> On Wed, 26 Nov 2025 10:00:36 +0100
> Alexandre Chartre <alexandre.chartre@oracle.com> wrote:
> 
>> On 11/25/25 19:16, Nathan Chancellor wrote:
>>> Hi Alexandre,
>>>
>>> On Fri, Nov 21, 2025 at 10:53:13AM +0100, Alexandre Chartre wrote:
>>>> objtool executes the objdump command to disassemble code. Use libopcodes
>>>> instead to have more control about the disassembly scope and output.
>>>> If libopcodes is not present then objtool is built without disassembly
>>>> support.
>>>>
>>>> Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
>>> ...
>>>> diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/include/objtool/arch.h
>>>> index d89f8b5ec14e3..18c0e69ee6170 100644
>>>> --- a/tools/objtool/include/objtool/arch.h
>>>> +++ b/tools/objtool/include/objtool/arch.h
>>>> @@ -103,4 +103,13 @@ bool arch_absolute_reloc(struct elf *elf, struct reloc *reloc);
>>>>    unsigned int arch_reloc_size(struct reloc *reloc);
>>>>    unsigned long arch_jump_table_sym_offset(struct reloc *reloc, struct reloc *table);
>>>>    
>>>> +#ifdef DISAS
>>>> +
>>>> +#include <bfd.h>
>>>
>>> This include of bfd.h breaks the build for me:
>>>
>>>     $ make -skj"$(nproc)" ARCH=x86_64 CROSS_COMPILE=x86_64-linux- clean defconfig bzImage
>>>     In file included from tools/objtool/include/objtool/arch.h:108,
>>>                      from check.c:14:
>>>     /usr/include/bfd.h:35:2: error: #error config.h must be included before this header
>>>        35 | #error config.h must be included before this header
>>>           |  ^~~~~
>>>     ...
>>>
>>> where my bfd.h has:
>>>
>>>     #ifndef __BFD_H_SEEN__
>>>     #define __BFD_H_SEEN__
>>>
>>>     /* PR 14072: Ensure that config.h is included first.  */
>>>     #if !defined PACKAGE && !defined PACKAGE_VERSION
>>>     #error config.h must be included before this header
>>>     #endif
>>
>> This check is effectively present in the bfd.h file generated from the
>> binutils source code. However it is not present in the bfd.h file provided
>> by the binutils RPM. I think this explained why we haven't seen this issue
>> so far.
>>
>> For history, this was introduced in 2012 for bug 14072. Then there was
>> complaints reported in bug 14243 and 15920. But it was decided not to
>> remove this change, and the suggested fix was to define PACKAGE when
>> including bfd.h.
>>
>> https://sourceware.org/bugzilla/show_bug.cgi?id=14072
>> https://sourceware.org/bugzilla/show_bug.cgi?id=14243
>> https://sourceware.org/bugzilla/show_bug.cgi?id=15920
>>
>> And Redhat has fixed the issue for the binutils RPM by removing this test:
>>
>> https://sourceware.org/bugzilla/show_bug.cgi?id=14243
>>
>>
>>> Something like this cures it for me but I am not sure if that is a
>>> proper fix or not since I see config.h in my binutils build folder has
>>> many other defines.
>>>
>>> Cheers,
>>> Nathan
>>>
>>> diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
>>> index df793ca6fc1a..96df4a73da23 100644
>>> --- a/tools/objtool/Makefile
>>> +++ b/tools/objtool/Makefile
>>> @@ -87,7 +87,7 @@ BUILD_DISAS := n
>>>    
>>>    ifeq ($(feature-libbfd),1)
>>>    	BUILD_DISAS := y
>>> -	OBJTOOL_CFLAGS += -DDISAS
>>> +	OBJTOOL_CFLAGS += -DDISAS -DPACKAGE="objtool-disas"
>>>    	OBJTOOL_LDFLAGS += -lopcodes
>>>    endif
>>>      
>>
>> This is the proper fix (as indicated in the binutils bugs), and this is
>> what the other kernel tools using bfd.h (bpf and perf) do. I will create
>> a patch with your suggestion.
> 
> ISTM that defining it just before including bfd.h is cleaner and more obvious.
> 

bfd.h is included at two different places: in arch.h and disas.c (and disas.c
includes arch.h). So we would need to ensure that PACKAGE always has the same
definition otherwise we will have a redefine error.

So I think it is simpler to have a global definition with -DPACKAGE.
Similarly, bpf has multiple includes and uses -DPACKAGE=bpf, while perf has
a single include and use a single define before the include.

alex.