This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] gdb: Replace operator new / operator new[]


If xmalloc fails allocating memory, usually because something tried a
huge allocation, like xmalloc(-1) or some such, GDB asks the user what
to do:

  .../src/gdb/utils.c:1079: internal-error: virtual memory exhausted.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  Quit this debugging session? (y or n)

If the user says "n", that throws a QUIT exception, which is caught by
one of the multiple CATCH(RETURN_MASK_ALL) blocks somewhere up the
stack.

The default implementations of operator new / operator new[] call
malloc directly, and on memory allocation throw std::bad_alloc.
Currently, if that happens, since nothing catches std::bad_alloc, the
exception escapes out of main, and GDB aborts from unhandled
exception.

This patch replaces the default operator new variants with versions
that, just like xmalloc:

 #1 - Raise an internal-error on memory allocation failure.

 #2 - Throw a QUIT gdb_exception, so that the exact same CATCH blocks
      continue handling memory allocation problems.

A minor complication of #2 is that operator new can _only_ throw
std:bad_alloc, or something that extends it:

  void* operator new (std::size_t size) throw (std::bad_alloc);

That means that if we let a gdb QUIT exception escape from within
operator new, the C++ runtime aborts due to unexpected exception
thrown.

So to bridge the gap, this patch adds a new gdb_quit_bad_alloc
exception type that inherits both std::bad_alloc and gdb_exception,
and throws _that_.

If we decide that we should be catching memory allocation errors in
fewer places than all the places we currently catch them (everywhere
we use RETURN_MASK_ALL currently), then we could change operator new
to throw plain std::bad_alloc then.  But I'm considering such a change
as separate matter from this one -- it'd make sense to do the same to
xmalloc at the same time, for instance.

Meanwhile, this allows using new/new[] instead of xmalloc/XNEW/etc.
without losing the "virtual memory exhausted" internal-error
safeguard.

Tested on x86_64 Fedora 23.

gdb/ChangeLog:
2016-09-22  Pedro Alves  <palves@redhat.com>

	* Makefile.in (SFILES): Add common/new-op.c.
	(COMMON_OBS): Add common/new-op.o.
	(new-op.o): New rule.
	* common/common-exceptions.h: Include <new>.
	(struct gdb_quit_bad_alloc): New type.
	* common/new-op.c: New file.

gdb/gdbserver/ChangeLog:
2016-09-22  Pedro Alves  <palves@redhat.com>

	* Makefile.in (SFILES): Add common/new-op.c.
	(OBS): Add common/new-op.o.
	(new-op.o): New rule.
---
 gdb/Makefile.in                |  8 +++-
 gdb/common/common-exceptions.h | 20 ++++++++++
 gdb/common/new-op.c            | 85 ++++++++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/Makefile.in      |  8 +++-
 4 files changed, 117 insertions(+), 4 deletions(-)
 create mode 100644 gdb/common/new-op.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 354705e..5a0093c 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -892,7 +892,7 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
 	target/waitstatus.c common/print-utils.c common/rsp-low.c \
 	common/errors.c common/common-debug.c common/common-exceptions.c \
 	common/btrace-common.c common/fileio.c common/common-regcache.c \
-	common/signals-state-save-restore.c \
+	common/signals-state-save-restore.c common/new-op.c \
 	$(SUBDIR_GCC_COMPILE_SRCS)
 
 LINTFILES = $(SFILES) $(YYFILES) $(CONFIG_SRCS) init.c
@@ -1087,7 +1087,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	format.o registry.o btrace.o record-btrace.o waitstatus.o \
 	print-utils.o rsp-low.o errors.o common-debug.o debug.o \
 	common-exceptions.o btrace-common.o fileio.o \
-	common-regcache.o \
+	common-regcache.o new-op.o \
 	$(SUBDIR_GCC_COMPILE_OBS)
 
 TSOBS = inflow.o
@@ -2288,6 +2288,10 @@ signals-state-save-restore.o: $(srcdir)/common/signals-state-save-restore.c
 	$(COMPILE) $(srcdir)/common/signals-state-save-restore.c
 	$(POSTCOMPILE)
 
+new-op.o: ${srcdir}/common/new-op.c
+	$(COMPILE) $(srcdir)/common/new-op.c
+	$(POSTCOMPILE)
+
 #
 # gdb/target/ dependencies
 #
diff --git a/gdb/common/common-exceptions.h b/gdb/common/common-exceptions.h
index c494de2..6bf7e40 100644
--- a/gdb/common/common-exceptions.h
+++ b/gdb/common/common-exceptions.h
@@ -21,6 +21,7 @@
 #define COMMON_EXCEPTIONS_H
 
 #include <setjmp.h>
+#include <new>
 
 /* Reasons for calling throw_exceptions().  NOTE: all reason values
    must be less than zero.  enum value 0 is reserved for internal use
@@ -283,6 +284,25 @@ struct gdb_exception_RETURN_MASK_QUIT : public gdb_exception_RETURN_MASK_ALL
 
 #endif /* GDB_XCPT_TRY || GDB_XCPT_RAW_TRY */
 
+/* An exception type that inherits from both std::bad_alloc and a gdb
+   exception.  This is necessary because operator new can only throw
+   std::bad_alloc, and OTOH, we want exceptions thrown due to memory
+   allocation error to be caught by all the CATCH/RETURN_MASK_ALL
+   spread around the codebase.  */
+
+struct gdb_quit_bad_alloc
+  : public gdb_exception_RETURN_MASK_QUIT,
+    public std::bad_alloc
+{
+  explicit gdb_quit_bad_alloc (gdb_exception ex)
+    : std::bad_alloc ()
+  {
+    gdb_exception *self = this;
+
+    *self = ex;
+  }
+};
+
 /* *INDENT-ON* */
 
 /* Throw an exception (as described by "struct gdb_exception").  When
diff --git a/gdb/common/new-op.c b/gdb/common/new-op.c
new file mode 100644
index 0000000..c3b73c1
--- /dev/null
+++ b/gdb/common/new-op.c
@@ -0,0 +1,85 @@
+/* Replace operator new/new[], for GDB, the GNU debugger.
+
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "common-defs.h"
+#include "host-defs.h"
+#include <new>
+
+/* Override operator new / operator [], in order to internal_error on
+   allocation failure and thus query the user for abort/core
+   dump/continue, just like xmalloc does.  We don't do this from a
+   new-handler function instead (std::set_new_handler) because we want
+   to catch allocation errors from within global constructors too.
+
+   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
+   nothrow variants and call malloc directly.  */
+
+void *
+operator new (std::size_t sz)
+{
+  /* malloc (0) is unpredictable; avoid it.  */
+  if (sz == 0)
+    sz = 1;
+
+  void *p = malloc (sz);	/* ARI: malloc */
+  if (p == NULL)
+    {
+      /* If the user decides to continue debugging, throw a
+	 gdb_quit_bad_alloc exception instead of a regular QUIT
+	 gdb_exception.  The former extends both std::bad_alloc and a
+	 QUIT gdb_exception.  This is necessary because operator new
+	 can only ever throw std::bad_alloc, or something that extends
+	 it.  */
+      TRY
+	{
+	  malloc_failure (sz);
+	}
+      CATCH (ex, RETURN_MASK_ALL)
+	{
+	  do_cleanups (all_cleanups ());
+
+	  throw gdb_quit_bad_alloc (ex);
+	}
+      END_CATCH
+    }
+  return p;
+}
+
+void *
+operator new (std::size_t sz, const std::nothrow_t&)
+{
+  /* malloc (0) is unpredictable; avoid it.  */
+  if (sz == 0)
+    sz = 1;
+  return malloc (sz);		/* ARI: malloc */
+}
+
+void *
+operator new[] (std::size_t sz)
+{
+   return ::operator new (sz);
+}
+
+void*
+operator new[] (size_t sz, const std::nothrow_t&)
+{
+  return ::operator new (sz, std::nothrow);
+}
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 309b496..6d5abd3 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -180,7 +180,8 @@ SFILES=	$(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
 	$(srcdir)/common/btrace-common.c \
 	$(srcdir)/common/fileio.c $(srcdir)/nat/linux-namespaces.c \
 	$(srcdir)/arch/arm.c $(srcdir)/common/common-regcache.c \
-	$(srcdir)/arch/arm-linux.c $(srcdir)/arch/arm-get-next-pcs.c
+	$(srcdir)/arch/arm-linux.c $(srcdir)/arch/arm-get-next-pcs.c \
+	$(srcdir)/common/new-op.c
 
 DEPFILES = @GDBSERVER_DEPFILES@
 
@@ -195,7 +196,7 @@ OBS = agent.o ax.o inferiors.o regcache.o remote-utils.o server.o signals.o \
       common-utils.o ptid.o buffer.o format.o filestuff.o dll.o notif.o \
       tdesc.o print-utils.o rsp-low.o errors.o common-debug.o cleanups.o \
       common-exceptions.o symbol.o btrace-common.o fileio.o common-regcache.o \
-      signals-state-save-restore.o \
+      signals-state-save-restore.o new-op.o \
       $(XML_BUILTIN) $(DEPFILES) $(LIBOBJS)
 GDBREPLAY_OBS = gdbreplay.o version.o
 GDBSERVER_LIBS = @GDBSERVER_LIBS@
@@ -723,6 +724,9 @@ common-regcache.o: ../common/common-regcache.c
 signals-state-save-restore.o: ../common/signals-state-save-restore.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
+new-op.o: ../common/new-op.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
 
 # Arch object files rules form ../arch
 
-- 
2.5.5


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]