[PATCH v4] tools lib traceevent: str addresses in heterogeneous arch environments

[PATCH v4] tools lib traceevent: str addresses in heterogeneous arch environments

[Kapileshwar Singh]

When a trace recorded on a 32-bit device is processed with a 64-bit
binary, the higher 32-bits of the address need to ignored

The lack of this results in the output of the 64-bit pointer
value to the trace as the 32-bit address lookup fails in find_printk.

Before:
burn-1778  [003]   548.600305: bputs:   0xc0046db2s: 2cec5c058d98c

After:
burn-1778  [003]   548.600305: bputs:   0xc0046db2s: RT throttling activated

The problem occurs in PRINT_FEILD when the field is recognized as a pointer
to a string (of the type const char *)

Heterogeneous architectures cases below can arise and should be handled:

* Traces recorded using 32-bit addresses processed on a 64-bit machine
* Traces recorded using 64-bit addresses processed on a 32-bit machine

Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Javi Merino <javi.merino@arm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Reported-by: Juri Lelli <juri.lelli@arm.com>
Signed-off-by: Kapileshwar Singh <kapileshwar.singh@arm.com>
---
 tools/lib/traceevent/event-parse.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index cc25f059ab3d..e622d2efeccf 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -3721,7 +3721,7 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
 	struct format_field *field;
 	struct printk_map *printk;
 	long long val, fval;
-	unsigned long addr;
+	unsigned long long addr;
 	char *str;
 	unsigned char *hex;
 	int print;
@@ -3754,13 +3754,31 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
 		 */
 		if (!(field->flags & FIELD_IS_ARRAY) &&
 		    field->size == pevent->long_size) {
-			addr = *(unsigned long *)(data + field->offset);
+
+			/* Handle heterogeneous recording and processing
+			 * architectures
+			 *
+			 * CASE I:
+			 * Traces recorded on 32-bit devices (32-bit
+			 * addressing) and processed on 64-bit devices:
+			 * In this case, the higher 32-bits of the address
+			 * need to be ignored.
+			 *
+			 * CASE II:
+			 * Traces recorded on 64 bit devices and processed
+			 * on 32-bit devices. In this case 64 bits must be
+			 * read.
+			 */
+			addr = (pevent->long_size == 8) ?
+				*(unsigned long long *)(data + field->offset) :
+				(unsigned long long)*(unsigned int *)(data + field->offset);
+
 			/* Check if it matches a print format */
 			printk = find_printk(pevent, addr);
 			if (printk)
 				trace_seq_puts(s, printk->printk);
 			else
-				trace_seq_printf(s, "%lx", addr);
+				trace_seq_printf(s, "%llx", addr);
 			break;
 		}
 		str = malloc(len + 1);
-- 
1.9.1

Re: [PATCH v4] tools lib traceevent: str addresses in heterogeneous arch environments

[Steven Rostedt]

On Mon, 21 Sep 2015 15:26:23 +0100
Kapileshwar Singh <kapileshwar.singh@arm.com> wrote:

> When a trace recorded on a 32-bit device is processed with a 64-bit
> binary, the higher 32-bits of the address need to ignored
> 
> The lack of this results in the output of the 64-bit pointer
> value to the trace as the 32-bit address lookup fails in find_printk.
> 
> Before:
> burn-1778  [003]   548.600305: bputs:   0xc0046db2s: 2cec5c058d98c
> 
> After:
> burn-1778  [003]   548.600305: bputs:   0xc0046db2s: RT throttling activated
> 
> The problem occurs in PRINT_FEILD when the field is recognized as a pointer
> to a string (of the type const char *)
> 
> Heterogeneous architectures cases below can arise and should be handled:
> 
> * Traces recorded using 32-bit addresses processed on a 64-bit machine
> * Traces recorded using 64-bit addresses processed on a 32-bit machine
> 
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Javi Merino <javi.merino@arm.com>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Reported-by: Juri Lelli <juri.lelli@arm.com>
> Signed-off-by: Kapileshwar Singh <kapileshwar.singh@arm.com>
> ---
>  tools/lib/traceevent/event-parse.c | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> index cc25f059ab3d..e622d2efeccf 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -3721,7 +3721,7 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
>  	struct format_field *field;
>  	struct printk_map *printk;
>  	long long val, fval;
> -	unsigned long addr;
> +	unsigned long long addr;
>  	char *str;
>  	unsigned char *hex;
>  	int print;
> @@ -3754,13 +3754,31 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
>  		 */
>  		if (!(field->flags & FIELD_IS_ARRAY) &&
>  		    field->size == pevent->long_size) {
> -			addr = *(unsigned long *)(data + field->offset);
> +
> +			/* Handle heterogeneous recording and processing
> +			 * architectures
> +			 *
> +			 * CASE I:
> +			 * Traces recorded on 32-bit devices (32-bit
> +			 * addressing) and processed on 64-bit devices:
> +			 * In this case, the higher 32-bits of the address
> +			 * need to be ignored.

I just re-read this, and realized that it's not quite accurate. It
should say something like "In this case, only 32 bits should be read.",
as the above describes more of Namhyung's version which was incorrect.

-- Steve


> +			 *
> +			 * CASE II:
> +			 * Traces recorded on 64 bit devices and processed
> +			 * on 32-bit devices. In this case 64 bits must be
> +			 * read.
> +			 */
> +			addr = (pevent->long_size == 8) ?
> +				*(unsigned long long *)(data + field->offset) :
> +				(unsigned long long)*(unsigned int *)(data + field->offset);
> +
>  			/* Check if it matches a print format */
>  			printk = find_printk(pevent, addr);
>  			if (printk)
>  				trace_seq_puts(s, printk->printk);
>  			else
> -				trace_seq_printf(s, "%lx", addr);
> +				trace_seq_printf(s, "%llx", addr);
>  			break;
>  		}
>  		str = malloc(len + 1);

Re: [PATCH v4] tools lib traceevent: str addresses in heterogeneous arch environments

[Kapileshwar Singh]

Hi Steve,

On 21/09/15 16:02, Steven Rostedt wrote:
> On Mon, 21 Sep 2015 15:26:23 +0100
> Kapileshwar Singh <kapileshwar.singh@arm.com> wrote:
> 
>> When a trace recorded on a 32-bit device is processed with a 64-bit
>> binary, the higher 32-bits of the address need to ignored
>>
>> The lack of this results in the output of the 64-bit pointer
>> value to the trace as the 32-bit address lookup fails in find_printk.
>>
>> Before:
>> burn-1778  [003]   548.600305: bputs:   0xc0046db2s: 2cec5c058d98c
>>
>> After:
>> burn-1778  [003]   548.600305: bputs:   0xc0046db2s: RT throttling activated
>>
>> The problem occurs in PRINT_FEILD when the field is recognized as a pointer
>> to a string (of the type const char *)
>>
>> Heterogeneous architectures cases below can arise and should be handled:
>>
>> * Traces recorded using 32-bit addresses processed on a 64-bit machine
>> * Traces recorded using 64-bit addresses processed on a 32-bit machine
>>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Javi Merino <javi.merino@arm.com>
>> Cc: David Ahern <dsahern@gmail.com>
>> Cc: Jiri Olsa <jolsa@kernel.org>
>> Reported-by: Juri Lelli <juri.lelli@arm.com>
>> Signed-off-by: Kapileshwar Singh <kapileshwar.singh@arm.com>
>> ---
>>  tools/lib/traceevent/event-parse.c | 24 +++++++++++++++++++++---
>>  1 file changed, 21 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
>> index cc25f059ab3d..e622d2efeccf 100644
>> --- a/tools/lib/traceevent/event-parse.c
>> +++ b/tools/lib/traceevent/event-parse.c
>> @@ -3721,7 +3721,7 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
>>  	struct format_field *field;
>>  	struct printk_map *printk;
>>  	long long val, fval;
>> -	unsigned long addr;
>> +	unsigned long long addr;
>>  	char *str;
>>  	unsigned char *hex;
>>  	int print;
>> @@ -3754,13 +3754,31 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
>>  		 */
>>  		if (!(field->flags & FIELD_IS_ARRAY) &&
>>  		    field->size == pevent->long_size) {
>> -			addr = *(unsigned long *)(data + field->offset);
>> +
>> +			/* Handle heterogeneous recording and processing
>> +			 * architectures
>> +			 *
>> +			 * CASE I:
>> +			 * Traces recorded on 32-bit devices (32-bit
>> +			 * addressing) and processed on 64-bit devices:
>> +			 * In this case, the higher 32-bits of the address
>> +			 * need to be ignored.
> 
> I just re-read this, and realized that it's not quite accurate. It
> should say something like "In this case, only 32 bits should be read.",
> as the above describes more of Namhyung's version which was incorrect.

Will correct the comment and send an updated patch.

Regards, 
KP
> 
> -- Steve
> 
> 
>> +			 *
>> +			 * CASE II:
>> +			 * Traces recorded on 64 bit devices and processed
>> +			 * on 32-bit devices. In this case 64 bits must be
>> +			 * read.
>> +			 */
>> +			addr = (pevent->long_size == 8) ?
>> +				*(unsigned long long *)(data + field->offset) :
>> +				(unsigned long long)*(unsigned int *)(data + field->offset);
>> +
>>  			/* Check if it matches a print format */
>>  			printk = find_printk(pevent, addr);
>>  			if (printk)
>>  				trace_seq_puts(s, printk->printk);
>>  			else
>> -				trace_seq_printf(s, "%lx", addr);
>> +				trace_seq_printf(s, "%llx", addr);
>>  			break;
>>  		}
>>  		str = malloc(len + 1);
>