Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] Don't override operator new if GDB is built with -fsanitize=address
@ 2016-10-25  9:21 Yao Qi
  2016-10-25  9:39 ` Pedro Alves
  0 siblings, 1 reply; 6+ messages in thread
From: Yao Qi @ 2016-10-25  9:21 UTC (permalink / raw)
  To: gdb-patches

Nowadays, if we build GDB with -fsanitize=address, we can get the asan
error below,

(gdb) quit
=================================================================
==9723==ERROR: AddressSanitizer: alloc-dealloc-mismatch (malloc vs operator delete) on 0x60200003bf70
    #0 0x7f88f3837527 in operator delete(void*) (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x55527)
    #1 0xac8e13 in __gnu_cxx::new_allocator<void (*)()>::deallocate(void (**)(), unsigned long) /usr/include/c++/4.9/ext/new_allocator.h:110
    #2 0xac8cc2 in __gnu_cxx::__alloc_traits<std::allocator<void (*)()> >::deallocate(std::allocator<void (*)()>&, void (**)(), unsigned long) /usr/include/c++/4.9/ext/alloc_traits.h:185
....
0x60200003bf70 is located 0 bytes inside of 8-byte region [0x60200003bf70,0x60200003bf78)
allocated by thread T0 here:
    #0 0x7f88f38367ef in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x547ef)
    #1 0xbd2762 in operator new(unsigned long) /home/yao/SourceCode/gnu/gdb/git/gdb/common/new-op.c:42
    #2 0xac8edc in __gnu_cxx::new_allocator<void (*)()>::allocate(unsigned long, void const*) /usr/include/c++/4.9/ext/new_allocator.h:104
    #3 0xac8d81 in __gnu_cxx::__alloc_traits<std::allocator<void (*)()> >::allocate(std::allocator<void (*)()>&, unsigned long) /usr/include/c++/4.9/ext/alloc_traits.h:182

The reason for this is that we override operator new but don't override
operator delete.  This patch does the override only if the code is NOT
compiled with asan.

gdb:

2016-10-25  Yao Qi  <yao.qi@linaro.org>

	PR gdb/20716
	* common/new-op.c (__has_feature): New macro.
	Don't override operator new if asan is used.
---
 gdb/common/new-op.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/gdb/common/new-op.c b/gdb/common/new-op.c
index 5ba4d6e..f04c5cb 100644
--- a/gdb/common/new-op.c
+++ b/gdb/common/new-op.c
@@ -17,6 +17,12 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+/* GCC does not understand __has_feature.  */
+#if !defined(__has_feature)
+# define __has_feature(x) 0
+#endif
+
+#if !__has_feature(address_sanitizer) && !defined(__SANITIZE_ADDRESS__)
 #include "common-defs.h"
 #include "host-defs.h"
 #include <new>
@@ -83,3 +89,4 @@ operator new[] (std::size_t sz, const std::nothrow_t&)
 {
   return ::operator new (sz, std::nothrow);
 }
+#endif
-- 
1.9.1


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

* Re: [PATCH] Don't override operator new if GDB is built with -fsanitize=address
  2016-10-25  9:21 [PATCH] Don't override operator new if GDB is built with -fsanitize=address Yao Qi
@ 2016-10-25  9:39 ` Pedro Alves
       [not found]   ` <CAH=s-POJSmo1rm2JaxmSG7GwqYqRxgv0tMYJKYcix8ddcvctFw@mail.gmail.com>
  0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2016-10-25  9:39 UTC (permalink / raw)
  To: Yao Qi, gdb-patches

On 10/25/2016 10:21 AM, Yao Qi wrote:
> Nowadays, if we build GDB with -fsanitize=address, we can get the asan
> error below,
> 
> (gdb) quit
> =================================================================
> ==9723==ERROR: AddressSanitizer: alloc-dealloc-mismatch (malloc vs operator delete) on 0x60200003bf70
>     #0 0x7f88f3837527 in operator delete(void*) (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x55527)
>     #1 0xac8e13 in __gnu_cxx::new_allocator<void (*)()>::deallocate(void (**)(), unsigned long) /usr/include/c++/4.9/ext/new_allocator.h:110
>     #2 0xac8cc2 in __gnu_cxx::__alloc_traits<std::allocator<void (*)()> >::deallocate(std::allocator<void (*)()>&, void (**)(), unsigned long) /usr/include/c++/4.9/ext/alloc_traits.h:185
> ....
> 0x60200003bf70 is located 0 bytes inside of 8-byte region [0x60200003bf70,0x60200003bf78)
> allocated by thread T0 here:
>     #0 0x7f88f38367ef in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x547ef)
>     #1 0xbd2762 in operator new(unsigned long) /home/yao/SourceCode/gnu/gdb/git/gdb/common/new-op.c:42
>     #2 0xac8edc in __gnu_cxx::new_allocator<void (*)()>::allocate(unsigned long, void const*) /usr/include/c++/4.9/ext/new_allocator.h:104
>     #3 0xac8d81 in __gnu_cxx::__alloc_traits<std::allocator<void (*)()> >::allocate(std::allocator<void (*)()>&, unsigned long) /usr/include/c++/4.9/ext/alloc_traits.h:182
> 
> The reason for this is that we override operator new but don't override
> operator delete.  This patch does the override only if the code is NOT
> compiled with asan.
> 
> gdb:
> 
> 2016-10-25  Yao Qi  <yao.qi@linaro.org>
> 
> 	PR gdb/20716
> 	* common/new-op.c (__has_feature): New macro.
> 	Don't override operator new if asan is used.

LGTM.

Thanks,
Pedro Alves


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

* Re: [PATCH] Don't override operator new if GDB is built with -fsanitize=address
       [not found]   ` <CAH=s-POJSmo1rm2JaxmSG7GwqYqRxgv0tMYJKYcix8ddcvctFw@mail.gmail.com>
@ 2016-10-25 10:38     ` Pedro Alves
  2016-10-25 10:40       ` Pedro Alves
  0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2016-10-25 10:38 UTC (permalink / raw)
  To: Yao Qi, GDB Patches

[Re-adding the list.]

On 10/25/2016 11:15 AM, Yao Qi wrote:
> On Tue, Oct 25, 2016 at 10:39 AM, Pedro Alves <palves@redhat.com> wrote:
>>
>> LGTM.
>>
> 
> Patch is pushed in.
> 

How about this follow up?

From e58eb16a5f27b1ee39c45642a80da7364763a07b Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 25 Oct 2016 11:20:03 +0100
Subject: [PATCH] new-op.c: Add comment about -fsanitize=address

gdb/ChangeLog:
2016-10-25  Pedro Alves  <palves@redhat.com>

	* common/new-op.c: Add comment about -fsanitize=address.
---
 gdb/common/new-op.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gdb/common/new-op.c b/gdb/common/new-op.c
index f04c5cb..1eb4f94 100644
--- a/gdb/common/new-op.c
+++ b/gdb/common/new-op.c
@@ -33,6 +33,12 @@
    new-handler function instead (std::set_new_handler) because we want
    to catch allocation errors from within global constructors too.
 +   Skip overriding if building with -fsanitize=address though.
+   Address sanitizer wants to override operator new/delete too in
+   order to detect malloc+delete and new+free mismatches.  Our
+   versions would mask out ASan's, with the result of losing that
+   useful mismatch detection.
+
    Note that C++ implementations could either have their throw
    versions call the nothrow versions (libstdc++), or the other way
    around (clang/libc++).  For that reason, we replace both throw and
-- 
2.5.5



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

* Re: [PATCH] Don't override operator new if GDB is built with -fsanitize=address
  2016-10-25 10:38     ` Pedro Alves
@ 2016-10-25 10:40       ` Pedro Alves
  2016-10-25 11:39         ` Yao Qi
  0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2016-10-25 10:40 UTC (permalink / raw)
  To: Yao Qi, GDB Patches

On 10/25/2016 11:38 AM, Pedro Alves wrote:
> How about this follow up?

On 10/25/2016 11:38 AM, Pedro Alves wrote:

> +++ b/gdb/common/new-op.c
> @@ -33,6 +33,12 @@
>     new-handler function instead (std::set_new_handler) because we want
>     to catch allocation errors from within global constructors too.
>  +   Skip overriding if building with -fsanitize=address though.
> +   Address sanitizer wants to override operator new/delete too in

Bah, looks like "edit as new" with Thunderbird messed
up the patch...  Here it is again...

From e58eb16a5f27b1ee39c45642a80da7364763a07b Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 25 Oct 2016 11:20:03 +0100
Subject: [PATCH] new-op.c: Add comment about -fsanitize=address

gdb/ChangeLog:
2016-10-25  Pedro Alves  <palves@redhat.com>

	* common/new-op.c: Add comment about -fsanitize=address.
---
 gdb/common/new-op.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gdb/common/new-op.c b/gdb/common/new-op.c
index f04c5cb..1eb4f94 100644
--- a/gdb/common/new-op.c
+++ b/gdb/common/new-op.c
@@ -33,6 +33,12 @@
    new-handler function instead (std::set_new_handler) because we want
    to catch allocation errors from within global constructors too.
 
+   Skip overriding if building with -fsanitize=address though.
+   Address sanitizer wants to override operator new/delete too in
+   order to detect malloc+delete and new+free mismatches.  Our
+   versions would mask out ASan's, with the result of losing that
+   useful mismatch detection.
+
    Note that C++ implementations could either have their throw
    versions call the nothrow versions (libstdc++), or the other way
    around (clang/libc++).  For that reason, we replace both throw and
-- 
2.5.5



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

* Re: [PATCH] Don't override operator new if GDB is built with -fsanitize=address
  2016-10-25 10:40       ` Pedro Alves
@ 2016-10-25 11:39         ` Yao Qi
  2016-10-25 12:33           ` Pedro Alves
  0 siblings, 1 reply; 6+ messages in thread
From: Yao Qi @ 2016-10-25 11:39 UTC (permalink / raw)
  To: Pedro Alves; +Cc: GDB Patches

On Tue, Oct 25, 2016 at 11:40 AM, Pedro Alves <palves@redhat.com> wrote:
>
> gdb/ChangeLog:
> 2016-10-25  Pedro Alves  <palves@redhat.com>
>
>         * common/new-op.c: Add comment about -fsanitize=address.

Patch is good to me.

-- 
Yao (齐尧)


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

* Re: [PATCH] Don't override operator new if GDB is built with -fsanitize=address
  2016-10-25 11:39         ` Yao Qi
@ 2016-10-25 12:33           ` Pedro Alves
  0 siblings, 0 replies; 6+ messages in thread
From: Pedro Alves @ 2016-10-25 12:33 UTC (permalink / raw)
  To: Yao Qi; +Cc: GDB Patches

On 10/25/2016 12:39 PM, Yao Qi wrote:
> On Tue, Oct 25, 2016 at 11:40 AM, Pedro Alves <palves@redhat.com> wrote:
>>
>> gdb/ChangeLog:
>> 2016-10-25  Pedro Alves  <palves@redhat.com>
>>
>>         * common/new-op.c: Add comment about -fsanitize=address.
> 
> Patch is good to me.
> 

Pushed.

Thanks,
Pedro Alves


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

end of thread, other threads:[~2016-10-25 12:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-25  9:21 [PATCH] Don't override operator new if GDB is built with -fsanitize=address Yao Qi
2016-10-25  9:39 ` Pedro Alves
     [not found]   ` <CAH=s-POJSmo1rm2JaxmSG7GwqYqRxgv0tMYJKYcix8ddcvctFw@mail.gmail.com>
2016-10-25 10:38     ` Pedro Alves
2016-10-25 10:40       ` Pedro Alves
2016-10-25 11:39         ` Yao Qi
2016-10-25 12:33           ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox