[PATCH 00/15 v2] Common code cleanups

[PATCH 00/15 v2] Common code cleanups

[Gary Benson]

Hi all,

This series is version 2 of the common code cleanups patch I posted
last Wednesday.  I'll repeat the description of the series from the
previous [PATCH 00/15] email:

  The directories "common", "nat" and "target" contain code shared
  between GDB and gdbserver.  Each C file is built at least twice,
  once for GDB and once for gdbserver, and many of the files use
  "#ifdef GDBSERVER" to source headers from GDB or gdbserver as
  appropriate.  This means that while the code is shared, the
  supporting definitions are not, and in some cases these are
  different in non-trivial ways.  In other places, GDBSERVER checks
  are used to cope where GDB and gdbserver do the same thing in
  different ways.

  This series reduces the number of GDBSERVER checks from 34 to 11.
  Tom Tromey started this work back in January, and I've updated it
  and extended it a little.  Most of the remaining checks are to
  select the correct gnulib config header.

I've mailed details of changes I've made to the individual patches to
this list and to the reviewers who suggested them already, so I won't
go over them again.

Built and regtested on x86-64 RHEL6.5.  mips-linux-watch.h changes
checked by cross-building gdbserver with the Sourcery CodeBench for
MIPS GNU/Linux toolchain.

Ok to commit?

Thanks,
Gary

--
http://gbenson.net/

[PATCH 15/15 v2] Finally remove GDBSERVER (mostly) from linux-btrace.c

[Gary Benson]

This applies the usual GDBSERVER treatment to linux-btrace.c.
Now it is used only to find the gnulib header.

gdb/
2014-07-16  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* nat/linux-btrace.c: Don't include defs.h or server.h.
	* nat/linux-btrace.h (struct target_ops): Declare.
---
 gdb/ChangeLog          |    6 ++++++
 gdb/nat/linux-btrace.c |   13 ++++++++++---
 gdb/nat/linux-btrace.h |    2 ++
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/gdb/nat/linux-btrace.c b/gdb/nat/linux-btrace.c
index 3b4180f..30d678c 100644
--- a/gdb/nat/linux-btrace.c
+++ b/gdb/nat/linux-btrace.c
@@ -19,17 +19,24 @@
    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 <config.h>
+
 #ifdef GDBSERVER
-#include "server.h"
+#include "build-gnulib-gdbserver/config.h"
 #else
-#include "defs.h"
+#include "build-gnulib/config.h"
 #endif
 
+#include "common-types.h"
+#include "common-utils.h"
+#include "target/waitstatus.h"
+#include "errors.h"
+#include "gdb_locale.h"
+#include "gdb_signals.h"
 #include "linux-btrace.h"
 #include "common-utils.h"
 #include "gdb_assert.h"
 #include "regcache.h"
-#include "gdbthread.h"
 #include "gdb_wait.h"
 #include "i386-cpuid.h"
 #include "common-regcache.h"
diff --git a/gdb/nat/linux-btrace.h b/gdb/nat/linux-btrace.h
index 12e9b60..e237eea 100644
--- a/gdb/nat/linux-btrace.h
+++ b/gdb/nat/linux-btrace.h
@@ -22,6 +22,8 @@
 #ifndef LINUX_BTRACE_H
 #define LINUX_BTRACE_H
 
+struct target_ops;
+
 #include "btrace-common.h"
 #include "config.h"
 #include "vec.h"
-- 
1.7.1

[PATCH 04/15 v2] Introduce common-types.h

[Gary Benson]

This introduces common-types.h.  This file defines various standard
types used by gdb and gdbserver.

Currently these types are conditionally defined based on GDBSERVER.
The long term goal is to remove all such tests; however, this is
difficult as currently gdb uses definitions from BFD.  In the meantime
this is still a step in the right direction.

gdb/
2014-07-16  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* common/common-types.h: New file.
	* nat/linux-ptrace.c: Include common-types.h.
	* defs.h: Include common-types.h.
	(gdb_byte, CORE_ADDR, CORE_ADDR_MAX, LONGEST, ULONGEST): Remove.

gdb/gdbserver/
2014-07-16  Tom Tromey  <tromey@redhat.com>

	* server.h: Include common-types.h.  Move gdb_assert.h include
	earlier.  Add static assertion.
	(gdb_byte, CORE_ADDR, LONGEST, ULONGEST): Remove.
---
 gdb/ChangeLog             |    8 ++++++
 gdb/common/common-types.h |   61 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/defs.h                |   30 +---------------------
 gdb/gdbserver/ChangeLog   |    6 ++++
 gdb/gdbserver/server.h    |   14 ++--------
 gdb/nat/linux-ptrace.c    |    1 +
 6 files changed, 80 insertions(+), 40 deletions(-)
 create mode 100644 gdb/common/common-types.h

diff --git a/gdb/common/common-types.h b/gdb/common/common-types.h
new file mode 100644
index 0000000..9fa1c24
--- /dev/null
+++ b/gdb/common/common-types.h
@@ -0,0 +1,61 @@
+/* Declarations for common types.
+
+   Copyright (C) 1986-2014 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/>.  */
+
+#ifndef COMMON_TYPES_H
+#define COMMON_TYPES_H
+
+#ifdef GDBSERVER
+
+/* * A byte from the program being debugged.  */
+typedef unsigned char gdb_byte;
+
+typedef unsigned long long CORE_ADDR;
+
+typedef long long LONGEST;
+typedef unsigned long long ULONGEST;
+
+#else /* GDBSERVER */
+
+#include "bfd.h"
+
+/* * A byte from the program being debugged.  */
+typedef bfd_byte gdb_byte;
+
+/* * An address in the program being debugged.  Host byte order.  */
+typedef bfd_vma CORE_ADDR;
+
+/* This is to make sure that LONGEST is at least as big as CORE_ADDR.  */
+
+#ifdef BFD64
+
+typedef BFD_HOST_64_BIT LONGEST;
+typedef BFD_HOST_U_64_BIT ULONGEST;
+
+#else /* No BFD64 */
+
+typedef long long LONGEST;
+typedef unsigned long long ULONGEST;
+
+#endif /* No BFD64 */
+#endif /* GDBSERVER */
+
+/* * The largest CORE_ADDR value.  */
+#define CORE_ADDR_MAX (~ (CORE_ADDR) 0)
+
+#endif /* COMMON_TYPES_H */
diff --git a/gdb/defs.h b/gdb/defs.h
index 511279a..d2a9447 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -96,35 +96,7 @@
 
 #include "libiberty.h"
 #include "hashtab.h"
-
-/* Rather than duplicate all the logic in BFD for figuring out what
-   types to use (which can be pretty complicated), symply define them
-   in terms of the corresponding type from BFD.  */
-
-#include "bfd.h"
-
-/* * A byte from the program being debugged.  */
-typedef bfd_byte gdb_byte;
-
-/* * An address in the program being debugged.  Host byte order.  */
-typedef bfd_vma CORE_ADDR;
-
-/* * The largest CORE_ADDR value.  */
-#define CORE_ADDR_MAX (~ (CORE_ADDR) 0)
-
-/* This is to make sure that LONGEST is at least as big as CORE_ADDR.  */
-
-#ifdef BFD64
-
-#define LONGEST BFD_HOST_64_BIT
-#define ULONGEST BFD_HOST_U_64_BIT
-
-#else /* No BFD64 */
-
-#define LONGEST long long
-#define ULONGEST unsigned long long
-
-#endif /* No BFD64 */
+#include "common-types.h"
 
 #ifndef min
 #define min(a, b) ((a) < (b) ? (a) : (b))
diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
index 2d55513..67214fa 100644
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -77,20 +77,14 @@ int vsnprintf(char *str, size_t size, const char *format, va_list ap);
 #  define PROG "gdbserver"
 #endif
 
-/* A type used for binary buffers.  */
-typedef unsigned char gdb_byte;
-
 #include "ptid.h"
 #include "buffer.h"
 #include "xml-utils.h"
 #include "gdb_locale.h"
+#include "common-types.h"
+#include "gdb_assert.h"
 
-/* FIXME: This should probably be autoconf'd for.  It's an integer type at
-   least the size of a (void *).  */
-typedef unsigned long long CORE_ADDR;
-
-typedef long long LONGEST;
-typedef unsigned long long ULONGEST;
+gdb_static_assert (sizeof (CORE_ADDR) >= sizeof (void *));
 
 #include "regcache.h"
 #include "gdb/signals.h"
@@ -147,8 +141,6 @@ extern int handle_target_event (int err, gdb_client_data client_data);
 #include "utils.h"
 #include "debug.h"
 
-#include "gdb_assert.h"
-
 /* Maximum number of bytes to read/write at once.  The value here
    is chosen to fill up a packet (the headers account for the 32).  */
 #define MAXBUFBYTES(N) (((N)-32)/2)
diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c
index 59c9bfa..e3462ec 100644
--- a/gdb/nat/linux-ptrace.c
+++ b/gdb/nat/linux-ptrace.c
@@ -29,6 +29,7 @@
 #include "buffer.h"
 #include "gdb_assert.h"
 #include "gdb_wait.h"
+#include "common-types.h"
 
 #include <stdint.h>
 
-- 
1.7.1

[PATCH 09/15 v2] Mostly remove GDBSERVER from linux-waitpid.c

[Gary Benson]

This commit mostly removes the use of GDBSERVER from
nat/linux-waitpid.c.  A use remains for some debugging
code that I will remove when the Linux thread_db code
is refactored.

gdb/
2014-07-16  Gary Benson  <gbenson@redhat.com>

	* nat/linux-waitpid.c: Don't include server.h or defs.h.
	(linux_debug) [debug_threads]: New declaration.
---
 gdb/ChangeLog           |    5 +++++
 gdb/nat/linux-waitpid.c |   13 +++++++------
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/gdb/nat/linux-waitpid.c b/gdb/nat/linux-waitpid.c
index 5159f03..8c01b4a 100644
--- a/gdb/nat/linux-waitpid.c
+++ b/gdb/nat/linux-waitpid.c
@@ -17,12 +17,11 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifdef GDBSERVER
-#include "server.h"
-#else
-#include "defs.h"
-#include "signal.h"
-#endif
+#include "config.h"
+
+#include <stdio.h>
+#include <stdarg.h>
+#include <errno.h>
 
 #include "linux-nat.h"
 #include "linux-waitpid.h"
@@ -37,6 +36,8 @@ static inline void
 linux_debug (const char *format, ...)
 {
 #ifdef GDBSERVER
+  extern int debug_threads;
+
   if (debug_threads)
     {
       va_list args;
-- 
1.7.1

[PATCH 14/15 v2] Introduce get_thread_regcache_for_ptid

[Gary Benson]

This introduces get_thread_regcache_for_ptid so that we can simplify
nat/linux-btrace.c.  A better long term solution would be unify the
regcache code, but this is sufficient for now.

gdb/
2014-07-16  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* common/common-regcache.h: New file.
	* regcache.c: Include the above.
	(get_thread_regcache_for_ptid): New function.
	* nat/linux-btrace.c: Include common-regcache.h.
	(perf_event_read_bts): Use get_thread_regcache_for_ptid.

gdb/gdbserver/
2014-07-16  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* regcache.c: Include common-regcache.h.
	(get_thread_regcache_for_ptid): New function.
---
 gdb/ChangeLog                |    9 +++++++++
 gdb/common/common-regcache.h |   28 ++++++++++++++++++++++++++++
 gdb/gdbserver/ChangeLog      |    6 ++++++
 gdb/gdbserver/regcache.c     |    9 +++++++++
 gdb/nat/linux-btrace.c       |    7 ++-----
 gdb/regcache.c               |    8 ++++++++
 6 files changed, 62 insertions(+), 5 deletions(-)
 create mode 100644 gdb/common/common-regcache.h

diff --git a/gdb/common/common-regcache.h b/gdb/common/common-regcache.h
new file mode 100644
index 0000000..2ef7cb1
--- /dev/null
+++ b/gdb/common/common-regcache.h
@@ -0,0 +1,28 @@
+/* Cache and manage the values of registers
+
+   Copyright (C) 1986-2014 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/>.  */
+
+#ifndef COMMON_REGCACHE_H
+#define COMMON_REGCACHE_H
+
+/* A hack until we have an independent regcache.  This must be
+   provided by the user.  */
+
+extern struct regcache *get_thread_regcache_for_ptid (ptid_t ptid);
+
+#endif /* COMMON_REGCACHE_H */
diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
index bed10b4..eb72322 100644
--- a/gdb/gdbserver/regcache.c
+++ b/gdb/gdbserver/regcache.c
@@ -21,6 +21,7 @@
 #include "gdbthread.h"
 #include "tdesc.h"
 #include "rsp-low.h"
+#include "common-regcache.h"
 
 #include <stdlib.h>
 #include <string.h>
@@ -65,6 +66,14 @@ get_thread_regcache (struct thread_info *thread, int fetch)
   return regcache;
 }
 
+/* See common/linux-btrace.h.  */
+
+struct regcache *
+get_thread_regcache_for_ptid (ptid_t ptid)
+{
+  return get_thread_regcache (find_thread_ptid (ptid), 1);
+}
+
 void
 regcache_invalidate_thread (struct thread_info *thread)
 {
diff --git a/gdb/nat/linux-btrace.c b/gdb/nat/linux-btrace.c
index 188220b..3b4180f 100644
--- a/gdb/nat/linux-btrace.c
+++ b/gdb/nat/linux-btrace.c
@@ -32,6 +32,7 @@
 #include "gdbthread.h"
 #include "gdb_wait.h"
 #include "i386-cpuid.h"
+#include "common-regcache.h"
 
 #ifdef HAVE_SYS_SYSCALL_H
 #include <sys/syscall.h>
@@ -184,11 +185,7 @@ perf_event_read_bts (struct btrace_target_info* tinfo, const uint8_t *begin,
   gdb_assert (start <= end);
 
   /* The first block ends at the current pc.  */
-#ifdef GDBSERVER
-  regcache = get_thread_regcache (find_thread_ptid (tinfo->ptid), 1);
-#else
-  regcache = get_thread_regcache (tinfo->ptid);
-#endif
+  regcache = get_thread_regcache_for_ptid (tinfo->ptid);
   block.end = regcache_read_pc (regcache);
 
   /* The buffer may contain a partial record as its last entry (i.e. when the
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 5ee90b0..e66a40e 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -30,6 +30,7 @@
 #include "exceptions.h"
 #include "remote.h"
 #include "valprint.h"
+#include "common-regcache.h"
 
 /*
  * DATA STRUCTURE
@@ -537,6 +538,13 @@ get_current_regcache (void)
   return get_thread_regcache (inferior_ptid);
 }
 
+/* See common/linux-btrace.h.  */
+
+struct regcache *
+get_thread_regcache_for_ptid (ptid_t ptid)
+{
+  return get_thread_regcache (ptid);
+}
 
 /* Observer for the target_changed event.  */
 
-- 
1.7.1

[PATCH 01/15 v2] Introduce common/errors.h

[Gary Benson]

This introduces common/errors.h.  This holds some error- and
warning-related declarations that can be used by the code in common.
Clients of the "common" code must provide definitions for these
functions.

gdb/
2014-07-16  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* common/errors.h: New file.
	* utils.h: Include errors.h.
	(perror_with_name, error, fatal, warning): Don't declare.
	* common/common-utils.h: Include errors.h.
	(malloc_failure, internal_error): Don't declare.

gdb/gdbserver/
2014-07-16  Tom Tromey  <tromey@redhat.com>

	* utils.h: Include errors.h.
	(perror_with_name, error, fatal, warning): Don't declare.
---
 gdb/ChangeLog             |    9 +++++++
 gdb/common/common-utils.h |    5 +---
 gdb/common/errors.h       |   55 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/ChangeLog   |    5 ++++
 gdb/gdbserver/utils.h     |    5 +---
 gdb/utils.h               |   10 +-------
 6 files changed, 72 insertions(+), 17 deletions(-)
 create mode 100644 gdb/common/errors.h

diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h
index 063698d..53be2f8 100644
--- a/gdb/common/common-utils.h
+++ b/gdb/common/common-utils.h
@@ -24,6 +24,7 @@
 #include "ansidecl.h"
 #include <stddef.h>
 #include <stdarg.h>
+#include "errors.h"
 
 /* If possible, define FUNCTION_NAME, a macro containing the name of
    the function being defined.  Since this macro may not always be
@@ -43,10 +44,6 @@
 #endif
 #endif
 
-extern void malloc_failure (long size) ATTRIBUTE_NORETURN;
-extern void internal_error (const char *file, int line, const char *, ...)
-     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 4);
-
 /* xmalloc(), xrealloc() and xcalloc() have already been declared in
    "libiberty.h". */
 
diff --git a/gdb/common/errors.h b/gdb/common/errors.h
new file mode 100644
index 0000000..5f998e3
--- /dev/null
+++ b/gdb/common/errors.h
@@ -0,0 +1,55 @@
+/* Declarations for error-reporting facilities.
+
+   Copyright (C) 1986-2014 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/>.  */
+
+#ifndef COMMON_ERRORS_H
+#define COMMON_ERRORS_H
+
+/* The declarations in this file are, for the time being, separately
+   implemented by gdb and gdbserver.  However they share a common
+   definition so that they can be used by code in common/.  */
+
+/* Like "perror" but throws an exception with the appropriate
+   text.  */
+
+extern void perror_with_name (const char *) ATTRIBUTE_NORETURN;
+
+/* Throw an exception.  */
+
+extern void error (const char *fmt, ...)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
+
+/* Cause a fatal error.  */
+
+extern void fatal (const char *fmt, ...)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
+
+/* Issue a warning.  */
+
+extern void warning (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
+
+/* Internal error.  */
+
+extern void internal_error (const char *file, int line, const char *, ...)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 4);
+
+/* Memory allocation failure.  */
+
+extern void malloc_failure (long size) ATTRIBUTE_NORETURN;
+
+#endif /* COMMON_ERRORS_H */
diff --git a/gdb/gdbserver/utils.h b/gdb/gdbserver/utils.h
index 906924b..819fa35 100644
--- a/gdb/gdbserver/utils.h
+++ b/gdb/gdbserver/utils.h
@@ -20,11 +20,8 @@
 #define UTILS_H
 
 #include "print-utils.h"
+#include "errors.h"
 
-void perror_with_name (const char *string) ATTRIBUTE_NORETURN;
-void error (const char *string,...) ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
-void fatal (const char *string,...) ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
-void warning (const char *string,...) ATTRIBUTE_PRINTF (1, 2);
 char *paddress (CORE_ADDR addr);
 char *pfildes (gdb_fildes_t fd);
 
diff --git a/gdb/utils.h b/gdb/utils.h
index a91f551..d9fe7e3 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -24,6 +24,7 @@
 #include "cleanups.h"
 #include "exceptions.h"
 #include "print-utils.h"
+#include "errors.h"
 
 extern void initialize_utils (void);
 
@@ -269,7 +270,6 @@ extern void fprintf_symbol_filtered (struct ui_file *, const char *,
 
 extern void throw_perror_with_name (enum errors errcode, const char *string)
   ATTRIBUTE_NORETURN;
-extern void perror_with_name (const char *) ATTRIBUTE_NORETURN;
 
 extern void perror_warning_with_name (const char *string);
 
@@ -286,17 +286,11 @@ extern char *warning_pre_print;
 extern void verror (const char *fmt, va_list ap)
      ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
 
-extern void error (const char *fmt, ...)
-     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
-
 extern void error_stream (struct ui_file *) ATTRIBUTE_NORETURN;
 
 extern void vfatal (const char *fmt, va_list ap)
      ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
 
-extern void fatal (const char *fmt, ...)
-     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
-
 extern void internal_verror (const char *file, int line, const char *,
 			     va_list ap)
      ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 0);
@@ -308,8 +302,6 @@ extern void internal_vwarning (const char *file, int line,
 extern void internal_warning (const char *file, int line,
 			      const char *, ...) ATTRIBUTE_PRINTF (3, 4);
 
-extern void warning (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
-
 extern void vwarning (const char *, va_list args) ATTRIBUTE_PRINTF (1, 0);
 
 extern void demangler_vwarning (const char *file, int line,
-- 
1.7.1

[PATCH 02/15 v2] Remove some GDBSERVER checks from linux-ptrace

[Gary Benson]

This patch removes some GDBSERVER checks from nat/linux-ptrace.c.
Currently the code uses a compile-time check to decide whether some
flags should be used.  This changes the code to instead let users of
the module specify an additional set of flags; and then changes gdb's
linux-nat.c to call this function.  At some later date, when the back
ends are fully merged, we will be able to remove this function again.

gdb/
2014-07-16  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* nat/linux-ptrace.c (additional_flags): New global.
	(linux_test_for_tracesysgood, linux_test_for_tracefork): Use
	additional_flags; don't check GDBSERVER.
	(linux_ptrace_set_additional_flags): New function.
	* nat/linux-ptrace.h (linux_ptrace_set_additional_flags):
	Declare.
	* linux-nat.c (_initialize_linux_nat): Call
	linux_ptrace_set_additional_flags.
---
 gdb/ChangeLog          |   12 +++++++++
 gdb/linux-nat.c        |    8 ++++++
 gdb/nat/linux-ptrace.c |   64 ++++++++++++++++++++++++-----------------------
 gdb/nat/linux-ptrace.h |    1 +
 4 files changed, 54 insertions(+), 31 deletions(-)

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index c738abf..de31229 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -5033,6 +5033,14 @@ Enables printf debugging output."),
   sigdelset (&suspend_mask, SIGCHLD);
 
   sigemptyset (&blocked_mask);
+
+  /* Do not enable PTRACE_O_TRACEEXIT until GDB is more prepared to
+     support read-only process state.  */
+  linux_ptrace_set_additional_flags (PTRACE_O_TRACESYSGOOD
+				     | PTRACE_O_TRACEVFORKDONE
+				     | PTRACE_O_TRACEVFORK
+				     | PTRACE_O_TRACEFORK
+				     | PTRACE_O_TRACEEXEC);
 }
 \f
 
diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c
index 3ad2113..59c9bfa 100644
--- a/gdb/nat/linux-ptrace.c
+++ b/gdb/nat/linux-ptrace.c
@@ -37,6 +37,10 @@
    there are no supported features.  */
 static int current_ptrace_options = -1;
 
+/* Additional flags to test.  */
+
+static int additional_flags;
+
 /* Find all possible reasons we could fail to attach PID and append
    these as strings to the already initialized BUFFER.  '\0'
    termination of BUFFER must be done by the caller.  */
@@ -359,16 +363,15 @@ linux_check_ptrace_features (void)
 static void
 linux_test_for_tracesysgood (int child_pid)
 {
-#ifdef GDBSERVER
-  /* gdbserver does not support PTRACE_O_TRACESYSGOOD.  */
-#else
-  int ret;
+  if ((additional_flags & PTRACE_O_TRACESYSGOOD) != 0)
+    {
+      int ret;
 
-  ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
-		(PTRACE_TYPE_ARG4) PTRACE_O_TRACESYSGOOD);
-  if (ret == 0)
-    current_ptrace_options |= PTRACE_O_TRACESYSGOOD;
-#endif
+      ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
+		    (PTRACE_TYPE_ARG4) PTRACE_O_TRACESYSGOOD);
+      if (ret != 0)
+	additional_flags &= ~PTRACE_O_TRACESYSGOOD;
+    }
 }
 
 /* Determine if PTRACE_O_TRACEFORK can be used to follow fork
@@ -388,16 +391,15 @@ linux_test_for_tracefork (int child_pid)
   if (ret != 0)
     return;
 
-#ifdef GDBSERVER
-  /* gdbserver does not support PTRACE_O_TRACEVFORKDONE yet.  */
-#else
-  /* Check if the target supports PTRACE_O_TRACEVFORKDONE.  */
-  ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
-		(PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK
-				    | PTRACE_O_TRACEVFORKDONE));
-  if (ret == 0)
-    current_ptrace_options |= PTRACE_O_TRACEVFORKDONE;
-#endif
+  if ((additional_flags & PTRACE_O_TRACEVFORKDONE) != 0)
+    {
+      /* Check if the target supports PTRACE_O_TRACEVFORKDONE.  */
+      ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
+		    (PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK
+					| PTRACE_O_TRACEVFORKDONE));
+      if (ret != 0)
+	additional_flags &= ~PTRACE_O_TRACEVFORKDONE;
+    }
 
   /* Setting PTRACE_O_TRACEFORK did not cause an error, however we
      don't know for sure that the feature is available; old
@@ -433,18 +435,7 @@ linux_test_for_tracefork (int child_pid)
 
 	  /* We got the PID from the grandchild, which means fork
 	     tracing is supported.  */
-#ifdef GDBSERVER
-	  /* Do not enable all the options for now since gdbserver does not
-	     properly support them.  This restriction will be lifted when
-	     gdbserver is augmented to support them.  */
-	  current_ptrace_options |= PTRACE_O_TRACECLONE;
-#else
-	  current_ptrace_options |= PTRACE_O_TRACEFORK | PTRACE_O_TRACEVFORK
-	    | PTRACE_O_TRACECLONE | PTRACE_O_TRACEEXEC;
-
-	  /* Do not enable PTRACE_O_TRACEEXIT until GDB is more prepared to
-	     support read-only process state.  */
-#endif
+	  current_ptrace_options |= PTRACE_O_TRACECLONE | additional_flags;
 
 	  /* Do some cleanup and kill the grandchild.  */
 	  my_waitpid (second_pid, &second_status, 0);
@@ -551,3 +542,14 @@ linux_ptrace_init_warnings (void)
 
   linux_ptrace_test_ret_to_nx ();
 }
