[PATCH] gdb: fix sparc memcpy fortify error

[PATCH] gdb: fix sparc memcpy fortify error

[Mike Frysinger]

Building on an x86_64-linux system with --enable-targets=all fails on the
sparc code with a fortify error:

cc1: warnings being treated as errors
In file included from /usr/include/string.h:640,
                 from gnulib/string.h:23,
                 from ../../gdb/gdb_string.h:25,
                 from ../../gdb/vec.h:25,
                 from ../../gdb/memattr.h:24,
                 from ../../gdb/target.h:60,
                 from ../../gdb/exec.h:23,
                 from ../../gdb/gdbcore.h:31,
                 from ../../gdb/sparc-tdep.c:29:
In function 'memcpy',
    inlined from 'sparc32_store_return_value' at ../../gdb/sparc-tdep.c:1112,
    inlined from 'sparc32_return_value' at ../../gdb/sparc-tdep.c:1170:
/usr/include/bits/string3.h:52: error: call to __builtin___memcpy_chk will
	always overflow destination buffer
make: *** [sparc-tdep.o] Error 1

So have the buffer be the max of the lengths used with memcpy().

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
2010-03-18  Mike Frysinger  <vapier@gentoo.org>

	* gdb/sparc-tdep.c (sparc32_store_return_value): Declare the length
	of buf using the "len" variable.

 gdb/sparc-tdep.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
index f129a55..ea0cdd2 100644
--- a/gdb/sparc-tdep.c
+++ b/gdb/sparc-tdep.c
@@ -1101,7 +1101,7 @@ sparc32_store_return_value (struct type *type, struct regcache *regcache,
 			    const gdb_byte *valbuf)
 {
   int len = TYPE_LENGTH (type);
-  gdb_byte buf[8];
+  gdb_byte buf[max(len, 8)];
 
   gdb_assert (!sparc_structure_or_union_p (type));
   gdb_assert (!(sparc_floating_p (type) && len == 16));
-- 
1.7.0.2

Re: [PATCH] gdb: fix sparc memcpy fortify error

[Eli Zaretskii]

> From: Mike Frysinger <vapier@gentoo.org>
> Date: Thu, 18 Mar 2010 22:09:09 -0400
> 
> -  gdb_byte buf[8];
> +  gdb_byte buf[max(len, 8)];

Isn't this Standard C only under C9x?  If so, we don't yet require
that standard version, AFAIK.

Re: [PATCH] gdb: fix sparc memcpy fortify error

[Hui Zhu]

Hi,

I got this issue in a amd64 ubuntu 9.04.
I reported it before, someone tell me that this is the bug of gcc.

Now, I handle this issue in this pc is  remove all the sparc function
from init.c file.

Thanks,
Hui

On Fri, Mar 19, 2010 at 15:44, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Mike Frysinger <vapier@gentoo.org>
>> Date: Thu, 18 Mar 2010 22:09:09 -0400
>>
>> -  gdb_byte buf[8];
>> +  gdb_byte buf[max(len, 8)];
>
> Isn't this Standard C only under C9x?  If so, we don't yet require
> that standard version, AFAIK.
>

Re: [PATCH] gdb: fix sparc memcpy fortify error

[Andreas Schwab]

Hui Zhu <teawater@gmail.com> writes:

> I reported it before, someone tell me that this is the bug of gcc.

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37060

Andreas.

-- 
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."

Re: [PATCH] gdb: fix sparc memcpy fortify error

[Mark Kettenis]

> From: Mike Frysinger <vapier@gentoo.org>
> Date: Thu, 18 Mar 2010 22:09:09 -0400
> 
> Building on an x86_64-linux system with --enable-targets=all fails on the
> sparc code with a fortify error:
> 
> cc1: warnings being treated as errors
> In file included from /usr/include/string.h:640,
>                  from gnulib/string.h:23,
>                  from ../../gdb/gdb_string.h:25,
>                  from ../../gdb/vec.h:25,
>                  from ../../gdb/memattr.h:24,
>                  from ../../gdb/target.h:60,
>                  from ../../gdb/exec.h:23,
>                  from ../../gdb/gdbcore.h:31,
>                  from ../../gdb/sparc-tdep.c:29:
> In function 'memcpy',
>     inlined from 'sparc32_store_return_value' at ../../gdb/sparc-tdep.c:1112,
>     inlined from 'sparc32_return_value' at ../../gdb/sparc-tdep.c:1170:
> /usr/include/bits/string3.h:52: error: call to __builtin___memcpy_chk will
> 	always overflow destination buffer
> make: *** [sparc-tdep.o] Error 1

That's a false positive I'm afraid.  I agree that it isn't trivial to
see that there is no buffer overflow here.  Unfortunately your
solution is a bit problematic:

> 2010-03-18  Mike Frysinger  <vapier@gentoo.org>
> 
> 	* gdb/sparc-tdep.c (sparc32_store_return_value): Declare the length
> 	of buf using the "len" variable.
> 
>  gdb/sparc-tdep.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
> index f129a55..ea0cdd2 100644
> --- a/gdb/sparc-tdep.c
> +++ b/gdb/sparc-tdep.c
> @@ -1101,7 +1101,7 @@ sparc32_store_return_value (struct type *type, struct regcache *regcache,
>  			    const gdb_byte *valbuf)
>  {
>    int len = TYPE_LENGTH (type);
> -  gdb_byte buf[8];
> +  gdb_byte buf[max(len, 8)];

Sorry, but variable sized arrays aren't C90.

Does it help if you replace

>    gdb_assert (!(sparc_floating_p (type) && len == 16));

with

     gdb_assert (len <= 8);

?

Re: [PATCH] gdb: fix sparc memcpy fortify error

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 238 bytes --]

On Friday 19 March 2010 05:25:49 Mark Kettenis wrote:
> Does it help if you replace
> 
> >    gdb_assert (!(sparc_floating_p (type) && len == 16));
> 
> with
> 
>      gdb_assert (len <= 8);

yes, that works on my system
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] gdb: fix sparc memcpy fortify error

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 283 bytes --]

On Friday 19 March 2010 04:18:06 Hui Zhu wrote:
> I got this issue in a amd64 ubuntu 9.04.
> I reported it before, someone tell me that this is the bug of gcc.

hrm, i searched a little before, but i guess i didnt use the right keywords to 
find the previous discussion
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH] gdb: workaround sparc memcpy fortify error

[Mike Frysinger]

Building on an x86_64-linux system with --enable-targets=all fails on the
sparc code with a fortify error:

cc1: warnings being treated as errors
In file included from /usr/include/string.h:640,
                 from gnulib/string.h:23,
                 from ../../gdb/gdb_string.h:25,
                 from ../../gdb/vec.h:25,
                 from ../../gdb/memattr.h:24,
                 from ../../gdb/target.h:60,
                 from ../../gdb/exec.h:23,
                 from ../../gdb/gdbcore.h:31,
                 from ../../gdb/sparc-tdep.c:29:
In function 'memcpy',
    inlined from 'sparc32_store_return_value' at ../../gdb/sparc-tdep.c:1112,
    inlined from 'sparc32_return_value' at ../../gdb/sparc-tdep.c:1170:
/usr/include/bits/string3.h:52: error: call to __builtin___memcpy_chk will
	always overflow destination buffer
make: *** [sparc-tdep.o] Error 1

This is due to the gcc optimizer bug PR37060, so tweak the gdb_assert ()
to avoid the issue.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
2010-03-19  Mike Frysinger  <vapier@gentoo.org>

	* gdb/sparc-tdep.c (sparc32_store_return_value): Change gdb_assert ()
	to len <= 8.

 gdb/sparc-tdep.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
index f129a55..ab45ddd 100644
--- a/gdb/sparc-tdep.c
+++ b/gdb/sparc-tdep.c
@@ -1062,7 +1062,7 @@ sparc32_extract_return_value (struct type *type, struct regcache *regcache,
   gdb_byte buf[8];
 
   gdb_assert (!sparc_structure_or_union_p (type));
-  gdb_assert (!(sparc_floating_p (type) && len == 16));
+  gdb_assert (len <= 8);
 
   if (sparc_floating_p (type))
     {
-- 
1.7.0.2

Re: [PATCH] gdb: workaround sparc memcpy fortify error

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 172 bytes --]

On Friday 19 March 2010 16:11:30 Mike Frysinger wrote:
> This is due to the gcc optimizer bug PR37060, so tweak the gdb_assert ()
> to avoid the issue.

is this OK ?
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] gdb: workaround sparc memcpy fortify error

[Joel Brobecker]

> > This is due to the gcc optimizer bug PR37060, so tweak the gdb_assert ()
> > to avoid the issue.
> 
> is this OK ?

ENOPATCH?

-- 
Joel

Re: [PATCH] gdb: workaround sparc memcpy fortify error

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

>> > This is due to the gcc optimizer bug PR37060, so tweak the gdb_assert ()
>> > to avoid the issue.
>> 
>> is this OK ?

Joel> ENOPATCH?

It was a ping for http://permalink.gmane.org/gmane.comp.gdb.patches/56350

Tom

Re: [PATCH] gdb: workaround sparc memcpy fortify error

[Joel Brobecker]

> Joel> ENOPATCH?
> 
> It was a ping for http://permalink.gmane.org/gmane.comp.gdb.patches/56350

Aha - Mark Kettenis seemed to be OK with this, since he's the one
who suggested it. Mark, is this patch still OK?

2010-03-19  Mike Frysinger  <vapier <at> gentoo.org>

        * gdb/sparc-tdep.c (sparc32_store_return_value): Change gdb_assert ()
        to len <= 8.

gdb/sparc-tdep.c |    2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
index f129a55..ab45ddd 100644
--- a/gdb/sparc-tdep.c
+++ b/gdb/sparc-tdep.c
@@ -1062,7 +1062,7 @@ sparc32_extract_return_value (struct type *type, struct regcache *regcache,
   gdb_byte buf[8];

   gdb_assert (!sparc_structure_or_union_p (type));
-  gdb_assert (!(sparc_floating_p (type) && len == 16));
+  gdb_assert (len <= 8);

   if (sparc_floating_p (type))
     {

-- 
Joel

Re: [PATCH] gdb: workaround sparc memcpy fortify error

[Mark Kettenis]

> X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 	tests=BAYES_00,TW_CP,TW_EG
> X-Spam-Check-By: sourceware.org
> Date: Wed, 21 Apr 2010 12:01:41 -0400
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Mike Frysinger <vapier@gentoo.org>, gdb-patches@sourceware.org
> Content-Disposition: inline
> Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm
> Sender: gdb-patches-owner@sourceware.org
> X-XS4ALL-DNSBL-Checked: mxdrop118.xs4all.nl checked 209.132.180.131 against DNS blacklists
> X-CNFS-Analysis: v=1.1 cv=a8sYchbnJd2dMYUqqjolUMD0rF/qLqJCTuzWyWz0xZo= c=1
> 	sm=0 a=_NDgYHZHEuYA:10 a=ORa4HqFjfvEA:10 a=kj9zAlcOel0A:10
> 	a=vbYRN7G9ZuyAWxq09MFwFw==:17 a=TSbVqHtbAAAA:8 a=7mOBRU54AAAA:8
> 	a=v8Wh4-vryUNciPy22swA:9 a=PQkedLpdK-Zry27wWXRhssVy98EA:4
> 	a=CjuIK1q_8ugA:10 a=6pLrmOwY6VMA:10 a=vbYRN7G9ZuyAWxq09MFwFw==:117
> X-Virus-Scanned: by XS4ALL Virus Scanner
> X-XS4ALL-Spam-Score: 0.0 () none
> X-XS4ALL-Spam: NO
> Envelope-To: m.m.kettenis@xs4all.nl
> 
> > Joel> ENOPATCH?
> > 
> > It was a ping for http://permalink.gmane.org/gmane.comp.gdb.patches/56350
> 
> Aha - Mark Kettenis seemed to be OK with this, since he's the one
> who suggested it. Mark, is this patch still OK?

Hmm, can we keep the existing gdb_assert() and just add the len <= 8
one?  With that change, this is fine with me.

> 2010-03-19  Mike Frysinger  <vapier <at> gentoo.org>
> 
>         * gdb/sparc-tdep.c (sparc32_store_return_value): Change gdb_assert ()
>         to len <= 8.
> 
> gdb/sparc-tdep.c |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c
> index f129a55..ab45ddd 100644
> --- a/gdb/sparc-tdep.c
> +++ b/gdb/sparc-tdep.c
> @@ -1062,7 +1062,7 @@ sparc32_extract_return_value (struct type *type, struct regcache *regcache,
>    gdb_byte buf[8];
> 
>    gdb_assert (!sparc_structure_or_union_p (type));
> -  gdb_assert (!(sparc_floating_p (type) && len == 16));
> +  gdb_assert (len <= 8);
> 
>    if (sparc_floating_p (type))
>      {
> 
> -- 
> Joel
> 

Re: [PATCH] gdb: workaround sparc memcpy fortify error

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 482 bytes --]

On Wednesday 21 April 2010 12:08:54 Mark Kettenis wrote:
> > > It was a ping for
> > > http://permalink.gmane.org/gmane.comp.gdb.patches/56350
> > 
> > Aha - Mark Kettenis seemed to be OK with this, since he's the one
> > who suggested it. Mark, is this patch still OK?
> 
> Hmm, can we keep the existing gdb_assert() and just add the len <= 8
> one?  With that change, this is fine with me.

fine by me.  i can also add a comment about the gcc PR if people want.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] gdb: workaround sparc memcpy fortify error

[Mark Kettenis]

> From: Mike Frysinger <vapier@gentoo.org>
> Date: Wed, 21 Apr 2010 15:40:43 -0400
> 
> On Wednesday 21 April 2010 12:08:54 Mark Kettenis wrote:
> > > > It was a ping for
> > > > http://permalink.gmane.org/gmane.comp.gdb.patches/56350
> > >=20
> > > Aha - Mark Kettenis seemed to be OK with this, since he's the one
> > > who suggested it. Mark, is this patch still OK?
> >=20
> > Hmm, can we keep the existing gdb_assert() and just add the len <=3D 8
> > one?  With that change, this is fine with me.
> 
> fine by me.  i can also add a comment about the gcc PR if people want.

Nah, I don't think that's really necessary in this case.

Re: [PATCH] gdb: workaround sparc memcpy fortify error

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 752 bytes --]

this is what i ended up committing then.  thanks guys.
-mike

2010-04-21  Mike Frysinger  <vapier@gentoo.org>

	* gdb/sparc-tdep.c (sparc32_store_return_value): Add gdb_assert ()
	for len <= 8.

Index: gdb/sparc-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc-tdep.c,v
retrieving revision 1.211
diff -u -p -r1.211 sparc-tdep.c
--- gdb/sparc-tdep.c	1 Jan 2010 07:31:42 -0000	1.211
+++ gdb/sparc-tdep.c	21 Apr 2010 20:10:16 -0000
@@ -1105,6 +1105,7 @@ sparc32_store_return_value (struct type 
 
   gdb_assert (!sparc_structure_or_union_p (type));
   gdb_assert (!(sparc_floating_p (type) && len == 16));
+  gdb_assert (len <= 8);
 
   if (sparc_floating_p (type))
     {

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]