public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [patch] Fix LD_PRELOAD=/usr/lib64/libasan.so.6 gdb
@ 2021-05-02 13:34 Jan Kratochvil
  2021-05-02 13:39 ` Simon Marchi
  0 siblings, 1 reply; 5+ messages in thread
From: Jan Kratochvil @ 2021-05-02 13:34 UTC (permalink / raw)
  To: gdb-patches

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

Hi,

AddressSanitizer: alloc-dealloc-mismatch (malloc vs operator delete) on 0x613000000040
https://bugzilla.redhat.com/show_bug.cgi?id=1510413

The gdb binary had so far on Fedora 34 x86_64:

                 U operator delete[](void*)
                 U operator delete[](void*, unsigned long)
                 U operator delete(void*, unsigned long)

While now there are defined all these - but IMO it is a bit unpredictable
which will be used in the future:

00000000000000c0 T operator delete[](void*)
00000000000000d0 T operator delete[](void*, std::nothrow_t const&)
00000000000000e0 T operator delete[](void*, unsigned long)
0000000000000090 T operator delete(void*)
00000000000000a0 T operator delete(void*, std::nothrow_t const&)
00000000000000b0 T operator delete(void*, unsigned long)


Jan

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

gdbsupport/
2021-05-02  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* new-op.cc (opertor delete 6x): New.

diff --git a/gdbsupport/new-op.cc b/gdbsupport/new-op.cc
index 5ab19621a43..f70d3ef191d 100644
--- a/gdbsupport/new-op.cc
+++ b/gdbsupport/new-op.cc
@@ -92,4 +92,44 @@ operator new[] (std::size_t sz, const std::nothrow_t&) noexcept
 {
   return ::operator new (sz, std::nothrow);
 }
+
+/* Define also operators delete as one can LD_PRELOAD=libasan.so.*
+   without recompiling the program with -fsanitize=address . */
+
+void
+operator delete (void *p)
+{
+  free (p);
+}
+
+void
+operator delete (void *p, const std::nothrow_t&) noexcept
+{
+  return ::operator delete (p);
+}
+
+void
+operator delete (void *p, std::size_t) noexcept
+{
+  return ::operator delete (p, std::nothrow);
+}
+
+void
+operator delete[] (void *p)
+{
+  return ::operator delete (p);
+}
+
+void
+operator delete[] (void *p, const std::nothrow_t&) noexcept
+{
+  return ::operator delete (p, std::nothrow);
+}
+
+void
+operator delete[] (void *p, std::size_t) noexcept
+{
+  return ::operator delete[] (p, std::nothrow);
+}
+
 #endif

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

* Re: [patch] Fix LD_PRELOAD=/usr/lib64/libasan.so.6 gdb
  2021-05-02 13:34 [patch] Fix LD_PRELOAD=/usr/lib64/libasan.so.6 gdb Jan Kratochvil
@ 2021-05-02 13:39 ` Simon Marchi
  2021-05-02 13:56   ` Jan Kratochvil
  0 siblings, 1 reply; 5+ messages in thread
From: Simon Marchi @ 2021-05-02 13:39 UTC (permalink / raw)
  To: Jan Kratochvil, gdb-patches

On 2021-05-02 9:34 a.m., Jan Kratochvil via Gdb-patches wrote:
> Hi,
> 
> AddressSanitizer: alloc-dealloc-mismatch (malloc vs operator delete) on 0x613000000040
> https://bugzilla.redhat.com/show_bug.cgi?id=1510413
> 
> The gdb binary had so far on Fedora 34 x86_64:
> 
>                  U operator delete[](void*)
>                  U operator delete[](void*, unsigned long)
>                  U operator delete(void*, unsigned long)
> 
> While now there are defined all these - but IMO it is a bit unpredictable

This last sentence doesn't sound grammatically correct (and I don't
understand what you mean).

> which will be used in the future:
> 
> 00000000000000c0 T operator delete[](void*)
> 00000000000000d0 T operator delete[](void*, std::nothrow_t const&)
> 00000000000000e0 T operator delete[](void*, unsigned long)
> 0000000000000090 T operator delete(void*)
> 00000000000000a0 T operator delete(void*, std::nothrow_t const&)
> 00000000000000b0 T operator delete(void*, unsigned long)

Please make sure to include all the relevant information about the issue
you observed in the commit message.  It's really not clear by reading it
what's the problem and why your change fixes it.

Simon

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

* Re: [patch] Fix LD_PRELOAD=/usr/lib64/libasan.so.6 gdb
  2021-05-02 13:39 ` Simon Marchi