+
+/* Set additional ptrace flags to use.  Some such flags may be checked
+   by the implementation above.  This function must be called before
+   any other function in this file; otherwise the flags may not take
+   effect appropriately.  */
+
+void
+linux_ptrace_set_additional_flags (int flags)
+{
+  additional_flags = flags;
+}
diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
index cffb5ce..41b3198 100644
--- a/gdb/nat/linux-ptrace.h
+++ b/gdb/nat/linux-ptrace.h
@@ -91,5 +91,6 @@ extern int linux_supports_tracefork (void);
 extern int linux_supports_traceclone (void);
 extern int linux_supports_tracevforkdone (void);
 extern int linux_supports_tracesysgood (void);
+extern void linux_ptrace_set_additional_flags (int);
 
 #endif /* COMMON_LINUX_PTRACE_H */
-- 
1.7.1

[PATCH 05/15 v2] Introduce and use debug_printf and debug_vprintf

[Gary Benson]

This introduces debug_printf and debug_vprintf, a function that
clients of "common" are expected to implement.  gdbserver's
existing debug_printf is repurposed as debug_vprintf, and a new
wrapper is written.

common/agent.c is changed to use debug_vprintf rather than
defining the macro DEBUG_AGENT depending on GDBSERVER.

nat/i386-dregs.c is changed to use the externally-implemented
debug_printf, rather than defining it itself.

gdb/
2014-07-16  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* common/common-debug.h: New file.
	* debug.c: Likewise.
	* utils.h: Include common-debug.h.
	* common/agent.c (debug_agent_print): New function.
	(DEBUG_AGENT): Redefine.
	* nat/i386-dregs.c (debug_printf): Undefine.

gdb/gdbserver/
2014-07-16  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* utils.h: Include common-debug.h.
	* debug.h (debug_printf): Don't declare.
	* debug.c (debug_vprintf): New function.
	(debug_printf): Use the above.
---
 gdb/ChangeLog             |   10 ++++++++++
 gdb/Makefile.in           |    2 +-
 gdb/common/agent.c        |   24 +++++++++++++++---------
 gdb/common/common-debug.h |   35 +++++++++++++++++++++++++++++++++++
 gdb/debug.c               |   41 +++++++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/ChangeLog   |    8 ++++++++
 gdb/gdbserver/debug.c     |   23 ++++++++++++++++-------
 gdb/gdbserver/debug.h     |    1 -
 gdb/gdbserver/utils.h     |    1 +
 gdb/nat/i386-dregs.c      |    4 ----
 gdb/utils.h               |    1 +
 11 files changed, 128 insertions(+), 22 deletions(-)
 create mode 100644 gdb/common/common-debug.h
 create mode 100644 gdb/debug.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index ce15501..a5d2d6b 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1034,7 +1034,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	gdb_vecs.o jit.o progspace.o skip.o probe.o \
 	common-utils.o buffer.o ptid.o gdb-dlfcn.o common-agent.o \
 	format.o registry.o btrace.o record-btrace.o waitstatus.o \
-	print-utils.o rsp-low.o
+	print-utils.o rsp-low.o debug.o
 
 TSOBS = inflow.o
 
diff --git a/gdb/common/agent.c b/gdb/common/agent.c
index 54f861a..549b784 100644
--- a/gdb/common/agent.c
+++ b/gdb/common/agent.c
@@ -33,15 +33,21 @@
 
 int debug_agent = 0;
 
