linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Aditya Bodkhe <Aditya.Bodkhe1@ibm.com>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>,
	Ingo Molnar <mingo@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Kan Liang <kan.liang@linux.intel.com>,
	Clark Williams <williams@redhat.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-perf-users@vger.kernel.org"
	<linux-perf-users@vger.kernel.org>,
	Arnaldo Melo <acme@redhat.com>,
	Athira Rajeev <atrajeev@linux.vnet.ibm.com>,
	"Steinar H. Gunderson" <sesse@google.com>
Subject: Re: [PATCH 2/3] perf disasm: Define stubs for the LLVM and capstone disassemblers
Date: Wed, 13 Nov 2024 15:24:08 +0000	[thread overview]
Message-ID: <B78FB6DF-24E9-4A3C-91C9-535765EC0E2A@ibm.com> (raw)
In-Reply-To: <20241111151734.1018476-3-acme@kernel.org>



> On 11 Nov 2024, at 8:47 PM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> 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.
> 
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> Cc: Ian Rogers <irogers@google.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Kan Liang <kan.liang@linux.intel.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Steinar H. Gunderson <sesse@google.com>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
> tools/perf/util/disasm.c | 40 ++++++++++++++++++++++++++++++++--------
> 1 file changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> index 36cf61602c17fe69..83df1da20a7b16cd 100644
> --- a/tools/perf/util/disasm.c
> +++ b/tools/perf/util/disasm.c
> @@ -1422,6 +1422,13 @@ read_symbol(const char *filename, struct map *map, struct symbol *sym,
> 	free(buf);
> 	return NULL;
> }
> +#else // defined(HAVE_LIBCAPSTONE_SUPPORT) || defined(HAVE_LIBLLVM_SUPPORT)
> +static void symbol__disassembler_missing(const char *disassembler, const char *filename,
> +					 struct symbol *sym)
> +{
> +	pr_debug("The %s disassembler isn't linked in for %s in %s\n",
> +		 disassembler, sym->name, filename);
> +}
> #endif
> 
> #ifdef HAVE_LIBCAPSTONE_SUPPORT
> @@ -1715,7 +1722,20 @@ static int symbol__disassemble_capstone(char *filename, struct symbol *sym,
> 	count = -1;
> 	goto out;
> }
> -#endif
> +#else // HAVE_LIBCAPSTONE_SUPPORT
> +static int symbol__disassemble_capstone(char *filename, struct symbol *sym,
> +					struct annotate_args *args)
> +	symbol__disassembler_missing("capstone", filename, sym);
> +	return -1;
> +}
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

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
> 
> 


  parent reply	other threads:[~2024-11-13 15:24 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-11 15:17 [PATCH 0/3 perf-tools-next] Selectable disassembler Arnaldo Carvalho de Melo
2024-11-11 15:17 ` [PATCH 1/3] perf disasm: Introduce symbol__disassemble_objdump() Arnaldo Carvalho de Melo
2024-11-11 16:15   ` Ian Rogers
2024-11-11 17:18     ` Arnaldo Carvalho de Melo
2024-11-11 15:17 ` [PATCH 2/3] perf disasm: Define stubs for the LLVM and capstone disassemblers Arnaldo Carvalho de Melo
2024-11-11 16:23   ` Ian Rogers
2024-11-11 17:20     ` Arnaldo Carvalho de Melo
2024-11-13 15:24   ` Aditya Bodkhe [this message]
2024-11-13 19:27     ` Arnaldo Carvalho de Melo
2024-11-15  5:27       ` Aditya Bodkhe
2024-11-11 15:17 ` [PATCH 3/3] perf disasm: Allow configuring what disassemblers to use Arnaldo Carvalho de Melo
2024-11-11 16:27   ` Ian Rogers
2024-11-11 17:24     ` Arnaldo Carvalho de Melo
2024-11-13 12:56       ` Steinar H. Gunderson
2024-11-13 19:14         ` Arnaldo Carvalho de Melo
2024-11-14  9:27           ` Steinar H. Gunderson
2025-01-23 22:31   ` Ian Rogers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=B78FB6DF-24E9-4A3C-91C9-535765EC0E2A@ibm.com \
    --to=aditya.bodkhe1@ibm.com \
    --cc=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=adrian.hunter@intel.com \
    --cc=atrajeev@linux.vnet.ibm.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=sesse@google.com \
    --cc=tglx@linutronix.de \
    --cc=williams@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).