@ 2021-05-02 13:56   ` Jan Kratochvil
  2021-05-02 14:30     ` Simon Marchi
  0 siblings, 1 reply; 5+ messages in thread
From: Jan Kratochvil @ 2021-05-02 13:56 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On Sun, 02 May 2021 15:39:12 +0200, Simon Marchi wrote:
> Please make sure to include all the relevant information about the issue
> you observed in the commit message.  It's really not clear by reading it
> what's the problem and why your change fixes it.

I was not aware GDB has changed the commit log format:

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

Currently for a binary compiled normally (without -fsanitize=address) but with
LD_PRELOAD of ASAN one gets:

$ ASAN_OPTIONS=detect_leaks=0:alloc_dealloc_mismatch=1:abort_on_error=1:fast_unwind_on_malloc=0 LD_PRELOAD=/usr/lib64/libasan.so.6 gdb
=================================================================
==1909567==ERROR: AddressSanitizer: alloc-dealloc-mismatch (malloc vs operator delete []) on 0x602000001570
    #0 0x7f1c98e5efa7 in operator delete[](void*) (/usr/lib64/libasan.so.6+0xb0fa7)
...
0x602000001570 is located 0 bytes inside of 2-byte region [0x602000001570,0x602000001572)
allocated by thread T0 here:
    #0 0x7f1c98e5cd1f in __interceptor_malloc (/usr/lib64/libasan.so.6+0xaed1f)
    #1 0x557ee4a42e81 in operator new(unsigned long) (/usr/libexec/gdb+0x74ce81)
SUMMARY: AddressSanitizer: alloc-dealloc-mismatch (/usr/lib64/libasan.so.6+0xb0fa7) in operator delete[](void*)
==1909567==HINT: if you don't care about these errors you may set ASAN_OPTIONS=alloc_dealloc_mismatch=0
==1909567==ABORTING

Despite the code called properly operator new[] and operator delete[].
But GDB's new-op.cc provides its own operator new[] which gets translated into
malloc() (which gets recongized as operatore new(size_t)) but as it does not
translate also operators delete[] Address Sanitizer gets confused.

The question is how many variants of the delete operator need to be provided.
Currently GDB does not call the nothrow delete operators (but it calls nothrow
new operators).

gdbsupport/
2021-05-02  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* new-op.cc (opertor delete 6x): New.

diff --git a/gdbsupport/new-op.cc b/gdbsupport/new-op.cc
index 5ab19621a43..f70d3ef191d 100644
--- a/gdbsupport/new-op.cc
+++ b/gdbsupport/new-op.cc
@@ -92,4 +92,44 @@ operator new[] (std::size_t sz, const std::nothrow_t&) noexcept
 {
   return ::operator new (sz, std::nothrow);
 }
+
+/* Define also operators delete as one can LD_PRELOAD=libasan.so.*
+   without recompiling the program with -fsanitize=address . */
+
+void
+operator delete (void *p)
+{
+  free (p);
+}
+
+void
+operator delete (void *p, const std::nothrow_t&) noexcept
+{
+  return ::operator delete (p);
+}
+
+void
+operator delete (void *p, std::size_t) noexcept
+{
+  return ::operator delete (p, std::nothrow);
+}
+
+void
+operator delete[] (void *p)
+{
+  return ::operator delete (p);
+}
+
+void
+operator delete[] (void *p, const std::nothrow_t&) noexcept
+{
+  return ::operator delete (p, std::nothrow);
+}
+
+void
+operator delete[] (void *p, std::size_t) noexcept
+{
+  return ::operator delete[] (p, std::nothrow);
+}
+
 #endif


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

