2015-07-01 09:07:01

by He Kuang

[permalink] [raw]
Subject: [PATCH 1/4] perf record: Use %ld for long type sample counter

Since we post-process and count (long rec->samples) to show precise
number of samples instead of estimate on (u64 rec->bytes_written), the
format string of that should be changed to %ld accordingly, otherwise
the value don't show right on 32bit machine.

Before this patch:

$ perf record -e syscalls:sys_enter_write -- dd if=/dev/zero
of=/dev/null bs=4k count=1000
1000+0 records in
1000+0 records out
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.077 MB perf.data (1 samples) ]

After this patch:

$ perf record -e syscalls:sys_enter_write -- dd if=/dev/zero
of=/dev/null bs=4k count=1000
1000+0 records in
1000+0 records out
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.077 MB perf.data (1001 samples) ]

Signed-off-by: He Kuang <[email protected]>
---
tools/perf/builtin-record.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index de165a1..65e632d 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -737,7 +737,7 @@ out_child:

if (rec->samples && !rec->opts.full_auxtrace)
scnprintf(samples, sizeof(samples),
- " (%" PRIu64 " samples)", rec->samples);
+ " (%ld samples)", rec->samples);
else
samples[0] = '\0';

--
1.8.5.2


2015-07-01 09:06:52

by He Kuang

[permalink] [raw]
Subject: [PATCH 2/4] perf probe: Fix failure to probe online module

Kernel module has the type DSO_TYPE_USER, the check on dso type in
kernel_get_module_dso() disable openning debuginfo of online kernel
modules.

Problem can be reproduced as:

$ insmod test_bpf.ko

$ perf probe -v -m test_bpf --add='skb_is_nonlinear'

probe-definition(0): skb_is_nonlinear
symbol:skb_is_nonlinear file:(null) line:0 offset:0 return:0 lazy:(null)
0 arguments
Failed to find module test_bpf.
Could not open debuginfo. Try to use symbols.
Failed to find symbol skb_is_nonlinear in test_bpf
Error: Failed to add events. Reason: No such file or directory (Code: -2)

Infact kernel_get_module_dso() function is called in perf-probe, in this
condition, machine->dsos list contains only dsos which is either kmod or
kernel loaded by machine__create_kernel_maps(), so there's no need to
pick away the non-kmod user type dsos.

This patch removes the check.

After this patch:

$ perf probe -v -m test_bpf --add='skb_is_nonlinear'

probe-definition(0): skb_is_nonlinear
symbol:skb_is_nonlinear file:(null) line:0 offset:0 return:0 lazy:(null)
0 arguments
Open Debuginfo file: /lib/modules/4.1.0+/test_bpf.ko
Try to find probe point from debuginfo.
Matched function: skb_is_nonlinear
found inline addr: 0x10ab3
Probe point found: test_bpf_init+585
Found 1 probe_trace_events.
Opening /sys/kernel/debug/tracing/kprobe_events write=1
Added new event:
Writing event: p:probe/skb_is_nonlinear test_bpf:test_bpf_init+585
probe:skb_is_nonlinear (on skb_is_nonlinear in test_bpf)

You can now use it in all perf tools, such as:

perf record -e probe:skb_is_nonlinear -aR sleep 1

Signed-off-by: He Kuang <[email protected]>
---
tools/perf/util/probe-event.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 381f23a..8ad2743 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -275,8 +275,6 @@ static int kernel_get_module_dso(const char *module, struct dso **pdso)

