linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] [PERF] (Userspace Tools) Fix a compilation error with -fstack-protector and -Werror
@ 2010-10-18 23:24 Brian Gitonga Marete
  2010-10-18 23:38 ` Frederic Weisbecker
  0 siblings, 1 reply; 15+ messages in thread
From: Brian Gitonga Marete @ 2010-10-18 23:24 UTC (permalink / raw)
  To: LKML

The following patch fixes compilation of the perf user-space tools on,
for example, gcc version 4.3.3 (Ubuntu 4.3.3-5ubuntu4) . It should not
break anything else.

Signed-off-by: Brian Gitonga Marete <marete@toshnix.com>
---
 tools/perf/Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 1950e19..64eb2ea 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -288,7 +288,7 @@ endif
 -include feature-tests.mak

 ifeq ($(call try-cc,$(SOURCE_HELLO),-Werror -fstack-protector-all),y)
-	CFLAGS := $(CFLAGS) -fstack-protector-all
+	CFLAGS := $(CFLAGS) -fstack-protector-all --param ssp-buffer-size=1
 endif

-- 
Brian Gitonga Marete
Toshnix Systems
Tel: +254722151590

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

* Re: [PATCH] [PERF] (Userspace Tools) Fix a compilation error with -fstack-protector and -Werror
  2010-10-18 23:24 [PATCH] [PERF] (Userspace Tools) Fix a compilation error with -fstack-protector and -Werror Brian Gitonga Marete
@ 2010-10-18 23:38 ` Frederic Weisbecker
  2010-10-19  0:06   ` Brian Gitonga Marete
  0 siblings, 1 reply; 15+ messages in thread
From: Frederic Weisbecker @ 2010-10-18 23:38 UTC (permalink / raw)
  To: Brian Gitonga Marete
  Cc: LKML, Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra

On Tue, Oct 19, 2010 at 02:24:00AM +0300, Brian Gitonga Marete wrote:
> The following patch fixes compilation of the perf user-space tools on,
> for example, gcc version 4.3.3 (Ubuntu 4.3.3-5ubuntu4) . It should not
> break anything else.



Hi,

What kind of warning have you encountered and why it fixes it?
Can you describe that in your changelog?

Thanks.



> 
> Signed-off-by: Brian Gitonga Marete <marete@toshnix.com>
> ---
>  tools/perf/Makefile |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index 1950e19..64eb2ea 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -288,7 +288,7 @@ endif
>  -include feature-tests.mak
> 
>  ifeq ($(call try-cc,$(SOURCE_HELLO),-Werror -fstack-protector-all),y)
> -	CFLAGS := $(CFLAGS) -fstack-protector-all
> +	CFLAGS := $(CFLAGS) -fstack-protector-all --param ssp-buffer-size=1
>  endif
> 
> -- 
> Brian Gitonga Marete
> Toshnix Systems
> Tel: +254722151590
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


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

* Re: [PATCH] [PERF] (Userspace Tools) Fix a compilation error with -fstack-protector and -Werror
  2010-10-18 23:38 ` Frederic Weisbecker
@ 2010-10-19  0:06   ` Brian Gitonga Marete
  2010-10-19  0:20     ` Frederic Weisbecker
  2010-10-19  6:40     ` Ingo Molnar
  0 siblings, 2 replies; 15+ messages in thread
From: Brian Gitonga Marete @ 2010-10-19  0:06 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: LKML, Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra

On Tue, Oct 19, 2010 at 2:38 AM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> On Tue, Oct 19, 2010 at 02:24:00AM +0300, Brian Gitonga Marete wrote:
>> The following patch fixes compilation of the perf user-space tools on,
>> for example, gcc version 4.3.3 (Ubuntu 4.3.3-5ubuntu4) . It should not
>> break anything else.
>
>
>
> Hi,
>
> What kind of warning have you encountered and why it fixes it?
> Can you describe that in your changelog?
>

Hello Frederic,

Some versions of gcc, e.g. gcc version 4.3.3 (Ubuntu 4.3.3-5ubuntu4), have
the (default) minimum size of buffers protected by `-fstack-protector' set
to 8. But in perf, there exist much smaller automatic buffers.

This in combination with the -fstack-protector-all, -Werror and
-Wstack-protector causes the compile to fail for such a compiler.

The error encountered with the above-mentioned compiler is:

gcc -o util/ui/util.o -c -ggdb3 -Wall -Wextra -std=gnu99 -Werror -O6
-D_FORTIFY_SOURCE=2 -Wformat -Wformat-security -Wformat-y2k -Wshadow
-Winit-self -Wpacked -Wredundant-decls -Wstack-protector
-Wstrict-aliasing=3 -Wswitch-default -Wswitch-enum -Wno-system-headers
-Wundef -Wvolatile-register-var -Wwrite-strings -Wbad-function-cast
-Wmissing-declarations -Wmissing-prototypes -Wnested-externs
-Wold-style-definition -Wstrict-prototypes
-Wdeclaration-after-statement  -fstack-protector-all
-D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Iutil/include
-Iarch/x86/include -DLIBELF_NO_MMAP -I/usr/include/elfutils
-DDWARF_SUPPORT -I/usr/include/slang -DSHA1_HEADER='<openssl/sha.h>'
util/ui/util.c
cc1: warnings being treated as errors
util/ui/util.c: In function ‘ui__dialog_yesno’:
util/ui/util.c:110: error: not protecting function: no buffer at least
8 bytes long

Signed-off-by: Brian Gitonga Marete <marete@toshnix.com>
---
 tools/perf/Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 1950e19..64eb2ea 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -288,7 +288,7 @@ endif
 -include feature-tests.mak

 ifeq ($(call try-cc,$(SOURCE_HELLO),-Werror -fstack-protector-all),y)
-	CFLAGS := $(CFLAGS) -fstack-protector-all
+	CFLAGS := $(CFLAGS) -fstack-protector-all --param ssp-buffer-size=1
 endif


-- 
1.6.0.4


-- 
Brian Gitonga Marete
Toshnix Systems
Tel: +254722151590

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

* Re: [PATCH] [PERF] (Userspace Tools) Fix a compilation error with -fstack-protector and -Werror
  2010-10-19  0:06   ` Brian Gitonga Marete
@ 2010-10-19  0:20     ` Frederic Weisbecker
  2010-10-19  6:40     ` Ingo Molnar
  1 sibling, 0 replies; 15+ messages in thread
From: Frederic Weisbecker @ 2010-10-19  0:20 UTC (permalink / raw)
  To: Brian Gitonga Marete, Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: LKML, Peter Zijlstra

On Tue, Oct 19, 2010 at 03:06:41AM +0300, Brian Gitonga Marete wrote:
> On Tue, Oct 19, 2010 at 2:38 AM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > On Tue, Oct 19, 2010 at 02:24:00AM +0300, Brian Gitonga Marete wrote:
> >> The following patch fixes compilation of the perf user-space tools on,
> >> for example, gcc version 4.3.3 (Ubuntu 4.3.3-5ubuntu4) . It should not
> >> break anything else.
> >
> >
> >
> > Hi,
> >
> > What kind of warning have you encountered and why it fixes it?
> > Can you describe that in your changelog?
> >
> 
> Hello Frederic,
> 
> Some versions of gcc, e.g. gcc version 4.3.3 (Ubuntu 4.3.3-5ubuntu4), have
> the (default) minimum size of buffers protected by `-fstack-protector' set
> to 8. But in perf, there exist much smaller automatic buffers.
> 
> This in combination with the -fstack-protector-all, -Werror and
> -Wstack-protector causes the compile to fail for such a compiler.
> 
> The error encountered with the above-mentioned compiler is:
> 
> gcc -o util/ui/util.o -c -ggdb3 -Wall -Wextra -std=gnu99 -Werror -O6
> -D_FORTIFY_SOURCE=2 -Wformat -Wformat-security -Wformat-y2k -Wshadow
> -Winit-self -Wpacked -Wredundant-decls -Wstack-protector
> -Wstrict-aliasing=3 -Wswitch-default -Wswitch-enum -Wno-system-headers
> -Wundef -Wvolatile-register-var -Wwrite-strings -Wbad-function-cast
> -Wmissing-declarations -Wmissing-prototypes -Wnested-externs
> -Wold-style-definition -Wstrict-prototypes
> -Wdeclaration-after-statement  -fstack-protector-all
> -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Iutil/include
> -Iarch/x86/include -DLIBELF_NO_MMAP -I/usr/include/elfutils
> -DDWARF_SUPPORT -I/usr/include/slang -DSHA1_HEADER='<openssl/sha.h>'
> util/ui/util.c
> cc1: warnings being treated as errors
> util/ui/util.c: In function ‘ui__dialog_yesno’:
> util/ui/util.c:110: error: not protecting function: no buffer at least
> 8 bytes long


Doh! So that was the reason of this warning. Yeah looks like a right fix.
And that fixes the issue for me.

Tested-by: Frederic Weisbecker <fweisbec@gmail.com>

Thanks!


> 
> Signed-off-by: Brian Gitonga Marete <marete@toshnix.com>
> ---
>  tools/perf/Makefile |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index 1950e19..64eb2ea 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -288,7 +288,7 @@ endif
>  -include feature-tests.mak
> 
>  ifeq ($(call try-cc,$(SOURCE_HELLO),-Werror -fstack-protector-all),y)
> -	CFLAGS := $(CFLAGS) -fstack-protector-all
> +	CFLAGS := $(CFLAGS) -fstack-protector-all --param ssp-buffer-size=1
>  endif
> 
> 
> -- 
> 1.6.0.4
> 
> 
> -- 
> Brian Gitonga Marete
> Toshnix Systems
> Tel: +254722151590


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

* Re: [PATCH] [PERF] (Userspace Tools) Fix a compilation error with -fstack-protector and -Werror
  2010-10-19  0:06   ` Brian Gitonga Marete
  2010-10-19  0:20     ` Frederic Weisbecker
@ 2010-10-19  6:40     ` Ingo Molnar
  2010-10-19  9:03       ` Américo Wang
  2010-10-19 11:12       ` Brian Gitonga Marete
  1 sibling, 2 replies; 15+ messages in thread
From: Ingo Molnar @ 2010-10-19  6:40 UTC (permalink / raw)
  To: Brian Gitonga Marete
  Cc: Frederic Weisbecker, LKML, Arnaldo Carvalho de Melo, Peter Zijlstra


* Brian Gitonga Marete <marete@toshnix.com> wrote:

> On Tue, Oct 19, 2010 at 2:38 AM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > On Tue, Oct 19, 2010 at 02:24:00AM +0300, Brian Gitonga Marete wrote:
> >> The following patch fixes compilation of the perf user-space tools on,
> >> for example, gcc version 4.3.3 (Ubuntu 4.3.3-5ubuntu4) . It should not
> >> break anything else.
> >
> >
> >
> > Hi,
> >
> > What kind of warning have you encountered and why it fixes it?
> > Can you describe that in your changelog?
> >
> 
> Hello Frederic,
> 
> Some versions of gcc, e.g. gcc version 4.3.3 (Ubuntu 4.3.3-5ubuntu4), have the 
> (default) minimum size of buffers protected by `-fstack-protector' set to 8. But 
> in perf, there exist much smaller automatic buffers.

Hm, it's this code:

        /* newtWinChoice should really be accepting const char pointers... */
        char yes[] = "Yes", no[] = "No";
        return newtWinChoice(NULL, yes, no, (char *)msg) == 1;

I.e. the code is messy and GCC is right to warn about it. Hence it would be somewhat 
bad to actually remove the warning that pointed out some dodgy piece of code.

Even if marking it const doesnt work due to the external libnewt API, we could at 
least put 'yes' and 'no' into file scope and mark them static?

	Ingo

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

* Re: [PATCH] [PERF] (Userspace Tools) Fix a compilation error with -fstack-protector and -Werror
  2010-10-19  6:40     ` Ingo Molnar
@ 2010-10-19  9:03       ` Américo Wang
  2010-10-19 11:12       ` Brian Gitonga Marete
  1 sibling, 0 replies; 15+ messages in thread
From: Américo Wang @ 2010-10-19  9:03 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Brian Gitonga Marete, Frederic Weisbecker, LKML,
	Arnaldo Carvalho de Melo, Peter Zijlstra

On Tue, Oct 19, 2010 at 08:40:00AM +0200, Ingo Molnar wrote:
>
>* Brian Gitonga Marete <marete@toshnix.com> wrote:
>
>> On Tue, Oct 19, 2010 at 2:38 AM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
>> > On Tue, Oct 19, 2010 at 02:24:00AM +0300, Brian Gitonga Marete wrote:
>> >> The following patch fixes compilation of the perf user-space tools on,
>> >> for example, gcc version 4.3.3 (Ubuntu 4.3.3-5ubuntu4) . It should not
>> >> break anything else.
>> >
>> >
>> >
>> > Hi,
>> >
>> > What kind of warning have you encountered and why it fixes it?
>> > Can you describe that in your changelog?
>> >
>> 
>> Hello Frederic,
>> 
>> Some versions of gcc, e.g. gcc version 4.3.3 (Ubuntu 4.3.3-5ubuntu4), have the 
>> (default) minimum size of buffers protected by `-fstack-protector' set to 8. But 
>> in perf, there exist much smaller automatic buffers.
>
>Hm, it's this code:
>
>        /* newtWinChoice should really be accepting const char pointers... */
>        char yes[] = "Yes", no[] = "No";
>        return newtWinChoice(NULL, yes, no, (char *)msg) == 1;
>
>I.e. the code is messy and GCC is right to warn about it. Hence it would be somewhat 
>bad to actually remove the warning that pointed out some dodgy piece of code.

Agreed.

>
>Even if marking it const doesnt work due to the external libnewt API, we could at 
>least put 'yes' and 'no' into file scope and mark them static?
>

How about making ui__dialog_yesno() a macro? ;) Like:

#define ui__dialog_yesno(msg) \
	({newWinChoice(NULL, "Yes", "No", (char *)(msg)) == 1;})

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

* Re: [PATCH] [PERF] (Userspace Tools) Fix a compilation error with -fstack-protector and -Werror
  2010-10-19  6:40     ` Ingo Molnar
  2010-10-19  9:03       ` Américo Wang
@ 2010-10-19 11:12       ` Brian Gitonga Marete
  2010-10-19 11:33         ` Brian Gitonga Marete
  1 sibling, 1 reply; 15+ messages in thread
From: Brian Gitonga Marete @ 2010-10-19 11:12 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Frederic Weisbecker, LKML, Arnaldo Carvalho de Melo, Peter Zijlstra

On Tue, Oct 19, 2010 at 9:40 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Brian Gitonga Marete <marete@toshnix.com> wrote:
>
>> On Tue, Oct 19, 2010 at 2:38 AM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
>> > On Tue, Oct 19, 2010 at 02:24:00AM +0300, Brian Gitonga Marete wrote:
>> >> The following patch fixes compilation of the perf user-space tools on,
>> >> for example, gcc version 4.3.3 (Ubuntu 4.3.3-5ubuntu4) . It should not
>> >> break anything else.
>> >
>> >
>> >
>> > Hi,
>> >
>> > What kind of warning have you encountered and why it fixes it?
>> > Can you describe that in your changelog?
>> >
>>
>> Hello Frederic,
>>
>> Some versions of gcc, e.g. gcc version 4.3.3 (Ubuntu 4.3.3-5ubuntu4), have the
>> (default) minimum size of buffers protected by `-fstack-protector' set to 8. But
>> in perf, there exist much smaller automatic buffers.
>
> Hm, it's this code:
>
>        /* newtWinChoice should really be accepting const char pointers... */
>        char yes[] = "Yes", no[] = "No";
>        return newtWinChoice(NULL, yes, no, (char *)msg) == 1;
>
> I.e. the code is messy and GCC is right to warn about it. Hence it would be somewhat
> bad to actually remove the warning that pointed out some dodgy piece of code.
>
> Even if marking it const doesnt work due to the external libnewt API, we could at
> least put 'yes' and 'no' into file scope and mark them static?

OK. Now that I actually look closely at that fragment I can see its
useless to create the automatic arrays. Local string literals would
also work (i.e. just pass `"Yes"' and `"No"' to newtWinChoice). But
can also do what you suggested if it is anticipated that they will be
used somewhere else within the file at some other time -- Currently
they are not.

Thanks.
-- 
Brian Gitonga Marete
Toshnix Systems
Tel: +254722151590

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

* Re: [PATCH] [PERF] (Userspace Tools) Fix a compilation error with -fstack-protector and -Werror
  2010-10-19 11:12       ` Brian Gitonga Marete
@ 2010-10-19 11:33         ` Brian Gitonga Marete
  2010-10-19 11:37           ` Brian Gitonga Marete
  2010-10-19 11:49           ` Ingo Molnar
  0 siblings, 2 replies; 15+ messages in thread
From: Brian Gitonga Marete @ 2010-10-19 11:33 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Frederic Weisbecker, LKML, Arnaldo Carvalho de Melo, Peter Zijlstra

On Tue, Oct 19, 2010 at 2:12 PM, Brian Gitonga Marete
<marete@toshnix.com> wrote:
> On Tue, Oct 19, 2010 at 9:40 AM, Ingo Molnar <mingo@elte.hu> wrote:
>>
>> * Brian Gitonga Marete <marete@toshnix.com> wrote:
>>
>>> On Tue, Oct 19, 2010 at 2:38 AM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
>>> > On Tue, Oct 19, 2010 at 02:24:00AM +0300, Brian Gitonga Marete wrote:
>>> >> The following patch fixes compilation of the perf user-space tools on,
>>> >> for example, gcc version 4.3.3 (Ubuntu 4.3.3-5ubuntu4) . It should not
>>> >> break anything else.
>>> >
>>> >
>>> >
>>> > Hi,
>>> >
>>> > What kind of warning have you encountered and why it fixes it?
>>> > Can you describe that in your changelog?
>>> >
>>>
>>> Hello Frederic,
>>>
>>> Some versions of gcc, e.g. gcc version 4.3.3 (Ubuntu 4.3.3-5ubuntu4), have the
>>> (default) minimum size of buffers protected by `-fstack-protector' set to 8. But
>>> in perf, there exist much smaller automatic buffers.
>>
>> Hm, it's this code:
>>
>>        /* newtWinChoice should really be accepting const char pointers... */
>>        char yes[] = "Yes", no[] = "No";
>>        return newtWinChoice(NULL, yes, no, (char *)msg) == 1;
>>
>> I.e. the code is messy and GCC is right to warn about it. Hence it would be somewhat
>> bad to actually remove the warning that pointed out some dodgy piece of code.
>>
>> Even if marking it const doesnt work due to the external libnewt API, we could at
>> least put 'yes' and 'no' into file scope and mark them static?
>
> OK. Now that I actually look closely at that fragment I can see its
> useless to create the automatic arrays. Local string literals would
> also work (i.e. just pass `"Yes"' and `"No"' to newtWinChoice). But
> can also do what you suggested if it is anticipated that they will be
> used somewhere else within the file at some other time -- Currently
> they are not.
>

Oops. Sorry. What I suggested won't work because of the
-Wwrite-strings default option. Which actually makes me understand why
the original author of the code made it the way it is. Your suggestion
of file-scope, static does solve the problem.


-- 
Brian Gitonga Marete
Toshnix Systems
Tel: +254722151590

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

