linux-stable.git/tools/perf/util/annotate-data.c, branch linux-6.11.y

perf annotate-data: Fix off-by-one in location range check

2024-10-04T14:38:03+00:00

[ Upstream commit 3ab0b8b238b5130ae3fa37ddaa329fc0e93b6b9a ]

The location list will have entries with half-open addressing like
[start, end) which means it doesn't include the end address.  So it
should skip entries at the end address and match to the next entry.

An example location list looks like this (from readelf -wo):

    00237876 ffffffff8110d32b (base address)
    0023787f v000000000000000 v000000000000002 views at 00237868 for:
             ffffffff8110d32b ffffffff8110d4eb (DW_OP_reg3 (rbx))     <<<--- 1
    00237885 v000000000000002 v000000000000000 views at 0023786a for:
             ffffffff8110d4eb ffffffff8110d50b (DW_OP_reg14 (r14))    <<<--- 2
    0023788c v000000000000000 v000000000000001 views at 0023786c for:
             ffffffff8110d50b ffffffff8110d7c4 (DW_OP_reg3 (rbx))
    00237893 v000000000000000 v000000000000000 views at 0023786e for:
             ffffffff8110d806 ffffffff8110d854 (DW_OP_reg3 (rbx))
    0023789a v000000000000000 v000000000000000 views at 00237870 for:
             ffffffff8110d876 ffffffff8110d88e (DW_OP_reg3 (rbx))

The first entry at 0023787f has [8110d32b, 8110d4eb) (omitting the
ffffffff at the beginning), and the second one has [8110d4eb, 8110d50b).

Fixes: 2bc3cf575a162a2c ("perf annotate-data: Improve debug message with location info")
Reviewed-by: Masami Hiramatsu 
Signed-off-by: Namhyung Kim 
Cc: Adrian Hunter 
Cc: Athira Rajeev 
Cc: Ian Rogers 
Cc: Ingo Molnar 
Cc: Jiri Olsa 
Cc: Kan Liang 
Cc: Masami Hiramatsu 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Link: https://lore.kernel.org/r/20240816235840.2754937-3-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo 
Signed-off-by: Sasha Levin

perf annotate-data: Ensure the number of type histograms

2024-05-11T16:03:13+00:00

Arnaldo reported that there is a case where nr_histograms and histograms
don't agree each other.

It ended up in a segfault trying to access a NULL histograms array.

Let's make sure to update the nr_histograms when the histograms array is
changed.

Reported-by: Arnaldo Carvalho de Melo 
Reviewed-by: Ian Rogers 
Signed-off-by: Namhyung Kim 
Tested-by: Arnaldo Carvalho de Melo 
Cc: Adrian Hunter 
Cc: Ingo Molnar 
Cc: Jiri Olsa 
Cc: Kan Liang 
Cc: Peter Zijlstra 
Link: https://lore.kernel.org/r/20240510210452.2449944-2-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo

perf annotate: Use zfree() to avoid possibly accessing dangling pointers

2024-05-07T15:43:53+00:00

When freeing a->b it is good practice to set a->b to NULL using
zfree(&a->b) so that when we have a bug where a reference to a freed 'a'
pointer is kept somewhere, we can more quickly cause a segfault if some
code tries to use a->b.

This is mostly done but some new cases were introduced recently, convert
them to zfree().

Cc: Adrian Hunter 
Cc: Ian Rogers 
Cc: Jiri Olsa 
Cc: Kan Liang 
Cc: Namhyung Kim 
Link: https://lore.kernel.org/lkml/ZjmbHHrjIm5YRIBv@x1
Signed-off-by: Arnaldo Carvalho de Melo

perf dso: Add reference count checking and accessor functions

2024-05-06T18:28:49+00:00

Add reference count checking to struct dso, this can help with
implementing correct reference counting discipline. To avoid
RC_CHK_ACCESS everywhere, add accessor functions for the variables in
struct dso.

The majority of the change is mechanical in nature and not easy to
split up.

Committer testing:

'perf test' up to this patch shows no regressions.

But:

  util/symbol.c: In function ‘dso__load_bfd_symbols’:
  util/symbol.c:1683:9: error: too few arguments to function ‘dso__set_adjust_symbols’
   1683 |         dso__set_adjust_symbols(dso);
        |         ^~~~~~~~~~~~~~~~~~~~~~~
  In file included from util/symbol.c:21:
  util/dso.h:268:20: note: declared here
    268 | static inline void dso__set_adjust_symbols(struct dso *dso, bool val)
        |                    ^~~~~~~~~~~~~~~~~~~~~~~
  make[6]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:106: /tmp/tmp.ZWHbQftdN6/util/symbol.o] Error 1
    MKDIR   /tmp/tmp.ZWHbQftdN6/tests/workloads/
  make[6]: *** Waiting for unfinished jobs....