-#ifdef GDBSERVER
-#define DEBUG_AGENT(fmt, args...)	\
-  if (debug_agent)			\
-    fprintf (stderr, fmt, ##args);
-#else
-#define DEBUG_AGENT(fmt, args...)	\
-  if (debug_agent)			\
-    fprintf_unfiltered (gdb_stdlog, fmt, ##args);
-#endif
+/* A stdarg wrapper for debug_vprintf.  */
+
+static void ATTRIBUTE_PRINTF (1, 2)
+debug_agent_print (const char *fmt, ...)
+{
+  va_list ap;
+
+  if (!debug_agent)
+    return;
+  va_start (ap, fmt);
+  debug_vprintf (fmt, ap);
+  va_end (ap);
+}
+
+#define DEBUG_AGENT debug_agent_print
 
 /* Global flag to determine using agent or not.  */
 int use_agent = 0;
diff --git a/gdb/common/common-debug.h b/gdb/common/common-debug.h
new file mode 100644
index 0000000..0467610
--- /dev/null
+++ b/gdb/common/common-debug.h
@@ -0,0 +1,35 @@
+/* Declarations for debug printing functions.
+
+   Copyright (C) 2014 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/>.  */
+
+#ifndef COMMON_DEBUG_H
+#define COMMON_DEBUG_H
+
+/* Print a formatted message to the appropriate channel for debugging
+   output for the client.  */
+
+extern void debug_printf (const char *format, ...)
+     ATTRIBUTE_PRINTF (1, 2);
+
+/* Print a formatted message to the appropriate channel for debugging
+   output for the client.  */
+
+extern void debug_vprintf (const char *format, va_list ap)
+     ATTRIBUTE_PRINTF (1, 0);
+
+#endif /* COMMON_DEBUG_H */
diff --git a/gdb/debug.c b/gdb/debug.c
new file mode 100644
index 0000000..cedf115
--- /dev/null
+++ b/gdb/debug.c
@@ -0,0 +1,41 @@
+/* Debug printing functions.
+
+   Copyright (C) 2014 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 "defs.h"
+#include "utils.h"
+
+/* See common/common-debug.h.  */
+
+void
+debug_vprintf (const char *fmt, va_list ap)
+{
+  vfprintf_unfiltered (gdb_stdlog, fmt, ap);
+}
+
+/* See common/common-debug.h.  */
+
+void
+debug_printf (const char *fmt, ...)
+{
+  va_list ap;
+
+  va_start (ap, fmt);
+  debug_vprintf (fmt, ap);
+  va_end (ap);
+}
diff --git a/gdb/gdbserver/debug.c b/gdb/gdbserver/debug.c
index c50af76..c008b17 100644
--- a/gdb/gdbserver/debug.c
+++ b/gdb/gdbserver/debug.c
@@ -33,9 +33,8 @@ int debug_timestamp;
    previous call ended with "\n".  */
 
 void
-debug_printf (const char *msg, ...)
+debug_vprintf (const char *format, va_list ap)
 {
-  va_list args;
 #if !defined (IN_PROCESS_AGENT)
   /* N.B. Not thread safe, and can't be used, as is, with IPA.  */
   static int new_line = 1;
@@ -53,16 +52,26 @@ debug_printf (const char *msg, ...)
     }
 #endif
 
-  va_start (args, msg);
-  vfprintf (stderr, msg, args);
-  va_end (args);
+  vfprintf (stderr, format, ap);
 
 #if !defined (IN_PROCESS_AGENT)
-  if (*msg)
-    new_line = msg[strlen (msg) - 1] == '\n';
+  if (*format)
+    new_line = format[strlen (format) - 1] == '\n';
 #endif
 }
 
+/* Print a debugging message using debug_vprintf.  */
+
+void
+debug_printf (const char *format, ...)
+{
+  va_list ap;
+
+  va_start (ap, format);
+  debug_vprintf (format, ap);
+  va_end (ap);
+}
+
 /* Flush debugging output.
    This is called, for example, when starting an inferior to ensure all debug
    output thus far appears before any inferior output.  */
diff --git a/gdb/gdbserver/debug.h b/gdb/gdbserver/debug.h
index 0f056ca..42a3f21 100644
--- a/gdb/gdbserver/debug.h
+++ b/gdb/gdbserver/debug.h
@@ -29,7 +29,6 @@
 extern int debug_threads;
 extern int debug_timestamp;
 
-void debug_printf (const char *msg, ...) ATTRIBUTE_PRINTF (1, 2);
 void debug_flush (void);
 void do_debug_enter (const char *function_name);
 void do_debug_exit (const char *function_name);
diff --git a/gdb/gdbserver/utils.h b/gdb/gdbserver/utils.h
index 819fa35..d48179d 100644
--- a/gdb/gdbserver/utils.h
+++ b/gdb/gdbserver/utils.h
@@ -21,6 +21,7 @@
 
 #include "print-utils.h"
 #include "errors.h"
+#include "common-debug.h"
 
 char *paddress (CORE_ADDR addr);
 char *pfildes (gdb_fildes_t fd);
diff --git a/gdb/nat/i386-dregs.c b/gdb/nat/i386-dregs.c
index 1fa5c19..e3272cd 100644
--- a/gdb/nat/i386-dregs.c
+++ b/gdb/nat/i386-dregs.c
@@ -178,10 +178,6 @@ typedef enum { WP_INSERT, WP_REMOVE, WP_COUNT } i386_wp_op_t;
 #ifndef GDBSERVER
 /* Whether or not to print the mirrored debug registers.  */
 extern int debug_hw_points;
-
-/* Print debugging messages.  */
-#define debug_printf(fmt, args...) \
-  fprintf_unfiltered (gdb_stdlog, fmt, ##args);
 #endif
 
 /* Print the values of the mirrored debug registers.  */
diff --git a/gdb/utils.h b/gdb/utils.h
index d9fe7e3..914d0b3 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -25,6 +25,7 @@
 #include "exceptions.h"
 #include "print-utils.h"
 #include "errors.h"
+#include "common-debug.h"
 
 extern void initialize_utils (void);
 
-- 
1.7.1

[PATCH 06/15 v2] Remove simple GDBSERVER uses from common, nat and target

[Gary Benson]

This removes various simple GDBSERVER uses from common, nat and
target.  The simple uses are just cases where the code includes defs.h
or server.h depending on GDBSERVER.  Instead, now the files include
the headers that they require.  Unfortunately we still need to check
GDBSERVER for some files to decide which gnulib config header to
import, but this is a step in the right direction.

gdb/
2014-07-16  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* common/buffer.c: Don't include server.h or defs.h; update
	includes.
	* common/common-utils.c: Don't include server.h or defs.h; update
	includes.
	* common/filestuff.c: Don't include server.h or defs.h; update
	includes.
	* common/filestuff.h: Include stdio.h.
	* common/format.c: Don't include server.h or defs.h; update
	includes.
	* common/gdb_vecs.c: Don't include server.h or defs.h; update
	includes.
	* common/print-utils.c: Don't include server.h or defs.h; update
	includes.
	* common/rsp-low.c: Don't include server.h or defs.h; update
	includes.
	* common/signals.c: Don't include server.h or defs.h; update
	includes.
	* common/vec.c: Don't include server.h or defs.h; update includes.
	* common/xml-utils.c: Don't include server.h or defs.h; update
	includes.
	* nat/linux-osdata.c: Don't include server.h or defs.h; update
	includes.
	* nat/linux-procfs.c: Don't include server.h or defs.h; update
	includes.
	* nat/linux-ptrace.c: Don't include server.h or defs.h; update
	includes.
	* nat/mips-linux-watch.h: Don't include server.h or defs.h; update
	includes.
	* target/waitstatus.c: Don't include server.h or defs.h; update
	includes.
---
 gdb/ChangeLog              |   34 ++++++++++++++++++++++++++++++++++
 gdb/common/buffer.c        |    9 ++++-----
 gdb/common/common-utils.c  |    9 ++++-----
 gdb/common/filestuff.c     |   12 +++++++++---
 gdb/common/filestuff.h     |    2 ++
 gdb/common/format.c        |   10 +++++-----
 gdb/common/gdb_vecs.c      |    9 ++++-----
 gdb/common/print-utils.c   |    8 +++-----
 gdb/common/rsp-low.c       |    8 +++-----
 gdb/common/signals.c       |   14 +++++++++++---
 gdb/common/vec.c           |    9 ++++-----
 gdb/common/xml-utils.c     |    7 ++-----
 gdb/nat/linux-osdata.c     |   11 +++++++++--
 gdb/nat/linux-procfs.c     |   14 +++++++++++---
 gdb/nat/linux-ptrace.c     |   16 +++++++++++++---
 gdb/nat/mips-linux-watch.c |    1 +
 gdb/nat/mips-linux-watch.h |    7 ++-----
 gdb/target/waitstatus.c    |    6 ------
 18 files changed, 121 insertions(+), 65 deletions(-)

diff --git a/gdb/common/buffer.c b/gdb/common/buffer.c
index 4d9edb8..fe7d4ca 100644
--- a/gdb/common/buffer.c
+++ b/gdb/common/buffer.c
@@ -17,11 +17,10 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifdef GDBSERVER
-#include "server.h"
-#else
-#include "defs.h"
-#endif
+#include "config.h"
+
+#include "libiberty.h"
+#include "common-utils.h"
 
 #include "xml-utils.h"
 #include "buffer.h"
diff --git a/gdb/common/common-utils.c b/gdb/common/common-utils.c
index 29fe2c5..c790075 100644
--- a/gdb/common/common-utils.c
+++ b/gdb/common/common-utils.c
@@ -17,12 +17,11 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifdef GDBSERVER
-#include "server.h"
-#else
-#include "defs.h"
-#endif
+#include "config.h"
 
+#include "libiberty.h"
+#include "common-utils.h"
+#include "gdb_locale.h"
 #include "gdb_assert.h"
 #include <string.h>
 
diff --git a/gdb/common/filestuff.c b/gdb/common/filestuff.c
index 4544926..b2adb22 100644
--- a/gdb/common/filestuff.c
+++ b/gdb/common/filestuff.c
@@ -16,15 +16,21 @@
    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 "config.h"
+
 #ifdef GDBSERVER
-#include "server.h"
+#include "build-gnulib-gdbserver/config.h"
 #else
-#include "defs.h"
-#include <string.h>
+#include "build-gnulib/config.h"
 #endif
+
+#include "common-utils.h"
+#include "gdb_locale.h"
 #include "filestuff.h"
 #include "gdb_vecs.h"
 
+#include <errno.h>
+#include <stdlib.h>
 #include <string.h>
 #include <fcntl.h>
 #include <unistd.h>
diff --git a/gdb/common/filestuff.h b/gdb/common/filestuff.h
index 70e09aa..3bc0d40 100644
--- a/gdb/common/filestuff.h
+++ b/gdb/common/filestuff.h
@@ -19,6 +19,8 @@
 #ifndef FILESTUFF_H
 #define FILESTUFF_H
 
+#include <stdio.h>
+
 /* Note all the file descriptors which are open when this is called.
    These file descriptors will not be closed by close_most_fds.  */
 
diff --git a/gdb/common/format.c b/gdb/common/format.c
index bddfbc6..b439720 100644
--- a/gdb/common/format.c
+++ b/gdb/common/format.c
@@ -17,14 +17,14 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifdef GDBSERVER
-#include "server.h"
-#else
-#include "defs.h"
-#endif
+#include "config.h"
 
 #include <string.h>
 
+#include "libiberty.h"
+#include "common-utils.h"
+#include "errors.h"
+#include "gdb_locale.h"
 #include "format.h"
 
 struct format_piece *
diff --git a/gdb/common/gdb_vecs.c b/gdb/common/gdb_vecs.c
index 4a3330f..e28256c 100644
--- a/gdb/common/gdb_vecs.c
+++ b/gdb/common/gdb_vecs.c
@@ -17,12 +17,11 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifdef GDBSERVER
-#include "server.h"
-#else
-#include "defs.h"
-#endif
+#include "config.h"
 
+#include "libiberty.h"
+#include "common-utils.h"
+#include "gdb_locale.h"
 #include "gdb_vecs.h"
 #include "host-defs.h"
 
diff --git a/gdb/common/print-utils.c b/gdb/common/print-utils.c
index 0e612a3..0026b1e 100644
--- a/gdb/common/print-utils.c
+++ b/gdb/common/print-utils.c
@@ -17,11 +17,9 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifdef GDBSERVER
-#include "server.h"
-#else
-#include "defs.h"
-#endif
+#include "common-utils.h"
+#include "common-types.h"
+#include "gdb_locale.h"
 
 #include "print-utils.h"
 
diff --git a/gdb/common/rsp-low.c b/gdb/common/rsp-low.c
index b777716..597113e 100644
--- a/gdb/common/rsp-low.c
+++ b/gdb/common/rsp-low.c
@@ -17,11 +17,9 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifdef GDBSERVER
-#include "server.h"
-#else
-#include "defs.h"
-#endif
+#include "common-utils.h"
+#include "common-types.h"
+#include "gdb_locale.h"
 
 #include <string.h>
 
diff --git a/gdb/common/signals.c b/gdb/common/signals.c
index 3c9cd41..0e2c30b 100644
--- a/gdb/common/signals.c
+++ b/gdb/common/signals.c
@@ -17,17 +17,25 @@
    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 "config.h"
+
 #ifdef GDBSERVER
-#include "server.h"
+#include "build-gnulib-gdbserver/config.h"
 #else
-#include "defs.h"
-#include <string.h>
+#include "build-gnulib/config.h"
 #endif
 
+#include <string.h>
+
 #ifdef HAVE_SIGNAL_H
 #include <signal.h>
 #endif
 
+#include "libiberty.h"
+#include "common-utils.h"
+#include "gdb_assert.h"
+#include "errors.h"
+#include "gdb_locale.h"
 #include "gdb_signals.h"
 #include "gdb_assert.h"
 
diff --git a/gdb/common/vec.c b/gdb/common/vec.c
index 4611e5f..c64e110 100644
--- a/gdb/common/vec.c
+++ b/gdb/common/vec.c
@@ -17,12 +17,11 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifdef GDBSERVER
-#include "server.h"
-#else
-#include "defs.h"
-#endif
+#include "config.h"
 
+#include "libiberty.h"
+#include "common-utils.h"
+#include "gdb_locale.h"
 #include "vec.h"
 
 struct vec_prefix
diff --git a/gdb/common/xml-utils.c b/gdb/common/xml-utils.c
index c6ceb69..ddc5965 100644
--- a/gdb/common/xml-utils.c
+++ b/gdb/common/xml-utils.c
@@ -17,12 +17,9 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifdef GDBSERVER
-#include "server.h"
-#else
-#include "defs.h"
-#endif
+#include "config.h"
 
+#include "libiberty.h"
 #include "xml-utils.h"
 
 #include <string.h>
diff --git a/gdb/nat/linux-osdata.c b/gdb/nat/linux-osdata.c
index dae637b..3635682 100644
--- a/gdb/nat/linux-osdata.c
+++ b/gdb/nat/linux-osdata.c
@@ -17,13 +17,20 @@
    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 "config.h"
+
 #ifdef GDBSERVER
-#include "server.h"
+#include "build-gnulib-gdbserver/config.h"
 #else
-#include "defs.h"
+#include "build-gnulib/config.h"
 #endif
 
+#include "libiberty.h"
+#include "common-types.h"
 #include "linux-osdata.h"
+#include "common-utils.h"
+#include "gdb_assert.h"
+#include "gdb_locale.h"
 
 #include <sys/types.h>
 #include <sys/sysinfo.h>
diff --git a/gdb/nat/linux-procfs.c b/gdb/nat/linux-procfs.c
index 1443a88..2628fad 100644
--- a/gdb/nat/linux-procfs.c
+++ b/gdb/nat/linux-procfs.c
@@ -16,13 +16,21 @@
    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 "config.h"
+
 #ifdef GDBSERVER
-#include "server.h"
+#include "build-gnulib-gdbserver/config.h"
 #else
-#include "defs.h"
-#include <string.h>
+#include "build-gnulib/config.h"
 #endif
 
+#include <string.h>
+#include <stdlib.h>
+
+#include "common-utils.h"
+#include "libiberty.h"
+#include "errors.h"
+#include "gdb_locale.h"
 #include "linux-procfs.h"
 #include "filestuff.h"
 
diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c
index e3462ec..4617211 100644
--- a/gdb/nat/linux-ptrace.c
+++ b/gdb/nat/linux-ptrace.c
@@ -16,13 +16,23 @@
    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 "config.h"
+
 #ifdef GDBSERVER
-#include "server.h"
+#include "build-gnulib-gdbserver/config.h"
 #else
-#include "defs.h"
-#include <string.h>
+#include "build-gnulib/config.h"
 #endif
 
+#include <string.h>
+#include <unistd.h>
+#include <errno.h>
+
+#include "libiberty.h"
+#include "common-utils.h"
+#include "gdb_locale.h"
+#include "errors.h"
+#include "gdb_assert.h"
 #include "linux-ptrace.h"
 #include "linux-procfs.h"
 #include "linux-waitpid.h"
diff --git a/gdb/nat/mips-linux-watch.c b/gdb/nat/mips-linux-watch.c
index acfc7f4..f57e2fd 100644
--- a/gdb/nat/mips-linux-watch.c
+++ b/gdb/nat/mips-linux-watch.c
@@ -18,6 +18,7 @@
 #include <sys/ptrace.h>
 #include "mips-linux-watch.h"
 #include "gdb_assert.h"
+#include "gdb_locale.h"
 
 /* Assuming usable watch registers REGS, return the irw_mask of
    register N.  */
diff --git a/gdb/nat/mips-linux-watch.h b/gdb/nat/mips-linux-watch.h
index c9f6932..b5ddd4b 100644
--- a/gdb/nat/mips-linux-watch.h
+++ b/gdb/nat/mips-linux-watch.h
@@ -18,11 +18,8 @@
 #ifndef MIPS_LINUX_WATCH_H
 #define MIPS_LINUX_WATCH_H 1
 
-#ifdef GDBSERVER
-#include "server.h"
-#else
-#include "defs.h"
-#endif
+#include "config.h"
+#include "common-types.h"
 
 #include <asm/ptrace.h>
 #include <stdint.h>
diff --git a/gdb/target/waitstatus.c b/gdb/target/waitstatus.c
index 4493555..9021477 100644
--- a/gdb/target/waitstatus.c
+++ b/gdb/target/waitstatus.c
@@ -17,12 +17,6 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifdef GDBSERVER
-#include "server.h"
-#else
-#include "defs.h"
-#endif
-
 #include "waitstatus.h"
 
 /* Return a pretty printed form of target_waitstatus.
-- 
1.7.1

[PATCH 03/15 v2] Make gdbserver CORE_ADDR unsigned

[Gary Benson]

gdbserver defines CORE_ADDR to be signed.  This seems erroneous to
me; and furthermore likely to cause problems in common/, as it is
different from gdb's definition.

gdb/gdbserver/
2014-07-16  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* server.h (CORE_ADDR): Now unsigned.
---
 gdb/gdbserver/ChangeLog |    5 +++++
 gdb/gdbserver/server.h  |    2 +-
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
index 6eb1a16..2d55513 100644
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -87,7 +87,7 @@ typedef unsigned char gdb_byte;
 
 /* FIXME: This should probably be autoconf'd for.  It's an integer type at
    least the size of a (void *).  */
-typedef long long CORE_ADDR;
+typedef unsigned long long CORE_ADDR;
 
 typedef long long LONGEST;
 typedef unsigned long long ULONGEST;
-- 
1.7.1

[PATCH 08/15 v2] Make btrace-common.h not use GDBSERVER

[Gary Benson]

This changes btrace-common.h not to include defs.h or server.h.
Instead it moves the defs.h include to btrace.c.

gdb/
2014-07-16  Tom Tromey  <tromey@redhat.com>

	* btrace.c: Include defs.h.
	* common/btrace-common.h: Don't include server.h or defs.h.
---
 gdb/ChangeLog              |    5 +++++
 gdb/btrace.c               |    1 +
 gdb/common/btrace-common.h |    6 ------
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/gdb/btrace.c b/gdb/btrace.c
index 87a171e..b5c3c26 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -19,6 +19,7 @@
    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 "defs.h"
 #include "btrace.h"
 #include "gdbthread.h"
 #include "exceptions.h"
diff --git a/gdb/common/btrace-common.h b/gdb/common/btrace-common.h
index 25617bb..339e684 100644
--- a/gdb/common/btrace-common.h
+++ b/gdb/common/btrace-common.h
@@ -26,12 +26,6 @@
    inferior.  For presentation purposes, the branch trace is represented as a
    list of sequential control-flow blocks, one such list per thread.  */
 
-#ifdef GDBSERVER
-#  include "server.h"
-#else
-#  include "defs.h"
-#endif
-
 #include "vec.h"
 
 /* A branch trace block.
-- 
1.7.1

[PATCH 13/15 v2] Finally remove GDBSERVER (mostly) from agent.c

[Gary Benson]

This reduces the use of the GDBSERVER define in agent.c to deciding
which gnulib header to include.

gdb/
2014-07-16  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* common/agent.c: Don't include defs.h or server.h; update
	includes.
---
 gdb/ChangeLog      |    6 ++++++
 gdb/common/agent.c |   19 ++++++++++++++-----
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/gdb/common/agent.c b/gdb/common/agent.c
index 798dd1d..0df4b19 100644
--- a/gdb/common/agent.c
+++ b/gdb/common/agent.c
@@ -17,15 +17,24 @@
    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 <config.h>
+
 #ifdef GDBSERVER
-#include "server.h"
+#include "build-gnulib-gdbserver/config.h"
 #else
-#include "defs.h"
-#include "target.h"
-#include "infrun.h"
-#include "objfiles.h"
+#include "build-gnulib/config.h"
 #endif
 
+#include <unistd.h>
+
+#include "ptid.h"
+#include "gdb_signals.h"
+#include "target/waitstatus.h"
+#include "common-types.h"
+#include "target/target.h"
+#include "target/symbol.h"
+#include "common-debug.h"
+
 #include <stdarg.h>
 #include <errno.h>
 
-- 
1.7.1

[PATCH 11/15 v2] More target unification

[Gary Benson]

This unifies a few more top-level target functions -- target_resume,
target_wait, and target_stop -- and the declaration of the variable
"non_stop".  This follows the usual pattern, where clients of "common"
are expected to define the objects appropriately, thus simplifying
common/agent.c a bit more.

gdb/
2014-07-16  Tom Tromey  <tromey@redhat.com>

	* target/target.h (target_resume, target_wait, target_stop)
	(non_stop): Moved from target.h.
	* target.h (target_resume, target_wait, target_stop, non_stop):
	Move to target/target.h.
	* common/agent.c (agent_run_command): Always use target_resume,
	target_stop, and target_wait.

gdb/gdbserver/
2014-07-16  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* target.c (target_wait, target_stop, target_resume): New
	functions.
---
 gdb/ChangeLog           |    9 +++++++++
 gdb/common/agent.c      |   27 +--------------------------
 gdb/gdbserver/ChangeLog |    6 ++++++
 gdb/gdbserver/target.c  |   34 ++++++++++++++++++++++++++++++++++
 gdb/target.h            |   31 -------------------------------
 gdb/target/target.h     |   39 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 89 insertions(+), 57 deletions(-)

diff --git a/gdb/common/agent.c b/gdb/common/agent.c
index 52de3d4..0fde2fd 100644
--- a/gdb/common/agent.c
+++ b/gdb/common/agent.c
@@ -230,18 +230,7 @@ agent_run_command (int pid, const char *cmd, int len)
   DEBUG_AGENT ("agent: resumed helper thread\n");
 
   /* Resume helper thread.  */
-#ifdef GDBSERVER
-{
-  struct thread_resume resume_info;
-
-  resume_info.thread = ptid;
-  resume_info.kind = resume_continue;
-  resume_info.sig = GDB_SIGNAL_0;
-  (*the_target->resume) (&resume_info, 1);
-}
-#else
- target_resume (ptid, 0, GDB_SIGNAL_0);
-#endif
+  target_resume (ptid, 0, GDB_SIGNAL_0);
 
   fd = gdb_connect_sync_socket (pid);
   if (fd >= 0)
@@ -277,25 +266,11 @@ agent_run_command (int pid, const char *cmd, int len)
       int was_non_stop = non_stop;
       /* Stop thread PTID.  */
       DEBUG_AGENT ("agent: stop helper thread\n");
-#ifdef GDBSERVER
-      {
-	struct thread_resume resume_info;
-
-	resume_info.thread = ptid;
-	resume_info.kind = resume_stop;
-	resume_info.sig = GDB_SIGNAL_0;
-	(*the_target->resume) (&resume_info, 1);
-      }
-
-      non_stop = 1;
-      mywait (ptid, &status, 0, 0);
-#else
       non_stop = 1;
       target_stop (ptid);
 
       memset (&status, 0, sizeof (status));
       target_wait (ptid, &status, 0);
-#endif
       non_stop = was_non_stop;
     }
 
diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index 44e1e11..6ba375c 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -134,6 +134,40 @@ mywait (ptid_t ptid, struct target_waitstatus *ourstatus, int options,
   return ret;
 }
 
+/* See target/target.h.  */
+
+ptid_t
+target_wait (ptid_t ptid, struct target_waitstatus *status, int options)
+{
+  return mywait (ptid, status, options, 0);
+}
+
+/* See target/target.h.  */
+
+void
+target_stop (ptid_t ptid)
+{
+  struct thread_resume resume_info;
+
+  resume_info.thread = ptid;
+  resume_info.kind = resume_stop;
+  resume_info.sig = GDB_SIGNAL_0;
+  (*the_target->resume) (&resume_info, 1);
+}
+
+/* See target/target.h.  */
+
+void
+target_resume (ptid_t ptid, int step, enum gdb_signal signal)
+{
+  struct thread_resume resume_info;
+
+  resume_info.thread = ptid;
+  resume_info.kind = step ? resume_step : resume_continue;
+  resume_info.sig = signal;
+  (*the_target->resume) (&resume_info, 1);
+}
+
 int
 start_non_stop (int nonstop)
 {
diff --git a/gdb/target.h b/gdb/target.h
index 8a2faca..2a4783c 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1191,31 +1191,6 @@ extern void target_detach (const char *, int);
 
 extern void target_disconnect (const char *, int);
 
-/* Resume execution of the target process PTID (or a group of
-   threads).  STEP says whether to single-step or to run free; SIGGNAL
-   is the signal to be given to the target, or GDB_SIGNAL_0 for no
-   signal.  The caller may not pass GDB_SIGNAL_DEFAULT.  A specific
-   PTID means `step/resume only this process id'.  A wildcard PTID
-   (all threads, or all threads of process) means `step/resume
-   INFERIOR_PTID, and let other threads (for which the wildcard PTID
-   matches) resume with their 'thread->suspend.stop_signal' signal
-   (usually GDB_SIGNAL_0) if it is in "pass" state, or with no signal
-   if in "no pass" state.  */
-
-extern void target_resume (ptid_t ptid, int step, enum gdb_signal signal);
-
-/* Wait for process pid to do something.  PTID = -1 to wait for any
-   pid to do something.  Return pid of child, or -1 in case of error;
-   store status through argument pointer STATUS.  Note that it is
-   _NOT_ OK to throw_exception() out of target_wait() without popping
-   the debugging target from the stack; GDB isn't prepared to get back
-   to the prompt with a debugging target but without the frame cache,
-   stop_pc, etc., set up.  OPTIONS is a bitwise OR of TARGET_W*
-   options.  */
-
-extern ptid_t target_wait (ptid_t ptid, struct target_waitstatus *status,
-			   int options);
-
 /* Fetch at least register REGNO, or all regs if regno == -1.  No result.  */
 
 extern void target_fetch_registers (struct regcache *regcache, int regno);
@@ -1557,12 +1532,6 @@ extern int target_thread_alive (ptid_t ptid);
 
 extern void target_find_new_threads (void);
 
-/* Make target stop in a continuable fashion.  (For instance, under
-   Unix, this should act like SIGSTOP).  This function is normally
-   used by GUIs to implement a stop button.  */
-
-extern void target_stop (ptid_t ptid);
-
 /* Send the specified COMMAND to the target's monitor
    (shell,interpreter) for execution.  The result of the query is
    placed in OUTBUF.  */
diff --git a/gdb/target/target.h b/gdb/target/target.h
index 4ac3440..1b34ce2 100644
--- a/gdb/target/target.h
+++ b/gdb/target/target.h
@@ -22,6 +22,37 @@
 
 /* This header is a stopgap until more code is shared.  */
 
+/* Resume execution of the target process PTID (or a group of
+   threads).  STEP says whether to single-step or to run free; SIGGNAL
+   is the signal to be given to the target, or GDB_SIGNAL_0 for no
+   signal.  The caller may not pass GDB_SIGNAL_DEFAULT.  A specific
+   PTID means `step/resume only this process id'.  A wildcard PTID
+   (all threads, or all threads of process) means `step/resume
+   INFERIOR_PTID, and let other threads (for which the wildcard PTID
+   matches) resume with their 'thread->suspend.stop_signal' signal
+   (usually GDB_SIGNAL_0) if it is in "pass" state, or with no signal
+   if in "no pass" state.  */
+
+extern void target_resume (ptid_t ptid, int step, enum gdb_signal signal);
+
+/* Wait for process pid to do something.  PTID = -1 to wait for any
+   pid to do something.  Return pid of child, or -1 in case of error;
+   store status through argument pointer STATUS.  Note that it is
+   _NOT_ OK to throw_exception() out of target_wait() without popping
+   the debugging target from the stack; GDB isn't prepared to get back
+   to the prompt with a debugging target but without the frame cache,
+   stop_pc, etc., set up.  OPTIONS is a bitwise OR of TARGET_W*
+   options.  */
+
+extern ptid_t target_wait (ptid_t ptid, struct target_waitstatus *status,
+			   int options);
+
+/* Make target stop in a continuable fashion.  (For instance, under
+   Unix, this should act like SIGSTOP).  This function is normally
+   used by GUIs to implement a stop button.  */
+
+extern void target_stop (ptid_t ptid);
+
 /* Read LEN bytes of target memory at address MEMADDR, placing the
    results in GDB's memory at MYADDR.  Return zero for success,
    nonzero if any error occurs.  Implementations of this function may
@@ -54,4 +85,12 @@ extern int target_read_uint32 (CORE_ADDR memaddr, unsigned int *result);
 extern int target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr,
 				ssize_t len);
 
+/* If set, the inferior should be controlled in non-stop mode.  In
+   this mode, each thread is controlled independently.  Execution
+   commands apply only to the selected thread by default, and stop
+   events stop only the thread that had the event -- the other threads
+   are kept running freely.  */
+
+extern int non_stop;
+
 #endif /* TARGET_COMMON_H */
-- 
1.7.1

[PATCH 10/15 v2] Add target/target.h

[Gary Benson]

This adds target/target.h.  This file declares some functions that the
"common" code can use and that the clients must implement.  It also
changes code in common to use these functions.

gdb/
2014-07-16  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* target.h: Include target/target.h.
	(target_read_memory, target_write_memory): Don't declare.
	* target.c (target_read_uint32): New function.
	* common/agent.c: Don't include server.h or defs.h; update
	includes.
	(agent_get_helper_thread_id): Use target_read_uint32.
	(agent_run_command): Always use target_write_memory,
	target_read_memory.
	(agent_capability_check): Use target_read_uint32.
	* target/target.h: New file.

gdb/gdbserver/
2014-07-16  Tom Tromey  <tromey@redhat.com>

	* target.c (target_read_memory, target_read_uint32)
	(target_write_memory): New functions.
---
 gdb/ChangeLog           |   14 +++++++++++
 gdb/common/agent.c      |   56 ++++++++++++---------------------------------
 gdb/gdbserver/ChangeLog |    5 ++++
 gdb/gdbserver/target.c  |   24 +++++++++++++++++++
 gdb/target.c            |   16 +++++++++++++
 gdb/target.h            |    7 +-----
 gdb/target/target.h     |   57 +++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 132 insertions(+), 47 deletions(-)
 create mode 100644 gdb/target/target.h

diff --git a/gdb/common/agent.c b/gdb/common/agent.c
index 549b784..52de3d4 100644
--- a/gdb/common/agent.c
+++ b/gdb/common/agent.c
@@ -26,6 +26,16 @@
 #include "objfiles.h"
 #endif
 
+#include <stdarg.h>
+#include <errno.h>
+
+#include "common-types.h"
+#include "target/target.h"
+#include "errors.h"
+#include "ptid.h"
+#include "gdb_locale.h"
+#include "common-utils.h"
+
 #include <string.h>
 #include <unistd.h>
 #include "agent.h"
@@ -128,23 +138,9 @@ agent_get_helper_thread_id (void)
 {
   if  (helper_thread_id == 0)
     {
-#ifdef GDBSERVER
-      if (read_inferior_memory (ipa_sym_addrs.addr_helper_thread_id,
-				(unsigned char *) &helper_thread_id,
-				sizeof helper_thread_id))
-#else
-      enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
-      gdb_byte buf[4];
-
-      if (target_read_memory (ipa_sym_addrs.addr_helper_thread_id,
-			      buf, sizeof buf) == 0)
-	helper_thread_id = extract_unsigned_integer (buf, sizeof buf,
-						     byte_order);
-      else
-#endif
-	{
-	  warning (_("Error reading helper thread's id in lib"));
-	}
+      if (target_read_uint32 (ipa_sym_addrs.addr_helper_thread_id,
+			      &helper_thread_id))
+	warning (_("Error reading helper thread's id in lib"));
     }
 
   return helper_thread_id;
@@ -222,13 +218,8 @@ agent_run_command (int pid, const char *cmd, int len)
   int tid = agent_get_helper_thread_id ();
   ptid_t ptid = ptid_build (pid, tid, 0);
 
-#ifdef GDBSERVER
-  int ret = write_inferior_memory (ipa_sym_addrs.addr_cmd_buf,
-				   (const unsigned char *) cmd, len);
-#else
   int ret = target_write_memory (ipa_sym_addrs.addr_cmd_buf,
 				 (gdb_byte *) cmd, len);
-#endif
 
   if (ret != 0)
     {
@@ -310,13 +301,8 @@ agent_run_command (int pid, const char *cmd, int len)
 
   if (fd >= 0)
     {
-#ifdef GDBSERVER
-      if (read_inferior_memory (ipa_sym_addrs.addr_cmd_buf,
-				(unsigned char *) cmd, IPA_CMD_BUF_SIZE))
-#else
       if (target_read_memory (ipa_sym_addrs.addr_cmd_buf, (gdb_byte *) cmd,
 			      IPA_CMD_BUF_SIZE))
-#endif
 	{
 	  warning (_("Error reading command response"));
 	  return -1;
@@ -336,20 +322,8 @@ agent_capability_check (enum agent_capa agent_capa)
 {
   if (agent_capability == 0)
     {
-#ifdef GDBSERVER
-      if (read_inferior_memory (ipa_sym_addrs.addr_capability,
-				(unsigned char *) &agent_capability,
-				sizeof agent_capability))
-#else
-      enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
-      gdb_byte buf[4];
-
-      if (target_read_memory (ipa_sym_addrs.addr_capability,
-			      buf, sizeof buf) == 0)
-	agent_capability = extract_unsigned_integer (buf, sizeof buf,
-						     byte_order);
-      else
-#endif
+      if (target_read_uint32 (ipa_sym_addrs.addr_capability,
+			      &agent_capability))
 	warning (_("Error reading capability of agent"));
     }
   return agent_capability & agent_capa;
diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index dcad5c9..44e1e11 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -48,6 +48,22 @@ read_inferior_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len)
   return res;
 }
 
+/* See target/target.h.  */
+
+int
+target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, int len)
+{
+  return read_inferior_memory (memaddr, myaddr, len);
+}
+
+/* See target/target.h.  */
+
+int
+target_read_uint32 (CORE_ADDR memaddr, unsigned int *result)
+{
+  return read_inferior_memory (memaddr, (gdb_byte *) result, sizeof (*result));
+}
+
 int
 write_inferior_memory (CORE_ADDR memaddr, const unsigned char *myaddr,
 		       int len)
@@ -71,6 +87,14 @@ write_inferior_memory (CORE_ADDR memaddr, const unsigned char *myaddr,
   return res;
 }
 
+/* See target/target.h.  */
+
+int
+target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr, ssize_t len)
+{
+  return write_inferior_memory (memaddr, myaddr, len);
+}
+
 ptid_t
 mywait (ptid_t ptid, struct target_waitstatus *ourstatus, int options,
 	int connected_wait)
diff --git a/gdb/target.c b/gdb/target.c
index 07d029a..d4907b6 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1306,6 +1306,22 @@ target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
     return TARGET_XFER_E_IO;
 }
 
+/* See target/target.h.  */
+
+int
+target_read_uint32 (CORE_ADDR memaddr, unsigned int *result)
+{
+  gdb_byte buf[4];
+  int r;
+
+  r = target_read_memory (memaddr, buf, sizeof buf);
+  if (r != 0)
+    return r;
+  *result = extract_unsigned_integer (buf, sizeof buf,
+				      gdbarch_byte_order (target_gdbarch ()));
+  return 0;
+}
+
 /* Like target_read_memory, but specify explicitly that this is a read
    from the target's raw memory.  That is, this read bypasses the
    dcache, breakpoint shadowing, etc.  */
diff --git a/gdb/target.h b/gdb/target.h
index 8bf160c..8a2faca 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -58,6 +58,7 @@ struct dcache_struct;
    it goes into the file stratum, which is always below the process
    stratum.  */
 
+#include "target/target.h"
 #include "target/resume.h"
 #include "target/wait.h"
 #include "target/waitstatus.h"
@@ -1278,9 +1279,6 @@ int target_supports_disable_randomization (void);
 
 extern int target_read_string (CORE_ADDR, char **, int, int *);
 
-extern int target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr,
-			       ssize_t len);
-
 extern int target_read_raw_memory (CORE_ADDR memaddr, gdb_byte *myaddr,
 				   ssize_t len);
 
@@ -1288,9 +1286,6 @@ extern int target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len);
 
 extern int target_read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len);
 