* Re: [PATCH] [PERF] (Userspace Tools) Fix a compilation error with -fstack-protector and -Werror
  2010-10-19 11:33         ` Brian Gitonga Marete
@ 2010-10-19 11:37           ` Brian Gitonga Marete
  2010-10-19 11:49           ` Ingo Molnar
  1 sibling, 0 replies; 15+ messages in thread
From: Brian Gitonga Marete @ 2010-10-19 11:37 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Frederic Weisbecker, LKML, Arnaldo Carvalho de Melo, Peter Zijlstra

On Tue, Oct 19, 2010 at 2:33 PM, Brian Gitonga Marete
<marete@toshnix.com> wrote:
> On Tue, Oct 19, 2010 at 2:12 PM, Brian Gitonga Marete
> <marete@toshnix.com> wrote:
>> On Tue, Oct 19, 2010 at 9:40 AM, Ingo Molnar <mingo@elte.hu> wrote:
>>>
>>> * Brian Gitonga Marete <marete@toshnix.com> wrote:
>>>
>>>> On Tue, Oct 19, 2010 at 2:38 AM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
>>>> > On Tue, Oct 19, 2010 at 02:24:00AM +0300, Brian Gitonga Marete wrote:
>>>> >> The following patch fixes compilation of the perf user-space tools on,
>>>> >> for example, gcc version 4.3.3 (Ubuntu 4.3.3-5ubuntu4) . It should not
>>>> >> break anything else.
>>>> >
>>>> >
>>>> >
>>>> > Hi,
>>>> >
>>>> > What kind of warning have you encountered and why it fixes it?
>>>> > Can you describe that in your changelog?
>>>> >
>>>>
>>>> Hello Frederic,
>>>>
>>>> Some versions of gcc, e.g. gcc version 4.3.3 (Ubuntu 4.3.3-5ubuntu4), have the
>>>> (default) minimum size of buffers protected by `-fstack-protector' set to 8. But
>>>> in perf, there exist much smaller automatic buffers.
>>>
>>> Hm, it's this code:
>>>
>>>        /* newtWinChoice should really be accepting const char pointers... */
>>>        char yes[] = "Yes", no[] = "No";
>>>        return newtWinChoice(NULL, yes, no, (char *)msg) == 1;
>>>
>>> I.e. the code is messy and GCC is right to warn about it. Hence it would be somewhat
>>> bad to actually remove the warning that pointed out some dodgy piece of code.
>>>
>>> Even if marking it const doesnt work due to the external libnewt API, we could at
>>> least put 'yes' and 'no' into file scope and mark them static?
>>
>> OK. Now that I actually look closely at that fragment I can see its
>> useless to create the automatic arrays. Local string literals would
>> also work (i.e. just pass `"Yes"' and `"No"' to newtWinChoice). But
>> can also do what you suggested if it is anticipated that they will be
>> used somewhere else within the file at some other time -- Currently
>> they are not.
>>
>
> Oops. Sorry. What I suggested won't work because of the
> -Wwrite-strings default option. Which actually makes me understand why
> the original author of the code made it the way it is. Your suggestion
> of file-scope, static does solve the problem.
>

As indeed does Americo's suggestion above.


-- 
Brian Gitonga Marete
Toshnix Systems
Tel: +254722151590

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

* Re: [PATCH] [PERF] (Userspace Tools) Fix a compilation error with -fstack-protector and -Werror
  2010-10-19 11:33         ` Brian Gitonga Marete
  2010-10-19 11:37           ` Brian Gitonga Marete
@ 2010-10-19 11:49           ` Ingo Molnar
  2010-10-19 13:11             ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 15+ messages in thread
From: Ingo Molnar @ 2010-10-19 11:49 UTC (permalink / raw)
  To: Brian Gitonga Marete
  Cc: Frederic Weisbecker, LKML, Arnaldo Carvalho de Melo, Peter Zijlstra


* Brian Gitonga Marete <marete@toshnix.com> wrote:

> > OK. Now that I actually look closely at that fragment I can see its useless to 
> > create the automatic arrays. Local string literals would also work (i.e. just 
> > pass `"Yes"' and `"No"' to newtWinChoice). But can also do what you suggested if 
> > it is anticipated that they will be used somewhere else within the file at some 
> > other time -- Currently they are not.
> 
> Oops. Sorry. What I suggested won't work because of the -Wwrite-strings default 
> option. Which actually makes me understand why the original author of the code 
> made it the way it is. Your suggestion of file-scope, static does solve the 
> problem.

Btw., -Wwrite-strings has proven to be a really useful warning in practice, in that 
it ensured that we propagate string immutability/const-ness as widely as possible. 
This resulted is cleaner perf code in the long run.

Here we cannot fix the Newt prototype (it's an existing library outside of our 
control) to take a const so we have to do the (mild) workaround of moving it to file 
scope. (if this becomes common then we'd have to re-evaluate the use of this 
warning)

I think Arnaldo has plans to get rid of the libnewt dependency altogether - that 
might be a fix too.

Thanks,

	Ingo

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

* Re: [PATCH] [PERF] (Userspace Tools) Fix a compilation error with -fstack-protector and -Werror
  2010-10-19 11:49           ` Ingo Molnar
@ 2010-10-19 13:11             ` Arnaldo Carvalho de Melo
  2010-10-24 21:23               ` [PATCH] Fix a compile error with -fstack-protector, -Wstack-protector " Brian Gitonga Marete
  2010-11-11 10:25               ` [PATCH] [PERF] (Userspace Tools) Fix a compilation error with -fstack-protector " Eric Dumazet
  0 siblings, 2 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-10-19 13:11 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Brian Gitonga Marete, Frederic Weisbecker, LKML, Peter Zijlstra

