linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ingo Molnar <mingo@kernel.org>, Thomas Gleixner <tglx@linutronix.de>
Cc: Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
	Clark Williams <williams@redhat.com>,
	linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	Jiri Olsa <jolsa@redhat.com>
Subject: [PATCH 19/63] perf probe: Fix wrong address verification
Date: Thu,  7 Nov 2019 15:59:27 -0300	[thread overview]
Message-ID: <20191107190011.23924-20-acme@kernel.org> (raw)
In-Reply-To: <20191107190011.23924-1-acme@kernel.org>

From: Masami Hiramatsu <mhiramat@kernel.org>

Since there are some DIE which has only ranges instead of the
combination of entrypc/highpc, address verification must use
dwarf_haspc() instead of dwarf_entrypc/dwarf_highpc.

Also, the ranges only DIE will have a partial code in different section
(e.g. unlikely code will be in text.unlikely as "FUNC.cold" symbol). In
that case, we can not use dwarf_entrypc() or die_entrypc(), because the
offset from original DIE can be a minus value.

Instead, this simply gets the symbol and offset from symtab.

Without this patch;

  # perf probe -D clear_tasks_mm_cpumask:1
  Failed to get entry address of clear_tasks_mm_cpumask
    Error: Failed to add events.

And with this patch:

  # perf probe -D clear_tasks_mm_cpumask:1
  p:probe/clear_tasks_mm_cpumask clear_tasks_mm_cpumask+0
  p:probe/clear_tasks_mm_cpumask_1 clear_tasks_mm_cpumask+5
  p:probe/clear_tasks_mm_cpumask_2 clear_tasks_mm_cpumask+8
  p:probe/clear_tasks_mm_cpumask_3 clear_tasks_mm_cpumask+16
  p:probe/clear_tasks_mm_cpumask_4 clear_tasks_mm_cpumask+82

Committer testing:

I managed to reproduce the above:

  [root@quaco ~]# perf probe -D clear_tasks_mm_cpumask:1
  p:probe/clear_tasks_mm_cpumask _text+919968
  p:probe/clear_tasks_mm_cpumask_1 _text+919973
  p:probe/clear_tasks_mm_cpumask_2 _text+919976
  [root@quaco ~]#