-extern int target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr,
-				ssize_t len);
-
 extern int target_write_raw_memory (CORE_ADDR memaddr, const gdb_byte *myaddr,
 				    ssize_t len);
 
diff --git a/gdb/target/target.h b/gdb/target/target.h
new file mode 100644
index 0000000..4ac3440
--- /dev/null
+++ b/gdb/target/target.h
@@ -0,0 +1,57 @@
+/* Declarations for common target functions.
+
+   Copyright (C) 1986-2014 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/>.  */
+
+#ifndef TARGET_COMMON_H
+#define TARGET_COMMON_H
+
+/* This header is a stopgap until more code is shared.  */
+
+/* Read LEN bytes of target memory at address MEMADDR, placing the
+   results in GDB's memory at MYADDR.  Return zero for success,
+   nonzero if any error occurs.  Implementations of this function may
+   define and use their own error codes, but functions in the common,
+   nat and target directories must treat the return code as opaque.
+   No guarantee is made about the contents of the data at MYADDR if
+   any error occurs.  */
+
+extern int target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr,
+			       ssize_t len);
+
+/* Read an unsigned 32-bit integer in the target's format from target
+   memory at address MEMADDR, storing the result in GDB's format in
+   GDB's memory at RESULT.  Return zero for success, nonzero if any
+   error occurs.  Implementations of this function may define and use
+   their own error codes, but functions in the common, nat and target
+   directories must treat the return code as opaque.  No guarantee is
+   made about the contents of the data at RESULT if any error
+   occurs.  */
+
+extern int target_read_uint32 (CORE_ADDR memaddr, unsigned int *result);
+
+/* Write LEN bytes from MYADDR to target memory at address MEMADDR.
+   Return zero for success, nonzero if any error occurs.
+   Implementations of this function may define and use their own error
+   codes, but functions in the common, nat and target directories must
+   treat the return code as opaque.  No guarantee is made about the
+   contents of the data at MEMADDR if any error occurs.  */
+
+extern int target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr,
+				ssize_t len);
+
+#endif /* TARGET_COMMON_H */
-- 
1.7.1

[PATCH 12/15 v2] Add target/symbol.h, update users

[Gary Benson]

This adds a new "target/symbol.h" file, that declares a symbol-lookup
function used by code in common.  It follows the usual pattern, where
clients of "common" have their own definitions of this function.  This
simplifies agent.c a bit.

gdb/
2014-07-16  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* target/symbol.h: New file.
	* target.h: Include target/symbol.h.
	* target.c (target_look_up_symbol): New function.
	* common/agent.c: Include target/symbol.h.
	(agent_look_up_symbols): Use target_look_up_symbol.

gdb/gdbserver/
2014-07-16  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* target.c: Include target/symbol.h.
	(target_look_up_symbol): New function.
---
 gdb/ChangeLog           |    9 +++++++++
 gdb/common/agent.c      |   13 ++-----------
 gdb/gdbserver/ChangeLog |    6 ++++++
 gdb/gdbserver/target.c  |   12 ++++++++++++
 gdb/target.c            |   14 ++++++++++++++
 gdb/target.h            |    1 +
 gdb/target/symbol.h     |   36 ++++++++++++++++++++++++++++++++++++
 7 files changed, 80 insertions(+), 11 deletions(-)
 create mode 100644 gdb/target/symbol.h

diff --git a/gdb/common/agent.c b/gdb/common/agent.c
index 0fde2fd..798dd1d 100644
--- a/gdb/common/agent.c
+++ b/gdb/common/agent.c
@@ -31,6 +31,7 @@
 
 #include "common-types.h"
 #include "target/target.h"
+#include "target/symbol.h"
 #include "errors.h"
 #include "ptid.h"
 #include "gdb_locale.h"
@@ -111,18 +112,8 @@ agent_look_up_symbols (void *arg)
     {
       CORE_ADDR *addrp =
 	(CORE_ADDR *) ((char *) &ipa_sym_addrs + symbol_list[i].offset);
-#ifdef GDBSERVER
-
-      if (look_up_one_symbol (symbol_list[i].name, addrp, 1) == 0)
-#else
-      struct bound_minimal_symbol sym =
-	lookup_minimal_symbol (symbol_list[i].name, NULL,
-			       (struct objfile *) arg);
 
-      if (sym.minsym != NULL)
-	*addrp = BMSYMBOL_VALUE_ADDRESS (sym);
-      else
-#endif
+      if (target_look_up_symbol (symbol_list[i].name, addrp, arg) == 0)
 	{
 	  DEBUG_AGENT ("symbol `%s' not found\n", symbol_list[i].name);
 	  return -1;
diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index 6ba375c..ad249a0 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -20,6 +20,7 @@
 
 #include "server.h"
 #include "tracepoint.h"
+#include "target/symbol.h"
 
 struct target_ops *the_target;
 
@@ -168,6 +169,17 @@ target_resume (ptid_t ptid, int step, enum gdb_signal signal)
   (*the_target->resume) (&resume_info, 1);
 }
 
+/* See target/symbol.h.  */
+
+int
+target_look_up_symbol (const char *name, CORE_ADDR *addr,
+		       struct objfile *objfile)
+{
+  gdb_assert (objfile == NULL);
+
+  return look_up_one_symbol (name, addr, 1);
+}
+
 int
 start_non_stop (int nonstop)
 {
diff --git a/gdb/target.c b/gdb/target.c
index d4907b6..44a324b 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1322,6 +1322,20 @@ target_read_uint32 (CORE_ADDR memaddr, unsigned int *result)
   return 0;
 }
 
+/* See target/symbol.h.  */
+
+int
+target_look_up_symbol (const char *name, CORE_ADDR *addr,
+		       struct objfile *objfile)
+{
+  struct bound_minimal_symbol sym
+    = lookup_minimal_symbol (name, NULL, objfile);
+
+  if (sym.minsym != NULL)
+    *addr = BMSYMBOL_VALUE_ADDRESS (sym);
+  return sym.minsym != NULL;
+}
+
 /* Like target_read_memory, but specify explicitly that this is a read
    from the target's raw memory.  That is, this read bypasses the
    dcache, breakpoint shadowing, etc.  */
diff --git a/gdb/target.h b/gdb/target.h
index 2a4783c..bb776e4 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -62,6 +62,7 @@ struct dcache_struct;
 #include "target/resume.h"
 #include "target/wait.h"
 #include "target/waitstatus.h"
+#include "target/symbol.h"
 #include "bfd.h"
 #include "symtab.h"
 #include "memattr.h"
diff --git a/gdb/target/symbol.h b/gdb/target/symbol.h
new file mode 100644
index 0000000..bb37b72
--- /dev/null
+++ b/gdb/target/symbol.h
@@ -0,0 +1,36 @@
+/* Declarations of target symbol functions.
+
+   Copyright (C) 1986-2014 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/>.  */
+
+#ifndef TARGET_SYMBOL_H
+#define TARGET_SYMBOL_H
+
+struct objfile;
+
+/* Find a symbol that matches NAME.  Limit the search to OBJFILE if
+   OBJFILE is non-NULL and the implementation supports limiting the
+   search to specific object files.  If a match is found, store the
+   matching symbol's address in ADDR and return nonzero.  Return zero
+   if no symbol matching NAME is found.  Raise an exception if OBJFILE
+   is non-NULL and the implementation does not support limiting
+   searches to specific object files.  */
+
+extern int target_look_up_symbol (const char *name, CORE_ADDR *addr,
+				  struct objfile *objfile);
+
+#endif /* TARGET_SYMBOL_H */
-- 
1.7.1

[PATCH 07/15 v2] Remove GDBSERVER use from nat/i386-dregs.c

[Gary Benson]

This removes the use of GDBSERVER from nat/i386-dregs.c.  Neither
defs.h or server.h are included: the specific files required are
listed instead.  Also, a declaration previously made only outside
of gdbserver is made unconditional.

gdb/
2014-07-16  Gary Benson  <gbenson@redhat.com>

	* gdb/nat/i386-dregs.c: Don't include server.h or defs.h; update
	includes.
	(debug_hw_points): Declare regardless of GDBSERVER.
---
 gdb/ChangeLog        |    6 ++++++
 gdb/nat/i386-dregs.c |   17 +++++++++--------
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/gdb/nat/i386-dregs.c b/gdb/nat/i386-dregs.c
index e3272cd..d43fbcf 100644
--- a/gdb/nat/i386-dregs.c
+++ b/gdb/nat/i386-dregs.c
@@ -17,12 +17,15 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifdef GDBSERVER
-#include "server.h"
-#else
-#include "defs.h"
-#include "inferior.h"
-#endif
+#include "config.h"
+#include "common-utils.h"
+#include "common-types.h"
+#include "break-common.h"
+#include "common-debug.h"
+#include "print-utils.h"
+#include "errors.h"
+#include "gdb_locale.h"
+#include "gdb_assert.h"
 #include "i386-dregs.h"
 
 /* Support for hardware watchpoints and breakpoints using the i386
@@ -175,10 +178,8 @@
 /* Types of operations supported by i386_handle_nonaligned_watchpoint.  */
 typedef enum { WP_INSERT, WP_REMOVE, WP_COUNT } i386_wp_op_t;
 
-#ifndef GDBSERVER
 /* Whether or not to print the mirrored debug registers.  */
 extern int debug_hw_points;
-#endif
 
 /* Print the values of the mirrored debug registers.  */
 
-- 
1.7.1

Re: [PATCH 01/15 v2] Introduce common/errors.h

[Doug Evans]

On Wed, Jul 16, 2014 at 7:17 AM, Gary Benson <gbenson@redhat.com> wrote:
> This introduces common/errors.h.  This holds some error- and
> warning-related declarations that can be used by the code in common.
> Clients of the "common" code must provide definitions for these
> functions.
>
> gdb/
> 2014-07-16  Tom Tromey  <tromey@redhat.com>
>             Gary Benson  <gbenson@redhat.com>
>
>         * common/errors.h: New file.
>         * utils.h: Include errors.h.
>         (perror_with_name, error, fatal, warning): Don't declare.
>         * common/common-utils.h: Include errors.h.
>         (malloc_failure, internal_error): Don't declare.
>
> gdb/gdbserver/
> 2014-07-16  Tom Tromey  <tromey@redhat.com>
>
>         * utils.h: Include errors.h.
>         (perror_with_name, error, fatal, warning): Don't declare.

Hi.  Comments inline.

> ---
>  gdb/ChangeLog             |    9 +++++++
>  gdb/common/common-utils.h |    5 +---
>  gdb/common/errors.h       |   55 +++++++++++++++++++++++++++++++++++++++++++++
>  gdb/gdbserver/ChangeLog   |    5 ++++
>  gdb/gdbserver/utils.h     |    5 +---
>  gdb/utils.h               |   10 +-------
>  6 files changed, 72 insertions(+), 17 deletions(-)
>  create mode 100644 gdb/common/errors.h
>
> diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h
> index 063698d..53be2f8 100644
> --- a/gdb/common/common-utils.h
> +++ b/gdb/common/common-utils.h
> @@ -24,6 +24,7 @@
>  #include "ansidecl.h"
>  #include <stddef.h>
>  #include <stdarg.h>
> +#include "errors.h"
>
>  /* If possible, define FUNCTION_NAME, a macro containing the name of
>     the function being defined.  Since this macro may not always be
> @@ -43,10 +44,6 @@
>  #endif
>  #endif
>
> -extern void malloc_failure (long size) ATTRIBUTE_NORETURN;
> -extern void internal_error (const char *file, int line, const char *, ...)
> -     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 4);
> -
>  /* xmalloc(), xrealloc() and xcalloc() have already been declared in
>     "libiberty.h". */
>
> diff --git a/gdb/common/errors.h b/gdb/common/errors.h
> new file mode 100644
> index 0000000..5f998e3
> --- /dev/null
> +++ b/gdb/common/errors.h
> @@ -0,0 +1,55 @@
> +/* Declarations for error-reporting facilities.
> +
> +   Copyright (C) 1986-2014 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/>.  */
> +
> +#ifndef COMMON_ERRORS_H
> +#define COMMON_ERRORS_H
> +
> +/* The declarations in this file are, for the time being, separately
> +   implemented by gdb and gdbserver.  However they share a common
> +   definition so that they can be used by code in common/.  */
> +
> +/* Like "perror" but throws an exception with the appropriate
> +   text.  */
> +
> +extern void perror_with_name (const char *) ATTRIBUTE_NORETURN;
> +
> +/* Throw an exception.  */
> +
> +extern void error (const char *fmt, ...)
> +     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
> +
> +/* Cause a fatal error.  */
> +
> +extern void fatal (const char *fmt, ...)
> +     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);

fatal is a confusing name IMO.  In gdb it just throws RETURN_QUIT.
That's not your fault of course, but I wonder if I can trouble you to
expand on the function comment a bit.  E.g., something like the
following?

/* Cause a fatal error.
   What this does in gdb is throw a RETURN_QUIT exception.  */

One could expand on the comment, but I didn't want to turn it into a FIXME.
[which is really what it should be, but that's a topic for another patch :-)]

> +
> +/* Issue a warning.  */
> +
> +extern void warning (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
> +
> +/* Internal error.  */
> +
> +extern void internal_error (const char *file, int line, const char *, ...)
> +     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 4);
> +
> +/* Memory allocation failure.  */
> +
> +extern void malloc_failure (long size) ATTRIBUTE_NORETURN;
> +
> +#endif /* COMMON_ERRORS_H */
> diff --git a/gdb/gdbserver/utils.h b/gdb/gdbserver/utils.h
> index 906924b..819fa35 100644
> --- a/gdb/gdbserver/utils.h
> +++ b/gdb/gdbserver/utils.h
> @@ -20,11 +20,8 @@
>  #define UTILS_H
>
>  #include "print-utils.h"
> +#include "errors.h"
>
> -void perror_with_name (const char *string) ATTRIBUTE_NORETURN;
> -void error (const char *string,...) ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
> -void fatal (const char *string,...) ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
> -void warning (const char *string,...) ATTRIBUTE_PRINTF (1, 2);
>  char *paddress (CORE_ADDR addr);
>  char *pfildes (gdb_fildes_t fd);
>
> diff --git a/gdb/utils.h b/gdb/utils.h
> index a91f551..d9fe7e3 100644
> --- a/gdb/utils.h
> +++ b/gdb/utils.h
> @@ -24,6 +24,7 @@
>  #include "cleanups.h"
>  #include "exceptions.h"
>  #include "print-utils.h"
> +#include "errors.h"
>
>  extern void initialize_utils (void);
>
> @@ -269,7 +270,6 @@ extern void fprintf_symbol_filtered (struct ui_file *, const char *,
>
>  extern void throw_perror_with_name (enum errors errcode, const char *string)
>    ATTRIBUTE_NORETURN;
> -extern void perror_with_name (const char *) ATTRIBUTE_NORETURN;
>
>  extern void perror_warning_with_name (const char *string);
>
> @@ -286,17 +286,11 @@ extern char *warning_pre_print;
>  extern void verror (const char *fmt, va_list ap)
>       ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
>
> -extern void error (const char *fmt, ...)
> -     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
> -
>  extern void error_stream (struct ui_file *) ATTRIBUTE_NORETURN;
>
>  extern void vfatal (const char *fmt, va_list ap)
>       ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
>
> -extern void fatal (const char *fmt, ...)
> -     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
> -

It's odd to separate out fatal and not vfatal.
[ditto for the others]
Can you add vfatal, et.al. to this patch?

>  extern void internal_verror (const char *file, int line, const char *,
>                              va_list ap)
>       ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 0);
> @@ -308,8 +302,6 @@ extern void internal_vwarning (const char *file, int line,
>  extern void internal_warning (const char *file, int line,
>                               const char *, ...) ATTRIBUTE_PRINTF (3, 4);
>
> -extern void warning (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
> -
>  extern void vwarning (const char *, va_list args) ATTRIBUTE_PRINTF (1, 0);
>
>  extern void demangler_vwarning (const char *file, int line,
> --
> 1.7.1
>

Re: [PATCH 02/15 v2] Remove some GDBSERVER checks from linux-ptrace

[Doug Evans]

On Wed, Jul 16, 2014 at 7:17 AM, Gary Benson <gbenson@redhat.com> wrote:
> This patch removes some GDBSERVER checks from nat/linux-ptrace.c.
> Currently the code uses a compile-time check to decide whether some
> flags should be used.  This changes the code to instead let users of
> the module specify an additional set of flags; and then changes gdb's
> linux-nat.c to call this function.  At some later date, when the back
> ends are fully merged, we will be able to remove this function again.
>
> gdb/
> 2014-07-16  Tom Tromey  <tromey@redhat.com>
>             Gary Benson  <gbenson@redhat.com>
>
>         * nat/linux-ptrace.c (additional_flags): New global.
>         (linux_test_for_tracesysgood, linux_test_for_tracefork): Use
>         additional_flags; don't check GDBSERVER.
>         (linux_ptrace_set_additional_flags): New function.
>         * nat/linux-ptrace.h (linux_ptrace_set_additional_flags):
>         Declare.
>         * linux-nat.c (_initialize_linux_nat): Call
>         linux_ptrace_set_additional_flags.

LGTM.

I had a thought, and please don't feel compelled to have to submit yet
another revision.
I'm just writing this down for completeness' sake.
Since linux_ptrace_set_additional_flags is supposed to be called
before any other related function, one could add an assert to the
other functions to verify this.

> [...]
> +/* Set additional ptrace flags to use.  Some such flags may be checked
> +   by the implementation above.  This function must be called before
> +   any other function in this file; otherwise the flags may not take
> +   effect appropriately.  */
> +
> +void
> +linux_ptrace_set_additional_flags (int flags)
> +{
> +  additional_flags = flags;
> +}

Re: [PATCH 03/15 v2] Make gdbserver CORE_ADDR unsigned

[Doug Evans]

On Wed, Jul 16, 2014 at 7:17 AM, Gary Benson <gbenson@redhat.com> wrote:
> gdbserver defines CORE_ADDR to be signed.  This seems erroneous to
> me; and furthermore likely to cause problems in common/, as it is
> different from gdb's definition.
>
> gdb/gdbserver/
> 2014-07-16  Tom Tromey  <tromey@redhat.com>
>             Gary Benson  <gbenson@redhat.com>
>
>         * server.h (CORE_ADDR): Now unsigned.

LGTM

Re: [PATCH 04/15 v2] Introduce common-types.h

[Doug Evans]

On Wed, Jul 16, 2014 at 7:17 AM, Gary Benson <gbenson@redhat.com> wrote:
> This introduces common-types.h.  This file defines various standard
> types used by gdb and gdbserver.
>
> Currently these types are conditionally defined based on GDBSERVER.
> The long term goal is to remove all such tests; however, this is
> difficult as currently gdb uses definitions from BFD.  In the meantime
> this is still a step in the right direction.
>
> gdb/
> 2014-07-16  Tom Tromey  <tromey@redhat.com>
>             Gary Benson  <gbenson@redhat.com>
>
>         * common/common-types.h: New file.
>         * nat/linux-ptrace.c: Include common-types.h.
>         * defs.h: Include common-types.h.
>         (gdb_byte, CORE_ADDR, CORE_ADDR_MAX, LONGEST, ULONGEST): Remove.
>
> gdb/gdbserver/
> 2014-07-16  Tom Tromey  <tromey@redhat.com>
>
>         * server.h: Include common-types.h.  Move gdb_assert.h include
>         earlier.  Add static assertion.
>         (gdb_byte, CORE_ADDR, LONGEST, ULONGEST): Remove.

LGTM

One nit for discussion's sake.
CORE_ADDR for gdbserver is a native value, whereas in gdb it's a
target value.  That's why the static assert (sizeof (CORE_ADDR) >=
sizeof (void *)) isn't in common code (IIUC - e.g., consider a
64-cross-32 gdb).  I don't think that'll ever be a problem, and if it
does become one we can address it then.  Just thinking out loud ...

[PATCH] Introduce common/errors.h

[Gary Benson]

From: Tom Tromey <tromey@redhat.com>

This introduces common/errors.h.  This holds some error- and
warning-related declarations that can be used by the code in common.
Clients of the "common" code must provide definitions for these
functions.

gdb/
2014-07-16  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* common/errors.h: New file.
	* common/errors.c: Likewise.
	* Makefile.in (HFILES_NO_SRCDIR) [common/errors.h]: New header.
	(COMMON_OBS) [errors.o]: New object file.
	(errors.o): New rule.
	* utils.h: Include errors.h.
	(perror_with_name, error, verror, fatal, vfatal, warning
	vwarning): Don't declare.
	* common/common-utils.h: Include errors.h.
	(malloc_failure, internal_error): Don't declare.

gdb/gdbserver/
2014-07-16  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* Makefile.in (SFILES) [common/errors.c]: New sourcefile.
	(OBS) [errors.o]: New object file.
	(IPA_OBS) [errors-ipa.o]: Likewise.
	(errors.o): New rule.
	(errors-ipa.o): Likewise.
	* utils.h: Include errors.h.
	(perror_with_name, error, fatal, warning): Don't declare.
	* utils.c (warning): Renamed and rewritten as...
	(vwarning): New function.
	(error): Renamed and rewritten as...
	(verror): New function.
	(fatal): Renamed and rewritten as...
	(vfatal): New function.
	(internal_error): Renamed and rewritten as...
	(internal_verror): New function.
---
 gdb/ChangeLog             |   14 ++++++++
 gdb/Makefile.in           |    8 +++-
 gdb/common/common-utils.h |    5 +--
 gdb/common/errors.c       |   71 +++++++++++++++++++++++++++++++++++++++
 gdb/common/errors.h       |   81 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/ChangeLog   |   19 ++++++++++
 gdb/gdbserver/Makefile.in |   16 +++++++--
 gdb/gdbserver/utils.c     |   35 +++++--------------
 gdb/gdbserver/utils.h     |    5 +--
 gdb/utils.c               |   46 -------------------------
 gdb/utils.h               |   22 +------------
 11 files changed, 217 insertions(+), 105 deletions(-)
 create mode 100644 gdb/common/errors.c
 create mode 100644 gdb/common/errors.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index ce15501..074ea78 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -935,7 +935,7 @@ gdb_bfd.h sparc-ravenscar-thread.h ppc-ravenscar-thread.h nat/linux-btrace.h \
 ctf.h nat/i386-cpuid.h nat/i386-gcc-cpuid.h target/resume.h \
 target/wait.h target/waitstatus.h nat/linux-nat.h nat/linux-waitpid.h \
 common/print-utils.h common/rsp-low.h nat/i386-dregs.h x86-linux-nat.h \
-i386-linux-nat.h
+i386-linux-nat.h common/errors.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
@@ -1034,7 +1034,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	gdb_vecs.o jit.o progspace.o skip.o probe.o \
 	common-utils.o buffer.o ptid.o gdb-dlfcn.o common-agent.o \
 	format.o registry.o btrace.o record-btrace.o waitstatus.o \
-	print-utils.o rsp-low.o
+	print-utils.o rsp-low.o errors.o
 
 TSOBS = inflow.o
 
@@ -2144,6 +2144,10 @@ rsp-low.o: ${srcdir}/common/rsp-low.c
 	$(COMPILE) $(srcdir)/common/rsp-low.c
 	$(POSTCOMPILE)
 
+errors.o: ${srcdir}/common/errors.c
+	$(COMPILE) $(srcdir)/common/errors.c
+	$(POSTCOMPILE)
+
 #
 # gdb/target/ dependencies
 #
diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h
index 063698d..53be2f8 100644
--- a/gdb/common/common-utils.h
+++ b/gdb/common/common-utils.h
@@ -24,6 +24,7 @@
 #include "ansidecl.h"
 #include <stddef.h>
 #include <stdarg.h>
+#include "errors.h"
 
 /* If possible, define FUNCTION_NAME, a macro containing the name of
    the function being defined.  Since this macro may not always be
@@ -43,10 +44,6 @@
 #endif
 #endif
 
-extern void malloc_failure (long size) ATTRIBUTE_NORETURN;
-extern void internal_error (const char *file, int line, const char *, ...)
-     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 4);
-
 /* xmalloc(), xrealloc() and xcalloc() have already been declared in
    "libiberty.h". */
 
diff --git a/gdb/common/errors.c b/gdb/common/errors.c
new file mode 100644
index 0000000..78a6d3e
--- /dev/null
+++ b/gdb/common/errors.c
@@ -0,0 +1,71 @@
+/* Error reporting facilities.
+
+   Copyright (C) 1986-2014 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 "config.h"
+#include "ansidecl.h"
+#include <stdarg.h>
+#include "errors.h"
+
+/* See common/errors.h.  */
+
+void
+warning (const char *fmt, ...)
+{
+  va_list ap;
+
+  va_start (ap, fmt);
+  vwarning (fmt, ap);
+  va_end (ap);
+}
+
+/* See common/errors.h.  */
+
+void
+error (const char *fmt, ...)
+{
+  va_list ap;
+
+  va_start (ap, fmt);
+  verror (fmt, ap);
+  va_end (ap);
+}
+
+/* See common/errors.h.  */
+
+void
+fatal (const char *fmt, ...)
+{
+  va_list ap;
+
+  va_start (ap, fmt);
+  vfatal (fmt, ap);
+  va_end (ap);
+}
+
+/* See common/errors.h.  */
+
+void
+internal_error (const char *file, int line, const char *fmt, ...)
+{
+  va_list ap;
+
+  va_start (ap, fmt);
+  internal_verror (file, line, fmt, ap);
+  va_end (ap);
+}
diff --git a/gdb/common/errors.h b/gdb/common/errors.h
new file mode 100644
index 0000000..4e8f4a7
--- /dev/null
+++ b/gdb/common/errors.h
@@ -0,0 +1,81 @@
+/* Declarations for error-reporting facilities.
+
+   Copyright (C) 1986-2014 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/>.  */
+
+#ifndef COMMON_ERRORS_H
+#define COMMON_ERRORS_H
+
+/* The declarations in this file are, for the time being, separately
+   implemented by gdb and gdbserver.  However they share a common
+   definition so that they can be used by code in common/.  */
+
+/* Issue a warning to the user.  */
+
+extern void warning (const char *fmt, ...) ATTRIBUTE_PRINTF (1, 2);
+
+extern void vwarning (const char *fmt, va_list args)
+     ATTRIBUTE_PRINTF (1, 0);
+
+/* Throw an error.  The current operation will be aborted.  The
+   message will be issued to the user.  The application will
+   return to a state where it is accepting commands from the user.  */
+
+extern void error (const char *fmt, ...)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
+
+extern void verror (const char *fmt, va_list args)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
+
+/* Throw an error, creating the message by combining STRING with the
+   system error message for errno.  The current operation will be
+   aborted.  The message will be issued to the user.  The application
+   will return to a state where it is accepting commands from the
+   user.  */
+
+extern void perror_with_name (const char *string) ATTRIBUTE_NORETURN;
+
+/* Throw a fatal error.  The current operation will be aborted.  The
+   message will be issued to the user.  The application will exit.  */
+
+extern void fatal (const char *fmt, ...)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
+
+extern void vfatal (const char *fmt, va_list args)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
+
+/* Throw an internal error.  This should be used to indicate
+   programming errors such as assertion failures, as opposed
+   to more general errors the user may encounter.  The current
+   operation will be aborted.  The message will be issued to
+   the user.  The application may offer the user the choice to
+   return to the command level, or the application may exit.  */
+
+extern void internal_error (const char *file, int line,
+			    const char *fmt, ...)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 4);
+
+extern void internal_verror (const char *file, int line,
+			     const char *fmt, va_list args)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 0);
+
+/* Special case internal error used to handle memory allocation
+   failures.  */
+
+extern void malloc_failure (long size) ATTRIBUTE_NORETURN;
+
+#endif /* COMMON_ERRORS_H */
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index f9a2f17..1faa00c 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -169,7 +169,7 @@ SFILES=	$(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
 	$(srcdir)/common/buffer.c $(srcdir)/nat/linux-btrace.c \
 	$(srcdir)/common/filestuff.c $(srcdir)/target/waitstatus.c \
 	$(srcdir)/nat/mips-linux-watch.c $(srcdir)/common/print-utils.c \
-	$(srcdir)/common/rsp-low.c
+	$(srcdir)/common/rsp-low.c $(srcdir)/common/errors.c
 
 DEPFILES = @GDBSERVER_DEPFILES@
 
@@ -182,7 +182,8 @@ OBS = agent.o ax.o inferiors.o regcache.o remote-utils.o server.o signals.o \
       target.o waitstatus.o utils.o debug.o version.o vec.o gdb_vecs.o \
       mem-break.o hostio.o event-loop.o tracepoint.o xml-utils.o \
       common-utils.o ptid.o buffer.o format.o filestuff.o dll.o notif.o \
-      tdesc.o print-utils.o rsp-low.o $(XML_BUILTIN) $(DEPFILES) $(LIBOBJS)
+      tdesc.o print-utils.o rsp-low.o errors.o $(XML_BUILTIN) $(DEPFILES) \
+      $(LIBOBJS)
 GDBREPLAY_OBS = gdbreplay.o version.o
 GDBSERVER_LIBS = @GDBSERVER_LIBS@
 XM_CLIBS = @LIBS@
@@ -300,7 +301,10 @@ gdbreplay$(EXEEXT): $(GDBREPLAY_OBS) $(LIBGNU) $(LIBIBERTY)
 	${CC-LD} $(INTERNAL_CFLAGS) $(INTERNAL_LDFLAGS) -o gdbreplay$(EXEEXT) $(GDBREPLAY_OBS) \
 	  $(XM_CLIBS) $(LIBGNU) $(LIBIBERTY)
 
-IPA_OBJS=ax-ipa.o tracepoint-ipa.o format-ipa.o utils-ipa.o regcache-ipa.o remote-utils-ipa.o common-utils-ipa.o tdesc-ipa.o print-utils-ipa.o rsp-low-ipa.o ${IPA_DEPFILES}
+IPA_OBJS=ax-ipa.o tracepoint-ipa.o format-ipa.o utils-ipa.o \
+	regcache-ipa.o remote-utils-ipa.o common-utils-ipa.o \
+	tdesc-ipa.o print-utils-ipa.o rsp-low-ipa.o errors-ipa.o \
+	${IPA_DEPFILES}
 
 IPA_LIB=libinproctrace.so
 
@@ -489,6 +493,9 @@ print-utils-ipa.o: ../common/print-utils.c
 rsp-low-ipa.o: ../common/rsp-low.c
 	$(IPAGENT_COMPILE) $<
 	$(POSTCOMPILE)
+errors-ipa.o: ../common/errors.c
+	$(IPAGENT_COMPILE) $<
+	$(POSTCOMPILE)
 
 ax.o: ax.c
 	$(COMPILE) $(WARN_CFLAGS_NO_FORMAT) $<
@@ -530,6 +537,9 @@ filestuff.o: ../common/filestuff.c
 agent.o: ../common/agent.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
+errors.o: ../common/errors.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
 waitstatus.o: ../target/waitstatus.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
diff --git a/gdb/gdbserver/utils.c b/gdb/gdbserver/utils.c
index 2d0b331..6c666e1 100644
--- a/gdb/gdbserver/utils.c
+++ b/gdb/gdbserver/utils.c
@@ -77,20 +77,17 @@ perror_with_name (const char *string)
   error ("%s.", combined);
 }
 
-/* Print an error message and return to command level.
-   STRING is the error message, used as a fprintf string,
-   and ARG is passed as an argument to it.  */
+/* Print an error message and return to command level.  */
 
 void
-error (const char *string,...)
+verror (const char *fmt, va_list args)
 {
 #ifndef IN_PROCESS_AGENT
   extern jmp_buf toplevel;
 #endif
-  va_list args;
-  va_start (args, string);
+
   fflush (stdout);
-  vfprintf (stderr, string, args);
+  vfprintf (stderr, fmt, args);
   fprintf (stderr, "\n");
 #ifndef IN_PROCESS_AGENT
   longjmp (toplevel, 1);
@@ -99,48 +96,36 @@ error (const char *string,...)
 #endif
 }
 
-/* Print an error message and exit reporting failure.
-   This is for a error that we cannot continue from.
-   STRING and ARG are passed to fprintf.  */
+/* Print an error message and exit reporting failure.  */
 
 /* VARARGS */
 void
-fatal (const char *string,...)
+vfatal (const char *fmt, va_list args)
 {
-  va_list args;
-  va_start (args, string);
   fprintf (stderr, PREFIX);
-  vfprintf (stderr, string, args);
+  vfprintf (stderr, fmt, args);
   fprintf (stderr, "\n");
-  va_end (args);
   exit (1);
 }
 
 /* VARARGS */
 void
-warning (const char *string,...)
+vwarning (const char *fmt, va_list args)
 {
-  va_list args;
-  va_start (args, string);
   fprintf (stderr, PREFIX);
-  vfprintf (stderr, string, args);
+  vfprintf (stderr, fmt, args);
   fprintf (stderr, "\n");
-  va_end (args);
 }
 
 /* Report a problem internal to GDBserver, and exit.  */
 
 void
-internal_error (const char *file, int line, const char *fmt, ...)
+internal_verror (const char *file, int line, const char *fmt, va_list args)
 {
-  va_list args;
-  va_start (args, fmt);
-
   fprintf (stderr,  "\
 %s:%d: A problem internal to " TOOLNAME " has been detected.\n", file, line);
   vfprintf (stderr, fmt, args);
   fprintf (stderr, "\n");
-  va_end (args);
   exit (1);
 }
 
diff --git a/gdb/gdbserver/utils.h b/gdb/gdbserver/utils.h
index 906924b..819fa35 100644
--- a/gdb/gdbserver/utils.h
+++ b/gdb/gdbserver/utils.h
@@ -20,11 +20,8 @@
 #define UTILS_H
 
 #include "print-utils.h"
+#include "errors.h"
 
-void perror_with_name (const char *string) ATTRIBUTE_NORETURN;
-void error (const char *string,...) ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
-void fatal (const char *string,...) ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
-void warning (const char *string,...) ATTRIBUTE_PRINTF (1, 2);
 char *paddress (CORE_ADDR addr);
 char *pfildes (gdb_fildes_t fd);
 
diff --git a/gdb/utils.c b/gdb/utils.c
index d324227..b2f9f0f 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -533,22 +533,6 @@ vwarning (const char *string, va_list args)
     }
 }
 
-/* Print a warning message.
-   The first argument STRING is the warning message, used as a fprintf string,
-   and the remaining args are passed as arguments to it.
-   The primary difference between warnings and errors is that a warning
-   does not force the return to command level.  */
-
-void
-warning (const char *string, ...)
-{
-  va_list args;
-
-  va_start (args, string);
-  vwarning (string, args);
-  va_end (args);
-}
-
 /* Print an error message and return to command level.
    The first argument STRING is the error message, used as a fprintf string,
    and the remaining args are passed as arguments to it.  */
@@ -559,16 +543,6 @@ verror (const char *string, va_list args)
   throw_verror (GENERIC_ERROR, string, args);
 }
 
-void
-error (const char *string, ...)
-{
-  va_list args;
-
-  va_start (args, string);
-  throw_verror (GENERIC_ERROR, string, args);
-  va_end (args);
-}
-
 /* Print an error message and quit.
    The first argument STRING is the error message, used as a fprintf string,
    and the remaining args are passed as arguments to it.  */
@@ -580,16 +554,6 @@ vfatal (const char *string, va_list args)
 }
 
 void
-fatal (const char *string, ...)
-{
-  va_list args;
-
-  va_start (args, string);
-  throw_vfatal (string, args);
-  va_end (args);
-}
-
-void
 error_stream (struct ui_file *stream)
 {
   char *message = ui_file_xstrdup (stream, NULL);
@@ -836,16 +800,6 @@ internal_verror (const char *file, int line, const char *fmt, va_list ap)
   fatal (_("Command aborted."));
 }
 
-void
-internal_error (const char *file, int line, const char *string, ...)
-{
-  va_list ap;
-
-  va_start (ap, string);
-  internal_verror (file, line, string, ap);
-  va_end (ap);
-}
-
 static struct internal_problem internal_warning_problem = {
   "internal-warning", 1, internal_problem_ask, 1, internal_problem_ask
 };
diff --git a/gdb/utils.h b/gdb/utils.h
index a91f551..e48fc43 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -24,6 +24,7 @@
 #include "cleanups.h"
 #include "exceptions.h"
 #include "print-utils.h"
+#include "errors.h"
 
 extern void initialize_utils (void);
 
@@ -269,7 +270,6 @@ extern void fprintf_symbol_filtered (struct ui_file *, const char *,
 
 extern void throw_perror_with_name (enum errors errcode, const char *string)
   ATTRIBUTE_NORETURN;
-extern void perror_with_name (const char *) ATTRIBUTE_NORETURN;
 
 extern void perror_warning_with_name (const char *string);
 
@@ -283,24 +283,8 @@ extern void (*deprecated_error_begin_hook) (void);
 
 extern char *warning_pre_print;
 
-extern void verror (const char *fmt, va_list ap)
-     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
-
-extern void error (const char *fmt, ...)
-     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
-
 extern void error_stream (struct ui_file *) ATTRIBUTE_NORETURN;
 
-extern void vfatal (const char *fmt, va_list ap)
-     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
-
-extern void fatal (const char *fmt, ...)
-     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
-
-extern void internal_verror (const char *file, int line, const char *,
-			     va_list ap)
-     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 0);
-
 extern void internal_vwarning (const char *file, int line,
 			       const char *, va_list ap)
      ATTRIBUTE_PRINTF (3, 0);
@@ -308,10 +292,6 @@ extern void internal_vwarning (const char *file, int line,
 extern void internal_warning (const char *file, int line,
 			      const char *, ...) ATTRIBUTE_PRINTF (3, 4);
 
-extern void warning (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
-
-extern void vwarning (const char *, va_list args) ATTRIBUTE_PRINTF (1, 0);
-
 extern void demangler_vwarning (const char *file, int line,
 			       const char *, va_list ap)
      ATTRIBUTE_PRINTF (3, 0);
-- 
1.7.1

Re: [PATCH] Introduce common/errors.h

[Gary Benson]

Please ignore this email... apparently git-send-email doesn't handle
being interrupted like you might expect!

Gary Benson wrote:
> From: Tom Tromey <tromey@redhat.com>
> 
> This introduces common/errors.h.  This holds some error- and
> warning-related declarations that can be used by the code in common.
> Clients of the "common" code must provide definitions for these
> functions.
> 
> gdb/
> 2014-07-16  Tom Tromey  <tromey@redhat.com>
> 	    Gary Benson  <gbenson@redhat.com>
> 
> 	* common/errors.h: New file.
> 	* common/errors.c: Likewise.
> 	* Makefile.in (HFILES_NO_SRCDIR) [common/errors.h]: New header.
> 	(COMMON_OBS) [errors.o]: New object file.
> 	(errors.o): New rule.
> 	* utils.h: Include errors.h.
> 	(perror_with_name, error, verror, fatal, vfatal, warning
> 	vwarning): Don't declare.
> 	* common/common-utils.h: Include errors.h.
> 	(malloc_failure, internal_error): Don't declare.
> 
> gdb/gdbserver/
> 2014-07-16  Tom Tromey  <tromey@redhat.com>
> 	    Gary Benson  <gbenson@redhat.com>
> 
> 	* Makefile.in (SFILES) [common/errors.c]: New sourcefile.
> 	(OBS) [errors.o]: New object file.
> 	(IPA_OBS) [errors-ipa.o]: Likewise.
> 	(errors.o): New rule.
> 	(errors-ipa.o): Likewise.
> 	* utils.h: Include errors.h.
> 	(perror_with_name, error, fatal, warning): Don't declare.
> 	* utils.c (warning): Renamed and rewritten as...
> 	(vwarning): New function.
> 	(error): Renamed and rewritten as...
> 	(verror): New function.
> 	(fatal): Renamed and rewritten as...
> 	(vfatal): New function.
> 	(internal_error): Renamed and rewritten as...
> 	(internal_verror): New function.
> ---
>  gdb/ChangeLog             |   14 ++++++++
>  gdb/Makefile.in           |    8 +++-
>  gdb/common/common-utils.h |    5 +--
>  gdb/common/errors.c       |   71 +++++++++++++++++++++++++++++++++++++++
>  gdb/common/errors.h       |   81 +++++++++++++++++++++++++++++++++++++++++++++
>  gdb/gdbserver/ChangeLog   |   19 ++++++++++
>  gdb/gdbserver/Makefile.in |   16 +++++++--
>  gdb/gdbserver/utils.c     |   35 +++++--------------
>  gdb/gdbserver/utils.h     |    5 +--
>  gdb/utils.c               |   46 -------------------------
>  gdb/utils.h               |   22 +------------
>  11 files changed, 217 insertions(+), 105 deletions(-)
>  create mode 100644 gdb/common/errors.c
>  create mode 100644 gdb/common/errors.h
> 
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index ce15501..074ea78 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -935,7 +935,7 @@ gdb_bfd.h sparc-ravenscar-thread.h ppc-ravenscar-thread.h nat/linux-btrace.h \
>  ctf.h nat/i386-cpuid.h nat/i386-gcc-cpuid.h target/resume.h \
>  target/wait.h target/waitstatus.h nat/linux-nat.h nat/linux-waitpid.h \
>  common/print-utils.h common/rsp-low.h nat/i386-dregs.h x86-linux-nat.h \
> -i386-linux-nat.h
> +i386-linux-nat.h common/errors.h
>  
>  # Header files that already have srcdir in them, or which are in objdir.
>  
> @@ -1034,7 +1034,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
>  	gdb_vecs.o jit.o progspace.o skip.o probe.o \
>  	common-utils.o buffer.o ptid.o gdb-dlfcn.o common-agent.o \
>  	format.o registry.o btrace.o record-btrace.o waitstatus.o \
> -	print-utils.o rsp-low.o
> +	print-utils.o rsp-low.o errors.o
>  
>  TSOBS = inflow.o
>  
> @@ -2144,6 +2144,10 @@ rsp-low.o: ${srcdir}/common/rsp-low.c
>  	$(COMPILE) $(srcdir)/common/rsp-low.c
>  	$(POSTCOMPILE)
>  
> +errors.o: ${srcdir}/common/errors.c
> +	$(COMPILE) $(srcdir)/common/errors.c
> +	$(POSTCOMPILE)
> +
>  #
>  # gdb/target/ dependencies
>  #
> diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h
> index 063698d..53be2f8 100644
> --- a/gdb/common/common-utils.h
> +++ b/gdb/common/common-utils.h
> @@ -24,6 +24,7 @@
>  #include "ansidecl.h"
>  #include <stddef.h>
>  #include <stdarg.h>
> +#include "errors.h"
>  
>  /* If possible, define FUNCTION_NAME, a macro containing the name of
>     the function being defined.  Since this macro may not always be
> @@ -43,10 +44,6 @@
>  #endif
>  #endif
>  
> -extern void malloc_failure (long size) ATTRIBUTE_NORETURN;
> -extern void internal_error (const char *file, int line, const char *, ...)
> -     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 4);
> -
>  /* xmalloc(), xrealloc() and xcalloc() have already been declared in
>     "libiberty.h". */
>  
> diff --git a/gdb/common/errors.c b/gdb/common/errors.c
> new file mode 100644
> index 0000000..78a6d3e
> --- /dev/null
> +++ b/gdb/common/errors.c
> @@ -0,0 +1,71 @@
> +/* Error reporting facilities.
> +
> +   Copyright (C) 1986-2014 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 "config.h"
> +#include "ansidecl.h"
> +#include <stdarg.h>
> +#include "errors.h"
> +
> +/* See common/errors.h.  */
> +
> +void
> +warning (const char *fmt, ...)
> +{
> +  va_list ap;
> +
> +  va_start (ap, fmt);
> +  vwarning (fmt, ap);
> +  va_end (ap);
> +}
> +
> +/* See common/errors.h.  */
> +
> +void
> +error (const char *fmt, ...)
> +{
> +  va_list ap;
> +
> +  va_start (ap, fmt);
> +  verror (fmt, ap);
> +  va_end (ap);
> +}
> +
> +/* See common/errors.h.  */
> +
> +void
> +fatal (const char *fmt, ...)
> +{
> +  va_list ap;
> +
> +  va_start (ap, fmt);
> +  vfatal (fmt, ap);
> +  va_end (ap);
> +}
> +
> +/* See common/errors.h.  */
> +
> +void
> +internal_error (const char *file, int line, const char *fmt, ...)
> +{
> +  va_list ap;
> +
> +  va_start (ap, fmt);
> +  internal_verror (file, line, fmt, ap);
> +  va_end (ap);
> +}
> diff --git a/gdb/common/errors.h b/gdb/common/errors.h
> new file mode 100644
> index 0000000..4e8f4a7
> --- /dev/null
> +++ b/gdb/common/errors.h
> @@ -0,0 +1,81 @@
> +/* Declarations for error-reporting facilities.
> +
> +   Copyright (C) 1986-2014 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/>.  */
> +
> +#ifndef COMMON_ERRORS_H
> +#define COMMON_ERRORS_H
> +
> +/* The declarations in this file are, for the time being, separately
> +   implemented by gdb and gdbserver.  However they share a common
> +   definition so that they can be used by code in common/.  */
> +
> +/* Issue a warning to the user.  */
> +
> +extern void warning (const char *fmt, ...) ATTRIBUTE_PRINTF (1, 2);
> +
> +extern void vwarning (const char *fmt, va_list args)
> +     ATTRIBUTE_PRINTF (1, 0);
> +
> +/* Throw an error.  The current operation will be aborted.  The
> +   message will be issued to the user.  The application will
> +   return to a state where it is accepting commands from the user.  */
> +
> +extern void error (const char *fmt, ...)
> +     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
> +
> +extern void verror (const char *fmt, va_list args)
> +     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
> +
> +/* Throw an error, creating the message by combining STRING with the
> +   system error message for errno.  The current operation will be
> +   aborted.  The message will be issued to the user.  The application
> +   will return to a state where it is accepting commands from the
> +   user.  */
> +
> +extern void perror_with_name (const char *string) ATTRIBUTE_NORETURN;
> +
> +/* Throw a fatal error.  The current operation will be aborted.  The
> +   message will be issued to the user.  The application will exit.  */
> +
> +extern void fatal (const char *fmt, ...)
> +     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
> +
> +extern void vfatal (const char *fmt, va_list args)
> +     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
> +
> +/* Throw an internal error.  This should be used to indicate
> +   programming errors such as assertion failures, as opposed
> +   to more general errors the user may encounter.  The current
> +   operation will be aborted.  The message will be issued to
> +   the user.  The application may offer the user the choice to
> +   return to the command level, or the application may exit.  */
> +
> +extern void internal_error (const char *file, int line,
> +			    const char *fmt, ...)
> +     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 4);
> +
> +extern void internal_verror (const char *file, int line,
> +			     const char *fmt, va_list args)
> +     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 0);
> +
> +/* Special case internal error used to handle memory allocation
> +   failures.  */
> +
> +extern void malloc_failure (long size) ATTRIBUTE_NORETURN;
> +
> +#endif /* COMMON_ERRORS_H */
> diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
> index f9a2f17..1faa00c 100644
> --- a/gdb/gdbserver/Makefile.in
> +++ b/gdb/gdbserver/Makefile.in
> @@ -169,7 +169,7 @@ SFILES=	$(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
>  	$(srcdir)/common/buffer.c $(srcdir)/nat/linux-btrace.c \
>  	$(srcdir)/common/filestuff.c $(srcdir)/target/waitstatus.c \
>  	$(srcdir)/nat/mips-linux-watch.c $(srcdir)/common/print-utils.c \
> -	$(srcdir)/common/rsp-low.c
> +	$(srcdir)/common/rsp-low.c $(srcdir)/common/errors.c
>  
>  DEPFILES = @GDBSERVER_DEPFILES@
>  
> @@ -182,7 +182,8 @@ OBS = agent.o ax.o inferiors.o regcache.o remote-utils.o server.o signals.o \
>        target.o waitstatus.o utils.o debug.o version.o vec.o gdb_vecs.o \
>        mem-break.o hostio.o event-loop.o tracepoint.o xml-utils.o \
>        common-utils.o ptid.o buffer.o format.o filestuff.o dll.o notif.o \
> -      tdesc.o print-utils.o rsp-low.o $(XML_BUILTIN) $(DEPFILES) $(LIBOBJS)
> +      tdesc.o print-utils.o rsp-low.o errors.o $(XML_BUILTIN) $(DEPFILES) \
> +      $(LIBOBJS)
>  GDBREPLAY_OBS = gdbreplay.o version.o
>  GDBSERVER_LIBS = @GDBSERVER_LIBS@
>  XM_CLIBS = @LIBS@
> @@ -300,7 +301,10 @@ gdbreplay$(EXEEXT): $(GDBREPLAY_OBS) $(LIBGNU) $(LIBIBERTY)
>  	${CC-LD} $(INTERNAL_CFLAGS) $(INTERNAL_LDFLAGS) -o gdbreplay$(EXEEXT) $(GDBREPLAY_OBS) \
>  	  $(XM_CLIBS) $(LIBGNU) $(LIBIBERTY)
>  
> -IPA_OBJS=ax-ipa.o tracepoint-ipa.o format-ipa.o utils-ipa.o regcache-ipa.o remote-utils-ipa.o common-utils-ipa.o tdesc-ipa.o print-utils-ipa.o rsp-low-ipa.o ${IPA_DEPFILES}
> +IPA_OBJS=ax-ipa.o tracepoint-ipa.o format-ipa.o utils-ipa.o \
> +	regcache-ipa.o remote-utils-ipa.o common-utils-ipa.o \
> +	tdesc-ipa.o print-utils-ipa.o rsp-low-ipa.o errors-ipa.o \
> +	${IPA_DEPFILES}
>  
>  IPA_LIB=libinproctrace.so
>  
> @@ -489,6 +493,9 @@ print-utils-ipa.o: ../common/print-utils.c
>  rsp-low-ipa.o: ../common/rsp-low.c
>  	$(IPAGENT_COMPILE) $<
>  	$(POSTCOMPILE)
> +errors-ipa.o: ../common/errors.c
> +	$(IPAGENT_COMPILE) $<
> +	$(POSTCOMPILE)
>  
>  ax.o: ax.c
>  	$(COMPILE) $(WARN_CFLAGS_NO_FORMAT) $<
> @@ -530,6 +537,9 @@ filestuff.o: ../common/filestuff.c
>  agent.o: ../common/agent.c
>  	$(COMPILE) $<
>  	$(POSTCOMPILE)
> +errors.o: ../common/errors.c
> +	$(COMPILE) $<
> +	$(POSTCOMPILE)
>  waitstatus.o: ../target/waitstatus.c
>  	$(COMPILE) $<
>  	$(POSTCOMPILE)
> diff --git a/gdb/gdbserver/utils.c b/gdb/gdbserver/utils.c
> index 2d0b331..6c666e1 100644
> --- a/gdb/gdbserver/utils.c
> +++ b/gdb/gdbserver/utils.c
> @@ -77,20 +77,17 @@ perror_with_name (const char *string)
>    error ("%s.", combined);
>  }
>  
> -/* Print an error message and return to command level.
> -   STRING is the error message, used as a fprintf string,
> -   and ARG is passed as an argument to it.  */
> +/* Print an error message and return to command level.  */
>  
>  void
> -error (const char *string,...)
> +verror (const char *fmt, va_list args)
>  {
>  #ifndef IN_PROCESS_AGENT
>    extern jmp_buf toplevel;
>  #endif
> -  va_list args;
> -  va_start (args, string);
> +
>    fflush (stdout);
> -  vfprintf (stderr, string, args);
> +  vfprintf (stderr, fmt, args);
>    fprintf (stderr, "\n");
>  #ifndef IN_PROCESS_AGENT
>    longjmp (toplevel, 1);
> @@ -99,48 +96,36 @@ error (const char *string,...)
>  #endif
>  }
>  
> -/* Print an error message and exit reporting failure.
> -   This is for a error that we cannot continue from.
> -   STRING and ARG are passed to fprintf.  */
> +/* Print an error message and exit reporting failure.  */
>  
>  /* VARARGS */
>  void
> -fatal (const char *string,...)
> +vfatal (const char *fmt, va_list args)
>  {
> -  va_list args;
> -  va_start (args, string);
>    fprintf (stderr, PREFIX);
> -  vfprintf (stderr, string, args);
> +  vfprintf (stderr, fmt, args);
>    fprintf (stderr, "\n");
> -  va_end (args);
>    exit (1);
>  }
>  
>  /* VARARGS */
>  void
> -warning (const char *string,...)
> +vwarning (const char *fmt, va_list args)
>  {
> -  va_list args;
> -  va_start (args, string);
>    fprintf (stderr, PREFIX);
> -  vfprintf (stderr, string, args);
> +  vfprintf (stderr, fmt, args);
>    fprintf (stderr, "\n");
> -  va_end (args);
>  }
>  
>  /* Report a problem internal to GDBserver, and exit.  */
>  
>  void
> -internal_error (const char *file, int line, const char *fmt, ...)
> +internal_verror (const char *file, int line, const char *fmt, va_list args)
>  {
> -  va_list args;
> -  va_start (args, fmt);
> -
>    fprintf (stderr,  "\
>  %s:%d: A problem internal to " TOOLNAME " has been detected.\n", file, line);
>    vfprintf (stderr, fmt, args);
>    fprintf (stderr, "\n");
> -  va_end (args);
>    exit (1);
>  }
>  
> diff --git a/gdb/gdbserver/utils.h b/gdb/gdbserver/utils.h
> index 906924b..819fa35 100644
> --- a/gdb/gdbserver/utils.h
> +++ b/gdb/gdbserver/utils.h
> @@ -20,11 +20,8 @@
>  #define UTILS_H
>  
>  #include "print-utils.h"
> +#include "errors.h"
>  
> -void perror_with_name (const char *string) ATTRIBUTE_NORETURN;
> -void error (const char *string,...) ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
> -void fatal (const char *string,...) ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
> -void warning (const char *string,...) ATTRIBUTE_PRINTF (1, 2);
>  char *paddress (CORE_ADDR addr);
>  char *pfildes (gdb_fildes_t fd);
>  
> diff --git a/gdb/utils.c b/gdb/utils.c
> index d324227..b2f9f0f 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -533,22 +533,6 @@ vwarning (const char *string, va_list args)
>      }
>  }
>  
> -/* Print a warning message.
> -   The first argument STRING is the warning message, used as a fprintf string,
> -   and the remaining args are passed as arguments to it.
> -   The primary difference between warnings and errors is that a warning
> -   does not force the return to command level.  */
> -
> -void
> -warning (const char *string, ...)
> -{
> -  va_list args;
> -
> -  va_start (args, string);
> -  vwarning (string, args);
> -  va_end (args);
> -}
> -
>  /* Print an error message and return to command level.
>     The first argument STRING is the error message, used as a fprintf string,
>     and the remaining args are passed as arguments to it.  */
> @@ -559,16 +543,6 @@ verror (const char *string, va_list args)
>    throw_verror (GENERIC_ERROR, string, args);
>  }
>  
> -void
> -error (const char *string, ...)
> -{
> -  va_list args;
> -
> -  va_start (args, string);
> -  throw_verror (GENERIC_ERROR, string, args);
> -  va_end (args);
> -}
> -
>  /* Print an error message and quit.
>     The first argument STRING is the error message, used as a fprintf string,
>     and the remaining args are passed as arguments to it.  */
> @@ -580,16 +554,6 @@ vfatal (const char *string, va_list args)
>  }
>  
>  void
> -fatal (const char *string, ...)
> -{
> -  va_list args;
> -
> -  va_start (args, string);
> -  throw_vfatal (string, args);
> -  va_end (args);
> -}
> -
> -void
>  error_stream (struct ui_file *stream)
>  {
>    char *message = ui_file_xstrdup (stream, NULL);
> @@ -836,16 +800,6 @@ internal_verror (const char *file, int line, const char *fmt, va_list ap)
>    fatal (_("Command aborted."));
>  }
>  
> -void
> -internal_error (const char *file, int line, const char *string, ...)
> -{
> -  va_list ap;
> -
> -  va_start (ap, string);
> -  internal_verror (file, line, string, ap);
> -  va_end (ap);
> -}
> -
>  static struct internal_problem internal_warning_problem = {
>    "internal-warning", 1, internal_problem_ask, 1, internal_problem_ask
>  };
> diff --git a/gdb/utils.h b/gdb/utils.h
> index a91f551..e48fc43 100644
> --- a/gdb/utils.h
> +++ b/gdb/utils.h
> @@ -24,6 +24,7 @@
>  #include "cleanups.h"
>  #include "exceptions.h"
>  #include "print-utils.h"
> +#include "errors.h"
>  
>  extern void initialize_utils (void);
>  
> @@ -269,7 +270,6 @@ extern void fprintf_symbol_filtered (struct ui_file *, const char *,
>  
>  extern void throw_perror_with_name (enum errors errcode, const char *string)
>    ATTRIBUTE_NORETURN;
> -extern void perror_with_name (const char *) ATTRIBUTE_NORETURN;
>  
>  extern void perror_warning_with_name (const char *string);
>  
> @@ -283,24 +283,8 @@ extern void (*deprecated_error_begin_hook) (void);
>  
>  extern char *warning_pre_print;
>  
> -extern void verror (const char *fmt, va_list ap)
> -     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
> -
> -extern void error (const char *fmt, ...)
> -     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
> -
>  extern void error_stream (struct ui_file *) ATTRIBUTE_NORETURN;
>  
> -extern void vfatal (const char *fmt, va_list ap)
> -     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
> -
> -extern void fatal (const char *fmt, ...)
> -     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
> -
> -extern void internal_verror (const char *file, int line, const char *,
> -			     va_list ap)
> -     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 0);
> -
>  extern void internal_vwarning (const char *file, int line,
>  			       const char *, va_list ap)
>       ATTRIBUTE_PRINTF (3, 0);
> @@ -308,10 +292,6 @@ extern void internal_vwarning (const char *file, int line,
>  extern void internal_warning (const char *file, int line,
>  			      const char *, ...) ATTRIBUTE_PRINTF (3, 4);
>  
> -extern void warning (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
> -
> -extern void vwarning (const char *, va_list args) ATTRIBUTE_PRINTF (1, 0);
> -
>  extern void demangler_vwarning (const char *file, int line,
>  			       const char *, va_list ap)
>       ATTRIBUTE_PRINTF (3, 0);
> -- 
> 1.7.1
> 

-- 
http://gbenson.net/

[PATCH 01/15 v3] Introduce common/errors.h

[Gary Benson]

Doug Evans wrote:
> fatal is a confusing name IMO.  In gdb it just throws RETURN_QUIT.
> That's not your fault of course, but I wonder if I can trouble you
> to expand on the function comment a bit.  E.g., something like the
> following?
> 
> /* Cause a fatal error.
>    What this does in gdb is throw a RETURN_QUIT exception.  */
> 
> One could expand on the comment, but I didn't want to turn it into
> a FIXME.  [which is really what it should be, but that's a topic
> for another patch :-)]

I have rewritten the documentation on all these functions.  I don't
want to mention details of GDB's and/or gdbserver's implementation
in this file, so what I have done is described what callers should
expect the functions to do.  I hope this is clear enough.

> It's odd to separate out fatal and not vfatal.
> [ditto for the others]
> Can you add vfatal, et.al. to this patch?

I've done that now.  I made the non-v-prefixed functions part of
the common codebase; users of the common codebase are required to
implement the v-prefixed functions only.

Thanks,
Gary

--
gdb/
2014-07-16  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* common/errors.h: New file.
	* common/errors.c: Likewise.
	* Makefile.in (HFILES_NO_SRCDIR) [common/errors.h]: New header.
	(COMMON_OBS) [errors.o]: New object file.
	(errors.o): New rule.
	* utils.h: Include errors.h.
	(perror_with_name, error, verror, fatal, vfatal, warning
	vwarning): Don't declare.
	* common/common-utils.h: Include errors.h.
	(malloc_failure, internal_error): Don't declare.

gdb/gdbserver/
2014-07-16  Tom Tromey  <tromey@redhat.com>
	    Gary Benson  <gbenson@redhat.com>

	* Makefile.in (SFILES) [common/errors.c]: New sourcefile.
	(OBS) [errors.o]: New object file.
	(IPA_OBS) [errors-ipa.o]: Likewise.
	(errors.o): New rule.
	(errors-ipa.o): Likewise.
	* utils.h: Include errors.h.
	(perror_with_name, error, fatal, warning): Don't declare.
	* utils.c (warning): Renamed and rewritten as...
	(vwarning): New function.
	(error): Renamed and rewritten as...
	(verror): New function.
	(fatal): Renamed and rewritten as...
	(vfatal): New function.
	(internal_error): Renamed and rewritten as...
	(internal_verror): New function.
---
 gdb/ChangeLog             |   14 ++++++++
 gdb/Makefile.in           |    8 +++-
 gdb/common/common-utils.h |    5 +--
 gdb/common/errors.c       |   71 +++++++++++++++++++++++++++++++++++++++
 gdb/common/errors.h       |   81 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/ChangeLog   |   19 ++++++++++
 gdb/gdbserver/Makefile.in |   16 +++++++--
 gdb/gdbserver/utils.c     |   35 +++++--------------
 gdb/gdbserver/utils.h     |    5 +--
 gdb/utils.c               |   46 -------------------------
 gdb/utils.h               |   22 +------------
 11 files changed, 217 insertions(+), 105 deletions(-)
 create mode 100644 gdb/common/errors.c
 create mode 100644 gdb/common/errors.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index ce15501..074ea78 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -935,7 +935,7 @@ gdb_bfd.h sparc-ravenscar-thread.h ppc-ravenscar-thread.h nat/linux-btrace.h \
 ctf.h nat/i386-cpuid.h nat/i386-gcc-cpuid.h target/resume.h \
 target/wait.h target/waitstatus.h nat/linux-nat.h nat/linux-waitpid.h \
 common/print-utils.h common/rsp-low.h nat/i386-dregs.h x86-linux-nat.h \
-i386-linux-nat.h
+i386-linux-nat.h common/errors.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
@@ -1034,7 +1034,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	gdb_vecs.o jit.o progspace.o skip.o probe.o \
 	common-utils.o buffer.o ptid.o gdb-dlfcn.o common-agent.o \
 	format.o registry.o btrace.o record-btrace.o waitstatus.o \
-	print-utils.o rsp-low.o
+	print-utils.o rsp-low.o errors.o
 
 TSOBS = inflow.o
 
@@ -2144,6 +2144,10 @@ rsp-low.o: ${srcdir}/common/rsp-low.c
 	$(COMPILE) $(srcdir)/common/rsp-low.c
 	$(POSTCOMPILE)
 
+errors.o: ${srcdir}/common/errors.c
+	$(COMPILE) $(srcdir)/common/errors.c
+	$(POSTCOMPILE)
+
 #
 # gdb/target/ dependencies
 #
diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h
index 063698d..53be2f8 100644
--- a/gdb/common/common-utils.h
+++ b/gdb/common/common-utils.h
@@ -24,6 +24,7 @@
 #include "ansidecl.h"
 #include <stddef.h>
 #include <stdarg.h>
+#include "errors.h"
 
 /* If possible, define FUNCTION_NAME, a macro containing the name of
    the function being defined.  Since this macro may not always be
@@ -43,10 +44,6 @@
 #endif
 #endif
 
-extern void malloc_failure (long size) ATTRIBUTE_NORETURN;
-extern void internal_error (const char *file, int line, const char *, ...)
-     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 4);
-
 /* xmalloc(), xrealloc() and xcalloc() have already been declared in
    "libiberty.h". */
 
diff --git a/gdb/common/errors.c b/gdb/common/errors.c
new file mode 100644
index 0000000..78a6d3e
--- /dev/null
+++ b/gdb/common/errors.c
@@ -0,0 +1,71 @@
+/* Error reporting facilities.
+
+   Copyright (C) 1986-2014 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 "config.h"
+#include "ansidecl.h"
+#include <stdarg.h>
+#include "errors.h"
+
+/* See common/errors.h.  */
+
+void
+warning (const char *fmt, ...)
+{
+  va_list ap;
+
+  va_start (ap, fmt);
+  vwarning (fmt, ap);
+  va_end (ap);
+}
+
+/* See common/errors.h.  */
+
+void
+error (const char *fmt, ...)
+{
+  va_list ap;
+
+  va_start (ap, fmt);
+  verror (fmt, ap);
+  va_end (ap);
+}
+
+/* See common/errors.h.  */
+
+void
+fatal (const char *fmt, ...)
+{
+  va_list ap;
+
+  va_start (ap, fmt);
+  vfatal (fmt, ap);
+  va_end (ap);
+}
+
+/* See common/errors.h.  */
+
+void
+internal_error (const char *file, int line, const char *fmt, ...)
+{
+  va_list ap;
+
+  va_start (ap, fmt);
+  internal_verror (file, line, fmt, ap);
+  va_end (ap);
+}
diff --git a/gdb/common/errors.h b/gdb/common/errors.h
new file mode 100644
index 0000000..4e8f4a7
--- /dev/null
+++ b/gdb/common/errors.h
@@ -0,0 +1,81 @@
+/* Declarations for error-reporting facilities.
+
+   Copyright (C) 1986-2014 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/>.  */
+
+#ifndef COMMON_ERRORS_H
+#define COMMON_ERRORS_H
+
+/* The declarations in this file are, for the time being, separately
+   implemented by gdb and gdbserver.  However they share a common
+   definition so that they can be used by code in common/.  */
+
+/* Issue a warning to the user.  */
+
+extern void warning (const char *fmt, ...) ATTRIBUTE_PRINTF (1, 2);
+
+extern void vwarning (const char *fmt, va_list args)
+     ATTRIBUTE_PRINTF (1, 0);
+
+/* Throw an error.  The current operation will be aborted.  The
+   message will be issued to the user.  The application will
+   return to a state where it is accepting commands from the user.  */
+
+extern void error (const char *fmt, ...)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
+
+extern void verror (const char *fmt, va_list args)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
+
+/* Throw an error, creating the message by combining STRING with the
+   system error message for errno.  The current operation will be
+   aborted.  The message will be issued to the user.  The application
+   will return to a state where it is accepting commands from the
+   user.  */
+
+extern void perror_with_name (const char *string) ATTRIBUTE_NORETURN;
+
+/* Throw a fatal error.  The current operation will be aborted.  The
+   message will be issued to the user.  The application will exit.  */
+
+extern void fatal (const char *fmt, ...)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
+
+extern void vfatal (const char *fmt, va_list args)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
+
+/* Throw an internal error.  This should be used to indicate
+   programming errors such as assertion failures, as opposed
+   to more general errors the user may encounter.  The current
+   operation will be aborted.  The message will be issued to
+   the user.  The application may offer the user the choice to
+   return to the command level, or the application may exit.  */
+
+extern void internal_error (const char *file, int line,
+			    const char *fmt, ...)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 4);
+
+extern void internal_verror (const char *file, int line,
+			     const char *fmt, va_list args)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 0);
+
+/* Special case internal error used to handle memory allocation
+   failures.  */
+
+extern void malloc_failure (long size) ATTRIBUTE_NORETURN;
+
+#endif /* COMMON_ERRORS_H */
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index f9a2f17..1faa00c 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -169,7 +169,7 @@ SFILES=	$(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
 	$(srcdir)/common/buffer.c $(srcdir)/nat/linux-btrace.c \
 	$(srcdir)/common/filestuff.c $(srcdir)/target/waitstatus.c \
 	$(srcdir)/nat/mips-linux-watch.c $(srcdir)/common/print-utils.c \
-	$(srcdir)/common/rsp-low.c
+	$(srcdir)/common/rsp-low.c $(srcdir)/common/errors.c
 
 DEPFILES = @GDBSERVER_DEPFILES@
 
@@ -182,7 +182,8 @@ OBS = agent.o ax.o inferiors.o regcache.o remote-utils.o server.o signals.o \
       target.o waitstatus.o utils.o debug.o version.o vec.o gdb_vecs.o \
       mem-break.o hostio.o event-loop.o tracepoint.o xml-utils.o \
       common-utils.o ptid.o buffer.o format.o filestuff.o dll.o notif.o \
-      tdesc.o print-utils.o rsp-low.o $(XML_BUILTIN) $(DEPFILES) $(LIBOBJS)
+      tdesc.o print-utils.o rsp-low.o errors.o $(XML_BUILTIN) $(DEPFILES) \
+      $(LIBOBJS)
 GDBREPLAY_OBS = gdbreplay.o version.o
 GDBSERVER_LIBS = @GDBSERVER_LIBS@
 XM_CLIBS = @LIBS@
@@ -300,7 +301,10 @@ gdbreplay$(EXEEXT): $(GDBREPLAY_OBS) $(LIBGNU) $(LIBIBERTY)
 	${CC-LD} $(INTERNAL_CFLAGS) $(INTERNAL_LDFLAGS) -o gdbreplay$(EXEEXT) $(GDBREPLAY_OBS) \
 	  $(XM_CLIBS) $(LIBGNU) $(LIBIBERTY)
 
-IPA_OBJS=ax-ipa.o tracepoint-ipa.o format-ipa.o utils-ipa.o regcache-ipa.o remote-utils-ipa.o common-utils-ipa.o tdesc-ipa.o print-utils-ipa.o rsp-low-ipa.o ${IPA_DEPFILES}
+IPA_OBJS=ax-ipa.o tracepoint-ipa.o format-ipa.o utils-ipa.o \
+	regcache-ipa.o remote-utils-ipa.o common-utils-ipa.o \
+	tdesc-ipa.o print-utils-ipa.o rsp-low-ipa.o errors-ipa.o \
+	${IPA_DEPFILES}
 
 IPA_LIB=libinproctrace.so
 
@@ -489,6 +493,9 @@ print-utils-ipa.o: ../common/print-utils.c
 rsp-low-ipa.o: ../common/rsp-low.c
 	$(IPAGENT_COMPILE) $<
 	$(POSTCOMPILE)
+errors-ipa.o: ../common/errors.c
+	$(IPAGENT_COMPILE) $<
+	$(POSTCOMPILE)
 
 ax.o: ax.c
 	$(COMPILE) $(WARN_CFLAGS_NO_FORMAT) $<
@@ -530,6 +537,9 @@ filestuff.o: ../common/filestuff.c
 agent.o: ../common/agent.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
+errors.o: ../common/errors.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
 waitstatus.o: ../target/waitstatus.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
diff --git a/gdb/gdbserver/utils.c b/gdb/gdbserver/utils.c
index 2d0b331..6c666e1 100644
--- a/gdb/gdbserver/utils.c
+++ b/gdb/gdbserver/utils.c
@@ -77,20 +77,17 @@ perror_with_name (const char *string)
   error ("%s.", combined);
 }
 