if (module) {
list_for_each_entry(dso, &host_machine->dsos.head, node) {
- if (!dso->kernel)
- continue;
if (strncmp(dso->short_name + 1, module,
dso->short_name_len - 2) == 0)
goto found;
--
1.8.5.2

2015-07-01 09:07:27

by He Kuang

[permalink] [raw]
Subject: [PATCH 3/4] perf probe: Enable --range option according to libdw version

The option --range uses functions in libdw (elfutils version >= 1.57),
this patch check if elfutils version meets the requirements before
enable this feature, so that perf can be built with old libdw.

Reported-by: Alexei Starovoitov <[email protected]>
Signed-off-by: He Kuang <[email protected]>
---
tools/perf/builtin-probe.c | 2 ++
tools/perf/util/dwarf-aux.c | 2 ++
tools/perf/util/dwarf-aux.h | 13 ++++++++++++-
3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 1272559..2760c06 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -372,8 +372,10 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
"Show accessible variables on PROBEDEF", opt_show_vars),
OPT_BOOLEAN('\0', "externs", &probe_conf.show_ext_vars,
"Show external variables too (with --vars only)"),
+#if _ELFUTILS_PREREQ(0, 157)
OPT_BOOLEAN('\0', "range", &probe_conf.show_location_range,
"Show variables location range in scope (with --vars only)"),
+#endif
OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
"file", "vmlinux pathname"),
OPT_STRING('s', "source", &symbol_conf.source_prefix,
diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 57f3ef4..2cb4c82 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -950,6 +950,7 @@ int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf)
return 0;
}

+#if _ELFUTILS_PREREQ(0, 157)
/**
* die_get_var_innermost_scope - Get innermost scope range of given variable DIE
* @sp_die: a subprogram DIE
@@ -1071,3 +1072,4 @@ int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)

return ret;
}
+#endif /* _ELFUTILS_PREREQ(0, 157) */
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index c42ec36..ade0c50 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -125,6 +125,17 @@ extern int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf);

/* Get the name and type of given variable DIE, stored as "type\tname" */
extern int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf);
+#if _ELFUTILS_PREREQ(0, 157)
extern int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
- struct strbuf *buf);
+ struct strbuf *buf);
+#else
+static inline
+int die_get_var_range(Dwarf_Die *sp_die __maybe_unused,
+ Dwarf_Die *vr_die __maybe_unused,
+ struct strbuf *buf __maybe_unused)
+{
+ return -ENOTSUP;
+}
+
+#endif /* _ELFUTILS_PREREQ(0, 157) */
#endif
--
1.8.5.2

2015-07-01 09:07:20

by He Kuang

[permalink] [raw]
Subject: [PATCH 4/4] perf probe: Add failure check when show variable range

Change improper type size_t to diffptr_t to make consistent with libdw
and handle error code.

