All of lore.kernel.org
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Arnaldo Carvalho de Melo <acme@kernel.org>,
	Ian Rogers <irogers@google.com>,
	Kan Liang <kan.liang@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-perf-users@vger.kernel.org,
	Masami Hiramatsu <mhiramat@kernel.org>
Subject: [PATCH 3/4] perf dwarf-aux: Check variable address range properly
Date: Fri, 12 Apr 2024 11:33:09 -0700	[thread overview]
Message-ID: <20240412183310.2518474-4-namhyung@kernel.org> (raw)
In-Reply-To: <20240412183310.2518474-1-namhyung@kernel.org>

In match_var_offset(), it just checked the end address of the variable
with the given offset because it assumed the register holds a pointer
to the data type and the offset starts from the base.

But I found some cases that the stack pointer (rsp = reg7) register is
used to pointer a stack variable while the frame base is maintained by a
different register (rbp = reg6).  In that case, it cannot simply use the
stack pointer as it cannot guarantee that it points to the frame base.
So it needs to check the both boundaries of the variable location.

Before:
  -----------------------------------------------------------
  find data type for 0x7c(reg7) at tcp_getsockopt+0xb62
  CU for net/ipv4/tcp.c (die:0x7b5f516)
  frame base: cfa=0 fbreg=6
  no pointer or no type
  check variable "tss" failed (die: 0x7b95801)
   variable location: base reg7, offset=0x110
   type='struct scm_timestamping_internal' size=0x30 (die:0x7b8c126)

So the current code just checks register number for the non-PC and
non-FB registers and assuming it has offset 0.  But this variable has
offset 0x110 so it should not match to this.

After:
  -----------------------------------------------------------
  find data type for 0x7c(reg7) at tcp_getsockopt+0xb62
  CU for net/ipv4/tcp.c (die:0x7b5f516)
  frame base: cfa=0 fbreg=6
  no pointer or no type
  check variable "zc" failed (die: 0x7b9580a)
   variable location: base=reg7, offset=0x40
   type='struct tcp_zerocopy_receive' size=0x40 (die:7b947f4)

Now it find the correct variable "zc".  It was located at reg7 + 0x40
and the size if 0x40 which means it should cover [0x40, 0x80).  And the
access was for reg7 + 0x7c so it found the right one.  But it still
failed to use the variable and it would be handled in the next patch.

Cc: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/dwarf-aux.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index b361fd7ebd56..40cfbdfe2d75 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -1372,6 +1372,9 @@ static bool match_var_offset(Dwarf_Die *die_mem, struct find_var_data *data,
 		return true;
 	}
 
+	if (addr_offset < addr_type)
+		return false;
+
 	if (die_get_real_type(die_mem, &type_die) == NULL)
 		return false;
 
@@ -1446,7 +1449,6 @@ static int __die_find_var_reg_cb(Dwarf_Die *die_mem, void *arg)
 
 		/* Local variables accessed using frame base register */
 		if (data->is_fbreg && ops->atom == DW_OP_fbreg &&
-		    data->offset >= (int)ops->number &&
 		    check_allowed_ops(ops, nops) &&
 		    match_var_offset(die_mem, data, data->offset, ops->number,
 				     /*is_pointer=*/false))
@@ -1537,9 +1539,6 @@ static int __die_find_var_addr_cb(Dwarf_Die *die_mem, void *arg)
 		if (ops->atom != DW_OP_addr)
 			continue;
 
-		if (data->addr < ops->number)
-			continue;
-
 		if (check_allowed_ops(ops, nops) &&
 		    match_var_offset(die_mem, data, data->addr, ops->number,
 				     /*is_pointer=*/false))
-- 
2.44.0.683.g7961c838ac-goog


  parent reply	other threads:[~2024-04-12 18:33 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-12 18:33 [PATCH 0/4] perf annotate-data: A couple of small updates Namhyung Kim
2024-04-12 18:33 ` [PATCH 1/4] perf annotate-data: Improve debug message with location info Namhyung Kim
2024-04-16 14:23   ` Arnaldo Carvalho de Melo
2024-04-12 18:33 ` [PATCH 2/4] perf dwarf-aux: Check pointer offset when checking variables Namhyung Kim
2024-04-12 18:33 ` Namhyung Kim [this message]
2024-04-12 18:33 ` [PATCH 4/4] perf annotate-data: Handle RSP if it's not the FB register Namhyung Kim
2024-04-16 21:06 ` [PATCH 0/4] perf annotate-data: A couple of small updates Arnaldo Carvalho de Melo

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=20240412183310.2518474-4-namhyung@kernel.org \
    --to=namhyung@kernel.org \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.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=mhiramat@kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.