-/* Print an error message and return to command level.
-   STRING is the error message, used as a fprintf string,
-   and ARG is passed as an argument to it.  */
+/* Print an error message and return to command level.  */
 
 void
-error (const char *string,...)
+verror (const char *fmt, va_list args)
 {
 #ifndef IN_PROCESS_AGENT
   extern jmp_buf toplevel;
 #endif
-  va_list args;
-  va_start (args, string);
+
   fflush (stdout);
-  vfprintf (stderr, string, args);
+  vfprintf (stderr, fmt, args);
   fprintf (stderr, "\n");
 #ifndef IN_PROCESS_AGENT
   longjmp (toplevel, 1);
@@ -99,48 +96,36 @@ error (const char *string,...)
 #endif
 }
 
-/* Print an error message and exit reporting failure.
-   This is for a error that we cannot continue from.
-   STRING and ARG are passed to fprintf.  */
+/* Print an error message and exit reporting failure.  */
 
 /* VARARGS */
 void
-fatal (const char *string,...)
+vfatal (const char *fmt, va_list args)
 {
-  va_list args;
-  va_start (args, string);
   fprintf (stderr, PREFIX);
-  vfprintf (stderr, string, args);
+  vfprintf (stderr, fmt, args);
   fprintf (stderr, "\n");
-  va_end (args);
   exit (1);
 }
 
 /* VARARGS */
 void
-warning (const char *string,...)
+vwarning (const char *fmt, va_list args)
 {
-  va_list args;
-  va_start (args, string);
   fprintf (stderr, PREFIX);
-  vfprintf (stderr, string, args);
+  vfprintf (stderr, fmt, args);
   fprintf (stderr, "\n");
-  va_end (args);
 }
 
 /* Report a problem internal to GDBserver, and exit.  */
 
 void
-internal_error (const char *file, int line, const char *fmt, ...)
+internal_verror (const char *file, int line, const char *fmt, va_list args)
 {
-  va_list args;
-  va_start (args, fmt);
-
   fprintf (stderr,  "\
 %s:%d: A problem internal to " TOOLNAME " has been detected.\n", file, line);
   vfprintf (stderr, fmt, args);
   fprintf (stderr, "\n");
-  va_end (args);
   exit (1);
 }
 
diff --git a/gdb/gdbserver/utils.h b/gdb/gdbserver/utils.h
index 906924b..819fa35 100644
--- a/gdb/gdbserver/utils.h
+++ b/gdb/gdbserver/utils.h
@@ -20,11 +20,8 @@
 #define UTILS_H
 
 #include "print-utils.h"
+#include "errors.h"
 
-void perror_with_name (const char *string) ATTRIBUTE_NORETURN;
-void error (const char *string,...) ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
-void fatal (const char *string,...) ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
-void warning (const char *string,...) ATTRIBUTE_PRINTF (1, 2);
 char *paddress (CORE_ADDR addr);
 char *pfildes (gdb_fildes_t fd);
 
diff --git a/gdb/utils.c b/gdb/utils.c
index d324227..b2f9f0f 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -533,22 +533,6 @@ vwarning (const char *string, va_list args)
     }
 }
 
-/* Print a warning message.
-   The first argument STRING is the warning message, used as a fprintf string,
-   and the remaining args are passed as arguments to it.
-   The primary difference between warnings and errors is that a warning
-   does not force the return to command level.  */
-
-void
-warning (const char *string, ...)
-{
-  va_list args;
-
-  va_start (args, string);
-  vwarning (string, args);
-  va_end (args);
-}
-
 /* Print an error message and return to command level.
    The first argument STRING is the error message, used as a fprintf string,
    and the remaining args are passed as arguments to it.  */
@@ -559,16 +543,6 @@ verror (const char *string, va_list args)
   throw_verror (GENERIC_ERROR, string, args);
 }
 
-void
-error (const char *string, ...)
-{
-  va_list args;
-
-  va_start (args, string);
-  throw_verror (GENERIC_ERROR, string, args);
-  va_end (args);
-}
-
 /* Print an error message and quit.
    The first argument STRING is the error message, used as a fprintf string,
    and the remaining args are passed as arguments to it.  */
@@ -580,16 +554,6 @@ vfatal (const char *string, va_list args)
 }
 
 void
-fatal (const char *string, ...)
-{
-  va_list args;
-
-  va_start (args, string);
-  throw_vfatal (string, args);
-  va_end (args);
-}
-
-void
 error_stream (struct ui_file *stream)
 {
   char *message = ui_file_xstrdup (stream, NULL);
@@ -836,16 +800,6 @@ internal_verror (const char *file, int line, const char *fmt, va_list ap)
   fatal (_("Command aborted."));
 }
 
-void
-internal_error (const char *file, int line, const char *string, ...)
-{
-  va_list ap;
-
-  va_start (ap, string);
-  internal_verror (file, line, string, ap);
-  va_end (ap);
-}
-
 static struct internal_problem internal_warning_problem = {
   "internal-warning", 1, internal_problem_ask, 1, internal_problem_ask
 };
diff --git a/gdb/utils.h b/gdb/utils.h
index a91f551..e48fc43 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -24,6 +24,7 @@
 #include "cleanups.h"
 #include "exceptions.h"
 #include "print-utils.h"
+#include "errors.h"
 
 extern void initialize_utils (void);
 
@@ -269,7 +270,6 @@ extern void fprintf_symbol_filtered (struct ui_file *, const char *,
 
 extern void throw_perror_with_name (enum errors errcode, const char *string)
   ATTRIBUTE_NORETURN;
-extern void perror_with_name (const char *) ATTRIBUTE_NORETURN;
 
 extern void perror_warning_with_name (const char *string);
 
@@ -283,24 +283,8 @@ extern void (*deprecated_error_begin_hook) (void);
 
 extern char *warning_pre_print;
 
-extern void verror (const char *fmt, va_list ap)
-     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
-
-extern void error (const char *fmt, ...)
-     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
-
 extern void error_stream (struct ui_file *) ATTRIBUTE_NORETURN;
 
-extern void vfatal (const char *fmt, va_list ap)
-     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
-
-extern void fatal (const char *fmt, ...)
-     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
-
-extern void internal_verror (const char *file, int line, const char *,
-			     va_list ap)
-     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 0);
-
 extern void internal_vwarning (const char *file, int line,
 			       const char *, va_list ap)
      ATTRIBUTE_PRINTF (3, 0);
