public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCHv5] gdb: Replace memcpy with std::copy to avoid some g++ warnings on sparc
@ 2023-01-25  8:55 Mark Wielaard
  2023-01-25 12:00 ` Hannes Domani
  0 siblings, 1 reply; 3+ messages in thread
From: Mark Wielaard @ 2023-01-25  8:55 UTC (permalink / raw)
  To: gdb-patches
  Cc: Simon Marchi, Sam James, Tom Tromey, John Baldwin, Mark Wielaard

For some reason g++ 12.2.1 on sparc produces spurious warnings for
stringop-overread and restrict in fbsd-tdep.c for some memcpy calls.
Use std::copy to avoid those.

In function ‘void* memcpy(void*, const void*, size_t)’,
    inlined from ‘gdb::optional<std::vector<unsigned char, gdb::default_init_allocator<unsigned char, std::allocator<unsigned char> > > > fbsd_make_note_desc(target_object, uint32_t)’ at ../../binutils-gdb/gdb/fbsd-tdep.c:666:10:
/usr/include/bits/string_fortified.h:29:33: error: ‘void* __builtin_memcpy(void*, const void*, long unsigned int)’ specified bound 18446744073709551612 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]

In function ‘void* memcpy(void*, const void*, size_t)’,
    inlined from ‘gdb::optional<std::vector<unsigned char, gdb::default_init_allocator<unsigned char, std::allocator<unsigned char> > > > fbsd_make_note_desc(target_object, uint32_t)’ at ../../binutils-gdb/gdb/fbsd-tdep.c:673:10:
/usr/include/bits/string_fortified.h:29:33: error: ‘void* __builtin_memcpy(void*, const void*, long unsigned int)’ accessing 18446744073709551612 bytes at offsets 0 and 0 overlaps 9223372036854775801 bytes at offset -9223372036854775805 [-Werror=restrict]

gdb/ChangeLog:

	* fbsd-tdep.c (fbsd_make_note_desc): Use std::copy instead
	of memcpy.
---

 V5: Use buf->begin (), buf->end () in second std::copy
 V4: Fix std::copy argument typo
 V3: Drop diagnostic suppressions just use std::copy
 V2: Fix typos and add example errors to commit messages

 gdb/fbsd-tdep.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
index 203390d9880..d03ea419aa8 100644
--- a/gdb/fbsd-tdep.c
+++ b/gdb/fbsd-tdep.c
@@ -662,8 +662,8 @@ fbsd_make_note_desc (enum target_object object, uint32_t structsize)
     return buf;
 
   gdb::byte_vector desc (sizeof (structsize) + buf->size ());
-  memcpy (desc.data (), &structsize, sizeof (structsize));
-  memcpy (desc.data () + sizeof (structsize), buf->data (), buf->size ());
+  std::copy (&structsize, &structsize + sizeof (structsize), desc.data ());
+  std::copy (buf->begin (), buf->end (), desc.data () + sizeof (structsize));
   return desc;
 }
 
-- 
2.31.1


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

* Re: [PATCHv5] gdb: Replace memcpy with std::copy to avoid some g++ warnings on sparc
  2023-01-25  8:55 [PATCHv5] gdb: Replace memcpy with std::copy to avoid some g++ warnings on sparc Mark Wielaard
@ 2023-01-25 12:00 ` Hannes Domani
  2023-01-25 21:20   ` Mark Wielaard
  0 siblings, 1 reply; 3+ messages in thread
From: Hannes Domani @ 2023-01-25 12:00 UTC (permalink / raw)
  To: gdb-patches, Mark Wielaard
  Cc: Simon Marchi, Sam James, Tom Tromey, John Baldwin

 Am Mittwoch, 25. Januar 2023 um 09:56:03 MEZ hat Mark Wielaard <mark@klomp.org> Folgendes geschrieben:

> For some reason g++ 12.2.1 on sparc produces spurious warnings for
> stringop-overread and restrict in fbsd-tdep.c for some memcpy calls.
> Use std::copy to avoid those.
>
> In function ‘void* memcpy(void*, const void*, size_t)’,
>     inlined from ‘gdb::optional<std::vector<unsigned char, gdb::default_init_allocator<unsigned char, std::allocator<unsigned char> > > > fbsd_make_note_desc(target_object, uint32_t)’ at ../../binutils-gdb/gdb/fbsd-tdep.c:666:10:
> /usr/include/bits/string_fortified.h:29:33: error: ‘void* __builtin_memcpy(void*, const void*, long unsigned int)’ specified bound 18446744073709551612 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
>
> In function ‘void* memcpy(void*, const void*, size_t)’,
>     inlined from ‘gdb::optional<std::vector<unsigned char, gdb::default_init_allocator<unsigned char, std::allocator<unsigned char> > > > fbsd_make_note_desc(target_object, uint32_t)’ at ../../binutils-gdb/gdb/fbsd-tdep.c:673:10:
> /usr/include/bits/string_fortified.h:29:33: error: ‘void* __builtin_memcpy(void*, const void*, long unsigned int)’ accessing 18446744073709551612 bytes at offsets 0 and 0 overlaps 9223372036854775801 bytes at offset -9223372036854775805 [-Werror=restrict]
>
> gdb/ChangeLog:
>
>     * fbsd-tdep.c (fbsd_make_note_desc): Use std::copy instead
>     of memcpy.
> ---
>
> V5: Use buf->begin (), buf->end () in second std::copy
> V4: Fix std::copy argument typo
> V3: Drop diagnostic suppressions just use std::copy
> V2: Fix typos and add example errors to commit messages
>
> gdb/fbsd-tdep.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
> index 203390d9880..d03ea419aa8 100644
> --- a/gdb/fbsd-tdep.c
> +++ b/gdb/fbsd-tdep.c
> @@ -662,8 +662,8 @@ fbsd_make_note_desc (enum target_object object, uint32_t structsize)
>     return buf;
>
>   gdb::byte_vector desc (sizeof (structsize) + buf->size ());
> -  memcpy (desc.data (), &structsize, sizeof (structsize));
> -  memcpy (desc.data () + sizeof (structsize), buf->data (), buf->size ());
> +  std::copy (&structsize, &structsize + sizeof (structsize), desc.data ());

structsize is of type uint32_t, so wouldn't that effectively copy 4*4 bytes, instead of just 4?

Shouldn't it rather look like this?:

  std::copy (&structsize, &structsize + 1, desc.data ());


> +  std::copy (buf->begin (), buf->end (), desc.data () + sizeof (structsize));
>   return desc;
> }
>
> --
> 2.31.1


Hannes

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

* Re: [PATCHv5] gdb: Replace memcpy with std::copy to avoid some g++ warnings on sparc
  2023-01-25 12:00 ` Hannes Domani