Em Tue, Oct 19, 2010 at 01:49:04PM +0200, Ingo Molnar escreveu:
> 
> * Brian Gitonga Marete <marete@toshnix.com> wrote:
> 
> > > OK. Now that I actually look closely at that fragment I can see its useless to 
> > > create the automatic arrays. Local string literals would also work (i.e. just 
> > > pass `"Yes"' and `"No"' to newtWinChoice). But can also do what you suggested if 
> > > it is anticipated that they will be used somewhere else within the file at some 
> > > other time -- Currently they are not.
> > 
> > Oops. Sorry. What I suggested won't work because of the -Wwrite-strings default 
> > option. Which actually makes me understand why the original author of the code 
> > made it the way it is. Your suggestion of file-scope, static does solve the 
> > problem.
> 
> Btw., -Wwrite-strings has proven to be a really useful warning in practice, in that 
> it ensured that we propagate string immutability/const-ness as widely as possible. 
> This resulted is cleaner perf code in the long run.
> 
> Here we cannot fix the Newt prototype (it's an existing library outside of our 
> control) to take a const so we have to do the (mild) workaround of moving it to file 
> scope. (if this becomes common then we'd have to re-evaluate the use of this 
> warning)
> 
> I think Arnaldo has plans to get rid of the libnewt dependency altogether - that 
> might be a fix too.

Yeah, but for now I'll just reap the results of this long discussion
about this issue. :)

- Arnaldo

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

* [PATCH] Fix a compile error with -fstack-protector, -Wstack-protector and -Werror.
  2010-10-19 13:11             ` Arnaldo Carvalho de Melo
@ 2010-10-24 21:23               ` Brian Gitonga Marete
  2010-11-11 10:25               ` [PATCH] [PERF] (Userspace Tools) Fix a compilation error with -fstack-protector " Eric Dumazet
  1 sibling, 0 replies; 15+ messages in thread
From: Brian Gitonga Marete @ 2010-10-24 21:23 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Brian Gitonga Marete, Ingo Molnar, Frederic Weisbecker, LKML,
	Peter Zijlstra

The following is as per Ingo's suggestion. Agains 2.6.36. Thanks.

In the function ui__dialog_yesno(), automatic buffers have to be used as long
as we are compiling perf with the -Wwrite-stings and -Werror options. This is because the
Newt library does not declare the string parameters in newtWinChoice() as
constant when it should.

But the automatic buffers are not large enough to benefit from ssp at least on some gcc versions, e.g. the one that comes with Ubuntu 9.0.4. This along with -Wstack-protector causes a compile warning which is turned into an error by
-Werror.

This patch works around this problem by making the buffers global and static.

Signed-off-by: Brian Gitonga Marete <marete@toshnix.com>
---
 tools/perf/util/ui/util.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/ui/util.c b/tools/perf/util/ui/util.c
index 04600e2..c7dc022 100644
--- a/tools/perf/util/ui/util.c
+++ b/tools/perf/util/ui/util.c
@@ -11,6 +11,9 @@
 #include "helpline.h"
 #include "util.h"
 
+static char yes[] = "Yes";
+static char no[] = "No";
+
 newtComponent newt_form__new(void);
 
 static void newt_form__set_exit_keys(newtComponent self)
@@ -109,6 +112,5 @@ out_destroy_form:
 bool ui__dialog_yesno(const char *msg)
 {
 	/* newtWinChoice should really be accepting const char pointers... */
-	char yes[] = "Yes", no[] = "No";
 	return newtWinChoice(NULL, yes, no, (char *)msg) == 1;
 }
-- 
1.7.1.1


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

* Re: [PATCH] [PERF] (Userspace Tools) Fix a compilation error with -fstack-protector and -Werror
  2010-10-19 13:11             ` Arnaldo Carvalho de Melo
  2010-10-24 21:23               ` [PATCH] Fix a compile error with -fstack-protector, -Wstack-protector " Brian Gitonga Marete
@ 2010-11-11 10:25               ` Eric Dumazet
  2010-11-11 17:05                 ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 15+ messages in thread
From: Eric Dumazet @ 2010-11-11 10:25 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Brian Gitonga Marete, Frederic Weisbecker, LKML,
	Peter Zijlstra

