linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] perf: fix wrong DEBUG configuration
@ 2015-05-19 13:41 Martin Liška
  2015-05-19 14:04 ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 26+ messages in thread
From: Martin Liška @ 2015-05-19 13:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Paul Mackerras, Peter Zijlstra

Following patch is fix for wrong DEBUG configuration.

Signed-off-by: Martin Liska <mliska@suse.cz>
---
  tools/perf/arch/common.c            | 2 +-
  tools/perf/config/Makefile          | 2 +-
  tools/perf/util/symbol.c            | 2 +-
  tools/perf/util/trace-event-parse.c | 2 +-
  4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
index 49776f1..b7bb42c 100644
--- a/tools/perf/arch/common.c
+++ b/tools/perf/arch/common.c
@@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
  static bool lookup_path(char *name)
  {
  	bool found = false;
-	char *path, *tmp;
+	char *path, *tmp = NULL;
  	char buf[PATH_MAX];
  	char *env = getenv("PATH");
  
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 59a98c6..b708ae0 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -129,7 +129,7 @@ ifndef DEBUG
  endif
  
  ifeq ($(DEBUG),0)
-  CFLAGS += -O6
+  CFLAGS += -Og
  endif
  
  ifdef PARSER_DEBUG
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 201f6c4c..29792d2 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -397,7 +397,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
  					    const char *name)
  {
  	struct rb_node *n;
-	struct symbol_name_rb_node *s;
+	struct symbol_name_rb_node *s = NULL;
  
  	if (symbols == NULL)
  		return NULL;
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index 25d6c73..d495741 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
  	char *line;
  	char *next = NULL;
  	char *addr_str;
-	char *fmt;
+	char *fmt = NULL;
  
  	line = strtok_r(file, "\n", &next);
  	while (line) {
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-19 13:41 [PATCH] perf: fix wrong DEBUG configuration Martin Liška
@ 2015-05-19 14:04 ` Arnaldo Carvalho de Melo
  2015-05-20 13:07   ` Martin Liška
  0 siblings, 1 reply; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-19 14:04 UTC (permalink / raw)
  To: Martin Liška
  Cc: linux-kernel, Ingo Molnar, Paul Mackerras, Peter Zijlstra

Em Tue, May 19, 2015 at 03:41:03PM +0200, Martin Liška escreveu:
> Following patch is fix for wrong DEBUG configuration.

Can you please state, in the changelog, what you think is wrong and how
your proposed change fixes it, so that after reading your intent I can
go and look at the code to see if it matches that?

- Arnaldo
 
> Signed-off-by: Martin Liska <mliska@suse.cz>
> ---
>  tools/perf/arch/common.c            | 2 +-
>  tools/perf/config/Makefile          | 2 +-
>  tools/perf/util/symbol.c            | 2 +-
>  tools/perf/util/trace-event-parse.c | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
> index 49776f1..b7bb42c 100644
> --- a/tools/perf/arch/common.c
> +++ b/tools/perf/arch/common.c
> @@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
>  static bool lookup_path(char *name)
>  {
>  	bool found = false;
> -	char *path, *tmp;
> +	char *path, *tmp = NULL;
>  	char buf[PATH_MAX];
>  	char *env = getenv("PATH");
> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> index 59a98c6..b708ae0 100644
> --- a/tools/perf/config/Makefile
> +++ b/tools/perf/config/Makefile
> @@ -129,7 +129,7 @@ ifndef DEBUG
>  endif
>  ifeq ($(DEBUG),0)
> -  CFLAGS += -O6
> +  CFLAGS += -Og
>  endif
>  ifdef PARSER_DEBUG
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 201f6c4c..29792d2 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -397,7 +397,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
>  					    const char *name)
>  {
>  	struct rb_node *n;
> -	struct symbol_name_rb_node *s;
> +	struct symbol_name_rb_node *s = NULL;
>  	if (symbols == NULL)
>  		return NULL;
> diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
> index 25d6c73..d495741 100644
> --- a/tools/perf/util/trace-event-parse.c
> +++ b/tools/perf/util/trace-event-parse.c
> @@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
>  	char *line;
>  	char *next = NULL;
>  	char *addr_str;
> -	char *fmt;
> +	char *fmt = NULL;
>  	line = strtok_r(file, "\n", &next);
>  	while (line) {
> -- 
> 2.1.4

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-19 14:04 ` Arnaldo Carvalho de Melo
@ 2015-05-20 13:07   ` Martin Liška
  2015-05-20 13:17     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 26+ messages in thread
From: Martin Liška @ 2015-05-20 13:07 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Ingo Molnar, Paul Mackerras, Peter Zijlstra

[-- Attachment #1: Type: text/plain, Size: 2612 bytes --]

On 05/19/2015 04:04 PM, Arnaldo Carvalho de Melo wrote:
> Em Tue, May 19, 2015 at 03:41:03PM +0200, Martin Liška escreveu:
>> Following patch is fix for wrong DEBUG configuration.
>
> Can you please state, in the changelog, what you think is wrong and how
> your proposed change fixes it, so that after reading your intent I can
> go and look at the code to see if it matches that?
>
> - Arnaldo
>
>> Signed-off-by: Martin Liska <mliska@suse.cz>
>> ---
>>   tools/perf/arch/common.c            | 2 +-
>>   tools/perf/config/Makefile          | 2 +-
>>   tools/perf/util/symbol.c            | 2 +-
>>   tools/perf/util/trace-event-parse.c | 2 +-
>>   4 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
>> index 49776f1..b7bb42c 100644
>> --- a/tools/perf/arch/common.c
>> +++ b/tools/perf/arch/common.c
>> @@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
>>   static bool lookup_path(char *name)
>>   {
>>   	bool found = false;
>> -	char *path, *tmp;
>> +	char *path, *tmp = NULL;
>>   	char buf[PATH_MAX];
>>   	char *env = getenv("PATH");
>> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
>> index 59a98c6..b708ae0 100644
>> --- a/tools/perf/config/Makefile
>> +++ b/tools/perf/config/Makefile
>> @@ -129,7 +129,7 @@ ifndef DEBUG
>>   endif
>>   ifeq ($(DEBUG),0)
>> -  CFLAGS += -O6
>> +  CFLAGS += -Og
>>   endif
>>   ifdef PARSER_DEBUG
>> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
>> index 201f6c4c..29792d2 100644
>> --- a/tools/perf/util/symbol.c
>> +++ b/tools/perf/util/symbol.c
>> @@ -397,7 +397,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
>>   					    const char *name)
>>   {
>>   	struct rb_node *n;
>> -	struct symbol_name_rb_node *s;
>> +	struct symbol_name_rb_node *s = NULL;
>>   	if (symbols == NULL)
>>   		return NULL;
>> diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
>> index 25d6c73..d495741 100644
>> --- a/tools/perf/util/trace-event-parse.c
>> +++ b/tools/perf/util/trace-event-parse.c
>> @@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
>>   	char *line;
>>   	char *next = NULL;
>>   	char *addr_str;
>> -	char *fmt;
>> +	char *fmt = NULL;
>>   	line = strtok_r(file, "\n", &next);
>>   	while (line) {
>> --
>> 2.1.4

Currently, GCC optimizes -O6 same as -O3 level. Right optimize debugging
experience is given by passing -Og to compiler.
Assign default value for pointers that are identified by compiler as
non-initialized.

Signed-off-by: Martin Liska <mliska@suse.cz>

[-- Attachment #2: 0001-perf-fix-wrong-DEBUG-configuration.patch --]
[-- Type: text/x-patch, Size: 2177 bytes --]

>From b5f2b42b279ab497e92d41e1f4021dd5cb6ae3ce Mon Sep 17 00:00:00 2001
From: mliska <mliska@suse.cz>
Date: Thu, 2 Apr 2015 15:24:49 +0200
Subject: [PATCH] perf: fix wrong DEBUG configuration

Currently, GCC optimizes -O6 same as -O3 level. Right optimize debugging
experience is given by passing -Og to compiler.
Assign default value for pointers that are identified by compiler as
non-initialized.

Signed-off-by: Martin Liska <mliska@suse.cz>
---
 tools/perf/arch/common.c            | 2 +-
 tools/perf/config/Makefile          | 2 +-
 tools/perf/util/symbol.c            | 2 +-
 tools/perf/util/trace-event-parse.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
index 49776f1..b7bb42c 100644
--- a/tools/perf/arch/common.c
+++ b/tools/perf/arch/common.c
@@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
 static bool lookup_path(char *name)
 {
 	bool found = false;
-	char *path, *tmp;
+	char *path, *tmp = NULL;
 	char buf[PATH_MAX];
 	char *env = getenv("PATH");
 
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index e3b3724..ca8d3e3 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -129,7 +129,7 @@ ifndef DEBUG
 endif
 
 ifeq ($(DEBUG),0)
-  CFLAGS += -O6
+  CFLAGS += -Og
 endif
 
 ifdef PARSER_DEBUG
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 82a31fd..a19fbd4 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -400,7 +400,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
 					    const char *name)
 {
 	struct rb_node *n;
-	struct symbol_name_rb_node *s;
+	struct symbol_name_rb_node *s = NULL;
 
 	if (symbols == NULL)
 		return NULL;
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index 25d6c73..d495741 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
 	char *line;
 	char *next = NULL;
 	char *addr_str;
-	char *fmt;
+	char *fmt = NULL;
 
 	line = strtok_r(file, "\n", &next);
 	while (line) {
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-20 13:07   ` Martin Liška
@ 2015-05-20 13:17     ` Arnaldo Carvalho de Melo
  2015-05-20 13:29       ` Martin Liška
  0 siblings, 1 reply; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-20 13:17 UTC (permalink / raw)
  To: Martin Liška
  Cc: linux-kernel, Ingo Molnar, Paul Mackerras, Peter Zijlstra

Em Wed, May 20, 2015 at 03:07:39PM +0200, Martin Liška escreveu:
> On 05/19/2015 04:04 PM, Arnaldo Carvalho de Melo wrote:
> >Em Tue, May 19, 2015 at 03:41:03PM +0200, Martin Liška escreveu:
> >>Following patch is fix for wrong DEBUG configuration.
> >
> >Can you please state, in the changelog, what you think is wrong and how
> >your proposed change fixes it, so that after reading your intent I can
> >go and look at the code to see if it matches that?

Ok, that improved things, thanks! A question:

> 
> Currently, GCC optimizes -O6 same as -O3 level. Right optimize debugging
> experience is given by passing -Og to compiler.

Is this is this -Og available in old gcc versions? When was it
introduced? Do you know?

- Arnaldo

> Assign default value for pointers that are identified by compiler as
> non-initialized.
> 
> Signed-off-by: Martin Liska <mliska@suse.cz>

> >From b5f2b42b279ab497e92d41e1f4021dd5cb6ae3ce Mon Sep 17 00:00:00 2001
> From: mliska <mliska@suse.cz>
> Date: Thu, 2 Apr 2015 15:24:49 +0200
> Subject: [PATCH] perf: fix wrong DEBUG configuration
> 
> Currently, GCC optimizes -O6 same as -O3 level. Right optimize debugging
> experience is given by passing -Og to compiler.
> Assign default value for pointers that are identified by compiler as
> non-initialized.
> 
> Signed-off-by: Martin Liska <mliska@suse.cz>
> ---
>  tools/perf/arch/common.c            | 2 +-
>  tools/perf/config/Makefile          | 2 +-
>  tools/perf/util/symbol.c            | 2 +-
>  tools/perf/util/trace-event-parse.c | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
> index 49776f1..b7bb42c 100644
> --- a/tools/perf/arch/common.c
> +++ b/tools/perf/arch/common.c
> @@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
>  static bool lookup_path(char *name)
>  {
>  	bool found = false;
> -	char *path, *tmp;
> +	char *path, *tmp = NULL;
>  	char buf[PATH_MAX];
>  	char *env = getenv("PATH");
>  
> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> index e3b3724..ca8d3e3 100644
> --- a/tools/perf/config/Makefile
> +++ b/tools/perf/config/Makefile
> @@ -129,7 +129,7 @@ ifndef DEBUG
>  endif
>  
>  ifeq ($(DEBUG),0)
> -  CFLAGS += -O6
> +  CFLAGS += -Og
>  endif
>  
>  ifdef PARSER_DEBUG
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 82a31fd..a19fbd4 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -400,7 +400,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
>  					    const char *name)
>  {
>  	struct rb_node *n;
> -	struct symbol_name_rb_node *s;
> +	struct symbol_name_rb_node *s = NULL;
>  
>  	if (symbols == NULL)
>  		return NULL;
> diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
> index 25d6c73..d495741 100644
> --- a/tools/perf/util/trace-event-parse.c
> +++ b/tools/perf/util/trace-event-parse.c
> @@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
>  	char *line;
>  	char *next = NULL;
>  	char *addr_str;
> -	char *fmt;
> +	char *fmt = NULL;
>  
>  	line = strtok_r(file, "\n", &next);
>  	while (line) {
> -- 
> 2.1.4
> 


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-20 13:17     ` Arnaldo Carvalho de Melo
@ 2015-05-20 13:29       ` Martin Liška
  2015-05-20 13:53         ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 26+ messages in thread
From: Martin Liška @ 2015-05-20 13:29 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Ingo Molnar, Paul Mackerras, Peter Zijlstra

On 05/20/2015 03:17 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, May 20, 2015 at 03:07:39PM +0200, Martin Liška escreveu:
>> On 05/19/2015 04:04 PM, Arnaldo Carvalho de Melo wrote:
>>> Em Tue, May 19, 2015 at 03:41:03PM +0200, Martin Liška escreveu:
>>>> Following patch is fix for wrong DEBUG configuration.
>>>
>>> Can you please state, in the changelog, what you think is wrong and how
>>> your proposed change fixes it, so that after reading your intent I can
>>> go and look at the code to see if it matches that?
>
> Ok, that improved things, thanks! A question:
>
>>
>> Currently, GCC optimizes -O6 same as -O3 level. Right optimize debugging
>> experience is given by passing -Og to compiler.
>
> Is this is this -Og available in old gcc versions? When was it
> introduced? Do you know?
>
> - Arnaldo

Hi

GCC 4.8.0 is the first version capable of the option: https://gcc.gnu.org/gcc-4.8/changes.html.
That can be problematic, which GCC version do you support in linux/perf?

Martin

>
>> Assign default value for pointers that are identified by compiler as
>> non-initialized.
>>
>> Signed-off-by: Martin Liska <mliska@suse.cz>
>
>> >From b5f2b42b279ab497e92d41e1f4021dd5cb6ae3ce Mon Sep 17 00:00:00 2001
>> From: mliska <mliska@suse.cz>
>> Date: Thu, 2 Apr 2015 15:24:49 +0200
>> Subject: [PATCH] perf: fix wrong DEBUG configuration
>>
>> Currently, GCC optimizes -O6 same as -O3 level. Right optimize debugging
>> experience is given by passing -Og to compiler.
>> Assign default value for pointers that are identified by compiler as
>> non-initialized.
>>
>> Signed-off-by: Martin Liska <mliska@suse.cz>
>> ---
>>   tools/perf/arch/common.c            | 2 +-
>>   tools/perf/config/Makefile          | 2 +-
>>   tools/perf/util/symbol.c            | 2 +-
>>   tools/perf/util/trace-event-parse.c | 2 +-
>>   4 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
>> index 49776f1..b7bb42c 100644
>> --- a/tools/perf/arch/common.c
>> +++ b/tools/perf/arch/common.c
>> @@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
>>   static bool lookup_path(char *name)
>>   {
>>   	bool found = false;
>> -	char *path, *tmp;
>> +	char *path, *tmp = NULL;
>>   	char buf[PATH_MAX];
>>   	char *env = getenv("PATH");
>>
>> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
>> index e3b3724..ca8d3e3 100644
>> --- a/tools/perf/config/Makefile
>> +++ b/tools/perf/config/Makefile
>> @@ -129,7 +129,7 @@ ifndef DEBUG
>>   endif
>>
>>   ifeq ($(DEBUG),0)
>> -  CFLAGS += -O6
>> +  CFLAGS += -Og
>>   endif
>>
>>   ifdef PARSER_DEBUG
>> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
>> index 82a31fd..a19fbd4 100644
>> --- a/tools/perf/util/symbol.c
>> +++ b/tools/perf/util/symbol.c
>> @@ -400,7 +400,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
>>   					    const char *name)
>>   {
>>   	struct rb_node *n;
>> -	struct symbol_name_rb_node *s;
>> +	struct symbol_name_rb_node *s = NULL;
>>
>>   	if (symbols == NULL)
>>   		return NULL;
>> diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
>> index 25d6c73..d495741 100644
>> --- a/tools/perf/util/trace-event-parse.c
>> +++ b/tools/perf/util/trace-event-parse.c
>> @@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
>>   	char *line;
>>   	char *next = NULL;
>>   	char *addr_str;
>> -	char *fmt;
>> +	char *fmt = NULL;
>>
>>   	line = strtok_r(file, "\n", &next);
>>   	while (line) {
>> --
>> 2.1.4
>>
>


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-20 13:29       ` Martin Liška
@ 2015-05-20 13:53         ` Arnaldo Carvalho de Melo
  2015-05-20 14:55           ` Ingo Molnar
  0 siblings, 1 reply; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-20 13:53 UTC (permalink / raw)
  To: Martin Liška
  Cc: linux-kernel, Ingo Molnar, Paul Mackerras, Peter Zijlstra

Em Wed, May 20, 2015 at 03:29:37PM +0200, Martin Liška escreveu:
> On 05/20/2015 03:17 PM, Arnaldo Carvalho de Melo wrote:
> >Em Wed, May 20, 2015 at 03:07:39PM +0200, Martin Liška escreveu:
> >>Currently, GCC optimizes -O6 same as -O3 level. Right optimize debugging
> >>experience is given by passing -Og to compiler.

> >Is this is this -Og available in old gcc versions? When was it
> >introduced? Do you know?
 
> GCC 4.8.0 is the first version capable of the option: https://gcc.gnu.org/gcc-4.8/changes.html.
> That can be problematic, which GCC version do you support in linux/perf?

So the rule has been: What are the kernel requirements for the
toolchain? tools/perf/ should build with that.

Looking at the kernel's README, that is...

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

COMPILING the kernel:

 - Make sure you have at least gcc 3.2 available.
   For more information, refer to Documentation/Changes.

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

The oldest toolchain I personally test from time to time is:

[acme@rhel5 ~]$ rpm -q gcc make binutils
gcc-4.1.2-55.el5
make-3.81-3.el5
binutils-2.17.50.0.6-26.el5
[acme@rhel5 ~]$

- Arnaldo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-20 13:53         ` Arnaldo Carvalho de Melo
@ 2015-05-20 14:55           ` Ingo Molnar
  2015-05-20 16:16             ` Martin Liška
  0 siblings, 1 reply; 26+ messages in thread
From: Ingo Molnar @ 2015-05-20 14:55 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Martin Liška, linux-kernel, Ingo Molnar, Paul Mackerras,
	Peter Zijlstra


* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Wed, May 20, 2015 at 03:29:37PM +0200, Martin Liška escreveu:
> > On 05/20/2015 03:17 PM, Arnaldo Carvalho de Melo wrote:
> > >Em Wed, May 20, 2015 at 03:07:39PM +0200, Martin Liška escreveu:
> > >>Currently, GCC optimizes -O6 same as -O3 level. Right optimize debugging
> > >>experience is given by passing -Og to compiler.
> 
> > >Is this is this -Og available in old gcc versions? When was it
> > >introduced? Do you know?
>  
> > GCC 4.8.0 is the first version capable of the option: https://gcc.gnu.org/gcc-4.8/changes.html.
> > That can be problematic, which GCC version do you support in linux/perf?
> 
> So the rule has been: What are the kernel requirements for the 
> toolchain? tools/perf/ should build with that.

So we could use -Og if it works, like Kbuild does it:

        KBUILD_CFLAGS += $(call cc-option,-mno-fp-ret-in-387)

the 'cc-option' Make function does some magic of silently calling GCC 
with that option and observing the result.

See:

  scripts/Kbuild.include:cc-option = $(call try-run,\

et al.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-20 14:55           ` Ingo Molnar
@ 2015-05-20 16:16             ` Martin Liška
  2015-05-21 15:05               ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 26+ messages in thread
From: Martin Liška @ 2015-05-20 16:16 UTC (permalink / raw)
  To: Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: linux-kernel, Ingo Molnar, Paul Mackerras, Peter Zijlstra

[-- Attachment #1: Type: text/plain, Size: 1476 bytes --]

On 05/20/2015 04:55 PM, Ingo Molnar wrote:
>
> * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
>> Em Wed, May 20, 2015 at 03:29:37PM +0200, Martin Liška escreveu:
>>> On 05/20/2015 03:17 PM, Arnaldo Carvalho de Melo wrote:
>>>> Em Wed, May 20, 2015 at 03:07:39PM +0200, Martin Liška escreveu:
>>>>> Currently, GCC optimizes -O6 same as -O3 level. Right optimize debugging
>>>>> experience is given by passing -Og to compiler.
>>
>>>> Is this is this -Og available in old gcc versions? When was it
>>>> introduced? Do you know?
>>
>>> GCC 4.8.0 is the first version capable of the option: https://gcc.gnu.org/gcc-4.8/changes.html.
>>> That can be problematic, which GCC version do you support in linux/perf?
>>
>> So the rule has been: What are the kernel requirements for the
>> toolchain? tools/perf/ should build with that.
>
> So we could use -Og if it works, like Kbuild does it:
>
>          KBUILD_CFLAGS += $(call cc-option,-mno-fp-ret-in-387)
>
> the 'cc-option' Make function does some magic of silently calling GCC
> with that option and observing the result.
>
> See:
>
>    scripts/Kbuild.include:cc-option = $(call try-run,\
>
> et al.
>
> Thanks,
>
> 	Ingo
>

Hi.

I am sorry, I did mistake in understanding of DEBUG variable.
Following patch should be fixed, except missing auto-detection
of -Og option.

Unfortunately, following hunk does not work, no -Ox is added to CFLAGS?

--  CFLAGS += -Og
++  CFLAGS += $(call cc-option,-Og,-O0)


Thanks,
Martin

[-- Attachment #2: perf-fix-wrong-DEBUG-configuration-v3.patch --]
[-- Type: text/x-patch, Size: 2232 bytes --]

>From 98ef10fcb27afc3ae7aaab3ebd157ab90b3b1afe Mon Sep 17 00:00:00 2001
From: mliska <mliska@suse.cz>
Date: Thu, 2 Apr 2015 15:24:49 +0200
Subject: [PATCH] perf: fix wrong DEBUG configuration

Currently, GCC optimizes -O6 same as -O3 level, thus change the value
to -O6.
Right optimize debugging experience is given by passing -Og to compiler.
Assign default value for pointers that are identified by compiler as
non-initialized.

Signed-off-by: Martin Liska <mliska@suse.cz>
---
 tools/perf/arch/common.c            | 2 +-
 tools/perf/config/Makefile          | 4 +++-
 tools/perf/util/symbol.c            | 2 +-
 tools/perf/util/trace-event-parse.c | 2 +-
 4 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
index 49776f1..b7bb42c 100644
--- a/tools/perf/arch/common.c
+++ b/tools/perf/arch/common.c
@@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
 static bool lookup_path(char *name)
 {
 	bool found = false;
-	char *path, *tmp;
+	char *path, *tmp = NULL;
 	char buf[PATH_MAX];
 	char *env = getenv("PATH");
 
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index e3b3724..6a29320 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -129,7 +129,9 @@ ifndef DEBUG
 endif
 
 ifeq ($(DEBUG),0)
-  CFLAGS += -O6
+  CFLAGS += -O3
+else
+  CFLAGS += -Og
 endif
 
 ifdef PARSER_DEBUG
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 82a31fd..a19fbd4 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -400,7 +400,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
 					    const char *name)
 {
 	struct rb_node *n;
-	struct symbol_name_rb_node *s;
+	struct symbol_name_rb_node *s = NULL;
 
 	if (symbols == NULL)
 		return NULL;
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index 25d6c73..d495741 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
 	char *line;
 	char *next = NULL;
 	char *addr_str;
-	char *fmt;
+	char *fmt = NULL;
 
 	line = strtok_r(file, "\n", &next);
 	while (line) {
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-20 16:16             ` Martin Liška
@ 2015-05-21 15:05               ` Arnaldo Carvalho de Melo
  2015-05-22  7:02                 ` Ingo Molnar
  0 siblings, 1 reply; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-21 15:05 UTC (permalink / raw)
  To: Martin Liška
  Cc: Ingo Molnar, linux-kernel, Ingo Molnar, Paul Mackerras, Peter Zijlstra

Em Wed, May 20, 2015 at 06:16:51PM +0200, Martin Liška escreveu:
> On 05/20/2015 04:55 PM, Ingo Molnar wrote:
> >* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> >>So the rule has been: What are the kernel requirements for the
> >>toolchain? tools/perf/ should build with that.
> >
> >So we could use -Og if it works, like Kbuild does it:
> >
> >         KBUILD_CFLAGS += $(call cc-option,-mno-fp-ret-in-387)
> >
> >the 'cc-option' Make function does some magic of silently calling GCC
> >with that option and observing the result.

> >See:

> >   scripts/Kbuild.include:cc-option = $(call try-run,\

> I am sorry, I did mistake in understanding of DEBUG variable.
> Following patch should be fixed, except missing auto-detection
> of -Og option.
> 
> Unfortunately, following hunk does not work, no -Ox is added to CFLAGS?
> 
> --  CFLAGS += -Og
> ++  CFLAGS += $(call cc-option,-Og,-O0)

I don't know if we have this cc-option, perhaps Ingo is suggesting we
get it in tools/build/? Or include scripts/Kbuild.include and then use
it?

I.e. we have checks to see if we can use, for instance
-fstack-protector-all, see tools/build/feature/Makefile, usong this
cc-option thing, importing it from Kbuild would solve the issue at
hand in a definitive way and in line with what we have been pursuing:
to make the tools/ build system be based on Kbuild.

- Arnaldo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-21 15:05               ` Arnaldo Carvalho de Melo
@ 2015-05-22  7:02                 ` Ingo Molnar
  2015-05-22 13:50                   ` Martin Liška
  2015-05-25  8:04                   ` Martin Liška
  0 siblings, 2 replies; 26+ messages in thread
From: Ingo Molnar @ 2015-05-22  7:02 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Martin Liška, linux-kernel, Ingo Molnar, Paul Mackerras,
	Peter Zijlstra


* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Wed, May 20, 2015 at 06:16:51PM +0200, Martin Liška escreveu:
> > On 05/20/2015 04:55 PM, Ingo Molnar wrote:
> > >* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > >>So the rule has been: What are the kernel requirements for the
> > >>toolchain? tools/perf/ should build with that.
> > >
> > >So we could use -Og if it works, like Kbuild does it:
> > >
> > >         KBUILD_CFLAGS += $(call cc-option,-mno-fp-ret-in-387)
> > >
> > >the 'cc-option' Make function does some magic of silently calling GCC
> > >with that option and observing the result.
> 
> > >See:
> 
> > >   scripts/Kbuild.include:cc-option = $(call try-run,\
> 
> > I am sorry, I did mistake in understanding of DEBUG variable.
> > Following patch should be fixed, except missing auto-detection
> > of -Og option.
> > 
> > Unfortunately, following hunk does not work, no -Ox is added to CFLAGS?
> > 
> > --  CFLAGS += -Og
> > ++  CFLAGS += $(call cc-option,-Og,-O0)
> 
> I don't know if we have this cc-option, perhaps Ingo is suggesting 
> we get it in tools/build/? Or include scripts/Kbuild.include and 
> then use it?
> 
> I.e. we have checks to see if we can use, for instance 
> -fstack-protector-all, see tools/build/feature/Makefile, using this 
> cc-option thing, importing it from Kbuild would solve the issue at 
> hand in a definitive way and in line with what we have been 
> pursuing: to make the tools/ build system be based on Kbuild.

So I'd suggest copying any necessary functions instead of outright 
including all of Kbuild in the tooling build system which creates 
non-trivial dependencies that is not necessarily tested as thoroughly 
on the kbuild side as on the tooling side.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-22  7:02                 ` Ingo Molnar
@ 2015-05-22 13:50                   ` Martin Liška
  2015-05-25  8:04                   ` Martin Liška
  1 sibling, 0 replies; 26+ messages in thread
From: Martin Liška @ 2015-05-22 13:50 UTC (permalink / raw)
  To: Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: linux-kernel, Ingo Molnar, Paul Mackerras, Peter Zijlstra

On 05/22/2015 09:02 AM, Ingo Molnar wrote:
>
> * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
>> Em Wed, May 20, 2015 at 06:16:51PM +0200, Martin Liška escreveu:
>>> On 05/20/2015 04:55 PM, Ingo Molnar wrote:
>>>> * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>>>>> So the rule has been: What are the kernel requirements for the
>>>>> toolchain? tools/perf/ should build with that.
>>>>
>>>> So we could use -Og if it works, like Kbuild does it:
>>>>
>>>>          KBUILD_CFLAGS += $(call cc-option,-mno-fp-ret-in-387)
>>>>
>>>> the 'cc-option' Make function does some magic of silently calling GCC
>>>> with that option and observing the result.
>>
>>>> See:
>>
>>>>    scripts/Kbuild.include:cc-option = $(call try-run,\
>>
>>> I am sorry, I did mistake in understanding of DEBUG variable.
>>> Following patch should be fixed, except missing auto-detection
>>> of -Og option.
>>>
>>> Unfortunately, following hunk does not work, no -Ox is added to CFLAGS?
>>>
>>> --  CFLAGS += -Og
>>> ++  CFLAGS += $(call cc-option,-Og,-O0)
>>
>> I don't know if we have this cc-option, perhaps Ingo is suggesting
>> we get it in tools/build/? Or include scripts/Kbuild.include and
>> then use it?
>>
>> I.e. we have checks to see if we can use, for instance
>> -fstack-protector-all, see tools/build/feature/Makefile, using this
>> cc-option thing, importing it from Kbuild would solve the issue at
>> hand in a definitive way and in line with what we have been
>> pursuing: to make the tools/ build system be based on Kbuild.
>
> So I'd suggest copying any necessary functions instead of outright
> including all of Kbuild in the tooling build system which creates
> non-trivial dependencies that is not necessarily tested as thoroughly
> on the kbuild side as on the tooling side.
>
> Thanks,
>
> 	Ingo
>

May I ask you Ingo for helping with that? It would be easy to test if you
grab my patch and add necessary bash functions.

Thank you,
Martin

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-22  7:02                 ` Ingo Molnar
  2015-05-22 13:50                   ` Martin Liška
@ 2015-05-25  8:04                   ` Martin Liška
  2015-05-25 10:47                     ` Ingo Molnar
  1 sibling, 1 reply; 26+ messages in thread
From: Martin Liška @ 2015-05-25  8:04 UTC (permalink / raw)
  To: Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: linux-kernel, Ingo Molnar, Paul Mackerras, Peter Zijlstra

[-- Attachment #1: Type: text/plain, Size: 1998 bytes --]

On 05/22/2015 09:02 AM, Ingo Molnar wrote:
>
> * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
>> Em Wed, May 20, 2015 at 06:16:51PM +0200, Martin Liška escreveu:
>>> On 05/20/2015 04:55 PM, Ingo Molnar wrote:
>>>> * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>>>>> So the rule has been: What are the kernel requirements for the
>>>>> toolchain? tools/perf/ should build with that.
>>>>
>>>> So we could use -Og if it works, like Kbuild does it:
>>>>
>>>>          KBUILD_CFLAGS += $(call cc-option,-mno-fp-ret-in-387)
>>>>
>>>> the 'cc-option' Make function does some magic of silently calling GCC
>>>> with that option and observing the result.
>>
>>>> See:
>>
>>>>    scripts/Kbuild.include:cc-option = $(call try-run,\
>>
>>> I am sorry, I did mistake in understanding of DEBUG variable.
>>> Following patch should be fixed, except missing auto-detection
>>> of -Og option.
>>>
>>> Unfortunately, following hunk does not work, no -Ox is added to CFLAGS?
>>>
>>> --  CFLAGS += -Og
>>> ++  CFLAGS += $(call cc-option,-Og,-O0)
>>
>> I don't know if we have this cc-option, perhaps Ingo is suggesting
>> we get it in tools/build/? Or include scripts/Kbuild.include and
>> then use it?
>>
>> I.e. we have checks to see if we can use, for instance
>> -fstack-protector-all, see tools/build/feature/Makefile, using this
>> cc-option thing, importing it from Kbuild would solve the issue at
>> hand in a definitive way and in line with what we have been
>> pursuing: to make the tools/ build system be based on Kbuild.
>
> So I'd suggest copying any necessary functions instead of outright
> including all of Kbuild in the tooling build system which creates
> non-trivial dependencies that is not necessarily tested as thoroughly
> on the kbuild side as on the tooling side.
>
> Thanks,
>
> 	Ingo
>

Hi.

Thanks for advice, I copied necessary functions to include/utilities.mak.
Following patch works for, tested both with a compiler capable of -Og and
a not capable one.

Thanks,
Martin

[-- Attachment #2: 0001-perf-fix-wrong-DEBUG-configuration-v4.patch --]
[-- Type: text/x-patch, Size: 3160 bytes --]

Currently, GCC optimizes -O6 same as -O3 level, thus change the value
to -O6.
Right optimize debugging experience is given by passing -Og to compiler.
Assign default value for pointers that are identified by compiler as
non-initialized.

Signed-off-by: Martin Liska <mliska@suse.cz>
---
 tools/perf/arch/common.c            |  2 +-
 tools/perf/config/Makefile          |  4 +++-
 tools/perf/config/utilities.mak     | 19 +++++++++++++++++++
 tools/perf/util/symbol.c            |  2 +-
 tools/perf/util/trace-event-parse.c |  2 +-
 5 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
index 49776f1..b7bb42c 100644
--- a/tools/perf/arch/common.c
+++ b/tools/perf/arch/common.c
@@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
 static bool lookup_path(char *name)
 {
 	bool found = false;
-	char *path, *tmp;
+	char *path, *tmp = NULL;
 	char buf[PATH_MAX];
 	char *env = getenv("PATH");
 
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index e3b3724..47e4ae1 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -129,7 +129,9 @@ ifndef DEBUG
 endif
 
 ifeq ($(DEBUG),0)
-  CFLAGS += -O6
+  CFLAGS += -O3
+else
+  CFLAGS += $(call cc-option,-Og,-O0)
 endif
 
 ifdef PARSER_DEBUG
diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
index c16ce83..0ebef09 100644
--- a/tools/perf/config/utilities.mak
+++ b/tools/perf/config/utilities.mak
@@ -177,3 +177,22 @@ $(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
 endef
 _ge_attempt = $(if $(get-executable),$(get-executable),$(call _gea_err,$(2)))
 _gea_err  = $(if $(1),$(error Please set '$(1)' appropriately))
+
+# try-run
+# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
+# Exit code chooses option. "$$TMP" is can be used as temporary file and
+# is automatically cleaned up.
+try-run = $(shell set -e;		\
+	TMP="$(TMPOUT).$$$$.tmp";	\
+	TMPO="$(TMPOUT).$$$$.o";	\
+	if ($(1)) >/dev/null 2>&1;	\
+	then echo "$(2)";		\
+	else echo "$(3)";		\
+	fi;				\
+	rm -f "$$TMP" "$$TMPO")
+
+# cc-option
+# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
+
+cc-option = $(call try-run,\
+	$(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 82a31fd..a19fbd4 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -400,7 +400,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
 					    const char *name)
 {
 	struct rb_node *n;
-	struct symbol_name_rb_node *s;
+	struct symbol_name_rb_node *s = NULL;
 
 	if (symbols == NULL)
 		return NULL;
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index 25d6c73..d495741 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
 	char *line;
 	char *next = NULL;
 	char *addr_str;
-	char *fmt;
+	char *fmt = NULL;
 
 	line = strtok_r(file, "\n", &next);
 	while (line) {
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-25  8:04                   ` Martin Liška
@ 2015-05-25 10:47                     ` Ingo Molnar
  2015-05-25 11:04                       ` Martin Liška
  0 siblings, 1 reply; 26+ messages in thread
From: Ingo Molnar @ 2015-05-25 10:47 UTC (permalink / raw)
  To: Martin Liška
  Cc: Arnaldo Carvalho de Melo, linux-kernel, Ingo Molnar,
	Paul Mackerras, Peter Zijlstra


* Martin Liška <mliska@suse.cz> wrote:

> Currently, GCC optimizes -O6 same as -O3 level, thus change the value
> to -O6.

s/to -O6
  to -O3

> Right optimize debugging experience is given by passing -Og to 
> compiler. Assign default value for pointers that are identified by 
> compiler as non-initialized.

s/Right optimize debugging experience is given/
  Correct debugging experience is given/

s/identified by compiler
  identified by the compiler

>  ifeq ($(DEBUG),0)
> -  CFLAGS += -O6
> +  CFLAGS += -O3
> +else
> +  CFLAGS += $(call cc-option,-Og,-O0)
>  endif

> +# try-run
> +# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
> +# Exit code chooses option. "$$TMP" is can be used as temporary file and
> +# is automatically cleaned up.
> +try-run = $(shell set -e;		\
> +	TMP="$(TMPOUT).$$$$.tmp";	\
> +	TMPO="$(TMPOUT).$$$$.o";	\
> +	if ($(1)) >/dev/null 2>&1;	\
> +	then echo "$(2)";		\
> +	else echo "$(3)";		\
> +	fi;				\
> +	rm -f "$$TMP" "$$TMPO")
> +
> +# cc-option
> +# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
> +
> +cc-option = $(call try-run,\
> +	$(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))

Looks good to me!

Acked-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-25 10:47                     ` Ingo Molnar
@ 2015-05-25 11:04                       ` Martin Liška
  2015-05-25 11:09                         ` Ingo Molnar
  0 siblings, 1 reply; 26+ messages in thread
From: Martin Liška @ 2015-05-25 11:04 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Arnaldo Carvalho de Melo, linux-kernel, Ingo Molnar,
	Paul Mackerras, Peter Zijlstra

[-- Attachment #1: Type: text/plain, Size: 1506 bytes --]

On 05/25/2015 12:47 PM, Ingo Molnar wrote:
>
> * Martin Liška <mliska@suse.cz> wrote:
>
>> Currently, GCC optimizes -O6 same as -O3 level, thus change the value
>> to -O6.
>
> s/to -O6
>    to -O3
>
>> Right optimize debugging experience is given by passing -Og to
>> compiler. Assign default value for pointers that are identified by
>> compiler as non-initialized.
>
> s/Right optimize debugging experience is given/
>    Correct debugging experience is given/
>
> s/identified by compiler
>    identified by the compiler
>
>>   ifeq ($(DEBUG),0)
>> -  CFLAGS += -O6
>> +  CFLAGS += -O3
>> +else
>> +  CFLAGS += $(call cc-option,-Og,-O0)
>>   endif
>
>> +# try-run
>> +# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
>> +# Exit code chooses option. "$$TMP" is can be used as temporary file and
>> +# is automatically cleaned up.
>> +try-run = $(shell set -e;		\
>> +	TMP="$(TMPOUT).$$$$.tmp";	\
>> +	TMPO="$(TMPOUT).$$$$.o";	\
>> +	if ($(1)) >/dev/null 2>&1;	\
>> +	then echo "$(2)";		\
>> +	else echo "$(3)";		\
>> +	fi;				\
>> +	rm -f "$$TMP" "$$TMPO")
>> +
>> +# cc-option
>> +# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
>> +
>> +cc-option = $(call try-run,\
>> +	$(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
>
> Looks good to me!
>
> Acked-by: Ingo Molnar <mingo@kernel.org>
>
> Thanks,
>
> 	Ingo
>

Thank you for review.

This is final version of the patch, where I appended your acknowledgment.

Martin

[-- Attachment #2: 0001-perf-fix-wrong-DEBUG-configuration-v5.patch --]
[-- Type: text/x-patch, Size: 3196 bytes --]

Currently, GCC optimizes -O6 same as -O3 level, thus change the value
to -O3.
Right debugging experience is given by passing -Og to compiler.
Assign default value for pointers that are identified by the compiler as
non-initialized.

Signed-off-by: Martin Liska <mliska@suse.cz>
Acked-by: Ingo Molnar <mingo@kernel.org>
---
 tools/perf/arch/common.c            |  2 +-
 tools/perf/config/Makefile          |  4 +++-
 tools/perf/config/utilities.mak     | 19 +++++++++++++++++++
 tools/perf/util/symbol.c            |  2 +-
 tools/perf/util/trace-event-parse.c |  2 +-
 5 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
index 49776f1..b7bb42c 100644
--- a/tools/perf/arch/common.c
+++ b/tools/perf/arch/common.c
@@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
 static bool lookup_path(char *name)
 {
 	bool found = false;
-	char *path, *tmp;
+	char *path, *tmp = NULL;
 	char buf[PATH_MAX];
 	char *env = getenv("PATH");
 
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index e3b3724..47e4ae1 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -129,7 +129,9 @@ ifndef DEBUG
 endif
 
 ifeq ($(DEBUG),0)
-  CFLAGS += -O6
+  CFLAGS += -O3
+else
+  CFLAGS += $(call cc-option,-Og,-O0)
 endif
 
 ifdef PARSER_DEBUG
diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
index c16ce83..0ebef09 100644
--- a/tools/perf/config/utilities.mak
+++ b/tools/perf/config/utilities.mak
@@ -177,3 +177,22 @@ $(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
 endef
 _ge_attempt = $(if $(get-executable),$(get-executable),$(call _gea_err,$(2)))
 _gea_err  = $(if $(1),$(error Please set '$(1)' appropriately))
+
+# try-run
+# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
+# Exit code chooses option. "$$TMP" is can be used as temporary file and
+# is automatically cleaned up.
+try-run = $(shell set -e;		\
+	TMP="$(TMPOUT).$$$$.tmp";	\
+	TMPO="$(TMPOUT).$$$$.o";	\
+	if ($(1)) >/dev/null 2>&1;	\
+	then echo "$(2)";		\
+	else echo "$(3)";		\
+	fi;				\
+	rm -f "$$TMP" "$$TMPO")
+
+# cc-option
+# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
+
+cc-option = $(call try-run,\
+	$(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 82a31fd..a19fbd4 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -400,7 +400,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
 					    const char *name)
 {
 	struct rb_node *n;
-	struct symbol_name_rb_node *s;
+	struct symbol_name_rb_node *s = NULL;
 
 	if (symbols == NULL)
 		return NULL;
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index 25d6c73..d495741 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
 	char *line;
 	char *next = NULL;
 	char *addr_str;
-	char *fmt;
+	char *fmt = NULL;
 
 	line = strtok_r(file, "\n", &next);
 	while (line) {
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-25 11:04                       ` Martin Liška
@ 2015-05-25 11:09                         ` Ingo Molnar
  2015-05-25 11:12                           ` Martin Liška
  0 siblings, 1 reply; 26+ messages in thread
From: Ingo Molnar @ 2015-05-25 11:09 UTC (permalink / raw)
  To: Martin Liška
  Cc: Arnaldo Carvalho de Melo, linux-kernel, Ingo Molnar,
	Paul Mackerras, Peter Zijlstra


* Martin Liška <mliska@suse.cz> wrote:

> >>Right optimize debugging experience is given by passing -Og to
> >>compiler. Assign default value for pointers that are identified by
> >>compiler as non-initialized.
> >
> >s/Right optimize debugging experience is given/
> >   Correct debugging experience is given/

> Right debugging experience is given by passing -Og to compiler.

So I fixed the spelling here once already :-/ If you want to use 
'right' in this context then use it like this:

  The right debugging experience is given by ...

Or you can use what I suggested first:

  Correct debugging experience is given by ...

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-25 11:09                         ` Ingo Molnar
@ 2015-05-25 11:12                           ` Martin Liška
  2015-05-25 13:52                             ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 26+ messages in thread
From: Martin Liška @ 2015-05-25 11:12 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Arnaldo Carvalho de Melo, linux-kernel, Ingo Molnar,
	Paul Mackerras, Peter Zijlstra

[-- Attachment #1: Type: text/plain, Size: 834 bytes --]

On 05/25/2015 01:09 PM, Ingo Molnar wrote:
>
> * Martin Liška <mliska@suse.cz> wrote:
>
>>>> Right optimize debugging experience is given by passing -Og to
>>>> compiler. Assign default value for pointers that are identified by
>>>> compiler as non-initialized.
>>>
>>> s/Right optimize debugging experience is given/
>>>    Correct debugging experience is given/
>
>> Right debugging experience is given by passing -Og to compiler.
>
> So I fixed the spelling here once already :-/ If you want to use
> 'right' in this context then use it like this:
>
>    The right debugging experience is given by ...
>
> Or you can use what I suggested first:
>
>    Correct debugging experience is given by ...
>
> Thanks,
>
> 	Ingo
>

Sorry Ingo for that, I overlooked that correction :)

Attached version should reflect that.

Thanks,
Martin

[-- Attachment #2: 0001-perf-fix-wrong-DEBUG-configuration-v6.patch --]
[-- Type: text/x-patch, Size: 3198 bytes --]

Currently, GCC optimizes -O6 same as -O3 level, thus change the value
to -O3.
Correct debugging experience is given by passing -Og to compiler.
Assign default value for pointers that are identified by the compiler as
non-initialized.

Signed-off-by: Martin Liska <mliska@suse.cz>
Acked-by: Ingo Molnar <mingo@kernel.org>
---
 tools/perf/arch/common.c            |  2 +-
 tools/perf/config/Makefile          |  4 +++-
 tools/perf/config/utilities.mak     | 19 +++++++++++++++++++
 tools/perf/util/symbol.c            |  2 +-
 tools/perf/util/trace-event-parse.c |  2 +-
 5 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
index 49776f1..b7bb42c 100644
--- a/tools/perf/arch/common.c
+++ b/tools/perf/arch/common.c
@@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
 static bool lookup_path(char *name)
 {
 	bool found = false;
-	char *path, *tmp;
+	char *path, *tmp = NULL;
 	char buf[PATH_MAX];
 	char *env = getenv("PATH");
 
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index e3b3724..47e4ae1 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -129,7 +129,9 @@ ifndef DEBUG
 endif
 
 ifeq ($(DEBUG),0)
-  CFLAGS += -O6
+  CFLAGS += -O3
+else
+  CFLAGS += $(call cc-option,-Og,-O0)
 endif
 
 ifdef PARSER_DEBUG
diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
index c16ce83..0ebef09 100644
--- a/tools/perf/config/utilities.mak
+++ b/tools/perf/config/utilities.mak
@@ -177,3 +177,22 @@ $(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
 endef
 _ge_attempt = $(if $(get-executable),$(get-executable),$(call _gea_err,$(2)))
 _gea_err  = $(if $(1),$(error Please set '$(1)' appropriately))
+
+# try-run
+# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
+# Exit code chooses option. "$$TMP" is can be used as temporary file and
+# is automatically cleaned up.
+try-run = $(shell set -e;		\
+	TMP="$(TMPOUT).$$$$.tmp";	\
+	TMPO="$(TMPOUT).$$$$.o";	\
+	if ($(1)) >/dev/null 2>&1;	\
+	then echo "$(2)";		\
+	else echo "$(3)";		\
+	fi;				\
+	rm -f "$$TMP" "$$TMPO")
+
+# cc-option
+# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
+
+cc-option = $(call try-run,\
+	$(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 82a31fd..a19fbd4 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -400,7 +400,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
 					    const char *name)
 {
 	struct rb_node *n;
-	struct symbol_name_rb_node *s;
+	struct symbol_name_rb_node *s = NULL;
 
 	if (symbols == NULL)
 		return NULL;
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index 25d6c73..d495741 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
 	char *line;
 	char *next = NULL;
 	char *addr_str;
-	char *fmt;
+	char *fmt = NULL;
 
 	line = strtok_r(file, "\n", &next);
 	while (line) {
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-25 11:12                           ` Martin Liška
@ 2015-05-25 13:52                             ` Arnaldo Carvalho de Melo
  2015-05-25 18:32                               ` Ingo Molnar
  0 siblings, 1 reply; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-25 13:52 UTC (permalink / raw)
  To: Martin Liška
  Cc: Ingo Molnar, linux-kernel, Ingo Molnar, Paul Mackerras, Peter Zijlstra

Em Mon, May 25, 2015 at 01:12:44PM +0200, Martin Liška escreveu:
> On 05/25/2015 01:09 PM, Ingo Molnar wrote:
> >
> >* Martin Liška <mliska@suse.cz> wrote:
> >
> >>>>Right optimize debugging experience is given by passing -Og to
> >>>>compiler. Assign default value for pointers that are identified by
> >>>>compiler as non-initialized.
> >>>
> >>>s/Right optimize debugging experience is given/
> >>>   Correct debugging experience is given/
> >
> >>Right debugging experience is given by passing -Og to compiler.
> >
> >So I fixed the spelling here once already :-/ If you want to use
> >'right' in this context then use it like this:
> >
> >   The right debugging experience is given by ...
> >
> >Or you can use what I suggested first:
> >
> >   Correct debugging experience is given by ...
> >
> >Thanks,
> >
> >	Ingo
> >
> 
> Sorry Ingo for that, I overlooked that correction :)

Are you ok with this Ingo?

I can apply, but there seems to be two patches folded here, one that
sets the possibly unitiliazed variables to NULL, and could be the first
in the series, and the other, that deals with multiple versions of gcc
and how should we ask something from them.

- Arnaldo
 
> Attached version should reflect that.
> 
> Thanks,
> Martin

> Currently, GCC optimizes -O6 same as -O3 level, thus change the value
> to -O3.
> Correct debugging experience is given by passing -Og to compiler.
> Assign default value for pointers that are identified by the compiler as
> non-initialized.
> 
> Signed-off-by: Martin Liska <mliska@suse.cz>
> Acked-by: Ingo Molnar <mingo@kernel.org>
> ---
>  tools/perf/arch/common.c            |  2 +-
>  tools/perf/config/Makefile          |  4 +++-
>  tools/perf/config/utilities.mak     | 19 +++++++++++++++++++
>  tools/perf/util/symbol.c            |  2 +-
>  tools/perf/util/trace-event-parse.c |  2 +-
>  5 files changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
> index 49776f1..b7bb42c 100644
> --- a/tools/perf/arch/common.c
> +++ b/tools/perf/arch/common.c
> @@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
>  static bool lookup_path(char *name)
>  {
>  	bool found = false;
> -	char *path, *tmp;
> +	char *path, *tmp = NULL;
>  	char buf[PATH_MAX];
>  	char *env = getenv("PATH");
>  
> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> index e3b3724..47e4ae1 100644
> --- a/tools/perf/config/Makefile
> +++ b/tools/perf/config/Makefile
> @@ -129,7 +129,9 @@ ifndef DEBUG
>  endif
>  
>  ifeq ($(DEBUG),0)
> -  CFLAGS += -O6
> +  CFLAGS += -O3
> +else
> +  CFLAGS += $(call cc-option,-Og,-O0)
>  endif
>  
>  ifdef PARSER_DEBUG
> diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
> index c16ce83..0ebef09 100644
> --- a/tools/perf/config/utilities.mak
> +++ b/tools/perf/config/utilities.mak
> @@ -177,3 +177,22 @@ $(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
>  endef
>  _ge_attempt = $(if $(get-executable),$(get-executable),$(call _gea_err,$(2)))
>  _gea_err  = $(if $(1),$(error Please set '$(1)' appropriately))
> +
> +# try-run
> +# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
> +# Exit code chooses option. "$$TMP" is can be used as temporary file and
> +# is automatically cleaned up.
> +try-run = $(shell set -e;		\
> +	TMP="$(TMPOUT).$$$$.tmp";	\
> +	TMPO="$(TMPOUT).$$$$.o";	\
> +	if ($(1)) >/dev/null 2>&1;	\
> +	then echo "$(2)";		\
> +	else echo "$(3)";		\
> +	fi;				\
> +	rm -f "$$TMP" "$$TMPO")
> +
> +# cc-option
> +# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
> +
> +cc-option = $(call try-run,\
> +	$(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 82a31fd..a19fbd4 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -400,7 +400,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
>  					    const char *name)
>  {
>  	struct rb_node *n;
> -	struct symbol_name_rb_node *s;
> +	struct symbol_name_rb_node *s = NULL;
>  
>  	if (symbols == NULL)
>  		return NULL;
> diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
> index 25d6c73..d495741 100644
> --- a/tools/perf/util/trace-event-parse.c
> +++ b/tools/perf/util/trace-event-parse.c
> @@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
>  	char *line;
>  	char *next = NULL;
>  	char *addr_str;
> -	char *fmt;
> +	char *fmt = NULL;
>  
>  	line = strtok_r(file, "\n", &next);
>  	while (line) {
> -- 
> 2.1.4
> 


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-25 13:52                             ` Arnaldo Carvalho de Melo
@ 2015-05-25 18:32                               ` Ingo Molnar
  2015-05-26  9:13                                 ` Martin Liška
  0 siblings, 1 reply; 26+ messages in thread
From: Ingo Molnar @ 2015-05-25 18:32 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Martin Liška, linux-kernel, Ingo Molnar, Paul Mackerras,
	Peter Zijlstra


* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Mon, May 25, 2015 at 01:12:44PM +0200, Martin Liška escreveu:
> > On 05/25/2015 01:09 PM, Ingo Molnar wrote:
> > >
> > >* Martin Liška <mliska@suse.cz> wrote:
> > >
> > >>>>Right optimize debugging experience is given by passing -Og to
> > >>>>compiler. Assign default value for pointers that are identified by
> > >>>>compiler as non-initialized.
> > >>>
> > >>>s/Right optimize debugging experience is given/
> > >>>   Correct debugging experience is given/
> > >
> > >>Right debugging experience is given by passing -Og to compiler.
> > >
> > >So I fixed the spelling here once already :-/ If you want to use
> > >'right' in this context then use it like this:
> > >
> > >   The right debugging experience is given by ...
> > >
> > >Or you can use what I suggested first:
> > >
> > >   Correct debugging experience is given by ...
> > >
> > >Thanks,
> > >
> > >	Ingo
> > >
> > 
> > Sorry Ingo for that, I overlooked that correction :)
> 
> Are you ok with this Ingo?

Yeah, certainly!

> I can apply, but there seems to be two patches folded here, one that 
> sets the possibly unitiliazed variables to NULL, and could be the 
> first in the series, and the other, that deals with multiple 
> versions of gcc and how should we ask something from them.

Yeah.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-25 18:32                               ` Ingo Molnar
@ 2015-05-26  9:13                                 ` Martin Liška
  2015-05-26 14:36                                   ` Arnaldo Carvalho de Melo
                                                     ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Martin Liška @ 2015-05-26  9:13 UTC (permalink / raw)
  To: Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: linux-kernel, Ingo Molnar, Paul Mackerras, Peter Zijlstra

[-- Attachment #1: Type: text/plain, Size: 1487 bytes --]

On 05/25/2015 08:32 PM, Ingo Molnar wrote:
>
> * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
>> Em Mon, May 25, 2015 at 01:12:44PM +0200, Martin Liška escreveu:
>>> On 05/25/2015 01:09 PM, Ingo Molnar wrote:
>>>>
>>>> * Martin Liška <mliska@suse.cz> wrote:
>>>>
>>>>>>> Right optimize debugging experience is given by passing -Og to
>>>>>>> compiler. Assign default value for pointers that are identified by
>>>>>>> compiler as non-initialized.
>>>>>>
>>>>>> s/Right optimize debugging experience is given/
>>>>>>    Correct debugging experience is given/
>>>>
>>>>> Right debugging experience is given by passing -Og to compiler.
>>>>
>>>> So I fixed the spelling here once already :-/ If you want to use
>>>> 'right' in this context then use it like this:
>>>>
>>>>    The right debugging experience is given by ...
>>>>
>>>> Or you can use what I suggested first:
>>>>
>>>>    Correct debugging experience is given by ...
>>>>
>>>> Thanks,
>>>>
>>>> 	Ingo
>>>>
>>>
>>> Sorry Ingo for that, I overlooked that correction :)
>>
>> Are you ok with this Ingo?
>
> Yeah, certainly!
>
>> I can apply, but there seems to be two patches folded here, one that
>> sets the possibly unitiliazed variables to NULL, and could be the
>> first in the series, and the other, that deals with multiple
>> versions of gcc and how should we ask something from them.
>
> Yeah.
>
> Thanks,
>
> 	Ingo
>

Hello.

As Arnaldo pointed, I split the patch to following 2 smaller patches.

Thanks,
Martin

[-- Attachment #2: 0001-perf-Fix-compiler-warnings.patch --]
[-- Type: text/x-patch, Size: 1575 bytes --]

Assign default value for pointers that are identified by the compiler as
non-initialized.

Signed-off-by: Martin Liska <mliska@suse.cz>
Acked-by: Ingo Molnar <mingo@kernel.org>
---
 tools/perf/arch/common.c            | 2 +-
 tools/perf/util/symbol.c            | 2 +-
 tools/perf/util/trace-event-parse.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
index 49776f1..b7bb42c 100644
--- a/tools/perf/arch/common.c
+++ b/tools/perf/arch/common.c
@@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
 static bool lookup_path(char *name)
 {
 	bool found = false;
-	char *path, *tmp;
+	char *path, *tmp = NULL;
 	char buf[PATH_MAX];
 	char *env = getenv("PATH");
 
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 82a31fd..a19fbd4 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -400,7 +400,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
 					    const char *name)
 {
 	struct rb_node *n;
-	struct symbol_name_rb_node *s;
+	struct symbol_name_rb_node *s = NULL;
 
 	if (symbols == NULL)
 		return NULL;
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index 25d6c73..d495741 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
 	char *line;
 	char *next = NULL;
 	char *addr_str;
-	char *fmt;
+	char *fmt = NULL;
 
 	line = strtok_r(file, "\n", &next);
 	while (line) {
-- 
2.1.4


[-- Attachment #3: 0002-perf-fix-wrong-DEBUG-configuration.patch --]
[-- Type: text/x-patch, Size: 1764 bytes --]

Currently, GCC optimizes -O6 same as -O3 level, thus change the value
to -O3.
Correct debugging experience is given by passing -Og to compiler.

Signed-off-by: Martin Liska <mliska@suse.cz>
Acked-by: Ingo Molnar <mingo@kernel.org>
---
 tools/perf/config/Makefile      |  4 +++-
 tools/perf/config/utilities.mak | 19 +++++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index e3b3724..47e4ae1 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -129,7 +129,9 @@ ifndef DEBUG
 endif
 
 ifeq ($(DEBUG),0)
-  CFLAGS += -O6
+  CFLAGS += -O3
+else
+  CFLAGS += $(call cc-option,-Og,-O0)
 endif
 
 ifdef PARSER_DEBUG
diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
index c16ce83..0ebef09 100644
--- a/tools/perf/config/utilities.mak
+++ b/tools/perf/config/utilities.mak
@@ -177,3 +177,22 @@ $(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
 endef
 _ge_attempt = $(if $(get-executable),$(get-executable),$(call _gea_err,$(2)))
 _gea_err  = $(if $(1),$(error Please set '$(1)' appropriately))
+
+# try-run
+# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
+# Exit code chooses option. "$$TMP" is can be used as temporary file and
+# is automatically cleaned up.
+try-run = $(shell set -e;		\
+	TMP="$(TMPOUT).$$$$.tmp";	\
+	TMPO="$(TMPOUT).$$$$.o";	\
+	if ($(1)) >/dev/null 2>&1;	\
+	then echo "$(2)";		\
+	else echo "$(3)";		\
+	fi;				\
+	rm -f "$$TMP" "$$TMPO")
+
+# cc-option
+# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
+
+cc-option = $(call try-run,\
+	$(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-26  9:13                                 ` Martin Liška
@ 2015-05-26 14:36                                   ` Arnaldo Carvalho de Melo
  2015-05-26 15:22                                     ` Arnaldo Carvalho de Melo
  2015-05-26 15:48                                     ` Martin Liška
  2015-05-27 16:51                                   ` [tip:perf/core] perf tools: Assign default value for some pointers tip-bot for Martin Liška
  2015-05-27 16:51                                   ` [tip:perf/core] perf tools: Improve setting of gcc debug option tip-bot for Martin Liska
  2 siblings, 2 replies; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-26 14:36 UTC (permalink / raw)
  To: Martin Liška
  Cc: Ingo Molnar, linux-kernel, Ingo Molnar, Paul Mackerras, Peter Zijlstra

Em Tue, May 26, 2015 at 11:13:32AM +0200, Martin Liška escreveu:
> On 05/25/2015 08:32 PM, Ingo Molnar wrote:
> >
> >* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> >
> >>Em Mon, May 25, 2015 at 01:12:44PM +0200, Martin Liška escreveu:
> >>>On 05/25/2015 01:09 PM, Ingo Molnar wrote:
> >>>>
> >>>>* Martin Liška <mliska@suse.cz> wrote:
> >>>>
> >>>>>>>Right optimize debugging experience is given by passing -Og to
> >>>>>>>compiler. Assign default value for pointers that are identified by
> >>>>>>>compiler as non-initialized.
> >>>>>>
> >>>>>>s/Right optimize debugging experience is given/
> >>>>>>   Correct debugging experience is given/
> >>>>
> >>>>>Right debugging experience is given by passing -Og to compiler.
> >>>>
> >>>>So I fixed the spelling here once already :-/ If you want to use
> >>>>'right' in this context then use it like this:
> >>>>
> >>>>   The right debugging experience is given by ...
> >>>>
> >>>>Or you can use what I suggested first:
> >>>>
> >>>>   Correct debugging experience is given by ...
> >>>>
> >>>>Thanks,
> >>>>
> >>>>	Ingo
> >>>>
> >>>
> >>>Sorry Ingo for that, I overlooked that correction :)
> >>
> >>Are you ok with this Ingo?
> >
> >Yeah, certainly!
> >
> >>I can apply, but there seems to be two patches folded here, one that
> >>sets the possibly unitiliazed variables to NULL, and could be the
> >>first in the series, and the other, that deals with multiple
> >>versions of gcc and how should we ask something from them.
> >
> >Yeah.
> >
> >Thanks,
> >
> >	Ingo
> >
> 
> Hello.
> 
> As Arnaldo pointed, I split the patch to following 2 smaller patches.

Thanks, but how did you generated the two attached patches? 

 [acme@ssdandy linux]$ git am /wb/0001-perf-Fix-compiler-warnings.patch 
  Patch format detection failed.

So I'll fix it up manually, next time please try:

  git format-patch -n HEAD^^..

Or equivalent,

- Arnaldo


 
> Thanks,
> Martin

> Assign default value for pointers that are identified by the compiler as
> non-initialized.
> 
> Signed-off-by: Martin Liska <mliska@suse.cz>
> Acked-by: Ingo Molnar <mingo@kernel.org>
> ---
>  tools/perf/arch/common.c            | 2 +-
>  tools/perf/util/symbol.c            | 2 +-
>  tools/perf/util/trace-event-parse.c | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
> index 49776f1..b7bb42c 100644
> --- a/tools/perf/arch/common.c
> +++ b/tools/perf/arch/common.c
> @@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
>  static bool lookup_path(char *name)
>  {
>  	bool found = false;
> -	char *path, *tmp;
> +	char *path, *tmp = NULL;
>  	char buf[PATH_MAX];
>  	char *env = getenv("PATH");
>  
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 82a31fd..a19fbd4 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -400,7 +400,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
>  					    const char *name)
>  {
>  	struct rb_node *n;
> -	struct symbol_name_rb_node *s;
> +	struct symbol_name_rb_node *s = NULL;
>  
>  	if (symbols == NULL)
>  		return NULL;
> diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
> index 25d6c73..d495741 100644
> --- a/tools/perf/util/trace-event-parse.c
> +++ b/tools/perf/util/trace-event-parse.c
> @@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
>  	char *line;
>  	char *next = NULL;
>  	char *addr_str;
> -	char *fmt;
> +	char *fmt = NULL;
>  
>  	line = strtok_r(file, "\n", &next);
>  	while (line) {
> -- 
> 2.1.4
> 

> Currently, GCC optimizes -O6 same as -O3 level, thus change the value
> to -O3.
> Correct debugging experience is given by passing -Og to compiler.
> 
> Signed-off-by: Martin Liska <mliska@suse.cz>
> Acked-by: Ingo Molnar <mingo@kernel.org>
> ---
>  tools/perf/config/Makefile      |  4 +++-
>  tools/perf/config/utilities.mak | 19 +++++++++++++++++++
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> index e3b3724..47e4ae1 100644
> --- a/tools/perf/config/Makefile
> +++ b/tools/perf/config/Makefile
> @@ -129,7 +129,9 @@ ifndef DEBUG
>  endif
>  
>  ifeq ($(DEBUG),0)
> -  CFLAGS += -O6
> +  CFLAGS += -O3
> +else
> +  CFLAGS += $(call cc-option,-Og,-O0)
>  endif
>  
>  ifdef PARSER_DEBUG
> diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
> index c16ce83..0ebef09 100644
> --- a/tools/perf/config/utilities.mak
> +++ b/tools/perf/config/utilities.mak
> @@ -177,3 +177,22 @@ $(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
>  endef
>  _ge_attempt = $(if $(get-executable),$(get-executable),$(call _gea_err,$(2)))
>  _gea_err  = $(if $(1),$(error Please set '$(1)' appropriately))
> +
> +# try-run
> +# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
> +# Exit code chooses option. "$$TMP" is can be used as temporary file and
> +# is automatically cleaned up.
> +try-run = $(shell set -e;		\
> +	TMP="$(TMPOUT).$$$$.tmp";	\
> +	TMPO="$(TMPOUT).$$$$.o";	\
> +	if ($(1)) >/dev/null 2>&1;	\
> +	then echo "$(2)";		\
> +	else echo "$(3)";		\
> +	fi;				\
> +	rm -f "$$TMP" "$$TMPO")
> +
> +# cc-option
> +# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
> +
> +cc-option = $(call try-run,\
> +	$(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
> -- 
> 2.1.4
> 


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-26 14:36                                   ` Arnaldo Carvalho de Melo
@ 2015-05-26 15:22                                     ` Arnaldo Carvalho de Melo
  2015-05-26 15:39                                       ` Martin Liška
  2015-05-26 15:48                                     ` Martin Liška
  1 sibling, 1 reply; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-26 15:22 UTC (permalink / raw)
  To: Martin Liška
  Cc: Ingo Molnar, linux-kernel, Ingo Molnar, Paul Mackerras, Peter Zijlstra

Em Tue, May 26, 2015 at 11:36:26AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, May 26, 2015 at 11:13:32AM +0200, Martin Liška escreveu:
> > As Arnaldo pointed, I split the patch to following 2 smaller patches.
> 
> Thanks, but how did you generated the two attached patches? 
> 
>  [acme@ssdandy linux]$ git am /wb/0001-perf-Fix-compiler-warnings.patch 
>   Patch format detection failed.
> 
> So I'll fix it up manually, next time please try:
> 
>   git format-patch -n HEAD^^..
> 
> Or equivalent,

I am also splitting the second patch into two, as it does two unrelated
things:

1. Conditionally use -Og instead of -O0 

2. Use -O3 instead of -O6, as -O6 is the same as -O3 currently

And I am not applying the second, as it doesn't fixes anything nor
improves the current situation, right?

- Arnaldo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-26 15:22                                     ` Arnaldo Carvalho de Melo
@ 2015-05-26 15:39                                       ` Martin Liška
  0 siblings, 0 replies; 26+ messages in thread
From: Martin Liška @ 2015-05-26 15:39 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, linux-kernel, Ingo Molnar, Paul Mackerras, Peter Zijlstra

On 05/26/2015 05:22 PM, Arnaldo Carvalho de Melo wrote:
> Em Tue, May 26, 2015 at 11:36:26AM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Tue, May 26, 2015 at 11:13:32AM +0200, Martin Liška escreveu:
>>> As Arnaldo pointed, I split the patch to following 2 smaller patches.
>>
>> Thanks, but how did you generated the two attached patches?
>>
>>   [acme@ssdandy linux]$ git am /wb/0001-perf-Fix-compiler-warnings.patch
>>    Patch format detection failed.
>>
>> So I'll fix it up manually, next time please try:
>>
>>    git format-patch -n HEAD^^..
>>
>> Or equivalent,
>
> I am also splitting the second patch into two, as it does two unrelated
> things:
>
> 1. Conditionally use -Og instead of -O0
>
> 2. Use -O3 instead of -O6, as -O6 is the same as -O3 currently
>
> And I am not applying the second, as it doesn't fixes anything nor
> improves the current situation, right?

Hi.

Works for me, the first one is more important.

Martin

>
> - Arnaldo
>


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-26 14:36                                   ` Arnaldo Carvalho de Melo
  2015-05-26 15:22                                     ` Arnaldo Carvalho de Melo
@ 2015-05-26 15:48                                     ` Martin Liška
  2015-05-26 15:53                                       ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 26+ messages in thread
From: Martin Liška @ 2015-05-26 15:48 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, linux-kernel, Ingo Molnar, Paul Mackerras, Peter Zijlstra

On 05/26/2015 04:36 PM, Arnaldo Carvalho de Melo wrote:
> Em Tue, May 26, 2015 at 11:13:32AM +0200, Martin Liška escreveu:
>> On 05/25/2015 08:32 PM, Ingo Molnar wrote:
>>>
>>> * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>>>
>>>> Em Mon, May 25, 2015 at 01:12:44PM +0200, Martin Liška escreveu:
>>>>> On 05/25/2015 01:09 PM, Ingo Molnar wrote:
>>>>>>
>>>>>> * Martin Liška <mliska@suse.cz> wrote:
>>>>>>
>>>>>>>>> Right optimize debugging experience is given by passing -Og to
>>>>>>>>> compiler. Assign default value for pointers that are identified by
>>>>>>>>> compiler as non-initialized.
>>>>>>>>
>>>>>>>> s/Right optimize debugging experience is given/
>>>>>>>>    Correct debugging experience is given/
>>>>>>
>>>>>>> Right debugging experience is given by passing -Og to compiler.
>>>>>>
>>>>>> So I fixed the spelling here once already :-/ If you want to use
>>>>>> 'right' in this context then use it like this:
>>>>>>
>>>>>>    The right debugging experience is given by ...
>>>>>>
>>>>>> Or you can use what I suggested first:
>>>>>>
>>>>>>    Correct debugging experience is given by ...
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> 	Ingo
>>>>>>
>>>>>
>>>>> Sorry Ingo for that, I overlooked that correction :)
>>>>
>>>> Are you ok with this Ingo?
>>>
>>> Yeah, certainly!
>>>
>>>> I can apply, but there seems to be two patches folded here, one that
>>>> sets the possibly unitiliazed variables to NULL, and could be the
>>>> first in the series, and the other, that deals with multiple
>>>> versions of gcc and how should we ask something from them.
>>>
>>> Yeah.
>>>
>>> Thanks,
>>>
>>> 	Ingo
>>>
>>
>> Hello.
>>
>> As Arnaldo pointed, I split the patch to following 2 smaller patches.
>
> Thanks, but how did you generated the two attached patches?
>
>   [acme@ssdandy linux]$ git am /wb/0001-perf-Fix-compiler-warnings.patch
>    Patch format detection failed.
>
> So I'll fix it up manually, next time please try:
>
>    git format-patch -n HEAD^^..
>
> Or equivalent,
>
> - Arnaldo

Hi.

That's quite strange, I've just rebuilt my branch to perf/perf/core and
I was given equal patches for:

'git format-patch -2'

Maybe that's related to erased patch header(From, Date, Subject).

Thanks for re-application,
Martin

>
>
>
>> Thanks,
>> Martin
>
>> Assign default value for pointers that are identified by the compiler as
>> non-initialized.
>>
>> Signed-off-by: Martin Liska <mliska@suse.cz>
>> Acked-by: Ingo Molnar <mingo@kernel.org>
>> ---
>>   tools/perf/arch/common.c            | 2 +-
>>   tools/perf/util/symbol.c            | 2 +-
>>   tools/perf/util/trace-event-parse.c | 2 +-
>>   3 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
>> index 49776f1..b7bb42c 100644
>> --- a/tools/perf/arch/common.c
>> +++ b/tools/perf/arch/common.c
>> @@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
>>   static bool lookup_path(char *name)
>>   {
>>   	bool found = false;
>> -	char *path, *tmp;
>> +	char *path, *tmp = NULL;
>>   	char buf[PATH_MAX];
>>   	char *env = getenv("PATH");
>>
>> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
>> index 82a31fd..a19fbd4 100644
>> --- a/tools/perf/util/symbol.c
>> +++ b/tools/perf/util/symbol.c
>> @@ -400,7 +400,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
>>   					    const char *name)
>>   {
>>   	struct rb_node *n;
>> -	struct symbol_name_rb_node *s;
>> +	struct symbol_name_rb_node *s = NULL;
>>
>>   	if (symbols == NULL)
>>   		return NULL;
>> diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
>> index 25d6c73..d495741 100644
>> --- a/tools/perf/util/trace-event-parse.c
>> +++ b/tools/perf/util/trace-event-parse.c
>> @@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
>>   	char *line;
>>   	char *next = NULL;
>>   	char *addr_str;
>> -	char *fmt;
>> +	char *fmt = NULL;
>>
>>   	line = strtok_r(file, "\n", &next);
>>   	while (line) {
>> --
>> 2.1.4
>>
>
>> Currently, GCC optimizes -O6 same as -O3 level, thus change the value
>> to -O3.
>> Correct debugging experience is given by passing -Og to compiler.
>>
>> Signed-off-by: Martin Liska <mliska@suse.cz>
>> Acked-by: Ingo Molnar <mingo@kernel.org>
>> ---
>>   tools/perf/config/Makefile      |  4 +++-
>>   tools/perf/config/utilities.mak | 19 +++++++++++++++++++
>>   2 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
>> index e3b3724..47e4ae1 100644
>> --- a/tools/perf/config/Makefile
>> +++ b/tools/perf/config/Makefile
>> @@ -129,7 +129,9 @@ ifndef DEBUG
>>   endif
>>
>>   ifeq ($(DEBUG),0)
>> -  CFLAGS += -O6
>> +  CFLAGS += -O3
>> +else
>> +  CFLAGS += $(call cc-option,-Og,-O0)
>>   endif
>>
>>   ifdef PARSER_DEBUG
>> diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
>> index c16ce83..0ebef09 100644
>> --- a/tools/perf/config/utilities.mak
>> +++ b/tools/perf/config/utilities.mak
>> @@ -177,3 +177,22 @@ $(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
>>   endef
>>   _ge_attempt = $(if $(get-executable),$(get-executable),$(call _gea_err,$(2)))
>>   _gea_err  = $(if $(1),$(error Please set '$(1)' appropriately))
>> +
>> +# try-run
>> +# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
>> +# Exit code chooses option. "$$TMP" is can be used as temporary file and
>> +# is automatically cleaned up.
>> +try-run = $(shell set -e;		\
>> +	TMP="$(TMPOUT).$$$$.tmp";	\
>> +	TMPO="$(TMPOUT).$$$$.o";	\
>> +	if ($(1)) >/dev/null 2>&1;	\
>> +	then echo "$(2)";		\
>> +	else echo "$(3)";		\
>> +	fi;				\
>> +	rm -f "$$TMP" "$$TMPO")
>> +
>> +# cc-option
>> +# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
>> +
>> +cc-option = $(call try-run,\
>> +	$(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
>> --
>> 2.1.4
>>
>


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf: fix wrong DEBUG configuration
  2015-05-26 15:48                                     ` Martin Liška
@ 2015-05-26 15:53                                       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-26 15:53 UTC (permalink / raw)
  To: Martin Liška
  Cc: Ingo Molnar, linux-kernel, Ingo Molnar, Paul Mackerras, Peter Zijlstra

Em Tue, May 26, 2015 at 05:48:27PM +0200, Martin Liška escreveu:
> On 05/26/2015 04:36 PM, Arnaldo Carvalho de Melo wrote:
> >Em Tue, May 26, 2015 at 11:13:32AM +0200, Martin Liška escreveu:
> >>>* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> >>>>Em Mon, May 25, 2015 at 01:12:44PM +0200, Martin Liška escreveu:
> >>As Arnaldo pointed, I split the patch to following 2 smaller patches.

> >Thanks, but how did you generated the two attached patches?

> >  [acme@ssdandy linux]$ git am /wb/0001-perf-Fix-compiler-warnings.patch
> >   Patch format detection failed.

> >So I'll fix it up manually, next time please try:

> >   git format-patch -n HEAD^^..

> >Or equivalent,

> That's quite strange, I've just rebuilt my branch to perf/perf/core and
> I was given equal patches for:

> 'git format-patch -2'

> Maybe that's related to erased patch header(From, Date, Subject).

Yes, I had to add at least From: and Subject:, but my scripts will pick
as well the Message-ID to create the Link: line, Cc: info to add the CC
and some other stuff that are present when fed with one message per
patch.

> Thanks for re-application,

You're welcome, keep on contributing!

- Arnaldo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [tip:perf/core] perf tools: Assign default value for some pointers
  2015-05-26  9:13                                 ` Martin Liška
  2015-05-26 14:36                                   ` Arnaldo Carvalho de Melo
@ 2015-05-27 16:51                                   ` tip-bot for Martin Liška
  2015-05-27 16:51                                   ` [tip:perf/core] perf tools: Improve setting of gcc debug option tip-bot for Martin Liska
  2 siblings, 0 replies; 26+ messages in thread
From: tip-bot for Martin Liška @ 2015-05-27 16:51 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: tglx, mliska, acme, linux-kernel, hpa, mingo

Commit-ID:  5bcaaca3e4d15ce39008a0b9c431c0ac4be784bd
Gitweb:     http://git.kernel.org/tip/5bcaaca3e4d15ce39008a0b9c431c0ac4be784bd
Author:     Martin Liška <mliska@suse.cz>
AuthorDate: Tue, 26 May 2015 11:41:37 -0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 27 May 2015 12:21:45 -0300

perf tools: Assign default value for some pointers

Assign default value for pointers that are identified by the compiler as
non-initialized.

Signed-off-by: Martin Liska <mliska@suse.cz>
Acked-by: Ingo Molnar <mingo@kernel.org>
Link: http://lkml.kernel.org/r/5564393C.1090104@suse.cz
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/arch/common.c            | 2 +-
 tools/perf/util/symbol.c            | 2 +-
 tools/perf/util/trace-event-parse.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
index 49776f1..b7bb42c 100644
--- a/tools/perf/arch/common.c
+++ b/tools/perf/arch/common.c
@@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
 static bool lookup_path(char *name)
 {
 	bool found = false;
-	char *path, *tmp;
+	char *path, *tmp = NULL;
 	char buf[PATH_MAX];
 	char *env = getenv("PATH");
 
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 00b6b17..b9e3eb5 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -398,7 +398,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
 					    const char *name)
 {
 	struct rb_node *n;
-	struct symbol_name_rb_node *s;
+	struct symbol_name_rb_node *s = NULL;
 
 	if (symbols == NULL)
 		return NULL;
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index 25d6c73..d495741 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
 	char *line;
 	char *next = NULL;
 	char *addr_str;
-	char *fmt;
+	char *fmt = NULL;
 
 	line = strtok_r(file, "\n", &next);
 	while (line) {

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [tip:perf/core] perf tools: Improve setting of gcc debug option
  2015-05-26  9:13                                 ` Martin Liška
  2015-05-26 14:36                                   ` Arnaldo Carvalho de Melo
  2015-05-27 16:51                                   ` [tip:perf/core] perf tools: Assign default value for some pointers tip-bot for Martin Liška
@ 2015-05-27 16:51                                   ` tip-bot for Martin Liska
  2 siblings, 0 replies; 26+ messages in thread
From: tip-bot for Martin Liska @ 2015-05-27 16:51 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: mingo, mliska, acme, linux-kernel, hpa, tglx

Commit-ID:  e8b7ea4356fdd3c4de5478f3418eb84f8dce2b61
Gitweb:     http://git.kernel.org/tip/e8b7ea4356fdd3c4de5478f3418eb84f8dce2b61
Author:     Martin Liska <mliska@suse.cz>
AuthorDate: Tue, 26 May 2015 12:23:24 -0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 27 May 2015 12:21:45 -0300

perf tools: Improve setting of gcc debug option

Correct debugging experience is given by passing -Og to compiler.

Do it in a way that supports older compilers

Signed-off-by: Martin Liska <mliska@suse.cz>
Acked-by: Ingo Molnar <mingo@kernel.org>
Link: http://lkml.kernel.org/r/5564393C.1090104@suse.cz
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/config/Makefile      |  2 ++
 tools/perf/config/utilities.mak | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index e3b3724..317001c 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -130,6 +130,8 @@ endif
 
 ifeq ($(DEBUG),0)
   CFLAGS += -O6
+else
+  CFLAGS += $(call cc-option,-Og,-O0)
 endif
 
 ifdef PARSER_DEBUG
diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
index c16ce83..0ebef09 100644
--- a/tools/perf/config/utilities.mak
+++ b/tools/perf/config/utilities.mak
@@ -177,3 +177,22 @@ $(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
 endef
 _ge_attempt = $(if $(get-executable),$(get-executable),$(call _gea_err,$(2)))
 _gea_err  = $(if $(1),$(error Please set '$(1)' appropriately))
+
+# try-run
+# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
+# Exit code chooses option. "$$TMP" is can be used as temporary file and
+# is automatically cleaned up.
+try-run = $(shell set -e;		\
+	TMP="$(TMPOUT).$$$$.tmp";	\
+	TMPO="$(TMPOUT).$$$$.o";	\
+	if ($(1)) >/dev/null 2>&1;	\
+	then echo "$(2)";		\
+	else echo "$(3)";		\
+	fi;				\
+	rm -f "$$TMP" "$$TMPO")
+
+# cc-option
+# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
+
+cc-option = $(call try-run,\
+	$(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))

^ permalink raw reply related	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2015-05-27 16:52 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-19 13:41 [PATCH] perf: fix wrong DEBUG configuration Martin Liška
2015-05-19 14:04 ` Arnaldo Carvalho de Melo
2015-05-20 13:07   ` Martin Liška
2015-05-20 13:17     ` Arnaldo Carvalho de Melo
2015-05-20 13:29       ` Martin Liška
2015-05-20 13:53         ` Arnaldo Carvalho de Melo
2015-05-20 14:55           ` Ingo Molnar
2015-05-20 16:16             ` Martin Liška
2015-05-21 15:05               ` Arnaldo Carvalho de Melo
2015-05-22  7:02                 ` Ingo Molnar
2015-05-22 13:50                   ` Martin Liška
2015-05-25  8:04                   ` Martin Liška
2015-05-25 10:47                     ` Ingo Molnar
2015-05-25 11:04                       ` Martin Liška
2015-05-25 11:09                         ` Ingo Molnar
2015-05-25 11:12                           ` Martin Liška
2015-05-25 13:52                             ` Arnaldo Carvalho de Melo
2015-05-25 18:32                               ` Ingo Molnar
2015-05-26  9:13                                 ` Martin Liška
2015-05-26 14:36                                   ` Arnaldo Carvalho de Melo
2015-05-26 15:22                                     ` Arnaldo Carvalho de Melo
2015-05-26 15:39                                       ` Martin Liška
2015-05-26 15:48                                     ` Martin Liška
2015-05-26 15:53                                       ` Arnaldo Carvalho de Melo
2015-05-27 16:51                                   ` [tip:perf/core] perf tools: Assign default value for some pointers tip-bot for Martin Liška
2015-05-27 16:51                                   ` [tip:perf/core] perf tools: Improve setting of gcc debug option tip-bot for Martin Liska

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).