This was updated:

  -       symbols__fixup_end(&dso->symbols, false);
  -       symbols__fixup_duplicate(&dso->symbols);
  -       dso->adjust_symbols = 1;
  +       symbols__fixup_end(dso__symbols(dso), false);
  +       symbols__fixup_duplicate(dso__symbols(dso));
  +       dso__set_adjust_symbols(dso);

But not build tested with BUILD_NONDISTRO and libbfd devel files installed
(binutils-devel on fedora).

Add the missing argument:

   	symbols__fixup_end(dso__symbols(dso), false);
   	symbols__fixup_duplicate(dso__symbols(dso));
  -	dso__set_adjust_symbols(dso);
  +	dso__set_adjust_symbols(dso, true);

Signed-off-by: Ian Rogers 
Tested-by: Arnaldo Carvalho de Melo 
Cc: Adrian Hunter 
Cc: Ahelenia Ziemiańska 
Cc: Alexander Shishkin 
Cc: Andi Kleen 
Cc: Athira Rajeev 
Cc: Ben Gainey 
Cc: Changbin Du 
Cc: Chengen Du 
Cc: Colin Ian King 
Cc: Dima Kogan 
Cc: Ilkka Koskinen 
Cc: Ingo Molnar 
Cc: James Clark 
Cc: Jiri Olsa 
Cc: K Prateek Nayak 
Cc: Kan Liang 
Cc: Leo Yan 
Cc: Li Dong 
Cc: Mark Rutland 
Cc: Masami Hiramatsu 
Cc: Namhyung Kim 
Cc: Paran Lee 
Cc: Peter Zijlstra 
Cc: Song Liu 
Cc: Sun Haiyong 
Cc: Thomas Richter 
Cc: Tiezhu Yang 
Cc: Yanteng Si 
Cc: zhaimingbing 
Link: https://lore.kernel.org/r/20240504213803.218974-6-irogers@google.com
Signed-off-by: Arnaldo Carvalho de Melo

perf annotate-data: Check kind of stack variables

2024-05-02T14:06:23+00:00

I sometimes see ("unknown type") in the result and it was because it
didn't check the type of stack variables properly during the instruction
tracking.  The stack can carry constant values (without type info) and
if the target instruction is accessing the stack location, it resulted
in the "unknown type".

Maybe we could pick one of integer types for the constant, but it
doesn't really mean anything useful.  Let's just drop the stack slot if
it doesn't have a valid type info.

Here's an example how it got the unknown type.
Note that 0xffffff48 = -0xb8.
  -----------------------------------------------------------
  find data type for 0xffffff48(reg6) at ...
  CU for ...
  frame base: cfa=0 fbreg=6
  scope: [2/2] (die:11cb97f)
  bb: [37 - 3a]
  var [37] reg15 type='int' size=0x4 (die:0x1180633)
  bb: [40 - 4b]
  mov [40] imm=0x1 -> reg13
  var [45] reg8 type='sigset_t*' size=0x8 (die:0x11a39ee)
  mov [45] imm=0x1 -> reg2                     <---  here reg2 has a constant
  bb: [215 - 237]
  mov [218] reg2 -> -0xb8(stack) constant      <---  and save it to the stack
  mov [225] reg13 -> -0xc4(stack) constant
  call [22f] find_task_by_vgpid
  call [22f] return -> reg0 type='struct task_struct*' size=0x8 (die:0x11881e8)
  bb: [5c8 - 5cf]
  bb: [2fb - 302]
  mov [2fb] -0xc4(stack) -> reg13 constant
  bb: [13b - 14d]
  mov [143] 0xd50(reg3) -> reg5 type='struct task_struct*' size=0x8 (die:0xa31f3c)
  bb: [153 - 153]
  chk [153] reg6 offset=0xffffff48 ok=0 kind=0 fbreg    <--- access here
  found by insn track: 0xffffff48(reg6) type-offset=0
   type='G^KU' size=0 (die:0xffffffffffffffff)

Signed-off-by: Namhyung Kim 
Cc: Adrian Hunter 
Cc: Ian Rogers 
Cc: Ingo Molnar 
Cc: Jiri Olsa 
Cc: Kan Liang 
Cc: Peter Zijlstra 
Link: https://lore.kernel.org/r/20240502060011.1838090-7-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo

perf annotate-data: Handle multi regs in find_data_type_block()

2024-05-02T14:05:10+00:00

The instruction tracking should be the same for the both registers.

Just do it once and compare the result with multi regs as with the
previous patches.

Then we don't need to call find_data_type_block() separately for each
reg.

Let's remove the 'reg' argument from the relevant functions.

Signed-off-by: Namhyung Kim 
Cc: Adrian Hunter 
Cc: Ian Rogers 
Cc: Ingo Molnar 
Cc: Jiri Olsa 
Cc: Kan Liang 
Cc: Peter Zijlstra 
Link: https://lore.kernel.org/r/20240502060011.1838090-6-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo

perf annotate-data: Check memory access with two registers

2024-05-02T13:54:31+00:00

The following instruction pattern is used to access a global variable.

  mov     $0x231c0, %rax
  movsql  %edi, %rcx
  mov     -0x7dc94ae0(,%rcx,8), %rcx
  cmpl    $0x0, 0xa60(%rcx,%rax,1)     <<<--- here

The first instruction set the address of the per-cpu variable (here, it
is 'runqueues' of type 'struct rq').  The second instruction seems like
a cpu number of the per-cpu base.  The third instruction get the base
offset of per-cpu area for that cpu.  The last instruction compares the
value of the per-cpu variable at the offset of 0xa60.

Signed-off-by: Namhyung Kim 
Cc: Adrian Hunter 
Cc: Ian Rogers 
Cc: Ingo Molnar 
Cc: Jiri Olsa 
Cc: Kan Liang 
Cc: Peter Zijlstra 
Link: https://lore.kernel.org/r/20240502060011.1838090-5-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo

perf annotate-data: Handle direct global variable access

2024-05-02T13:51:23+00:00

Like per-cpu base offset array, sometimes it accesses the global
variable directly using the offset.  Allow this type of instructions as
long as it finds a global variable for the address.

  movslq  %edi, %rcx
  mov     -0x7dc94ae0(,%rcx,8), %rcx   <<<--- here

As %rcx has a valid type (i.e. array index) from the first instruction,
it will be checked by the first case in check_matching_type().  But as
it's not a pointer type, the match will fail.  But in this case, it
should check if it accesses the kernel global array variable.

Signed-off-by: Namhyung Kim 
Cc: Adrian Hunter 
Cc: Ian Rogers 
Cc: Ingo Molnar 
Cc: Jiri Olsa 
Cc: Kan Liang 
Cc: Peter Zijlstra 
Link: https://lore.kernel.org/r/20240502060011.1838090-4-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo

perf annotate-data: Collect global variables in advance

2024-05-02T13:47:52+00:00

Currently it looks up global variables from the current CU using address
and name.  But it sometimes fails to find a variable as the variable can
come from a different CU - but it's still strange it failed to find a
declaration for some reason.

Anyway, it can collect all global variables from all CU once and then
lookup them later on.  This slightly improves the success rate of my
test data set.

Signed-off-by: Namhyung Kim 
Cc: Adrian Hunter 
Cc: Ian Rogers 
Cc: Ingo Molnar 
Cc: Jiri Olsa 
Cc: Kan Liang 
Cc: Peter Zijlstra 
Link: https://lore.kernel.org/r/20240502060011.1838090-3-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo

perf annotate-data: Handle RSP if it's not the FB register

2024-04-16T13:46:55+00:00

In some cases, the stack pointer on x86 (rsp = reg7) is used to point
variables on stack but it's not the frame base register.  Then it
should handle the register like normal registers (IOW not to access
the other stack variables using offset calculation) but it should not
assume it would have a pointer.

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 "zc" failed (die: 0x7b9580a)
   variable location: base=reg7, offset=0x40
   type='struct tcp_zerocopy_receive' size=0x40 (die:0x7b947f4)

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
  found "zc" in scope=3/3 (die: 0x7b957fc) type_offset=0x3c
   variable location: base=reg7, offset=0x40
   type='struct tcp_zerocopy_receive' size=0x40 (die:0x7b947f4)

Note that the type-offset was properly calculated to 0x3c as the
variable starts at 0x40.

Signed-off-by: Namhyung Kim 
Cc: Adrian Hunter 
Cc: Ian Rogers 
Cc: Ingo Molnar 
Cc: Jiri Olsa 
Cc: Kan Liang 
Cc: Peter Zijlstra 
Link: https://lore.kernel.org/r/20240412183310.2518474-5-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo