2012-08-27 07:44:05

by Feng Tang

[permalink] [raw]
Subject: [PATCH 0/3] perf tools: Fixes for some compile errors.

Hi Arnaldo and all,

I met 3 compiling problems on my x86_32 machine, and these are the fixes,
patch 1/3 only applies for perf/core branch of your git, while patch 2
applies to Linus' tree also. please review.

Thanks,
Feng

---
Feng Tang (3):
perf tools: Fix a compiling error in trace-event-perl.c for 32 bits
machine
Perf tools: Fix a compiling error in util/map.c
perf tools: Fix a misuse of for_each_set_bit() in session.c

tools/perf/util/map.c | 5 ++---
.../perf/util/scripting-engines/trace-event-perl.c | 2 +-
tools/perf/util/session.c | 9 ++++-----
3 files changed, 7 insertions(+), 9 deletions(-)


2012-08-27 07:44:06

by Feng Tang

[permalink] [raw]
Subject: [PATCH 1/3] perf tools: Fix a compiling error in trace-event-perl.c for 32 bits machine

On my x86_32 mahcine, there is a compile error:

CC util/scripting-engines/trace-event-perl.o
cc1: warnings being treated as errors
util/scripting-engines/trace-event-perl.c: In function ‘perl_process_tracepoint’:
util/scripting-engines/trace-event-perl.c:285: error: format ‘%d’ expects type ‘int’, but argument 2 has type ‘__u64’
make: *** [util/scripting-engines/trace-event-perl.o] Error 1

Fix it by using the "%lld" for __u64.

Signed-off-by: Feng Tang <[email protected]>
---
.../perf/util/scripting-engines/trace-event-perl.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/scripting-engines/trace-event-perl.c b/tools/perf/util/scripting-engines/trace-event-perl.c
index d280010..1cde6aa 100644
--- a/tools/perf/util/scripting-engines/trace-event-perl.c
+++ b/tools/perf/util/scripting-engines/trace-event-perl.c
@@ -282,7 +282,7 @@ static void perl_process_tracepoint(union perf_event *perf_event __unused,

event = find_cache_event(evsel);
if (!event)
- die("ug! no event found for type %d", evsel->attr.config);
+ die("ug! no event found for type %lld", evsel->attr.config);

pid = raw_field_value(event, "common_pid", data);

--
1.7.1

2012-08-27 07:44:57

by Feng Tang

[permalink] [raw]
Subject: [PATCH 3/3] perf tools: Fix a misuse of for_each_set_bit() in session.c

In regs_dump__printf() it use for_each_set_bit() for bit ops by
casting a (u64 *) to a (unsigned long *), this works for 64 bits
machine, but will fail on 32 bits ones.

Fix it by using the raw bit comparing method.

Signed-off-by: Feng Tang <[email protected]>
---
tools/perf/util/session.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index f7bb7ae..afcea6c 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -904,11 +904,10 @@ static void regs_dump__printf(u64 mask, u64 *regs)
{
unsigned rid, i = 0;

- for_each_set_bit(rid, (unsigned long *) &mask, sizeof(mask) * 8) {
- u64 val = regs[i++];
-
- printf(".... %-5s 0x%" PRIx64 "\n",
- perf_reg_name(rid), val);
+ for (rid = 0; rid < 64; rid++) {
+ if (mask & (1 << rid))
+ printf(".... %-5s 0x%" PRIx64 "\n",
+ perf_reg_name(rid), regs[i++]);
}
}

--
1.7.1

2012-08-27 07:44:54

by Feng Tang

[permalink] [raw]
Subject: [PATCH 2/3] Perf tools: Fix a compiling error in util/map.c

This patch fix a compile warning taken as error:

CC util/map.o
cc1: warnings being treated as errors
util/map.c: In function ‘map__fprintf_dsoname’:
util/map.c:240: error: ‘dsoname’ may be used uninitialized in this function
make: *** [util/map.o] Error 1

Signed-off-by: Feng Tang <[email protected]>
---
tools/perf/util/map.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 7d37159..5f90dda 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -237,15 +237,14 @@ size_t map__fprintf(struct map *self, FILE *fp)

size_t map__fprintf_dsoname(struct map *map, FILE *fp)
{
- const char *dsoname;
+ const char *dsoname = "[unknown]";

if (map && map->dso && (map->dso->name || map->dso->long_name)) {
if (symbol_conf.show_kernel_path && map->dso->long_name)
dsoname = map->dso->long_name;
else if (map->dso->name)
dsoname = map->dso->name;
- } else
- dsoname = "[unknown]";
+ }

return fprintf(fp, "%s", dsoname);
}
--
1.7.1

2012-08-27 08:06:30

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH 3/3] perf tools: Fix a misuse of for_each_set_bit() in session.c

Hi,

On Mon, 27 Aug 2012 15:38:27 +0800, Feng Tang wrote:
> In regs_dump__printf() it use for_each_set_bit() for bit ops by
> casting a (u64 *) to a (unsigned long *), this works for 64 bits
> machine, but will fail on 32 bits ones.
>
> Fix it by using the raw bit comparing method.

Did it really cause a build failure or a program error? If not, it
looks better to keep using for_each_set_bit() interface. How about
casting the @mask to (void *) ?

Thanks,
Namhyung