@@ -308,10 +292,6 @@ extern void internal_vwarning (const char *file, int line,
 extern void internal_warning (const char *file, int line,
 			      const char *, ...) ATTRIBUTE_PRINTF (3, 4);
 
-extern void warning (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
-
-extern void vwarning (const char *, va_list args) ATTRIBUTE_PRINTF (1, 0);
-
 extern void demangler_vwarning (const char *file, int line,
 			       const char *, va_list ap)
      ATTRIBUTE_PRINTF (3, 0);

Re: [PATCH 01/15 v3] Introduce common/errors.h

[Pedro Alves]

On 07/17/2014 02:47 PM, Gary Benson wrote:
> +/* Throw an error.  The current operation will be aborted.  The
> +   message will be issued to the user.  The application will
> +   return to a state where it is accepting commands from the user.  */

These comments aren't really true, though.  We have plenty of places
catching errors with TRY_CATCH and proceeding without returning to a
state where we're accepting commands from the user.

> +
> +extern void error (const char *fmt, ...)
> +     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
> +

(We should really bite the bullet and move exceptions.{h|c} and
cleanups.{h|c} to common/ and make gdbserver use them too.)

-- 
Pedro Alves

Re: [PATCH 01/15 v3] Introduce common/errors.h

[Gary Benson]

Pedro Alves wrote:
> On 07/17/2014 02:47 PM, Gary Benson wrote:
> > +/* Throw an error.  The current operation will be aborted.  The
> > +   message will be issued to the user.  The application will
> > +   return to a state where it is accepting commands from the user.  */
> 
> These comments aren't really true, though.  We have plenty of places
> catching errors with TRY_CATCH and proceeding without returning to a
> state where we're accepting commands from the user.

How about "Throw an error.  The current operation will be aborted.
The application may catch and process the error, or, if not, the
message will be issued to the user and the application will return
to a state where it is accepting commands from the user."

> (We should really bite the bullet and move exceptions.{h|c} and
> cleanups.{h|c} to common/ and make gdbserver use them too.)

Can I leave this for another series please?  This one's big enough
already.

Thanks,
Gary

-- 
http://gbenson.net/

Re: [PATCH 01/15 v3] Introduce common/errors.h

[Pedro Alves]

On 07/17/2014 04:39 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 07/17/2014 02:47 PM, Gary Benson wrote:
>>> +/* Throw an error.  The current operation will be aborted.  The
>>> +   message will be issued to the user.  The application will
>>> +   return to a state where it is accepting commands from the user.  */
>>
>> These comments aren't really true, though.  We have plenty of places
>> catching errors with TRY_CATCH and proceeding without returning to a
>> state where we're accepting commands from the user.
> 
> How about "Throw an error.  The current operation will be aborted.
> The application may catch and process the error, or, if not, the
> message will be issued to the user and the application will return
> to a state where it is accepting commands from the user."

Still infers what the application does with the error.  IMO, it's
best to describe the interface.  Like:

 Throw a generic error.  This function not return, instead
 it executes a long jump, aborting the current operation.
 The function takes printf-style arguments, which are used to
 construct the error message.

>> (We should really bite the bullet and move exceptions.{h|c} and
>> cleanups.{h|c} to common/ and make gdbserver use them too.)
> 
> Can I leave this for another series please?  This one's big enough
> already.

Yes, of course.  That's why it was a parenthesis.

-- 
Pedro Alves

Re: [PATCH 02/15 v2] Remove some GDBSERVER checks from linux-ptrace

[Pedro Alves]

On 07/16/2014 03:17 PM, Gary Benson wrote:
> This patch removes some GDBSERVER checks from nat/linux-ptrace.c.
> Currently the code uses a compile-time check to decide whether some
> flags should be used.  This changes the code to instead let users of
> the module specify an additional set of flags; and then changes gdb's
> linux-nat.c to call this function. 

> At some later date, when the back
> ends are fully merged, we will be able to remove this function again.


> @@ -37,6 +37,10 @@
>     there are no supported features.  */
>  static int current_ptrace_options = -1;
>  
> +/* Additional flags to test.  */
> +
> +static int additional_flags;
> +
>  /* Find all possible reasons we could fail to attach PID and append
>     these as strings to the already initialized BUFFER.  '\0'
>     termination of BUFFER must be done by the caller.  */
> @@ -359,16 +363,15 @@ linux_check_ptrace_features (void)
>  static void
>  linux_test_for_tracesysgood (int child_pid)
>  {
> -#ifdef GDBSERVER
> -  /* gdbserver does not support PTRACE_O_TRACESYSGOOD.  */
> -#else
> -  int ret;
> +  if ((additional_flags & PTRACE_O_TRACESYSGOOD) != 0)
> +    {

Please avoid this indenting unless necessary, by
reversing the logic:

  if ((additional_flags & PTRACE_O_TRACESYSGOOD) == 0)
     return;

As this will end up being undone once additional_flags is
removed again later on, this way avoid re-indenting back,
and complicating history / git blame.

> +      int ret;
>  
> -  ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
> -		(PTRACE_TYPE_ARG4) PTRACE_O_TRACESYSGOOD);
> -  if (ret == 0)
> -    current_ptrace_options |= PTRACE_O_TRACESYSGOOD;
> -#endif
> +      ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
> +		    (PTRACE_TYPE_ARG4) PTRACE_O_TRACESYSGOOD);
> +      if (ret != 0)
> +	additional_flags &= ~PTRACE_O_TRACESYSGOOD;

This isn't right.  The function should still update
current_ptrace_options (the effectively enabled options).

Otherwise ...

> +    }
>  }
>  
>  /* Determine if PTRACE_O_TRACEFORK can be used to follow fork
> @@ -388,16 +391,15 @@ linux_test_for_tracefork (int child_pid)
>    if (ret != 0)
>      return;
>  
> -#ifdef GDBSERVER
> -  /* gdbserver does not support PTRACE_O_TRACEVFORKDONE yet.  */
> -#else
> -  /* Check if the target supports PTRACE_O_TRACEVFORKDONE.  */
> -  ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
> -		(PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK
> -				    | PTRACE_O_TRACEVFORKDONE));
> -  if (ret == 0)
> -    current_ptrace_options |= PTRACE_O_TRACEVFORKDONE;
> -#endif
> +  if ((additional_flags & PTRACE_O_TRACEVFORKDONE) != 0)
> +    {
> +      /* Check if the target supports PTRACE_O_TRACEVFORKDONE.  */
> +      ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0,
> +		    (PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK
> +					| PTRACE_O_TRACEVFORKDONE));
> +      if (ret != 0)
> +	additional_flags &= ~PTRACE_O_TRACEVFORKDONE;
> +    }
>  
>    /* Setting PTRACE_O_TRACEFORK did not cause an error, however we
>       don't know for sure that the feature is available; old
> @@ -433,18 +435,7 @@ linux_test_for_tracefork (int child_pid)
>  
>  	  /* We got the PID from the grandchild, which means fork
>  	     tracing is supported.  */
> -#ifdef GDBSERVER
> -	  /* Do not enable all the options for now since gdbserver does not
> -	     properly support them.  This restriction will be lifted when
> -	     gdbserver is augmented to support them.  */
> -	  current_ptrace_options |= PTRACE_O_TRACECLONE;
> -#else
> -	  current_ptrace_options |= PTRACE_O_TRACEFORK | PTRACE_O_TRACEVFORK
> -	    | PTRACE_O_TRACECLONE | PTRACE_O_TRACEEXEC;
> -
> -	  /* Do not enable PTRACE_O_TRACEEXIT until GDB is more prepared to
> -	     support read-only process state.  */
> -#endif
> +	  current_ptrace_options |= PTRACE_O_TRACECLONE | additional_flags;

... note how PTRACE_O_TRACESYSGOOD is lost if PTRACE_O_TRACECLONE
isn't supported.

So don't clear additional_flags.  Instead add to current_ptrace_options
as options required in additional_flags are detected to be supported.

-- 
Pedro Alves

Re: [PATCH 03/15 v2] Make gdbserver CORE_ADDR unsigned

[Pedro Alves]

The only issue that I think might come out of this is
that on MIPS, addresses are signed, so pointers are sign
extended.  But we can definitely handle whatever fallout this
may cause, if any, when we see it.  Clearly if GDB's native
targets can handle that, so should gdbserver's, and in any
case the issue should be pretty localized.  Just pointing it
out FYI, to keep an eye out for it.

-- 
Pedro Alves

Re: [PATCH 03/15 v2] Make gdbserver CORE_ADDR unsigned

[Maciej W. Rozycki]

On Thu, 17 Jul 2014, Pedro Alves wrote:

> The only issue that I think might come out of this is
> that on MIPS, addresses are signed, so pointers are sign
> extended.  But we can definitely handle whatever fallout this
> may cause, if any, when we see it.  Clearly if GDB's native
> targets can handle that, so should gdbserver's, and in any
> case the issue should be pretty localized.  Just pointing it
> out FYI, to keep an eye out for it.

 Also SH64 AFAICT, it defines `elf_backend_sign_extend_vma' to true in 
bfd/elf32-sh64.c.  We have the following comment in gdb/mips-tdep.c:

/* MIPS believes that the PC has a sign extended value.  Perhaps the
   all registers should be sign extended for simplicity?  */

which is of course true in that we need to sign-extend all integer 
registers (that includes GPRs and CP0 registers; maybe some control 
registers as well); where applicable that is, i.e. debugging a strict 
32-bit ABI on 64-bit hardware.  Then on the other hand the values in these 
registers should already have been truncated to 32 bits and then 
sign-extended before they have been written in the first place.

 Overall it's tricky, hardware does not always enforce proper 
sign-extension required by the ABI, e.g. an o32 kernel-mode program is 
free to set GPRs or the PC to a value outside the range supported by the 
ABI (the Linux kernel sometimes takes advantage of this possibility), and 
we have no way I believe to make the user aware of this happening, while 
that might be The Bug they're after.

  Maciej

Re: [PATCH 01/15 v3] Introduce common/errors.h

[Gary Benson]

Pedro Alves wrote:
> On 07/17/2014 04:39 PM, Gary Benson wrote:
> > Pedro Alves wrote:
> > > On 07/17/2014 02:47 PM, Gary Benson wrote:
> > > > +/* Throw an error.  The current operation will be aborted.  The
> > > > +   message will be issued to the user.  The application will
> > > > +   return to a state where it is accepting commands from the user.  */
> > >
> > > These comments aren't really true, though.  We have plenty of
> > > places catching errors with TRY_CATCH and proceeding without
> > > returning to a state where we're accepting commands from the
> > > user.
> > 
> > How about "Throw an error.  The current operation will be aborted.
> > The application may catch and process the error, or, if not, the
> > message will be issued to the user and the application will return
> > to a state where it is accepting commands from the user."
> 
> Still infers what the application does with the error.  IMO, it's
> best to describe the interface.  Like:
> 
>  Throw a generic error.  This function not return, instead
>  it executes a long jump, aborting the current operation.
>  The function takes printf-style arguments, which are used to
>  construct the error message.

Surely "it executes a long jump" is describing what the application
does with the error?  Besides, it's not true for IPA, where error
just calls exit (1).

In the context of this whole file, you have:

 - warning (one type)
 - errors (three types)
 - special cases (perror_with_name, malloc_failure)

The only difference between the three types of error is what the
application does with them:

 - error (may be handled, may return to command level, may exit)
 - fatal (will exit)
 - internal_error (may return to command level, may exit)

I don't think you can properly document these functions without
spelling this out.

Also, the documentation in this file is not just for callers of the
functions, it has to document them for implementors too.

How about the following?

Cheers,
Gary

--
/* Display a warning message to the user.  The message is constructed
   using a printf-style format string and a printf- or vprintf-style
   argument list.  */

extern void warning (const char *fmt, ...) ATTRIBUTE_PRINTF (1, 2);

extern void vwarning (const char *fmt, va_list args)
     ATTRIBUTE_PRINTF (1, 0);

/* Throw a non-fatal error, constructing the message using a printf-
   style format string and a printf- or vprintf-style argument list.
   This function does not return.  The application may attempt to
   handle the error.  */

extern void error (const char *fmt, ...)
     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);

extern void verror (const char *fmt, va_list args)
     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);

/* Throw a non-fatal error, constructing the message by combining
   STRING with the system error message for errno.  This function does
   not return.  The application may attempt to handle the error.  */

extern void perror_with_name (const char *string) ATTRIBUTE_NORETURN;

/* Throw a fatal error, constructing the message using a printf-style
   format string and a printf- or vprintf-style argument list.  This
   function does not return.  The application will exit.  */

extern void fatal (const char *fmt, ...)
     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);

extern void vfatal (const char *fmt, va_list args)
     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);

/* Throw an internal error, constructing the message using a printf-
   style format string and a printf- or vprintf-style argument list.
   This function does not return.  Internal errors indicate
   programming errors such as assertion failures, as opposed to more
   general errors the user may encounter.  Internal errors are treated
   as fatal errors, except that the application may offer the user the
   choice to return to the command level rather than exiting.  */

extern void internal_error (const char *file, int line,
			    const char *fmt, ...)
     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 4);

extern void internal_verror (const char *file, int line,
			     const char *fmt, va_list args)
     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 0);

/* A special case of internal error used to handle memory allocation
   failures.  */

extern void malloc_failure (long size) ATTRIBUTE_NORETURN;

Re: [PATCH 01/15 v3] Introduce common/errors.h

[Doug Evans]

On Fri, Jul 18, 2014 at 10:06 AM, Gary Benson <gbenson@redhat.com> wrote:
> [...]
> /* Throw a fatal error, constructing the message using a printf-style
>    format string and a printf- or vprintf-style argument list.  This
>    function does not return.  The application will exit.  */
>
> extern void fatal (const char *fmt, ...)
>      ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
>
> extern void vfatal (const char *fmt, va_list args)
>      ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
> [...]

Remember gdb doesn't exit on fatal().
fatal() in gdb is essentially ^c (quit() calls fatal()!).

Can I repeat my request to please document this in the function comment.

Re: [PATCH 01/15 v3] Introduce common/errors.h

[Pedro Alves]

On 07/18/2014 10:06 AM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 07/17/2014 04:39 PM, Gary Benson wrote:
>>> Pedro Alves wrote:
>>>> On 07/17/2014 02:47 PM, Gary Benson wrote:
>>>>> +/* Throw an error.  The current operation will be aborted.  The
>>>>> +   message will be issued to the user.  The application will
>>>>> +   return to a state where it is accepting commands from the user.  */
>>>>
>>>> These comments aren't really true, though.  We have plenty of
>>>> places catching errors with TRY_CATCH and proceeding without
>>>> returning to a state where we're accepting commands from the
>>>> user.
>>>
>>> How about "Throw an error.  The current operation will be aborted.
>>> The application may catch and process the error, or, if not, the
>>> message will be issued to the user and the application will return
>>> to a state where it is accepting commands from the user."
>>
>> Still infers what the application does with the error.  IMO, it's
>> best to describe the interface.  Like:
>>
>>  Throw a generic error.  This function not return, instead
>>  it executes a long jump, aborting the current operation.
>>  The function takes printf-style arguments, which are used to
>>  construct the error message.
> 
> Surely "it executes a long jump" is describing what the application
> does with the error?

By "does with the error" I was talking about what the application
does after the error is thrown, like talking about accepting
commands from the user, not really the internal implementation.

> Besides, it's not true for IPA, where error
> just calls exit (1).

Yeah, forgot that, though that's conceptually the same as the IPA having
an outer TRY_CATCH and calling exit(1) there.

OK, thinking this through.

Conceptually, what matters to the user of the API is when to call which error
function variant, and this one is to be called on generic errors.  That
the IPA chooses the treat generic errors as fatal errors, it's IPA's
business.  That's the detail of what the application decides to do
with the error.

> 
> In the context of this whole file, you have:
> 
>  - warning (one type)
>  - errors (three types)
>  - special cases (perror_with_name, malloc_failure)
> 
> The only difference between the three types of error is what the
> application does with them:
> 
>  - error (may be handled, may return to command level, may exit)
>  - fatal (will exit)
>  - internal_error (may return to command level, may exit)

I think that's not the best way to look at this.  We're documenting
an API, the contract the shared/core code will rely on to decide which
function to call.  If the contract is different in GDB vs GDBserver
then we shall fix up the differences first.

So, what is the contract we want ?  Where do we want to get to?

I think it should be:

- warning (something odd detected, but we'll carry on anyway.)
- error (generic, non-predicatable, non-fatal condition detected.
         The requested operation can't proceed.  E.g.: trying to
         compute expressions with optimized values; invalid input;invalid
         conversion attempted; etc.)
- internal_error (faulty logic detected)

and then, I think we should just not use 'fatal' as is in common code,
and so not move that to common code.  It's just confusing and behaves
different in GDB and GDBserver.  We have a bunch of callers on the
GDBserver side, but can't those all be internal_error / gdb_assert?
Why not?  If not, then I propose really describing 'fatal' as
being a non-recoverable fatal error.

On the GDB side it's only used in two situations -- throw
a QUIT when the user wants to try recovering from an internal error,
and, throw a QUIT when the user presses ctrl-c.  It's not called
when a fatal / internal error is detected, like on the
GDBserver side.

I think we should rename the existing 'fatal' on the GDB side for
clarity to something like throw_quit or some such.  How GDB
recovers from internal_error is GDB's business.  It just happens
today, that results in a QUIT being thrown.

For situations in GDB where we might have a fatal error that is
truly not recoverable, and that should not even consult the user
on whether to quit/dump core, etc., then we wouldn't call the
existing 'fatal' function as it is.
E.g., if GDB's 'fatal' where truly a 'fatal'-like function like
GDBserver's, this in main.c:

    /* Install it.  */
    if (!interp_set (interp, 1))
      {
        fprintf_unfiltered (gdb_stderr,
			    "Interpreter `%s' failed to initialize.\n",
                            interpreter_p);
        exit (1);
      }

would be written as:

    /* Install it.  */
    if (!interp_set (interp, 1))
      {
        fatal ("Interpreter `%s' failed to initialize.\n",
               interpreter_p);
      }

with 'fatal' printing the error and exiting, just like gdbserver.

I'd support doing that.

> I don't think you can properly document these functions without
> spelling this out.
> 
> Also, the documentation in this file is not just for callers of the
> functions, it has to document them for implementors too.

The way to do that is to document the contract callers use to
determine the class of error at hand, not what the application
does with such class of error.  If we're thinking in terms of library,
then listing what current applications do just results in comments
getting stale as implementations change or more applications are added.

> /* Throw an internal error, constructing the message using a printf-
>    style format string and a printf- or vprintf-style argument list.
>    This function does not return.  Internal errors indicate
>    programming errors such as assertion failures, as opposed to more
>    general errors the user may encounter.  Internal errors are treated
>    as fatal errors, except that the application may offer the user the
>    choice to return to the command level rather than exiting.  */

Here we're talking about "the command level", which doesn't
even exist on GDBserver or the IPA.  And I'm now thinking that
saying "Throw" here isn't exactly the best to say.  Instead,
I think it'd be better to say something like "Call when an internal
error was detected.", and don't even talk about the command level.
Like:

 /* An internal error was detected.  Internal errors indicate
    programming errors such as assertion failures, as opposed to more
    general errors beyond the application's control.
    This function does not return, and constructs an error message
    using a printf-style argument list.  FILE and LINE indicate the
    file and line number where the programming error was detected.

Thanks,
-- 
Pedro Alves

Re: [PATCH 01/15 v3] Introduce common/errors.h

[Gary Benson]

Doug Evans wrote:
> On Fri, Jul 18, 2014 at 10:06 AM, Gary Benson <gbenson@redhat.com> wrote:
> > [...]
> > /* Throw a fatal error, constructing the message using a printf-style
> >    format string and a printf- or vprintf-style argument list.  This
> >    function does not return.  The application will exit.  */
> >
> > extern void fatal (const char *fmt, ...)
> >      ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
> >
> > extern void vfatal (const char *fmt, va_list args)
> >      ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
> > [...]
> 
> Remember gdb doesn't exit on fatal().
> fatal() in gdb is essentially ^c (quit() calls fatal()!).
> 
> Can I repeat my request to please document this in the function
> comment.

I wanted to avoid putting implementation details here, but I see this
isn't going to happen.  Is the attached ok?

Cheers,
Gary

-- 
/* Display a warning message to the user.  The message is constructed
   using a printf-style format string and a printf- or vprintf-style
   argument list.  */

extern void warning (const char *fmt, ...) ATTRIBUTE_PRINTF (1, 2);

extern void vwarning (const char *fmt, va_list args)
     ATTRIBUTE_PRINTF (1, 0);

/* Throw a generic error, constructing the message using a printf-
   style format string and a printf- or vprintf-style argument list.
   This function does not return.  */

extern void error (const char *fmt, ...)
     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);

extern void verror (const char *fmt, va_list args)
     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);

/* Throw a generic error, constructing the message by combining STRING
   with the system error message for errno.  This function does not
   return.  */

extern void perror_with_name (const char *string) ATTRIBUTE_NORETURN;

/* Throw a fatal error, constructing the message using a printf-style
   format string and a printf- or vprintf-style argument list.  This
   function does not return.  Fatal errors cause GDB to return to the
   command level.  Fatal errors cause gdbserver to exit.  */

extern void fatal (const char *fmt, ...)
     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);

extern void vfatal (const char *fmt, va_list args)
     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);

/* Throw an internal error, constructing the message using a printf-
   style format string and a printf- or vprintf-style argument list.
   This function does not return.  */

extern void internal_error (const char *file, int line,
			    const char *fmt, ...)
     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 4);

extern void internal_verror (const char *file, int line,
			     const char *fmt, va_list args)
     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (3, 0);

/* A special case of internal error used to handle memory allocation
   failures.  */

extern void malloc_failure (long size) ATTRIBUTE_NORETURN;

Re: [PATCH 01/15 v3] Introduce common/errors.h

[Doug Evans]

On Fri, Jul 18, 2014 at 11:44 AM, Gary Benson <gbenson@redhat.com> wrote:
> Doug Evans wrote:
>> On Fri, Jul 18, 2014 at 10:06 AM, Gary Benson <gbenson@redhat.com> wrote:
>> > [...]
>> > /* Throw a fatal error, constructing the message using a printf-style
>> >    format string and a printf- or vprintf-style argument list.  This
>> >    function does not return.  The application will exit.  */
>> >
>> > extern void fatal (const char *fmt, ...)
>> >      ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
>> >
>> > extern void vfatal (const char *fmt, va_list args)
>> >      ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);
>> > [...]
>>
>> Remember gdb doesn't exit on fatal().
>> fatal() in gdb is essentially ^c (quit() calls fatal()!).
>>
>> Can I repeat my request to please document this in the function
>> comment.
>
> I wanted to avoid putting implementation details here,

I realize that.

> but I see this
> isn't going to happen.  Is the attached ok?

I'm trying to not impose on you too much churn and back-and-forth in
what is clearly a stepping stone.
Eventually I think fatal needs to disappear from the gdb side (renamed
or whatever).
Until that happens (i.e., *if* you just want to keep the patch
basically as is), then at the least I don't want to lie to the reader,
and I want to make the reader aware of the issue.  That's the high
order bit for me as far as "fatal" goes (within the context of trying
to keep the patch basically as is).

If you want to take on the task of getting this 100% correct in this
pass (or at least within some fraction thereof :-)), then we can take
a different route.  Your choice IMO.  [And I mean that sincerely - I
*am* trying to help you make progress here without being too
pedantic.]

> /* Throw a fatal error, constructing the message using a printf-style
>    format string and a printf- or vprintf-style argument list.  This
>    function does not return.  Fatal errors cause GDB to return to the
>    command level.  Fatal errors cause gdbserver to exit.  */
>
> extern void fatal (const char *fmt, ...)
>      ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
>
> extern void vfatal (const char *fmt, va_list args)
>      ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 0);

I would tweak that a little, and instead just point out that gdb's
usage of the name "fatal" is broken.  I don't care too much about
the wording, I just want to make sure we don't lie to the reader
and make the reader aware of the issue.

/* Throw a fatal error, constructing the message using a printf-style
   format string and a printf- or vprintf-style argument list.  This
   function does not return.  Fatal errors cause the app to exit,
   except in the case of gdb where it just throws a RETURN_QUIT
   exception.  */