Signed-off-by: He Kuang <[email protected]>
---
tools/perf/util/dwarf-aux.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 2cb4c82..da96eb1 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -965,7 +965,7 @@ static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
{
Dwarf_Die *scopes;
int count;
- size_t offset = 0;
+ ptrdiff_t offset = 0;
Dwarf_Addr base;
Dwarf_Addr start, end;
Dwarf_Addr entry;
@@ -991,6 +991,9 @@ static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die,

while ((offset = dwarf_ranges(&scopes[1], offset, &base,
&start, &end)) > 0) {
+ if (offset < 0)
+ goto out;
+
start -= entry;
end -= entry;

@@ -1029,7 +1032,7 @@ int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)
Dwarf_Addr entry;
Dwarf_Op *op;
size_t nops;
- size_t offset = 0;
+ ptrdiff_t offset = 0;
Dwarf_Attribute attr;
bool first = true;
const char *name;
@@ -1048,6 +1051,9 @@ int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)
while ((offset = dwarf_getlocations(
&attr, offset, &base,
&start, &end, &op, &nops)) > 0) {
+ if (offset < 0)
+ return offset;
+
if (start == 0) {
/* Single Location Descriptions */
ret = die_get_var_innermost_scope(sp_die, vr_die, buf);
--
1.8.5.2

Subject: Re: [PATCH 4/4] perf probe: Add failure check when show variable range

On 2015/07/01 18:05, He Kuang wrote:
> Change improper type size_t to diffptr_t to make consistent with libdw
> and handle error code.
>
> Signed-off-by: He Kuang <[email protected]>
> ---
> tools/perf/util/dwarf-aux.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index 2cb4c82..da96eb1 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -965,7 +965,7 @@ static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
> {
> Dwarf_Die *scopes;
> int count;
> - size_t offset = 0;
> + ptrdiff_t offset = 0;
> Dwarf_Addr base;
> Dwarf_Addr start, end;
> Dwarf_Addr entry;
> @@ -991,6 +991,9 @@ static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
>
> while ((offset = dwarf_ranges(&scopes[1], offset, &base,
> &start, &end)) > 0) {
> + if (offset < 0)
> + goto out;

Hmm, here offset always > 0, see the condition carefully.
Perhaps, this should be placed at the right after the while loop.

> +
> start -= entry;
> end -= entry;
>
> @@ -1029,7 +1032,7 @@ int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)
> Dwarf_Addr entry;
> Dwarf_Op *op;
> size_t nops;
> - size_t offset = 0;
> + ptrdiff_t offset = 0;
> Dwarf_Attribute attr;
> bool first = true;
> const char *name;
> @@ -1048,6 +1051,9 @@ int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)
> while ((offset = dwarf_getlocations(
> &attr, offset, &base,
> &start, &end, &op, &nops)) > 0) {
> + if (offset < 0)
> + return offset;

Ditto.

Thanks,

> +
> if (start == 0) {
> /* Single Location Descriptions */
> ret = die_get_var_innermost_scope(sp_die, vr_die, buf);
>


--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]

Subject: Re: [PATCH 3/4] perf probe: Enable --range option according to libdw version

On 2015/07/01 18:05, He Kuang wrote:
> The option --range uses functions in libdw (elfutils version >= 1.57),
> this patch check if elfutils version meets the requirements before
> enable this feature, so that perf can be built with old libdw.

Is that related to any libdw bug ? or you need to use newer API?
Please tell us more precise report or actual URL/bug number.

Thank you,

>
> Reported-by: Alexei Starovoitov <[email protected]>
> Signed-off-by: He Kuang <[email protected]>
> ---
> tools/perf/builtin-probe.c | 2 ++
> tools/perf/util/dwarf-aux.c | 2 ++
> tools/perf/util/dwarf-aux.h | 13 ++++++++++++-
> 3 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index 1272559..2760c06 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -372,8 +372,10 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> "Show accessible variables on PROBEDEF", opt_show_vars),
> OPT_BOOLEAN('\0', "externs", &probe_conf.show_ext_vars,
> "Show external variables too (with --vars only)"),
> +#if _ELFUTILS_PREREQ(0, 157)
> OPT_BOOLEAN('\0', "range", &probe_conf.show_location_range,
> "Show variables location range in scope (with --vars only)"),
> +#endif
> OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
> "file", "vmlinux pathname"),
> OPT_STRING('s', "source", &symbol_conf.source_prefix,
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index 57f3ef4..2cb4c82 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -950,6 +950,7 @@ int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf)
> return 0;
> }
>
> +#if _ELFUTILS_PREREQ(0, 157)
> /**
> * die_get_var_innermost_scope - Get innermost scope range of given variable DIE
> * @sp_die: a subprogram DIE
> @@ -1071,3 +1072,4 @@ int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)
>
> return ret;
> }
> +#endif /* _ELFUTILS_PREREQ(0, 157) */
> diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
> index c42ec36..ade0c50 100644
> --- a/tools/perf/util/dwarf-aux.h
> +++ b/tools/perf/util/dwarf-aux.h
> @@ -125,6 +125,17 @@ extern int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf);
>
> /* Get the name and type of given variable DIE, stored as "type\tname" */
> extern int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf);
> +#if _ELFUTILS_PREREQ(0, 157)
> extern int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
> - struct strbuf *buf);
> + struct strbuf *buf);
> +#else
> +static inline
> +int die_get_var_range(Dwarf_Die *sp_die __maybe_unused,
> + Dwarf_Die *vr_die __maybe_unused,
> + struct strbuf *buf __maybe_unused)
> +{
> + return -ENOTSUP;
> +}
> +
> +#endif /* _ELFUTILS_PREREQ(0, 157) */
> #endif
>


--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]

Subject: Re: [PATCH 2/4] perf probe: Fix failure to probe online module