>
> Signed-off-by: Feng Tang <[email protected]>
> ---
> tools/perf/util/session.c | 9 ++++-----
> 1 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index f7bb7ae..afcea6c 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -904,11 +904,10 @@ static void regs_dump__printf(u64 mask, u64 *regs)
> {
> unsigned rid, i = 0;
>
> - for_each_set_bit(rid, (unsigned long *) &mask, sizeof(mask) * 8) {
> - u64 val = regs[i++];
> -
> - printf(".... %-5s 0x%" PRIx64 "\n",
> - perf_reg_name(rid), val);
> + for (rid = 0; rid < 64; rid++) {
> + if (mask & (1 << rid))
> + printf(".... %-5s 0x%" PRIx64 "\n",
> + perf_reg_name(rid), regs[i++]);
> }
> }

2012-08-27 08:23:52

by Feng Tang

[permalink] [raw]
Subject: Re: [PATCH 3/3] perf tools: Fix a misuse of for_each_set_bit() in session.c

Hi,

On Mon, 27 Aug 2012 16:59:07 +0900
Namhyung Kim <[email protected]> wrote:

> Hi,
>
> On Mon, 27 Aug 2012 15:38:27 +0800, Feng Tang wrote:
> > In regs_dump__printf() it use for_each_set_bit() for bit ops by
> > casting a (u64 *) to a (unsigned long *), this works for 64 bits
> > machine, but will fail on 32 bits ones.
> >
> > Fix it by using the raw bit comparing method.
>
> Did it really cause a build failure or a program error? If not, it
> looks better to keep using for_each_set_bit() interface. How about
> casting the @mask to (void *) ?

It's a compile error:

cc1: warnings being treated as errors
util/session.c: In function ‘perf_session_deliver_event’:
util/include/linux/bitops.h:104: error: dereferencing pointer ‘p’ does break strict-aliasing rules
util/include/linux/bitops.h:96: error: dereferencing pointer ‘p’ does break strict-aliasing rules
util/session.c:907: note: initialized from here
util/include/linux/bitops.h:96: note: initialized from here
make: *** [util/session.o] Error 1
make: *** Waiting for unfinished jobs....

Casting the @mask to (void *) still sees the same error

Thanks,
Feng

2012-08-27 15:34:26

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 1/3] perf tools: Fix a compiling error in trace-event-perl.c for 32 bits machine

Em Mon, Aug 27, 2012 at 03:38:25PM +0800, Feng Tang escreveu:
> On my x86_32 mahcine, there is a compile error:
>
> CC util/scripting-engines/trace-event-perl.o
> cc1: warnings being treated as errors
> util/scripting-engines/trace-event-perl.c: In function ‘perl_process_tracepoint’:
> util/scripting-engines/trace-event-perl.c:285: error: format ‘%d’ expects type ‘int’, but argument 2 has type ‘__u64’
> make: *** [util/scripting-engines/trace-event-perl.o] Error 1
>
> Fix it by using the "%lld" for __u64.
>
> Signed-off-by: Feng Tang <[email protected]>
> ---
> .../perf/util/scripting-engines/trace-event-perl.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/tools/perf/util/scripting-engines/trace-event-perl.c b/tools/perf/util/scripting-engines/trace-event-perl.c
> index d280010..1cde6aa 100644
> --- a/tools/perf/util/scripting-engines/trace-event-perl.c
> +++ b/tools/perf/util/scripting-engines/trace-event-perl.c
> @@ -282,7 +282,7 @@ static void perl_process_tracepoint(union perf_event *perf_event __unused,
>
> event = find_cache_event(evsel);
> if (!event)
> - die("ug! no event found for type %d", evsel->attr.config);
> + die("ug! no event found for type %lld", evsel->attr.config);

Please use PRIu64 instead, like in:

grep PRIu64 tools/perf/*/*.c

> pid = raw_field_value(event, "common_pid", data);
>
> --
> 1.7.1

2012-08-28 02:23:09

by Feng Tang

[permalink] [raw]
Subject: [PATCH v2 1/3] perf tools: Fix a compiling error in trace-event-perl.c for 32 bits machine

>From 8e2fe258906fc83869be10a1b4a40ef9acc4a2a6 Mon Sep 17 00:00:00 2001
From: Feng Tang <[email protected]>
Date: Mon, 27 Aug 2012 14:01:33 +0800
Subject: [PATCH 1/3] perf tools: Fix a compiling error in trace-event-perl.c for 32 bits machine

On my x86_32 mahcine, there is a compile error:

CC util/scripting-engines/trace-event-perl.o
cc1: warnings being treated as errors
util/scripting-engines/trace-event-perl.c: In function ‘perl_process_tracepoint’:
util/scripting-engines/trace-event-perl.c:285: error: format ‘%d’ expects type ‘int’, but argument 2 has type ‘__u64’
make: *** [util/scripting-engines/trace-event-perl.o] Error 1

Fix it by using the "%PRIu64" for __u64.

v2: use PRIu64 as suggested by Arnaldo.

Signed-off-by: Feng Tang <[email protected]>
---
.../perf/util/scripting-engines/trace-event-perl.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/scripting-engines/trace-event-perl.c b/tools/perf/util/scripting-engines/trace-event-perl.c
index d280010..09db03f 100644
--- a/tools/perf/util/scripting-engines/trace-event-perl.c
+++ b/tools/perf/util/scripting-engines/trace-event-perl.c
@@ -282,7 +282,7 @@ static void perl_process_tracepoint(union perf_event *perf_event __unused,

event = find_cache_event(evsel);
if (!event)
- die("ug! no event found for type %d", evsel->attr.config);
+ die("ug! no event found for type %" PRIu64, evsel->attr.config);

pid = raw_field_value(event, "common_pid", data);

--
1.7.1