@ 2023-01-25 21:20   ` Mark Wielaard
  0 siblings, 0 replies; 3+ messages in thread
From: Mark Wielaard @ 2023-01-25 21:20 UTC (permalink / raw)
  To: Hannes Domani
  Cc: gdb-patches, Simon Marchi, Sam James, Tom Tromey, John Baldwin

Hi Hannes,

On Wed, Jan 25, 2023 at 12:00:05PM +0000, Hannes Domani wrote:
>  Am Mittwoch, 25. Januar 2023 um 09:56:03 MEZ hat Mark Wielaard <mark@klomp.org> Folgendes geschrieben:
> 
> >   gdb::byte_vector desc (sizeof (structsize) + buf->size ());
> > -  memcpy (desc.data (), &structsize, sizeof (structsize));
> > -  memcpy (desc.data () + sizeof (structsize), buf->data (), buf->size ());
> > +  std::copy (&structsize, &structsize + sizeof (structsize), desc.data ());
> 
> structsize is of type uint32_t, so wouldn't that effectively copy 4*4 bytes, instead of just 4?
> 
> Shouldn't it rather look like this?:
> 
>   std::copy (&structsize, &structsize + 1, desc.data ());

Aha, yeah, std::copy acts on the whole elements of the input. Maybe
less confusing to just keep using memcpy here. It is only the second
memcpy that produces the issue on sparc. That also brings down the fix
to a simple oneliner. Will sent a v6.

Thanks,

Mark

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

end of thread, other threads:[~2023-01-25 21:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-25  8:55 [PATCHv5] gdb: Replace memcpy with std::copy to avoid some g++ warnings on sparc Mark Wielaard
2023-01-25 12:00 ` Hannes Domani
2023-01-25 21:20   ` Mark Wielaard

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 read-only IMAP folder(s) and NNTP newsgroup(s).