Le mardi 19 octobre 2010 à 11:11 -0200, Arnaldo Carvalho de Melo a
écrit :
> Em Tue, Oct 19, 2010 at 01:49:04PM +0200, Ingo Molnar escreveu:
> > 
> > * Brian Gitonga Marete <marete@toshnix.com> wrote:
> > 
> > > > OK. Now that I actually look closely at that fragment I can see its useless to 
> > > > create the automatic arrays. Local string literals would also work (i.e. just 
> > > > pass `"Yes"' and `"No"' to newtWinChoice). But can also do what you suggested if 
> > > > it is anticipated that they will be used somewhere else within the file at some 
> > > > other time -- Currently they are not.
> > > 
> > > Oops. Sorry. What I suggested won't work because of the -Wwrite-strings default 
> > > option. Which actually makes me understand why the original author of the code 
> > > made it the way it is. Your suggestion of file-scope, static does solve the 
> > > problem.
> > 
> > Btw., -Wwrite-strings has proven to be a really useful warning in practice, in that 
> > it ensured that we propagate string immutability/const-ness as widely as possible. 
> > This resulted is cleaner perf code in the long run.
> > 
> > Here we cannot fix the Newt prototype (it's an existing library outside of our 
> > control) to take a const so we have to do the (mild) workaround of moving it to file 
> > scope. (if this becomes common then we'd have to re-evaluate the use of this 
> > warning)
> > 
> > I think Arnaldo has plans to get rid of the libnewt dependency altogether - that 
> > might be a fix too.
> 
> Yeah, but for now I'll just reap the results of this long discussion
> about this issue. :)
> 

Hi Arnaldo

Sorry if I missed something, but current linux-2.6 tree has the problem.

Is the fix under control ?

Thanks !



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

* Re: [PATCH] [PERF] (Userspace Tools) Fix a compilation error with -fstack-protector and -Werror
  2010-11-11 10:25               ` [PATCH] [PERF] (Userspace Tools) Fix a compilation error with -fstack-protector " Eric Dumazet
@ 2010-11-11 17:05                 ` Arnaldo Carvalho de Melo
  2010-11-11 17:13                   ` Eric Dumazet
  0 siblings, 1 reply; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-11-11 17:05 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Ingo Molnar, Brian Gitonga Marete, Frederic Weisbecker, LKML,
	Peter Zijlstra, Cyrill Gorcunov

Em Thu, Nov 11, 2010 at 11:25:35AM +0100, Eric Dumazet escreveu:
> Hi Arnaldo
> 
> Sorry if I missed something, but current linux-2.6 tree has the problem.
> 
> Is the fix under control ?

Ingo merged this while I'm away, its in tip/urgent.

commit a3da8e451321c31d88cebd12c234d0aac2a1cc35
Author: Cyrill Gorcunov <gorcunov@gmail.com>
Date:   Sat Nov 6 11:47:24 2010 +0300

    perf, ui: Eliminate stack-smashing protection compiler complaint


- Arnaldo

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

* Re: [PATCH] [PERF] (Userspace Tools) Fix a compilation error with -fstack-protector and -Werror
  2010-11-11 17:05                 ` Arnaldo Carvalho de Melo
@ 2010-11-11 17:13                   ` Eric Dumazet
  0 siblings, 0 replies; 15+ messages in thread
From: Eric Dumazet @ 2010-11-11 17:13 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, Brian Gitonga Marete, Frederic Weisbecker, LKML,
	Peter Zijlstra, Cyrill Gorcunov

Le jeudi 11 novembre 2010 à 12:05 -0500, Arnaldo Carvalho de Melo a
écrit :

> Ingo merged this while I'm away, its in tip/urgent.

So far so good ;)

Thanks !



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

end of thread, other threads:[~2010-11-11 17:13 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-18 23:24 [PATCH] [PERF] (Userspace Tools) Fix a compilation error with -fstack-protector and -Werror Brian Gitonga Marete
2010-10-18 23:38 ` Frederic Weisbecker
2010-10-19  0:06   ` Brian Gitonga Marete
2010-10-19  0:20     ` Frederic Weisbecker
2010-10-19  6:40     ` Ingo Molnar
2010-10-19  9:03       ` Américo Wang
2010-10-19 11:12       ` Brian Gitonga Marete
2010-10-19 11:33         ` Brian Gitonga Marete
2010-10-19 11:37           ` Brian Gitonga Marete
2010-10-19 11:49           ` Ingo Molnar
2010-10-19 13:11             ` Arnaldo Carvalho de Melo
2010-10-24 21:23               ` [PATCH] Fix a compile error with -fstack-protector, -Wstack-protector " Brian Gitonga Marete
2010-11-11 10:25               ` [PATCH] [PERF] (Userspace Tools) Fix a compilation error with -fstack-protector " Eric Dumazet
2010-11-11 17:05                 ` Arnaldo Carvalho de Melo
2010-11-11 17:13                   ` Eric Dumazet

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).