Hello,
I am working on improving quality of the data type profiling and I
found some issues. One is when more than one variables are placed at
the same location. Then it should find the correct one based on the
given info rather than checking the first one and bailing out. This
one (patch 2) can go to the perf-tools tree.
Another issue is use of the stack pointe register when it's not the
frame base register. I found a case where rbp is used as the frame
base but rsp is also used to point some stack variables. And it
confuses itself how to interpret the type of the variable.
I think these are rare cases but it would depends on the code pattern
and compiler behavior. Anyway I can see a tiny improvement in my data
with this change. :)
Thanks,
Namhyung
Namhyung Kim (4):
perf annotate-data: Improve debug message with location info
perf dwarf-aux: Check pointer offset when checking variables
perf dwarf-aux: Check variable address range properly
perf annotate-data: Handle RSP if it's not the FB register
tools/perf/util/annotate-data.c | 126 +++++++++++++++++++++++++-------
tools/perf/util/dwarf-aux.c | 35 ++++++---
2 files changed, 125 insertions(+), 36 deletions(-)
base-commit: 0ffc8fca5c15a70f32c8aff12c566bbd3991bd0a
--
2.44.0.683.g7961c838ac-goog
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 <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
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
On Fri, Apr 12, 2024 at 11:33:06AM -0700, Namhyung Kim wrote:
> Hello,
>
> I am working on improving quality of the data type profiling and I
> found some issues. One is when more than one variables are placed at
> the same location. Then it should find the correct one based on the
> given info rather than checking the first one and bailing out. This
> one (patch 2) can go to the perf-tools tree.
>
> Another issue is use of the stack pointe register when it's not the
> frame base register. I found a case where rbp is used as the frame
> base but rsp is also used to point some stack variables. And it
> confuses itself how to interpret the type of the variable.
>
> I think these are rare cases but it would depends on the code pattern
> and compiler behavior. Anyway I can see a tiny improvement in my data
Thanks, applied to perf-tools-next,
- Arnaldo
> with this change. :)
> Thanks,
> Namhyung
>
>
> Namhyung Kim (4):
> perf annotate-data: Improve debug message with location info
> perf dwarf-aux: Check pointer offset when checking variables
> perf dwarf-aux: Check variable address range properly
> perf annotate-data: Handle RSP if it's not the FB register
>
> tools/perf/util/annotate-data.c | 126 +++++++++++++++++++++++++-------
> tools/perf/util/dwarf-aux.c | 35 ++++++---
> 2 files changed, 125 insertions(+), 36 deletions(-)
>
>
> base-commit: 0ffc8fca5c15a70f32c8aff12c566bbd3991bd0a
> --
> 2.44.0.683.g7961c838ac-goog