Re: [PATCH] Please define thread_info as struct thread_info (and other stuff)

[Thomas Schwinge <thomas@codesourcery.com>]

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

Hi!

On Mon, 17 Dec 2018 13:41:25 -0800, John Baldwin <jhb@FreeBSD.org> wrote:
> On 12/17/18 12:51 PM, Svante Signell wrote:
> > On Sun, 2018-12-16 at 16:10 -0700, Tom Tromey wrote:
> >>>>>>> "Svante" == Svante Signell <svante.signell@gmail.com> writes:
> >> Svante> Finally, I've found the problem (but no workaround yet): thread_info
> >> is an RPC on GNU/Hurd, and including mach.h in gdb/config/i386/nm-
> >> i386gnu.h:#include <mach.h> further includes <mach/mach_interface.h> which has
> >> the conflicting name Svante> of that RPC: kern_return_t thread_info
> >>
> >> Typical answers for this kind of thing are either to segregate the use
> >> of the system header somehow, or maybe namespacing or some other kind of
> >> renaming.  I haven't looked into the details much in this case I'm
> >> afraid.
> > 
> > As I see it you need to:
> > 
> > 1) Apply the patches submitted earlier in this thread using struct thread_info
> > consistently everywhere (simplest).

(We probably wouldn't be able to do that: necessary to work around some
compiler bug, I've seen a number of commits that changed "new-style C++
for iterators" away from 'for (struct thread_info *ti : [something])' to
just 'thread_info *ti' (without 'class' or 'struct').)

> > 2) Rename all usage of the struct thread_info to something else e.g. struct
> > gdb_thread_info (not future-proof though).
> > 3) Create a gdb namespace for all your code to avoid conflicts.
> > 4) Segregate the use of system header files as you write above. Dunno how to do
> > that though, but some of you should.
> 
> Normally code for native targets do 4).  I've had to be careful about which
> includes I use in native FreeBSD targets to avoid namespace collisions, etc.

Right.  I have now pushed to master the attached commit
cabb5f067daa9227bf0323cbf64c6065d6e4796f "[gdb, hurd] Work around
conflict between Mach's 'thread_info' function, and GDB's 'thread_info'
class".


Grüße
 Thomas



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gdb-hurd-Work-around-conflict-between-Mach-s-thread_.patch --]
[-- Type: text/x-diff, Size: 5377 bytes --]

From cabb5f067daa9227bf0323cbf64c6065d6e4796f Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Wed, 13 Feb 2019 12:02:20 +0100
Subject: [PATCH] [gdb, hurd] Work around conflict between Mach's 'thread_info'
 function, and GDB's 'thread_info' class

    In file included from ./nm.h:25:0,
                     from [...]/gdb/defs.h:423,
                     from [...]/gdb/gdb.c:19:
    [...]/gdb/regcache.h:35:46: warning: 'get_thread_regcache' initialized and declared 'extern'
     extern struct regcache *get_thread_regcache (thread_info *thread);
                                                  ^~~~~~~~~~~
    [...]/gdb/regcache.h:35:46: error: 'regcache* get_thread_regcache' redeclared as different kind of symbol
    [...]
    [...]/gdb/gdbarch.h:1203:69: error: 'thread_info' is not a type
     extern LONGEST gdbarch_get_syscall_number (struct gdbarch *gdbarch, thread_info *thread);
                                                                         ^~~~~~~~~~~

Fixed with a different (self-contained, more maintainable?) approach compared
to what has been done in commit 7aabaf9d4ad52a1df1f551908fbd8cafc5e7597a
"Create private_thread_info hierarchy", and commit
75cbc781e371279f4403045be93b07fd8fe7fde5 "gdb: For macOS, s/thread_info/struct
thread_info/".  We don't want to change all the GDB code to everywhere use
'class thread_info' or 'struct thread_info' instead of plain 'thread_info'.

	gdb/
	* config/i386/nm-i386gnu.h: Don't "#include" any files.
	* gnu-nat.h (mach_thread_info): New function.
	* gnu-nat.c (thread_takeover_sc_cmd): Use it.
---
 gdb/ChangeLog                |  4 ++++
 gdb/config/i386/nm-i386gnu.h |  5 -----
 gdb/gnu-nat.c                |  9 ++++++---
 gdb/gnu-nat.h                | 15 ++++++++++++++-
 gdb/i386-gnu-nat.c           |  4 +++-
 5 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e6d86e2552..c88216b94b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2019-02-14  Thomas Schwinge  <thomas@codesourcery.com>
 