But then when trying to actually put the probe in place, it fails if I
use :0 as the offset:

  [root@quaco ~]# perf probe -L clear_tasks_mm_cpumask | head -5
  <clear_tasks_mm_cpumask@/usr/src/debug/kernel-5.2.fc30/linux-5.2.18-200.fc30.x86_64/kernel/cpu.c:0>
        0  void clear_tasks_mm_cpumask(int cpu)
        1  {
        2  	struct task_struct *p;

  [root@quaco ~]# perf probe clear_tasks_mm_cpumask:0
  Probe point 'clear_tasks_mm_cpumask' not found.
    Error: Failed to add events.
  [root@quaco

The next patch is needed to fix this case.

Fixes: 576b523721b7 ("perf probe: Fix probing symbols with optimization suffix")
Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lore.kernel.org/lkml/157199318513.8075.10463906803299647907.stgit@devnote2
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-finder.c | 32 ++++++++++----------------------
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index cd9f95e5044e..2b6513e5725c 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -604,38 +604,26 @@ static int convert_to_trace_point(Dwarf_Die *sp_die, Dwfl_Module *mod,
 				  const char *function,
 				  struct probe_trace_point *tp)
 {
-	Dwarf_Addr eaddr, highaddr;
+	Dwarf_Addr eaddr;
 	GElf_Sym sym;
 	const char *symbol;
 
 	/* Verify the address is correct */
-	if (dwarf_entrypc(sp_die, &eaddr) != 0) {
-		pr_warning("Failed to get entry address of %s\n",
-			   dwarf_diename(sp_die));
-		return -ENOENT;
-	}
-	if (dwarf_highpc(sp_die, &highaddr) != 0) {
-		pr_warning("Failed to get end address of %s\n",
-			   dwarf_diename(sp_die));
-		return -ENOENT;
-	}
-	if (paddr > highaddr) {
-		pr_warning("Offset specified is greater than size of %s\n",
+	if (!dwarf_haspc(sp_die, paddr)) {
+		pr_warning("Specified offset is out of %s\n",
 			   dwarf_diename(sp_die));
 		return -EINVAL;
 	}
 
-	symbol = dwarf_diename(sp_die);
+	/* Try to get actual symbol name from symtab */
+	symbol = dwfl_module_addrsym(mod, paddr, &sym, NULL);
 	if (!symbol) {
-		/* Try to get the symbol name from symtab */
-		symbol = dwfl_module_addrsym(mod, paddr, &sym, NULL);
-		if (!symbol) {
-			pr_warning("Failed to find symbol at 0x%lx\n",
-				   (unsigned long)paddr);
-			return -ENOENT;
-		}
-		eaddr = sym.st_value;
+		pr_warning("Failed to find symbol at 0x%lx\n",
+			   (unsigned long)paddr);
+		return -ENOENT;
 	}
+	eaddr = sym.st_value;
+
 	tp->offset = (unsigned long)(paddr - eaddr);
 	tp->address = (unsigned long)paddr;
 	tp->symbol = strdup(symbol);
-- 
2.21.0


  parent reply	other threads:[~2019-11-07 19:03 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-07 18:59 [GIT PULL] perf/core improvements and fixes Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 01/63] perf data: Correctly identify directory data files Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 02/63] perf data: Move perf_dir_version into data.h Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 03/63] perf data: Rename directory "header" file to "data" Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 04/63] perf session: Fix indent in perf_session__new()" Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 05/63] perf data: Support single perf.data file directory Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 06/63] perf record: Put a copy of kcore into the perf.data directory Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 07/63] perf llvm: Make .o saving a debug message, not an info one Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 08/63] perf cs-etm: Fix definition of macro TO_CS_QUEUE_NR Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 09/63] perf evsel: Always preserve errno while cleaning up perf_event_open failures Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 10/63] perf evsel: Avoid close(-1) Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 11/63] perf tools: Move ALLOC_LIST into a function Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 12/63] perf tools: Avoid a malloc() for array events Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 13/63] perf tests: Fix a typo Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 14/63] perf kvm: Use evlist layer api when possible Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 15/63] perf probe: Fix to find range-only function instance Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 16/63] perf probe: Walk function lines in lexical blocks Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 17/63] perf probe: Fix to show function entry line as probe-able Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 18/63] perf jevents: Fix resource leak in process_mapfile() and main() Arnaldo Carvalho de Melo
2019-11-07 18:59 ` Arnaldo Carvalho de Melo [this message]
2019-11-07 18:59 ` [PATCH 20/63] perf probe: Fix to probe a function which has no entry pc Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 21/63] perf probe: Fix to probe an inline " Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 22/63] perf probe: Fix to list probe event with correct line number Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 23/63] perf probe: Fix to show inlined function callsite without entry_pc Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 24/63] perf probe: Fix to show ranges of variables in functions " Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 25/63] perf auxtrace: Add auxtrace_cache__remove() Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 26/63] perf dso: Refactor dso_cache__read() Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 27/63] perf dso: Add dso__data_write_cache_addr() Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 28/63] perf map: Check if the map still has some refcounts on exit Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 29/63] perf map: Allow map__next() to receive a NULL arg Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 30/63] perf maps: Add for_each_entry()/_safe() iterators Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 31/63] perf map_groups: Introduce for_each_entry() and for_each_entry_safe() iterators Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 32/63] libsubcmd: Move EXTRA_FLAGS to the end to allow overriding existing flags Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 33/63] libsubcmd: Use -O0 with DEBUG=1 Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 34/63] perf tools: Splice events onto evlist even on error Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 36/63] perf vendor events intel: Update all the Intel JSON metrics from TMAM 3.6 Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 37/63] perf env: Add perf_env__numa_node() Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 38/63] perf stat: Add --per-node agregation support Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 39/63] perf tools: Fix cross compile for ARM64 Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 40/63] perf inject: Make --strip keep evsels Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 41/63] perf parse: Add parse events handle error Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 42/63] perf parse: Ensure config and str in terms are unique Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 43/63] perf parse: Add destructors for parse event terms Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 44/63] perf parse: Before yyabort-ing free components Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 45/63] perf parse: If pmu configuration fails free terms Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 46/63] perf parse: Add a deep delete for parse event terms Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 47/63] perf symbols: Remove needless checks for map->groups->machine Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 48/63] perf machine: Add kernel_dso() method Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 49/63] perf annotate: Fix heap overflow Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 50/63] perf probe: Return a better scope DIE if there is no best scope Arnaldo Carvalho de Melo
2019-11-07 18:59 ` [PATCH 51/63] perf probe: Skip end-of-sequence and non statement lines Arnaldo Carvalho de Melo
2019-11-07 19:00 ` [PATCH 52/63] perf probe: Filter out instances except for inlined subroutine and subprogram Arnaldo Carvalho de Melo
2019-11-07 19:00 ` [PATCH 53/63] perf probe: Fix to show calling lines of inlined functions Arnaldo Carvalho de Melo
2019-11-07 19:00 ` [PATCH 54/63] perf probe: Skip overlapped location on searching variables Arnaldo Carvalho de Melo
2019-11-07 19:00 ` [PATCH 55/63] perf record: Add support for limit perf output file size Arnaldo Carvalho de Melo
2019-11-07 19:00 ` [PATCH 56/63] perf tests: Fix out of bounds memory access Arnaldo Carvalho de Melo
2019-12-16 16:07   ` Naresh Kamboju
2019-12-16 16:20     ` Greg Kroah-Hartman
2019-11-07 19:00 ` [PATCH 57/63] perf diff: Don't use hack to skip column length calculation Arnaldo Carvalho de Melo
2019-11-07 19:00 ` [PATCH 58/63] perf block: Cleanup and refactor block info functions Arnaldo Carvalho de Melo
2019-11-07 19:00 ` [PATCH 59/63] perf hist: Count the total cycles of all samples Arnaldo Carvalho de Melo
2019-11-07 19:00 ` [PATCH 60/63] perf hist: Support block formats with compare/sort/display Arnaldo Carvalho de Melo
2019-11-07 19:00 ` [PATCH 61/63] perf report: Sort by sampled cycles percent per block for stdio Arnaldo Carvalho de Melo
2019-11-07 19:00 ` [PATCH 62/63] perf report: Support --percent-limit for --total-cycles Arnaldo Carvalho de Melo
2019-11-07 19:00 ` [PATCH 63/63] perf report: Sort by sampled cycles percent per block for tui Arnaldo Carvalho de Melo
2019-11-12 11:08 ` [GIT PULL] perf/core improvements and fixes Ingo Molnar

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=20191107190011.23924-20-acme@kernel.org \
    --to=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=jolsa@kernel.org \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --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).