On 2015/07/01 18:05, He Kuang wrote:
> Kernel module has the type DSO_TYPE_USER, the check on dso type in
> kernel_get_module_dso() disable openning debuginfo of online kernel
> modules.
>
> Problem can be reproduced as:
>
> $ insmod test_bpf.ko
>
> $ perf probe -v -m test_bpf --add='skb_is_nonlinear'
>
> probe-definition(0): skb_is_nonlinear
> symbol:skb_is_nonlinear file:(null) line:0 offset:0 return:0 lazy:(null)
> 0 arguments
> Failed to find module test_bpf.
> Could not open debuginfo. Try to use symbols.
> Failed to find symbol skb_is_nonlinear in test_bpf
> Error: Failed to add events. Reason: No such file or directory (Code: -2)
>
> Infact kernel_get_module_dso() function is called in perf-probe, in this
> condition, machine->dsos list contains only dsos which is either kmod or
> kernel loaded by machine__create_kernel_maps(), so there's no need to
> pick away the non-kmod user type dsos.
>
> This patch removes the check.
>
> After this patch:
>
> $ perf probe -v -m test_bpf --add='skb_is_nonlinear'
>
> probe-definition(0): skb_is_nonlinear
> symbol:skb_is_nonlinear file:(null) line:0 offset:0 return:0 lazy:(null)
> 0 arguments
> Open Debuginfo file: /lib/modules/4.1.0+/test_bpf.ko
> Try to find probe point from debuginfo.
> Matched function: skb_is_nonlinear
> found inline addr: 0x10ab3
> Probe point found: test_bpf_init+585
> Found 1 probe_trace_events.
> Opening /sys/kernel/debug/tracing/kprobe_events write=1
> Added new event:
> Writing event: p:probe/skb_is_nonlinear test_bpf:test_bpf_init+585
> probe:skb_is_nonlinear (on skb_is_nonlinear in test_bpf)
>
> You can now use it in all perf tools, such as:
>
> perf record -e probe:skb_is_nonlinear -aR sleep 1

Oops, OK. Without debuginfo, that always fails. I've tested on my machine.

Acked-by: Masami Hiramatsu <[email protected]>

Thank you!

>
> Signed-off-by: He Kuang <[email protected]>
> ---
> tools/perf/util/probe-event.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 381f23a..8ad2743 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -275,8 +275,6 @@ static int kernel_get_module_dso(const char *module, struct dso **pdso)
>
> if (module) {
> list_for_each_entry(dso, &host_machine->dsos.head, node) {
> - if (!dso->kernel)
> - continue;
> if (strncmp(dso->short_name + 1, module,
> dso->short_name_len - 2) == 0)
> goto found;
>


--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]

2015-07-01 11:55:06

by He Kuang

[permalink] [raw]
Subject: [PATCH v2 1/2] perf probe: Enable --range option according to libdw version

The option --range uses function dwarf_getlocations() which was first
introduced to libdw in elfutils version 0.157. Without this API,
there's no easy way to get locations ranges by the givin attribute.

This patch check if elfutils version meets the requirements before
enable this feature, so that perf can be built with old libdw.

Reported-by: Alexei Starovoitov <[email protected]>
Signed-off-by: He Kuang <[email protected]>
---
tools/perf/builtin-probe.c | 2 ++
tools/perf/util/dwarf-aux.c | 2 ++
tools/perf/util/dwarf-aux.h | 13 ++++++++++++-
3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 1272559..2760c06 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -372,8 +372,10 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
"Show accessible variables on PROBEDEF", opt_show_vars),
OPT_BOOLEAN('\0', "externs", &probe_conf.show_ext_vars,
"Show external variables too (with --vars only)"),
+#if _ELFUTILS_PREREQ(0, 157)
OPT_BOOLEAN('\0', "range", &probe_conf.show_location_range,
"Show variables location range in scope (with --vars only)"),
+#endif
OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
"file", "vmlinux pathname"),
OPT_STRING('s', "source", &symbol_conf.source_prefix,
diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 57f3ef4..2cb4c82 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -950,6 +950,7 @@ int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf)
return 0;
}

+#if _ELFUTILS_PREREQ(0, 157)
/**
* die_get_var_innermost_scope - Get innermost scope range of given variable DIE
* @sp_die: a subprogram DIE
@@ -1071,3 +1072,4 @@ int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)

return ret;
}
+#endif /* _ELFUTILS_PREREQ(0, 157) */
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index c42ec36..ade0c50 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -125,6 +125,17 @@ extern int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf);

