perf pmu: clang points out: address of array 'alias->unit' will always evaluate to 'true'

perf pmu: clang points out: address of array 'alias->unit' will always evaluate to 'true'

[Arnaldo Carvalho de Melo]

util/pmu.c:948:28: error: address of array 'alias->unit' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
        if ((info->unit && alias->unit) ||
                        ~~ ~~~~~~~^~~~
util/pmu.c:953:13: error: address of array 'alias->unit' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
        if (alias->unit)
        ~~  ~~~~~~~^~~~
2 errors generated.


So, is this test about having something on that alias->unit array? I.e.
should this suffice?

[acme@jouet linux]$ cat clang.patch 
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 82a654dec666..49bfee0e3d9e 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -945,12 +945,12 @@ static int check_info_data(struct perf_pmu_alias *alias,
 	 * define unit, scale and snapshot, fail
 	 * if there's more than one.
 	 */
-	if ((info->unit && alias->unit) ||
+	if ((info->unit && alias->unit[0]) ||
 	    (info->scale && alias->scale) ||
 	    (info->snapshot && alias->snapshot))
 		return -EINVAL;
 
-	if (alias->unit)
+	if (alias->unit[0])
 		info->unit = alias->unit;
 
 	if (alias->scale)
[acme@jouet linux]$ 

Re: perf pmu: clang points out: address of array 'alias->unit' will always evaluate to 'true'

[Jiri Olsa]

On Tue, Feb 14, 2017 at 03:24:35PM -0300, Arnaldo Carvalho de Melo wrote:
> util/pmu.c:948:28: error: address of array 'alias->unit' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
>         if ((info->unit && alias->unit) ||
>                         ~~ ~~~~~~~^~~~
> util/pmu.c:953:13: error: address of array 'alias->unit' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
>         if (alias->unit)
>         ~~  ~~~~~~~^~~~
> 2 errors generated.
> 
> 
> So, is this test about having something on that alias->unit array? I.e.
> should this suffice?

yep, that seems right.. good catch

thanks,
jirka

> 
> [acme@jouet linux]$ cat clang.patch 
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 82a654dec666..49bfee0e3d9e 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -945,12 +945,12 @@ static int check_info_data(struct perf_pmu_alias *alias,
>  	 * define unit, scale and snapshot, fail
>  	 * if there's more than one.
>  	 */
> -	if ((info->unit && alias->unit) ||
> +	if ((info->unit && alias->unit[0]) ||
>  	    (info->scale && alias->scale) ||
>  	    (info->snapshot && alias->snapshot))
>  		return -EINVAL;
>  
> -	if (alias->unit)
> +	if (alias->unit[0])
>  		info->unit = alias->unit;
>  
>  	if (alias->scale)
> [acme@jouet linux]$ 

perf bison: clang points out: error: conflicting types for 'parse_events_error' always evaluate to 'true'

[Arnaldo Carvalho de Melo]

Em Wed, Feb 15, 2017 at 11:50:56AM +0100, Jiri Olsa escreveu:
> On Tue, Feb 14, 2017 at 03:24:35PM -0300, Arnaldo Carvalho de Melo wrote:
> > util/pmu.c:948:28: error: address of array 'alias->unit' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
> >         if ((info->unit && alias->unit) ||
> >                         ~~ ~~~~~~~^~~~
> > util/pmu.c:953:13: error: address of array 'alias->unit' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
> >         if (alias->unit)
> >         ~~  ~~~~~~~^~~~
> > 2 errors generated.
> > 
> > 
> > So, is this test about having something on that alias->unit array? I.e.
> > should this suffice?
> 
> yep, that seems right.. good catch

Thanks, adding your Acked-by then, ok?

How about this other one:

  CC       /tmp/build/perf/util/trace-event-info.o
util/parse-events.y:699:6: error: conflicting types for 'parse_events_error'
void parse_events_error(YYLTYPE *loc, void *data,
     ^
/tmp/build/perf/util/parse-events-bison.c:2224:7: note: previous implicit declaration is here
      yyerror (&yylloc, _data, scanner, YY_("syntax error"));
      ^
/tmp/build/perf/util/parse-events-bison.c:65:25: note: expanded from macro 'yyerror'
#define yyerror         parse_events_error
                        ^
1 error generated.

Re: perf bison: clang points out: error: conflicting types for 'parse_events_error' always evaluate to 'true'

[Jiri Olsa]

On Wed, Feb 15, 2017 at 09:41:02AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Feb 15, 2017 at 11:50:56AM +0100, Jiri Olsa escreveu:
> > On Tue, Feb 14, 2017 at 03:24:35PM -0300, Arnaldo Carvalho de Melo wrote:
> > > util/pmu.c:948:28: error: address of array 'alias->unit' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
> > >         if ((info->unit && alias->unit) ||
> > >                         ~~ ~~~~~~~^~~~
> > > util/pmu.c:953:13: error: address of array 'alias->unit' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
> > >         if (alias->unit)
> > >         ~~  ~~~~~~~^~~~
> > > 2 errors generated.
> > > 
> > > 
> > > So, is this test about having something on that alias->unit array? I.e.
> > > should this suffice?
> > 
> > yep, that seems right.. good catch
> 
> Thanks, adding your Acked-by then, ok?

yes

> 
> How about this other one:
> 
>   CC       /tmp/build/perf/util/trace-event-info.o
> util/parse-events.y:699:6: error: conflicting types for 'parse_events_error'
> void parse_events_error(YYLTYPE *loc, void *data,
>      ^
> /tmp/build/perf/util/parse-events-bison.c:2224:7: note: previous implicit declaration is here
>       yyerror (&yylloc, _data, scanner, YY_("syntax error"));
>       ^
> /tmp/build/perf/util/parse-events-bison.c:65:25: note: expanded from macro 'yyerror'
> #define yyerror         parse_events_error
>                         ^
> 1 error generated.
> 

hum, thats generated code.. I guess we need appropriate -Wno-... option ;-)

jirka

perf build with clang, modulo libpython: Re: perf bison: clang points out: error: conflicting types for 'parse_events_error' always evaluate to 'true'

[Arnaldo Carvalho de Melo]

Em Wed, Feb 15, 2017 at 01:52:23PM +0100, Jiri Olsa escreveu:
> On Wed, Feb 15, 2017 at 09:41:02AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Feb 15, 2017 at 11:50:56AM +0100, Jiri Olsa escreveu:
> > > On Tue, Feb 14, 2017 at 03:24:35PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > So, is this test about having something on that alias->unit array? I.e.
> > > > should this suffice?

> > > yep, that seems right.. good catch

> > Thanks, adding your Acked-by then, ok?
> 
> yes
 
Thanks!

> > How about this other one:

> > util/parse-events.y:699:6: error: conflicting types for 'parse_events_error'
> > void parse_events_error(YYLTYPE *loc, void *data,
> >      ^
> > /tmp/build/perf/util/parse-events-bison.c:2224:7: note: previous implicit declaration is here
> >       yyerror (&yylloc, _data, scanner, YY_("syntax error"));
> >       ^
> > /tmp/build/perf/util/parse-events-bison.c:65:25: note: expanded from macro 'yyerror'
> > #define yyerror         parse_events_error

> > 1 error generated.

> hum, thats generated code.. I guess we need appropriate -Wno-... option ;-)

That would be too easy, don't shot the messenger :-)

Fix at the end of this message, please ack, with it I managed to build tools/perf using
clang, albeit still with NO_LIBPYTHON=1:

$ readelf -wi ~/bin/perf  | head
Contents of the .debug_info section:

  Compilation Unit @ offset 0x0:
   Length:        0x828 (32-bit)
   Version:       4
   Abbrev Offset: 0x0
   Pointer Size:  8
 <0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
    <c>   DW_AT_producer    : (indirect string, offset: 0x0): clang version 4.0.0 (http://llvm.org/git/clang.git f5be8ba13adc4ba1011a7ccd60c844bd60427c1c) (http://llvm.org/git/llvm.git efca1a37676f4cd276d947658cf90b0fb625abfd)
    <10>   DW_AT_language    : 12	(ANSI C99)
$ 

$ ls -la ~/bin/perf
-rwxr-xr-x. 2 acme acme 10029688 Feb 15 09:57 /home/acme/bin/perf
$ size ~/bin/perf
   text	   data	    bss	    dec	    hex	filename
3447514	 831320	23901696	28180530	1ae0032	/home/acme/bin/perf
$ strip ~/bin/perf
$ ls -la ~/bin/perf
-rwxr-xr-x. 2 acme acme 4284968 Feb 15 10:00 /home/acme/bin/perf
$

Compare with with gcc, also with NO_LIBPYTHON=1:

[acme@jouet linux]$ readelf -wi ~/bin/perf  | head
Contents of the .debug_info section:

  Compilation Unit @ offset 0x0:
   Length:        0x1d43 (32-bit)
   Version:       4
   Abbrev Offset: 0x0
   Pointer Size:  8
 <0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
    <c>   DW_AT_producer    : (indirect string, offset: 0x1b27c): GNU C99 6.3.1 20161221 (Red Hat 6.3.1-1) -mtune=generic -march=x86-64 -ggdb3 -O6 -std=gnu99 -fno-omit-frame-pointer -funwind-tables -fstack-protector-all
    <10>   DW_AT_language    : 12	(ANSI C99)
[acme@jouet linux]$ ls -la ~/bin/perf
-rwxr-xr-x. 2 acme acme 16948144 Feb 15 10:02 /home/acme/bin/perf
[acme@jouet linux]$ size ~/bin/perf
   text	   data	    bss	    dec	    hex	filename
3671662	 836480	23902752	28410894	1b1840e	/home/acme/bin/perf
[acme@jouet linux]$ strip ~/bin/perf
[acme@jouet linux]$ ls -la ~/bin/perf
-rwxr-xr-x. 2 acme acme 4515280 Feb 15 10:02 /home/acme/bin/perf
[acme@jouet linux]$

to build with libpython I need to filter out the -spec=bla/bla/bla/bla
option generated automatically somewhere:

clang-4.0: error: argument unused during compilation: '-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1' [-Werror,-Wunused-command-line-argument]
clang-4.0: error: argument unused during compilation: '-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1' [-Werror,-Wunused-command-line-argument]
error: command 'clang' failed with exit status 1
cp: cannot stat '/tmp/build/perf/python_ext_build/lib/perf.so': No such file or directory
Makefile.perf:381: recipe for target '/tmp/build/perf/python/perf.so' failed
make[2]: *** [/tmp/build/perf/python/perf.so] Error 1
make[2]: *** Waiting for unfinished jobs....

---------------------------------------------------


diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 3a5196380609..a14b47ab3879 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -17,6 +17,8 @@
 #include "parse-events.h"
 #include "parse-events-bison.h"
 
+void parse_events_error(YYLTYPE *loc, void *data, void *scanner, char const *msg);
+
 #define ABORT_ON(val) \
 do { \
 	if (val) \

Re: perf build with clang, modulo libpython: Re: perf bison: clang points out: error: conflicting types for 'parse_events_error' always evaluate to 'true'

[Jiri Olsa]

On Wed, Feb 15, 2017 at 10:06:05AM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

> 
> clang-4.0: error: argument unused during compilation: '-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1' [-Werror,-Wunused-command-line-argument]
> clang-4.0: error: argument unused during compilation: '-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1' [-Werror,-Wunused-command-line-argument]
> error: command 'clang' failed with exit status 1
> cp: cannot stat '/tmp/build/perf/python_ext_build/lib/perf.so': No such file or directory
> Makefile.perf:381: recipe for target '/tmp/build/perf/python/perf.so' failed
> make[2]: *** [/tmp/build/perf/python/perf.so] Error 1
> make[2]: *** Waiting for unfinished jobs....
> 
> ---------------------------------------------------
> 
> 
> diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
> index 3a5196380609..a14b47ab3879 100644
> --- a/tools/perf/util/parse-events.y
> +++ b/tools/perf/util/parse-events.y
> @@ -17,6 +17,8 @@
>  #include "parse-events.h"
>  #include "parse-events-bison.h"
>  
> +void parse_events_error(YYLTYPE *loc, void *data, void *scanner, char const *msg);
> +
>  #define ABORT_ON(val) \
>  do { \
>  	if (val) \

if that helps.. seems harmless ;-) ack

jirka

[tip:perf/core] perf pmu: Fix check for unset alias->unit array

[tip-bot for Arnaldo Carvalho de Melo]

Commit-ID:  b30a7d1fc96d60fe2c02e375a56a046385701d17
Gitweb:     http://git.kernel.org/tip/b30a7d1fc96d60fe2c02e375a56a046385701d17
Author:     Arnaldo Carvalho de Melo <acme@redhat.com>
AuthorDate: Wed, 15 Feb 2017 10:06:20 -0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 15 Feb 2017 10:06:20 -0300

perf pmu: Fix check for unset alias->unit array

The alias->unit field is an array, so to check that it is not set we
should see if it is an empty string, i.e. alias->unit[0], instead of
checking alias->unit != NULL, as this will _always_ evaluate to 'true'.

Pointed out by clang.

Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/20170214182435.GD4458@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/pmu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 82a654de..49bfee0 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -945,12 +945,12 @@ static int check_info_data(struct perf_pmu_alias *alias,
 	 * define unit, scale and snapshot, fail
 	 * if there's more than one.
 	 */
-	if ((info->unit && alias->unit) ||
+	if ((info->unit && alias->unit[0]) ||
 	    (info->scale && alias->scale) ||
 	    (info->snapshot && alias->snapshot))
 		return -EINVAL;
 
-	if (alias->unit)
+	if (alias->unit[0])
 		info->unit = alias->unit;
 
 	if (alias->scale)

[tip:perf/core] perf tools: Add missing parse_events_error() prototype

[tip-bot for Arnaldo Carvalho de Melo]

Commit-ID:  34a0548f01626b01c9e98d9627812c3c9f6b6f7d
Gitweb:     http://git.kernel.org/tip/34a0548f01626b01c9e98d9627812c3c9f6b6f7d
Author:     Arnaldo Carvalho de Melo <acme@redhat.com>
AuthorDate: Wed, 15 Feb 2017 10:09:11 -0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 15 Feb 2017 11:20:49 -0300

perf tools: Add missing parse_events_error() prototype

As pointed out by clang, we were not providing a prototype for a
function before using it:

  util/parse-events.y:699:6: error: conflicting types for 'parse_events_error'
  void parse_events_error(YYLTYPE *loc, void *data,
       ^
  /tmp/build/perf/util/parse-events-bison.c:2224:7: note: previous implicit declaration is here
        yyerror (&yylloc, _data, scanner, YY_("syntax error"));
        ^
  /tmp/build/perf/util/parse-events-bison.c:65:25: note: expanded from macro 'yyerror'
  #define yyerror         parse_events_error

  1 error generated.

One line fix it.

Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/20170215130605.GC4020@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.y | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 3a51963..a14b47a 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -17,6 +17,8 @@
 #include "parse-events.h"
 #include "parse-events-bison.h"
 
+void parse_events_error(YYLTYPE *loc, void *data, void *scanner, char const *msg);
+
 #define ABORT_ON(val) \
 do { \
 	if (val) \