[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <6847C738-D2EB-4637-873C-20DF5FC64605@ibm.com>
Date: Fri, 15 Nov 2024 05:27:21 +0000
From: Aditya Bodkhe <Aditya.Bodkhe1@....com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
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 14 Nov 2024, at 12:57 AM, Arnaldo Carvalho de Melo <acme@...nel.org> wrote:
>
> 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.
>
Hi Arnaldo
Thanks for adding the fixes. I tested with tmp.perf-tools-next and it compiles fine
Tested-by: Aditya Bodkhe <adityab1@...ux.ibm.com <mailto:adityab1@...ux.ibm.com>>
> 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
Powered by blists - more mailing lists