linux-kernel - Re: [PATCH 2/3] perf disasm: Define stubs for the LLVM and capstone disassemblers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZzT9oHzYdfLZwmJ_@x1>
Date: Wed, 13 Nov 2024 16:27:28 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Aditya Bodkhe <Aditya.Bodkhe1@....com>
Cc: Masami Hiramatsu <mhiramat@...nel.org>,
	Namhyung Kim <namhyung@...nel.org>, Ingo Molnar <mingo@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>, Jiri Olsa <jolsa@...nel.org>,
	Ian Rogers <irogers@...gle.com>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Kan Liang <kan.liang@...ux.intel.com>,
	Clark Williams <williams@...hat.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-perf-users@...r.kernel.org" <linux-perf-users@...r.kernel.org>,
	Arnaldo Melo <acme@...hat.com>,
	Athira Rajeev <atrajeev@...ux.vnet.ibm.com>,
	"Steinar H. Gunderson" <sesse@...gle.com>
Subject: Re: [PATCH 2/3] perf disasm: Define stubs for the LLVM and capstone
 disassemblers

On Wed, Nov 13, 2024 at 03:24:08PM +0000, Aditya Bodkhe wrote:
> Hi Arnaldo
> I was testing this change in powerpc setup I see below compilation error
> 
> CC   util/disasm.o
> util/disasm.c: In function ‘symbol__disassemble_capstone’:
> util/disasm.c:1728:9: error: expected declaration specifiers before ‘symbol__disassembler_missing’
>  1728 |     symbol__disassembler_missing("capstone", filename, sym);
>    |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> util/disasm.c:1729:9: error: expected declaration specifiers before ‘return’
>  1729 |     return -1;
>    |     ^~~~~~
> util/disasm.c:1730:1: error: expected declaration specifiers before ‘}’ token
>  1730 | }
> 
> Below patch fixes the issue

Thanks, I've folded your fix into the patch since it is still in the
tmp.perf-tools-next branch, added a note to the patch to give you
and Masami credit, both sent fixes that I had to slightly adjust:

----
perf disasm: Define stubs for the LLVM and capstone disassemblers

This reduces the number of ifdefs in the main symbol__disassemble()
method and paves the way for allowing the user to configure the
disassemblers of preference.

Acked-by: Ian Rogers <irogers@...gle.com>
Cc: Adrian Hunter <adrian.hunter@...el.com>
Cc: Aditya Bodkhe <Aditya.Bodkhe1@....com>
Cc: Athira Rajeev <atrajeev@...ux.vnet.ibm.com>
Cc: Jiri Olsa <jolsa@...nel.org>
Cc: Kan Liang <kan.liang@...ux.intel.com>
Cc: Masami Hiramatsu (Google) <mhiramat@...nel.org>
Cc: Namhyung Kim <namhyung@...nel.org>
Cc: Steinar H. Gunderson <sesse@...gle.com>
Link: https://lore.kernel.org/r/20241111151734.1018476-3-acme@kernel.org
[ Applied fixes from Masami Hiramatsu and Aditya Bodkhe for when capstone devel files are not available ]
Link: https://lore.kernel.org/r/B78FB6DF-24E9-4A3C-91C9-535765EC0E2A@ibm.com
Link: https://lore.kernel.org/r/173145729034.2747044.453926054000880254.stgit@mhiramat.roam.corp.google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
----
 
> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> index df6c172c9c7f..da22b6ab9ecf 100644
> --- a/tools/perf/util/disasm.c
> +++ b/tools/perf/util/disasm.c
> @@ -1724,7 +1724,8 @@ static int symbol__disassemble_capstone(char *filename, struct symbol *sym,
>  }
>  #else // HAVE_LIBCAPSTONE_SUPPORT
>  static int symbol__disassemble_capstone(char *filename, struct symbol *sym,
> -                    struct annotate_args *args)
> +                    struct annotate_args *args __maybe_unused)
> +{
>     symbol__disassembler_missing("capstone", filename, sym);
>     return -1;
>  }
> 
> I tried with tmp.perf-tools-next , I have tested the above patch fixes the compilation error
> 
> Thanks 
> Aditya Bodkhe
> > +
> > +static int symbol__disassemble_capstone_powerpc(char *filename, struct symbol *sym,
> > +						struct annotate_args *args __maybe_unused)
> > +{
> > +	symbol__disassembler_missing("capstone powerpc", filename, sym);
> > +	return -1;
> > +}
> > +#endif // HAVE_LIBCAPSTONE_SUPPORT
> > 
> > static int symbol__disassemble_raw(char *filename, struct symbol *sym,
> > 					struct annotate_args *args)
> > @@ -1983,7 +2003,14 @@ static int symbol__disassemble_llvm(char *filename, struct symbol *sym,
> > 	free(line_storage);
> > 	return ret;
> > }
> > -#endif
> > +#else // HAVE_LIBLLVM_SUPPORT
> > +static int symbol__disassemble_llvm(char *filename, struct symbol *sym,
> > +				    struct annotate_args *args __maybe_unused)
> > +{
> > +	symbol__disassembler_missing("LLVM", filename, sym);
> > +	return -1;
> > +}
> > +#endif // HAVE_LIBLLVM_SUPPORT
> > 
> > /*
> >  * Possibly create a new version of line with tabs expanded. Returns the
> > @@ -2242,24 +2269,21 @@ int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
> > 			err = symbol__disassemble_raw(symfs_filename, sym, args);
> > 			if (err == 0)
> > 				goto out_remove_tmp;
> > -#ifdef HAVE_LIBCAPSTONE_SUPPORT
> > +
> > 			err = symbol__disassemble_capstone_powerpc(symfs_filename, sym, args);
> > 			if (err == 0)
> > 				goto out_remove_tmp;
> > -#endif
> > 		}
> > 	}
> > 
> > -#ifdef HAVE_LIBLLVM_SUPPORT
> > 	err = symbol__disassemble_llvm(symfs_filename, sym, args);
> > 	if (err == 0)
> > 		goto out_remove_tmp;
> > -#endif
> > -#ifdef HAVE_LIBCAPSTONE_SUPPORT
> > +
> > 	err = symbol__disassemble_capstone(symfs_filename, sym, args);
> > 	if (err == 0)
> > 		goto out_remove_tmp;
> > -#endif
> > +
> > 	err = symbol__disassemble_objdump(symfs_filename, sym, args);
> > 
> > out_remove_tmp:
> > -- 
> > 2.47.0
> > 
> > 
>