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: <20230417122943.2155502-1-anders.roxell@linaro.org> <20230417122943.2155502-2-anders.roxell@linaro.org>
In-Reply-To: <20230417122943.2155502-2-anders.roxell@linaro.org>
From:   Ian Rogers <irogers@google.com>
Date:   Mon, 17 Apr 2023 10:14:51 -0700
Message-ID: <CAP-5=fWUevSnyn5MtVO1p6cEVE8MvBTq4Qgth7RcPYueRERQKA@mail.gmail.com>
Subject: Re: [backport PATCH 1/2] tools perf: Fix compilation error with new binutils
To:     Anders Roxell <anders.roxell@linaro.org>
Cc:     stable@vger.kernel.org, acme@redhat.com, andres@anarazel.de,
        linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
        Quentin Monnet <quentin@isovalent.com>,
        Alexei Starovoitov <ast@kernel.org>,
        Ben Hutchings <benh@debian.org>, Jiri Olsa <jolsa@kernel.org>,
        Sedat Dilek <sedat.dilek@gmail.com>, bpf@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Apr 17, 2023 at 5:30=E2=80=AFAM Anders Roxell <anders.roxell@linaro=
.org> wrote:
>
> From: Andres Freund <andres@anarazel.de>
>
> binutils changed the signature of init_disassemble_info(), which now caus=
es
> compilation failures for tools/perf/util/annotate.c, e.g. on debian
> unstable.

Thanks, I believe the compilation issue may well be resolved by:
https://lore.kernel.org/lkml/20230311065753.3012826-8-irogers@google.com/
where binutils is made opt-in rather than opt-out.

> Relevant binutils commit:
>
>   https://sourceware.org/git/?p=3Dbinutils-gdb.git;a=3Dcommit;h=3D60a3da0=
0bd5407f07
>
> Wire up the feature test and switch to init_disassemble_info_compat(),
> which were introduced in prior commits, fixing the compilation failure.

I was kind of surprised to see no version check ifdef. Is
init_disassemble_info_compat is supported in older binutils?

Thanks,
Ian

> I verified that perf can still disassemble bpf programs by using bpftrace
> under load, recording a perf trace, and then annotating the bpf "function=
"
> with and without the changes. With old binutils there's no change in outp=
ut
> before/after this patch. When comparing the output from old binutils (2.3=
5)
> to new bintuils with the patch (upstream snapshot) there are a few output
> differences, but they are unrelated to this patch. An example hunk is:
>
>        1.15 :   55:mov    %rbp,%rdx
>        0.00 :   58:add    $0xfffffffffffffff8,%rdx
>        0.00 :   5c:xor    %ecx,%ecx
>   -    1.03 :   5e:callq  0xffffffffe12aca3c
>   +    1.03 :   5e:call   0xffffffffe12aca3c
>        0.00 :   63:xor    %eax,%eax
>   -    2.18 :   65:leaveq
>   -    2.82 :   66:retq
>   +    2.18 :   65:leave
>   +    2.82 :   66:ret
>
> Signed-off-by: Andres Freund <andres@anarazel.de>
> Acked-by: Quentin Monnet <quentin@isovalent.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Ben Hutchings <benh@debian.org>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Sedat Dilek <sedat.dilek@gmail.com>
> Cc: bpf@vger.kernel.org
> Link: http://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.a=
narazel.de
> Link: https://lore.kernel.org/r/20220801013834.156015-5-andres@anarazel.d=
e
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> ---
>  tools/perf/Makefile.config | 8 ++++++++
>  tools/perf/util/annotate.c | 7 ++++---
>  2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index 3e7706c251e9..55905571f87b 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -281,6 +281,7 @@ FEATURE_CHECK_LDFLAGS-libpython :=3D $(PYTHON_EMBED_L=
DOPTS)
>  FEATURE_CHECK_LDFLAGS-libaio =3D -lrt
>
>  FEATURE_CHECK_LDFLAGS-disassembler-four-args =3D -lbfd -lopcodes -ldl
> +FEATURE_CHECK_LDFLAGS-disassembler-init-styled =3D -lbfd -lopcodes -ldl
>
>  CORE_CFLAGS +=3D -fno-omit-frame-pointer
>  CORE_CFLAGS +=3D -ggdb3
> @@ -838,13 +839,16 @@ else
>    ifeq ($(feature-libbfd-liberty), 1)
>      EXTLIBS +=3D -lbfd -lopcodes -liberty
>      FEATURE_CHECK_LDFLAGS-disassembler-four-args +=3D -liberty -ldl
> +    FEATURE_CHECK_LDFLAGS-disassembler-init-styled +=3D -liberty -ldl
>    else
>      ifeq ($(feature-libbfd-liberty-z), 1)
>        EXTLIBS +=3D -lbfd -lopcodes -liberty -lz
>        FEATURE_CHECK_LDFLAGS-disassembler-four-args +=3D -liberty -lz -ld=
l
> +      FEATURE_CHECK_LDFLAGS-disassembler-init-styled +=3D -liberty -ldl
>      endif
>    endif
>    $(call feature_check,disassembler-four-args)
> +  $(call feature_check,disassembler-init-styled)
>  endif
>
>  ifeq ($(feature-libbfd-buildid), 1)
> @@ -957,6 +961,10 @@ ifeq ($(feature-disassembler-four-args), 1)
>      CFLAGS +=3D -DDISASM_FOUR_ARGS_SIGNATURE
>  endif
>
> +ifeq ($(feature-disassembler-init-styled), 1)
> +    CFLAGS +=3D -DDISASM_INIT_STYLED
> +endif
> +
>  ifeq (${IS_64_BIT}, 1)
>    ifndef NO_PERF_READ_VDSO32
>      $(call feature_check,compile-32)
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 308189454788..f2d1741b7610 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1684,6 +1684,7 @@ static int dso__disassemble_filename(struct dso *ds=
o, char *filename, size_t fil
>  #define PACKAGE "perf"
>  #include <bfd.h>
>  #include <dis-asm.h>
> +#include <tools/dis-asm-compat.h>
>
>  static int symbol__disassemble_bpf(struct symbol *sym,
>                                    struct annotate_args *args)
> @@ -1726,9 +1727,9 @@ static int symbol__disassemble_bpf(struct symbol *s=
ym,
>                 ret =3D errno;
>                 goto out;
>         }
> -       init_disassemble_info(&info, s,
> -                             (fprintf_ftype) fprintf);
> -
> +       init_disassemble_info_compat(&info, s,
> +                                    (fprintf_ftype) fprintf,
> +                                    fprintf_styled);
>         info.arch =3D bfd_get_arch(bfdf);
>         info.mach =3D bfd_get_mach(bfdf);
>
> --
> 2.39.2
>