+	* config/i386/nm-i386gnu.h: Don't "#include" any files.
+	* gnu-nat.h (mach_thread_info): New function.
+	* gnu-nat.c (thread_takeover_sc_cmd): Use it.
+
 	* config/i386/nm-i386gnu.h (gnu_target_pid_to_str): Remove.
 
 2019-02-14  Frederic Konrad  <konrad@adacore.com>
diff --git a/gdb/config/i386/nm-i386gnu.h b/gdb/config/i386/nm-i386gnu.h
index 8f36102da4..e859b23f2b 100644
--- a/gdb/config/i386/nm-i386gnu.h
+++ b/gdb/config/i386/nm-i386gnu.h
@@ -19,11 +19,6 @@
 #ifndef CONFIG_I386_NM_I386GNU_H
 #define CONFIG_I386_NM_I386GNU_H
 
-#include <unistd.h>
-#include <mach.h>
-#include <mach/exception.h>
-#include "regcache.h"
-
 /* Thread flavors used in re-setting the T bit.  */
 #define THREAD_STATE_FLAVOR		i386_REGS_SEGS_STATE
 #define THREAD_STATE_SIZE		i386_THREAD_STATE_COUNT
diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
index cb8331daaf..bd8fcb6e59 100644
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -20,6 +20,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/>.  */
 
+/* Include this first, to pick up the <mach.h> 'thread_info' diversion.  */
+#include "gnu-nat.h"
+
 /* Mach/Hurd headers are not yet ready for C++ compilation.  */
 extern "C"
 {
@@ -67,7 +70,6 @@ extern "C"
 #include "gdb_obstack.h"
 #include "tid-parse.h"
 
-#include "gnu-nat.h"
 #include "inf-child.h"
 
 /* MIG stubs are not yet ready for C++ compilation.  */
@@ -3429,8 +3431,9 @@ thread_takeover_sc_cmd (const char *args, int from_tty)
   thread_basic_info_data_t _info;
   thread_basic_info_t info = &_info;
   mach_msg_type_number_t info_len = THREAD_BASIC_INFO_COUNT;
-  kern_return_t err =
-  thread_info (thread->port, THREAD_BASIC_INFO, (int *) &info, &info_len);
+  kern_return_t err
+    = mach_thread_info (thread->port, THREAD_BASIC_INFO,
+			(int *) &info, &info_len);
   if (err)
     error (("%s."), safe_strerror (err));
   thread->sc = info->suspend_count;
diff --git a/gdb/gnu-nat.h b/gdb/gnu-nat.h
index 9df479c850..dbad0cac93 100644
--- a/gdb/gnu-nat.h
+++ b/gdb/gnu-nat.h
@@ -19,8 +19,21 @@
 #ifndef GNU_NAT_H
 #define GNU_NAT_H
 
-#include <unistd.h>
+#include "defs.h"
+
+/* Work around conflict between Mach's 'thread_info' function, and GDB's
+   'thread_info' class.  Make the former available as 'mach_thread_info'.  */
+#define thread_info mach_thread_info
+/* Mach headers are not yet ready for C++ compilation.  */
+extern "C"
+{
 #include <mach.h>
+}
+#undef thread_info
+/* Divert 'mach_thread_info' to the original Mach 'thread_info' function.  */
+extern __typeof__ (mach_thread_info) mach_thread_info asm ("thread_info");
+
+#include <unistd.h>
 
 struct inf;
 
diff --git a/gdb/i386-gnu-nat.c b/gdb/i386-gnu-nat.c
index ffba941eef..c23c4bc79c 100644
--- a/gdb/i386-gnu-nat.c
+++ b/gdb/i386-gnu-nat.c
@@ -17,6 +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/>.  */
 
+/* Include this first, to pick up the <mach.h> 'thread_info' diversion.  */
+#include "gnu-nat.h"
+
 /* Mach/Hurd headers are not yet ready for C++ compilation.  */
 extern "C"
 {
@@ -34,7 +37,6 @@ extern "C"
 
 #include "i386-tdep.h"
 
-#include "gnu-nat.h"
 #include "inf-child.h"
 #include "i387-tdep.h"
 
-- 
2.19.2