Re: [PATCH v2 2/4] perf probe: Fix list result when address is zero

["Wangnan (F)" <wangnan0@huawei.com>]

On 2015/8/26 17:29, 平松雅巳 / HIRAMATU，MASAMI wrote:
>> From: Wang Nan [mailto:wangnan0@huawei.com]
>>
>> When manually added uprobe point with zero address, 'perf probe -l'
>> reports error. For example:
>>
>>   # echo p:probe_libc/abs_0 /path/to/lib.bin:0x0 arg1=%ax > \
>>             /sys/kernel/debug/tracing/uprobe_events
>>
>>   # perf probe -l
>>   Error: Failed to show event list.
>>
>> Probing at 0x0 is possible and useful when lib.bin is not a normal
>> shared object but is manually mapped. However, in this case kernel
>> report:
>>
>>   # cat /sys/kernel/debug/tracing/uprobe_events
>>   p:probe_libc/abs_0 /tmp/oxygen_root/lib64/libc-2.18.so:0x          (null) arg1=%ax
>>
>> This patch supports the above kernel output.
> Former part is OK to me.
>
>> In addition, this patch also fixes convert_to_perf_probe_point() that,
>> if failed to find probe point from both of dwarf and map, in all cases
>> pp->function should be filled with a strdup() result, or the following
>> ENOMEM error code is incorrect. Remove !tp->module && !is_kprobe
>> condition so always use address to build function name if symbol is not
>> found.
> No, I think this should be included in 3/4, since until that perf probe
> doesn't support absolute address on kernel.

This is for manually introduced probing points.

convert_to_perf_probe_point is called by:

perf probe -l -> show_perf_probe_events -> __show_perf_probe_events --> convert_to_perf_probe_event

If there is a manually inserted probing point there, 'perf probe -l' reports an error.

Here is the result if I remove the second part:

$ git log --oneline  # I'm in 2/4
aae2c2c perf probe: Fix list result when address is zero
931dff5 perf probe: Prevent segfault when reading probe point with 
absolute address
...

$ git diff    # remove the second part
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 926bcec..de2c27e 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1888,7 +1888,7 @@ static int convert_to_perf_probe_point(struct 
probe_trace_point *tp,
         if (tp->symbol) {
                 pp->function = strdup(tp->symbol);
                 pp->offset = tp->offset;
-       } else {
+       } else if (!tp->module && !is_kprobe) {
                 ret = e_snprintf(buf, 128, "0x%" PRIx64, (u64)tp->address);
                 if (ret < 0)
                         return ret;


...
# echo 'p:probe_libc/abs_5 /lib64/libc.so.6:0x5' > 
/sys/kernel/debug/tracing/uprobe_events
# cat /sys/kernel/debug/tracing/uprobe_events
  p:probe_libc/abs_5 /lib64/libc.so.6:0x0000000000000005
# perf probe -l
   Error: Failed to show event list

Then add it back:

$ git reset --hard HEAD
...
# perf probe -l
  probe_libc:abs_5     (on 0x5 in /lib64/libc.so.6)

Thank you.

> Thank you,
>
>> Signed-off-by: Wang Nan <wangnan0@huawei.com>
>> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> ---
>>   tools/perf/util/probe-event.c | 30 ++++++++++++++++++++++++++----
>>   1 file changed, 26 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>> index ef325a2..3ce223d 100644
>> --- a/tools/perf/util/probe-event.c
>> +++ b/tools/perf/util/probe-event.c
>> @@ -1521,9 +1521,31 @@ int parse_probe_trace_command(const char *cmd, struct probe_trace_event *tev)
>>   	} else
>>   		p = argv[1];
>>   	fmt1_str = strtok_r(p, "+", &fmt);
>> -	if (fmt1_str[0] == '0')	/* only the address started with 0x */
>> -		tp->address = strtoul(fmt1_str, NULL, 0);
>> -	else {
>> +	/* only the address started with 0x */
>> +	if (fmt1_str[0] == '0')	{
>> +		/*
>> +		 * Fix a special case:
>> +		 * if address == 0, kernel reports something like:
>> +		 * p:probe_libc/abs_0 /lib/libc-2.18.so:0x          (null) arg1=%ax
>> +		 * Newer kernel may fix that, but we want to
>> +		 * support old kernel also.
>> +		 */
>> +		if (strcmp(fmt1_str, "0x") == 0) {
>> +			if (!argv[2] || strcmp(argv[2], "(null)")) {
>> +				ret = -EINVAL;
>> +				goto out;
>> +			}
>> +			tp->address = 0;
>> +
>> +			free(argv[2]);
>> +			for (i = 2; argv[i + 1] != NULL; i++)
>> +				argv[i] = argv[i + 1];
>> +
>> +			argv[i] = NULL;
>> +			argc -= 1;
>> +		} else
>> +			tp->address = strtoul(fmt1_str, NULL, 0);
>> +	} else {
>>   		/* Only the symbol-based probe has offset */
>>   		tp->symbol = strdup(fmt1_str);
>>   		if (tp->symbol == NULL) {
>> @@ -1868,7 +1890,7 @@ static int convert_to_perf_probe_point(struct probe_trace_point *tp,
>>   	if (tp->symbol) {
>>   		pp->function = strdup(tp->symbol);
>>   		pp->offset = tp->offset;
>> -	} else if (!tp->module && !is_kprobe) {
>> +	} else {
>>   		ret = e_snprintf(buf, 128, "0x%" PRIx64, (u64)tp->address);
>>   		if (ret < 0)
>>   			return ret;
>> --
>> 1.8.3.4