/* Get the name and type of given variable DIE, stored as "type\tname" */
extern int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf);
+#if _ELFUTILS_PREREQ(0, 157)
extern int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
- struct strbuf *buf);
+ struct strbuf *buf);
+#else
+static inline
+int die_get_var_range(Dwarf_Die *sp_die __maybe_unused,
+ Dwarf_Die *vr_die __maybe_unused,
+ struct strbuf *buf __maybe_unused)
+{
+ return -ENOTSUP;
+}
+
+#endif /* _ELFUTILS_PREREQ(0, 157) */
#endif
--
1.8.5.2

2015-07-01 11:54:45

by He Kuang

[permalink] [raw]
Subject: [PATCH v2 2/2] perf probe: Add failure check when show variable range

Change improper type size_t to diffptr_t to make consistent with libdw
and handle error code.

Signed-off-by: He Kuang <[email protected]>
---
tools/perf/util/dwarf-aux.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 2cb4c82..7658fb4 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -965,7 +965,7 @@ static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
{
Dwarf_Die *scopes;
int count;
- size_t offset = 0;
+ ptrdiff_t offset = 0;
Dwarf_Addr base;
Dwarf_Addr start, end;
Dwarf_Addr entry;
@@ -991,6 +991,7 @@ static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die,

while ((offset = dwarf_ranges(&scopes[1], offset, &base,
&start, &end)) > 0) {
+
start -= entry;
end -= entry;

@@ -1004,6 +1005,11 @@ static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
}
}

+ if (offset < 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+
if (!first)
strbuf_addf(buf, "]>");

@@ -1029,7 +1035,7 @@ int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)
Dwarf_Addr entry;
Dwarf_Op *op;
size_t nops;
- size_t offset = 0;
+ ptrdiff_t offset = 0;
Dwarf_Attribute attr;
bool first = true;
const char *name;
@@ -1067,6 +1073,9 @@ int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)
}
}

+ if (offset < 0)
+ return -EINVAL;
+
if (!first)
strbuf_addf(buf, "]>");

--
1.8.5.2

2015-07-01 12:01:01

by He Kuang

[permalink] [raw]
Subject: Re: [PATCH 4/4] perf probe: Add failure check when show variable range

hi, Masami

On 2015/7/1 19:00, Masami Hiramatsu wrote:
> On 2015/07/01 18:05, He Kuang wrote:
>> Change improper type size_t to diffptr_t to make consistent with libdw
>> and handle error code.
>>
>> Signed-off-by: He Kuang <[email protected]>
>> ---
>> tools/perf/util/dwarf-aux.c | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
>> index 2cb4c82..da96eb1 100644
>> --- a/tools/perf/util/dwarf-aux.c
>> +++ b/tools/perf/util/dwarf-aux.c
>> @@ -965,7 +965,7 @@ static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
>> {
>> Dwarf_Die *scopes;
>> int count;
>> - size_t offset = 0;
>> + ptrdiff_t offset = 0;
>> Dwarf_Addr base;
>> Dwarf_Addr start, end;
>> Dwarf_Addr entry;
>> @@ -991,6 +991,9 @@ static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
>>
>> while ((offset = dwarf_ranges(&scopes[1], offset, &base,
>> &start, &end)) > 0) {
>> + if (offset < 0)
>> + goto out;
>
> Hmm, here offset always > 0, see the condition carefully.
> Perhaps, this should be placed at the right after the while loop.

Sorry for this silly mistake. Modified and resent with patch 3/4,
more info is added in commit message of that patch.

Thank you.

>
>> +
>> start -= entry;
>> end -= entry;
>>
>> @@ -1029,7 +1032,7 @@ int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)
>> Dwarf_Addr entry;
>> Dwarf_Op *op;
>> size_t nops;
>> - size_t offset = 0;
>> + ptrdiff_t offset = 0;
>> Dwarf_Attribute attr;
>> bool first = true;
>> const char *name;
>> @@ -1048,6 +1051,9 @@ int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)
>> while ((offset = dwarf_getlocations(
>> &attr, offset, &base,
>> &start, &end, &op, &nops)) > 0) {
>> + if (offset < 0)
>> + return offset;
>
> Ditto.
>
> Thanks,
>
>> +
>> if (start == 0) {
>> /* Single Location Descriptions */
>> ret = die_get_var_innermost_scope(sp_die, vr_die, buf);
>>
>
>