Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de>
In-Reply-To: <20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de>
From:   Quentin Monnet <quentin@isovalent.com>
Date:   Wed, 22 Jun 2022 23:53:58 +0100
Message-ID: <CACdoK4LeRPkACejq87VLFgP-b=y1ZoRX3196f7xEVo-UWm8jWA@mail.gmail.com>
Subject: Re: init_disassemble_info() signature changes causes compile failures
To:     Andres Freund <andres@anarazel.de>
Cc:     open list <linux-kernel@vger.kernel.org>,
        bpf <bpf@vger.kernel.org>,
        Arnaldo Carvalho de Melo <acme@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, 22 Jun 2022 at 19:19, Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> binutils changed the signature of init_disassemble_info(), which now caus=
es
> perf and bpftool to fail to compile (e.g. on debian unstable).
>
> Relevant binutils commit: https://sourceware.org/git/?p=3Dbinutils-gdb.gi=
t;a=3Dcommitdiff;h=3D60a3da00bd5407f07d64dff82a4dae98230dfaac
>
> util/annotate.c: In function =E2=80=98symbol__disassemble_bpf=E2=80=99:
> util/annotate.c:1765:9: error: too few arguments to function =E2=80=98ini=
t_disassemble_info=E2=80=99
>  1765 |         init_disassemble_info(&info, s,
>       |         ^~~~~~~~~~~~~~~~~~~~~
> In file included from util/annotate.c:1718:
> /usr/include/dis-asm.h:472:13: note: declared here
>   472 | extern void init_disassemble_info (struct disassemble_info *dinfo=
, void *stream,
>       |             ^~~~~~~~~~~~~~~~~~~~~
>
> with equivalent failures in
>
> tools/bpf/bpf_jit_disasm.c
> tools/bpf/bpftool/jit_disasm.c

Hi Andres,

Too bad the libbfd API is changing again :/ But many thanks for
pinning down the relevant commit, and for the report!

> The fix is easy enough, add a wrapper around fprintf() that conforms to t=
he
> new signature.
>
> However I assume the necessary feature test and wrapper should only be ad=
ded
> once? I don't know the kernel stuff well enough to choose the right struc=
ture
> here.

We can probably find a common header for the wrapper under
tools/include/. One possibility could be a new header under
tools/include/tools/, like for libc_compat.h. Although personally I
don't mind too much about redefining the wrapper several times given
how short it is, and because maybe some tools could redefine it anyway
to use colour output in the future.

The feature test would better be shared, it would probably be similar
to what was done in the following commit to accommodate for a previous
change in libbfd:

    https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/comm=
it/?id=3Dfb982666e380c1632a74495b68b3c33a66e76430

> Attached is my local fix for perf. Obviously would need work to be a real
> solution.

Thanks a lot! Would you be willing to submit a patch for the feature
detection and wrapper?

Best regards,
Quentin