* Re: [patch] Fix LD_PRELOAD=/usr/lib64/libasan.so.6 gdb
  2021-05-02 13:56   ` Jan Kratochvil
@ 2021-05-02 14:30     ` Simon Marchi
  2021-05-02 14:41       ` Jan Kratochvil
  0 siblings, 1 reply; 5+ messages in thread
From: Simon Marchi @ 2021-05-02 14:30 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

On 2021-05-02 9:56 a.m., Jan Kratochvil wrote:> On Sun, 02 May 2021 15:39:12 +0200, Simon Marchi wrote:
>> Please make sure to include all the relevant information about the issue
>> you observed in the commit message.  It's really not clear by reading it
>> what's the problem and why your change fixes it.
> 
> I was not aware GDB has changed the commit log format:
> 
> ------------------------------------------------------------------------------
> 
> Currently for a binary compiled normally (without -fsanitize=address) but with
> LD_PRELOAD of ASAN one gets:
> 
> $ ASAN_OPTIONS=detect_leaks=0:alloc_dealloc_mismatch=1:abort_on_error=1:fast_unwind_on_malloc=0 LD_PRELOAD=/usr/lib64/libasan.so.6 gdb
> =================================================================
> ==1909567==ERROR: AddressSanitizer: alloc-dealloc-mismatch (malloc vs operator delete []) on 0x602000001570
>     #0 0x7f1c98e5efa7 in operator delete[](void*) (/usr/lib64/libasan.so.6+0xb0fa7)
> ...
> 0x602000001570 is located 0 bytes inside of 2-byte region [0x602000001570,0x602000001572)
> allocated by thread T0 here:
>     #0 0x7f1c98e5cd1f in __interceptor_malloc (/usr/lib64/libasan.so.6+0xaed1f)
>     #1 0x557ee4a42e81 in operator new(unsigned long) (/usr/libexec/gdb+0x74ce81)
> SUMMARY: AddressSanitizer: alloc-dealloc-mismatch (/usr/lib64/libasan.so.6+0xb0fa7) in operator delete[](void*)
> ==1909567==HINT: if you don't care about these errors you may set ASAN_OPTIONS=alloc_dealloc_mismatch=0
> ==1909567==ABORTING
> 
> Despite the code called properly operator new[] and operator delete[].
> But GDB's new-op.cc provides its own operator new[] which gets translated into
> malloc() (which gets recongized as operatore new(size_t)) but as it does not
> translate also operators delete[] Address Sanitizer gets confused.
> 
> The question is how many variants of the delete operator need to be provided.
> Currently GDB does not call the nothrow delete operators (but it calls nothrow
> new operators).

I'm not very familiar with the nothrow concept, so I can't really tell
whether this is OK.  I would give my LGTM to the patch, but because of
this bit let's wait a bit to see if anybody else has something to say.
If there's no news in a week, then that's ok to push.

> 
> gdbsupport/
> 2021-05-02  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* new-op.cc (opertor delete 6x): New.
> 
> diff --git a/gdbsupport/new-op.cc b/gdbsupport/new-op.cc
> index 5ab19621a43..f70d3ef191d 100644
> --- a/gdbsupport/new-op.cc
> +++ b/gdbsupport/new-op.cc
> @@ -92,4 +92,44 @@ operator new[] (std::size_t sz, const std::nothrow_t&) noexcept
>  {
>    return ::operator new (sz, std::nothrow);
>  }
> +
> +/* Define also operators delete as one can LD_PRELOAD=libasan.so.*
> +   without recompiling the program with -fsanitize=address . */

Here, just add the precision that it is to prevent
alloc_dealloc_mismatch errors that we do this.

Thanks,

Simon

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

* Re: [patch] Fix LD_PRELOAD=/usr/lib64/libasan.so.6 gdb
  2021-05-02 14:30     ` Simon Marchi
@ 2021-05-02 14:41       ` Jan Kratochvil
  0 siblings, 0 replies; 5+ messages in thread
From: Jan Kratochvil @ 2021-05-02 14:41 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On Sun, 02 May 2021 16:30:19 +0200, Simon Marchi wrote:
> On 2021-05-02 9:56 a.m., Jan Kratochvil wrote:> On Sun, 02 May 2021 15:39:12 +0200, Simon Marchi wrote:
> > The question is how many variants of the delete operator need to be provided.
> > Currently GDB does not call the nothrow delete operators (but it calls nothrow
> > new operators).
> 
> I'm not very familiar with the nothrow concept, so I can't really tell
> whether this is OK.

Updated below.


Jan


Currently for a binary compiled normally (without -fsanitize=address) but with
LD_PRELOAD of ASAN one gets:

$ ASAN_OPTIONS=detect_leaks=0:alloc_dealloc_mismatch=1:abort_on_error=1:fast_unwind_on_malloc=0 LD_PRELOAD=/usr/lib64/libasan.so.6 gdb
=================================================================
==1909567==ERROR: AddressSanitizer: alloc-dealloc-mismatch (malloc vs operator delete []) on 0x602000001570
    #0 0x7f1c98e5efa7 in operator delete[](void*) (/usr/lib64/libasan.so.6+0xb0fa7)
...
0x602000001570 is located 0 bytes inside of 2-byte region [0x602000001570,0x602000001572)
allocated by thread T0 here:
    #0 0x7f1c98e5cd1f in __interceptor_malloc (/usr/lib64/libasan.so.6+0xaed1f)
    #1 0x557ee4a42e81 in operator new(unsigned long) (/usr/libexec/gdb+0x74ce81)
SUMMARY: AddressSanitizer: alloc-dealloc-mismatch (/usr/lib64/libasan.so.6+0xb0fa7) in operator delete[](void*)
==1909567==HINT: if you don't care about these errors you may set ASAN_OPTIONS=alloc_dealloc_mismatch=0
==1909567==ABORTING

Despite the code called properly operator new[] and operator delete[].
But GDB's new-op.cc provides its own operator new[] which gets translated into
malloc() (which gets recongized as operatore new(size_t)) but as it does not
translate also operators delete[] Address Sanitizer gets confused.

The question is how many variants of the delete operator need to be provided.
There could be 14 operators new but there are only 4, GDB uses 3 of them.
There could be 16 operators delete but there are only 6, GDB uses 2 of them.
It depends on libraries and compiler which of the operators will get used.
Currently being used:
                 U operator new[](unsigned long)
                 U operator new(unsigned long)
                 U operator new(unsigned long, std::nothrow_t const&)
                 U operator delete[](void*)
                 U operator delete(void*, unsigned long)

gdbsupport/
2021-05-02  Jan Kratochvil  <jan.kratochvil@redhat.com>

        * new-op.cc (opertor delete 6x): New.

diff --git a/gdbsupport/new-op.cc b/gdbsupport/new-op.cc
index 5ab19621a43..2f4c71457b1 100644
--- a/gdbsupport/new-op.cc
+++ b/gdbsupport/new-op.cc
@@ -92,4 +92,46 @@ operator new[] (std::size_t sz, const std::nothrow_t&) noexcept
 {
   return ::operator new (sz, std::nothrow);
 }
+
+/* Define also operators delete as one can LD_PRELOAD=libasan.so.*
+   without recompiling the program with -fsanitize=address and then one would
+   get false positive alloc-dealloc-mismatch (malloc vs operator delete [])
+   errors from AddressSanitizers.  */
+
+void
+operator delete (void *p)
+{
+  free (p);
+}
+
+void
+operator delete (void *p, const std::nothrow_t&) noexcept
+{
+  return ::operator delete (p);
+}
+
+void
+operator delete (void *p, std::size_t) noexcept
+{
+  return ::operator delete (p, std::nothrow);
+}
+
+void
+operator delete[] (void *p)
+{
+  return ::operator delete (p);
+}
+
+void
+operator delete[] (void *p, const std::nothrow_t&) noexcept
+{
+  return ::operator delete (p, std::nothrow);
+}
+
+void
+operator delete[] (void *p, std::size_t) noexcept
+{
+  return ::operator delete[] (p, std::nothrow);
+}
+
 #endif


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

end of thread, other threads:[~2021-05-02 14:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-02 13:34 [patch] Fix LD_PRELOAD=/usr/lib64/libasan.so.6 gdb Jan Kratochvil
2021-05-02 13:39 ` Simon Marchi
2021-05-02 13:56   ` Jan Kratochvil
2021-05-02 14:30     ` Simon Marchi
2021-05-02 14:41       ` Jan Kratochvil

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