[PATCH] Don't override various Makefile variables for gnulib et al

Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed

* [PATCH] Don't override various Makefile variables for gnulib et al
@ 2019-11-22 23:30 Christian Biesinger via gdb-patches
  2019-11-26 18:01 ` [PATCH] add file desc to gdbserver client_state Stan Cox
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Christian Biesinger via gdb-patches @ 2019-11-22 23:30 UTC (permalink / raw)
  To: gdb-patches, gcc-patches; +Cc: Christian Biesinger

Normally the toplevel Makefile will pass various CC=foo and other
flags down to subdir Makefiles. However, for Gnulib this is a problem
because Gnulib's configure specifically sets CC to something that
includes a -std=gnu11 flag on some systems, and this override would
set it back to CC=gcc, leading to compile errors in a GDB build
with an updated Gnulib.

I don't believe this is needed outside of GCC, so this patch changes
Gnulib and other non-GCC modules to just not override any flags --
the values set during configure time should be fine. If a user
overrides them manually when invoking make, those will still work.

Under the same condition, I also removed the host_exports. I don't
understand why this is ever necessary (this is only after configure
has run).

The other option is to clear MAKEOVERRIDES in gnulib/Makefile.am, but
that means the user can't override any variables for this subdirectory.

ChangeLog:

2019-11-22  Christian Biesinger  <cbiesinger@google.com>

	* Makefile.def: Pass no_exports_and_flags to various non-GCC
	modules.
	* Makefile.in: Allow passing a no_exports_and_flags argument to
	"all" to suppress emitting exports and make flags. Useful when
	invoked via host_modules from Makefile.def.
	* Makefile.tpl: Regenerate.

Change-Id: I7d80328cf81c133ba6157eec7d10c422b6790723
---
 Makefile.def | 12 ++++++------
 Makefile.in  | 30 ++++++++++++------------------
 Makefile.tpl |  9 ++++++---
 3 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/Makefile.def b/Makefile.def
index 311feb9de3..e1ff065202 100644
--- a/Makefile.def
+++ b/Makefile.def
@@ -33,7 +33,7 @@ build_modules= { module= fixincludes; };
 build_modules= { module= libcpp;
                  extra_configure_flags='--disable-nls am_cv_func_iconv=no';};
 
-host_modules= { module= bfd; bootstrap=true; };
+host_modules= { module= bfd; bootstrap=true; no_exports_and_flags=true; };
 host_modules= { module= opcodes; bootstrap=true; };
 host_modules= { module= binutils; bootstrap=true; };
 host_modules= { module= bison; no_check_cross= true; };
@@ -105,15 +105,15 @@ host_modules= { module= libiconv;
 		missing= install-html;
 		missing= install-info; };
 host_modules= { module= m4; };
-host_modules= { module= readline; };
+host_modules= { module= readline; no_exports_and_flags=true; };
 host_modules= { module= sid; };
-host_modules= { module= sim; };
+host_modules= { module= sim; no_exports_and_flags=true; };
 host_modules= { module= texinfo; no_install= true; };
 host_modules= { module= zlib; no_install=true; no_check=true;
 		bootstrap=true;
 	        extra_configure_flags='@extra_host_zlib_configure_flags@';};
-host_modules= { module= gnulib; };
-host_modules= { module= gdb; };
+host_modules= { module= gnulib; no_exports_and_flags=true; };
+host_modules= { module= gdb; no_exports_and_flags=true; };
 host_modules= { module= expect; };
 host_modules= { module= guile; };
 host_modules= { module= tk; };
@@ -129,7 +129,7 @@ host_modules= { module= lto-plugin; bootstrap=true;
 		extra_make_flags='@extra_linker_plugin_flags@'; };
 host_modules= { module= libcc1; extra_configure_flags=--enable-shared; };
 host_modules= { module= gotools; };
-host_modules= { module= libctf; no_check=true;
+host_modules= { module= libctf; no_check=true; no_exports_and_flags=true;
 		bootstrap=true; };
 
 target_modules = { module= libstdc++-v3;
diff --git a/Makefile.in b/Makefile.in
index 1aabf6ede4..bd41753543 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -3414,10 +3414,9 @@ maybe-all-bfd: all-bfd
 all-bfd: configure-bfd
 	@r=`${PWD_COMMAND}`; export r; \
 	s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
-	$(HOST_EXPORTS)  \
+	 \
 	(cd $(HOST_SUBDIR)/bfd && \
-	  $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
-		$(TARGET-bfd))
+	  $(MAKE) $(TARGET-bfd))
 @endif bfd
 
 
@@ -25530,10 +25529,9 @@ all-readline: configure-readline
 	@: $(MAKE); $(unstage)
 	@r=`${PWD_COMMAND}`; export r; \
 	s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
-	$(HOST_EXPORTS)  \
+	 \
 	(cd $(HOST_SUBDIR)/readline && \
-	  $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
-		$(TARGET-readline))
+	  $(MAKE) $(TARGET-readline))
 @endif readline
 
 
@@ -26412,10 +26410,9 @@ all-sim: configure-sim
 	@: $(MAKE); $(unstage)
 	@r=`${PWD_COMMAND}`; export r; \
 	s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
-	$(HOST_EXPORTS)  \
+	 \
 	(cd $(HOST_SUBDIR)/sim && \
-	  $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
-		$(TARGET-sim))
+	  $(MAKE) $(TARGET-sim))
 @endif sim
 
 
@@ -28150,10 +28147,9 @@ all-gnulib: configure-gnulib
 	@: $(MAKE); $(unstage)
 	@r=`${PWD_COMMAND}`; export r; \
 	s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
-	$(HOST_EXPORTS)  \
+	 \
 	(cd $(HOST_SUBDIR)/gnulib && \
-	  $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
-		$(TARGET-gnulib))
+	  $(MAKE) $(TARGET-gnulib))
 @endif gnulib
 
 
@@ -28591,10 +28587,9 @@ all-gdb: configure-gdb
 	@: $(MAKE); $(unstage)
 	@r=`${PWD_COMMAND}`; export r; \
 	s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
-	$(HOST_EXPORTS)  \
+	 \
 	(cd $(HOST_SUBDIR)/gdb && \
-	  $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
-		$(TARGET-gdb))
+	  $(MAKE) $(TARGET-gdb))
 @endif gdb
 
 
@@ -33571,10 +33566,9 @@ maybe-all-libctf: all-libctf
 all-libctf: configure-libctf
 	@r=`${PWD_COMMAND}`; export r; \
 	s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
-	$(HOST_EXPORTS)  \
+	 \
 	(cd $(HOST_SUBDIR)/libctf && \
-	  $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
-		$(TARGET-libctf))
+	  $(MAKE) $(TARGET-libctf))
 @endif libctf
 
 
diff --git a/Makefile.tpl b/Makefile.tpl
index 5b118a8ba4..876ecf8dbd 100644
--- a/Makefile.tpl
+++ b/Makefile.tpl
@@ -1126,10 +1126,13 @@ all-[+prefix+][+module+]: configure-[+prefix+][+module+][+ IF bootstrap +][+ ELS
 	@: $(MAKE); $(unstage)[+ ENDIF bootstrap +]
 	@r=`${PWD_COMMAND}`; export r; \
 	s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
-	[+exports+] [+extra_exports+] \
+	[+ IF no_exports_and_flags +][+ ELSE
+	+][+exports+] [+extra_exports+][+
+	ENDIF no_exports_and_flags +] \
 	(cd [+subdir+]/[+module+] && \
-	  $(MAKE) $(BASE_FLAGS_TO_PASS) [+args+] [+stage1_args+] [+extra_make_flags+] \
-		$(TARGET-[+prefix+][+module+]))
+	  $(MAKE) [+ IF no_exports_and_flags +][+ ELSE
+	  +]$(BASE_FLAGS_TO_PASS) [+args+] [+stage1_args+] [+extra_make_flags+] \
+		[+ ENDIF no_exports_and_flags +]$(TARGET-[+prefix+][+module+]))
 @endif [+prefix+][+module+]
 
 [+ IF bootstrap +]

base-commit: 987012b89bce7f6385ed88585547f852a8005a3f
-- 
2.24.0.432.g9d3f5f5b63-goog


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

* [PATCH] add file desc to gdbserver client_state
  2019-11-22 23:30 [PATCH] Don't override various Makefile variables for gnulib et al Christian Biesinger via gdb-patches
@ 2019-11-26 18:01 ` Stan Cox
  2019-12-13 23:13   ` Tom Tromey
  2020-01-29 14:07 ` [PATCH] Don't override various Makefile variables for gnulib et al Christian Biesinger via gdb-patches
  2020-01-29 14:10 ` Christian Biesinger via gdb-patches
  2 siblings, 1 reply; 9+ messages in thread
From: Stan Cox @ 2019-11-26 18:01 UTC (permalink / raw)
  To: gdb-patches

This patch adds gdb_fildes_t support to gdbserver for later use by the 
gdbserver multi-client patch.  That patch enables multiple clients with 
each client associated with its corresponding file desc.   This patch 
adds the corresponding file desc to the client_state and adds a file 
desc parameter to the gdbserver I/O ops.  It also adds a minimal 
multi_client_states which currently just manages a single client_state 
but later will be expanded to be a collection of multiple client_states 
where each client_state is the state for an individual client.  The 
gdbserver multi-client patch is mentioned in: 
https://sourceware.org/ml/gdb-patches/2019-08/msg00577.html

gdbserver/ChangeLog:

2019-11-25  Stan Cox  <scox@redhat.com>

	* server.h (struct multi_client_states):  New.
	* server.c (g_client_states)
	(get_client_states, multi_client_sates::set_client_state):  New.
	(captured_main):  Set the client state for this remote file desc.
	* remote-utils.c: (get_remote_desc):  New to access remote file desc.
	(handle_accept_event):  Set the client state for this remote file desc.
	(write_prim, read_prim, readchar): Add gdb_fildes_t parameter.
	Change all callers.

---
  gdb/gdbserver/ChangeLog      | 11 ++++++++
  gdb/gdbserver/remote-utils.c | 55 +++++++++++++++++++++---------------
  gdb/gdbserver/remote-utils.h |  2 ++
  gdb/gdbserver/server.c       | 21 ++++++++++++++
  gdb/gdbserver/server.h       | 11 +++++++-
  5 files changed, 77 insertions(+), 23 deletions(-)

diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index d7da4b7aed..5c5a119863 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -93,7 +93,7 @@ enum {
     Either NOT_SCHEDULED or the callback id.  */
  static int readchar_callback = NOT_SCHEDULED;

-static int readchar (void);
+static int readchar (gdb_fildes_t);
  static void reset_readchar (void);
  static void reschedule (void);

@@ -115,6 +115,13 @@ static gdb_fildes_t listen_desc = INVALID_DESCRIPTOR;
  # define write(fd, buf, len) send (fd, (char *) buf, len, 0)
  #endif

+
+int
+get_remote_desc (void)
+{
+  return remote_desc;
+}
+
  int
  gdb_connected (void)
  {
@@ -156,6 +163,9 @@ handle_accept_event (int err, gdb_client_data 
client_data)
    if (remote_desc == -1)
      perror_with_name ("Accept failed");

+  struct multi_client_states & client_states = get_client_states();
+  client_states.set_client_state (remote_desc);
+
    /* Enable TCP keep alive process. */
    socklen_t tmp = 1;
    setsockopt (remote_desc, SOL_SOCKET, SO_KEEPALIVE,
@@ -607,12 +617,12 @@ read_ptid (const char *buf, const char **obuf)
     This may return less than COUNT.  */

  static int
-write_prim (const void *buf, int count)
+write_prim (gdb_fildes_t fd, const void *buf, int count)
  {
    if (remote_connection_is_stdio ())
      return write (fileno (stdout), buf, count);
    else
-    return write (remote_desc, buf, count);
+    return write (fd, buf, count);
  }

  /* Read COUNT bytes from the client and store in BUF.
@@ -620,12 +630,12 @@ write_prim (const void *buf, int count)
     This may return less than COUNT.  */

  static int
-read_prim (void *buf, int count)
+read_prim (gdb_fildes_t fd, void *buf, int count)
  {
    if (remote_connection_is_stdio ())
      return read (fileno (stdin), buf, count);
    else
-    return read (remote_desc, buf, count);
+    return read (fd, buf, count);
  }

  /* Send a packet to the remote machine, with error checking.
@@ -666,7 +676,7 @@ putpkt_binary_1 (char *buf, int cnt, int is_notif)

    do
      {
-      if (write_prim (buf2, p - buf2) != p - buf2)
+      if (write_prim (cs.file_desc, buf2, p - buf2) != p - buf2)
  	{
  	  perror ("putpkt(write)");
  	  free (buf2);
@@ -679,9 +689,9 @@ putpkt_binary_1 (char *buf, int cnt, int is_notif)
  	  if (remote_debug)
  	    {
  	      if (is_notif)
-		debug_printf ("putpkt (\"%s\"); [notif]\n", buf2);
+	  	debug_printf ("putpkt (%d, \"%s\"); [notif]\n", (int)cs.file_desc, 
buf2);
  	      else
-		debug_printf ("putpkt (\"%s\"); [noack mode]\n", buf2);
+		debug_printf ("putpkt (%d, \"%s\"); [noack mode]\n", 
(int)cs.file_desc, buf2);
  	      debug_flush ();
  	    }
  	  break;
@@ -689,11 +699,11 @@ putpkt_binary_1 (char *buf, int cnt, int is_notif)

        if (remote_debug)
  	{
-	  debug_printf ("putpkt (\"%s\"); [looking for ack]\n", buf2);
+	  debug_printf ("putpkt (%d, \"%s\"); [looking for ack]\n", 
(int)cs.file_desc, buf2);
  	  debug_flush ();
  	}

-      cc = readchar ();
+      cc = readchar (cs.file_desc);

        if (cc < 0)
  	{
@@ -761,7 +771,7 @@ input_interrupt (int unused)
        int cc;
        char c = 0;

-      cc = read_prim (&c, 1);
+      cc = read_prim (remote_desc, &c, 1);

        if (cc == 0)
  	{
@@ -891,13 +901,13 @@ static unsigned char *readchar_bufp;
  /* Returns next char from remote GDB.  -1 if error.  */

  static int
-readchar (void)
+readchar (gdb_fildes_t fd)
  {
    int ch;

    if (readchar_bufcnt == 0)
      {
-      readchar_bufcnt = read_prim (readchar_buf, sizeof (readchar_buf));
+      readchar_bufcnt = read_prim (fd, readchar_buf, sizeof 
(readchar_buf));

        if (readchar_bufcnt <= 0)
  	{
@@ -972,6 +982,7 @@ getpkt (char *buf)
    char *bp;
    unsigned char csum, c1, c2;
    int c;
+  gdb_fildes_t fd = cs.file_desc;

    while (1)
      {
@@ -979,7 +990,7 @@ getpkt (char *buf)

        while (1)
  	{
-	  c = readchar ();
+	  c = readchar (fd);

  	  /* The '\003' may appear before or after each packet, so
  	     check for an input interrupt.  */
@@ -1004,7 +1015,7 @@ getpkt (char *buf)
        bp = buf;
        while (1)
  	{
-	  c = readchar ();
+	  c = readchar (fd);
  	  if (c < 0)
  	    return -1;
  	  if (c == '#')
@@ -1014,8 +1025,8 @@ getpkt (char *buf)
  	}
        *bp = 0;

-      c1 = fromhex (readchar ());
-      c2 = fromhex (readchar ());
+      c1 = fromhex (readchar (fd));
+      c2 = fromhex (readchar (fd));

        if (csum == (c1 << 4) + c2)
  	break;
@@ -1032,7 +1043,7 @@ getpkt (char *buf)

        fprintf (stderr, "Bad checksum, sentsum=0x%x, csum=0x%x, buf=%s\n",
  	       (c1 << 4) + c2, csum, buf);
-      if (write_prim ("-", 1) != 1)
+      if (write_prim (fd, "-", 1) != 1)
  	return -1;
      }

@@ -1040,11 +1051,11 @@ getpkt (char *buf)
      {
        if (remote_debug)
  	{
-	  debug_printf ("getpkt (\"%s\");  [sending ack] \n", buf);
+	  debug_printf ("getpkt (%d, \"%s\");  [sending ack] \n", (int)fd, buf);
  	  debug_flush ();
  	}

-      if (write_prim ("+", 1) != 1)
+      if (write_prim (fd, "+", 1) != 1)
  	return -1;

        if (remote_debug)
@@ -1057,7 +1068,7 @@ getpkt (char *buf)
      {
        if (remote_debug)
  	{
-	  debug_printf ("getpkt (\"%s\");  [no ack sent] \n", buf);
+	  debug_printf ("getpkt (%d, \"%s\");  [no ack sent] \n", (int)fd, buf);
  	  debug_flush ();
  	}
      }
@@ -1074,7 +1085,7 @@ getpkt (char *buf)
    while (readchar_bufcnt > 0 && *readchar_bufp == '\003')
      {
        /* Consume the interrupt character in the buffer.  */
-      readchar ();
+      readchar (fd);
        (*the_target->request_interrupt) ();
      }

diff --git a/gdb/gdbserver/remote-utils.h b/gdb/gdbserver/remote-utils.h
index 4ca5d9435e..606592852c 100644
--- a/gdb/gdbserver/remote-utils.h
+++ b/gdb/gdbserver/remote-utils.h
@@ -19,6 +19,8 @@
  #ifndef GDBSERVER_REMOTE_UTILS_H
  #define GDBSERVER_REMOTE_UTILS_H

+int get_remote_desc (void);
+
  int gdb_connected (void);

  #define STDIO_CONNECTION_NAME "stdio"
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index c5f7176cff..a686038af7 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -152,6 +152,23 @@ static struct btrace_config current_btrace_conf;

  /* The client remote protocol state. */

+static struct multi_client_states g_client_states;
+
+multi_client_states&
+get_client_states (void)
+{
+  return g_client_states;
+}
+
+client_state&
+multi_client_states::set_client_state (gdb_fildes_t fd)
+{
+  client_state &cs = get_client_state ();
+
+  cs.file_desc = fd;
+  return cs;
+}
+
  static client_state g_client_state;

  client_state &
@@ -3556,6 +3573,10 @@ captured_main (int argc, char *argv[])
  #endif

    current_directory = getcwd (NULL, 0);
+
+  int remote_desc = get_remote_desc();
+  multi_client_states &client_states = get_client_states ();
+  client_states.set_client_state (remote_desc);
    client_state &cs = get_client_state ();

    if (current_directory == NULL)
diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
index e01c4f146e..52fae32e3e 100644
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -142,6 +142,8 @@ struct client_state
      own_buf ((char *) xmalloc (PBUFSIZ + 1))
    {}

+  gdb_fildes_t file_desc;
+
    /* The thread set with an `Hc' packet.  `Hc' is deprecated in favor of
       `vCont'.  Note the multi-process extensions made `vCont' a
       requirement, so `Hc pPID.TID' is pretty much undefined.  So
@@ -200,7 +202,14 @@ struct client_state

  };

-client_state &get_client_state ();
+struct multi_client_states
+{
+public:
+  client_state & set_client_state (gdb_fildes_t);
+};
+
+client_state & get_client_state ();
+struct multi_client_states & get_client_states (void);

  #include "gdbthread.h"
  #include "inferiors.h"
-- 
2.21.0


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

* Re: [PATCH] add file desc to gdbserver client_state
  2019-11-26 18:01 ` [PATCH] add file desc to gdbserver client_state Stan Cox
@ 2019-12-13 23:13   ` Tom Tromey
  2019-12-24  4:42     ` Stan Cox
  0 siblings, 1 reply; 9+ messages in thread
From: Tom Tromey @ 2019-12-13 23:13 UTC (permalink / raw)
  To: Stan Cox; +Cc: gdb-patches

>>>>> "Stan" == Stan Cox <scox@redhat.com> writes:

Stan> This patch adds gdb_fildes_t support to gdbserver for later use by the
Stan> gdbserver multi-client patch.  That patch enables multiple clients
Stan> with each client associated with its corresponding file desc.   This
Stan> patch adds the corresponding file desc to the client_state and adds a
Stan> file desc parameter to the gdbserver I/O ops.  It also adds a minimal
Stan> multi_client_states which currently just manages a single client_state
Stan> but later will be expanded to be a collection of multiple
Stan> client_states where each client_state is the state for an individual
Stan> client.  The gdbserver multi-client patch is mentioned in:
Stan> https://sourceware.org/ml/gdb-patches/2019-08/msg00577.html

Thanks.

This looks like a reasonable first step.  Hopefully a few things will
change in coming patches?  I'll note some below.  Also, I think in
general it needs a few more comments and some formatting cleanup.

Stan> +
Stan> +int
Stan> +get_remote_desc (void)
Stan> +{

In gdb new functions need a comment -- usually a description in the
header and then something like "see mumble.h" at the implementation.

Will remote_desc be going away?  And hopefully get_remote_desc is also
just a stepping stone?

Stan> +  struct multi_client_states & client_states = get_client_states();

Should be "&client_states", without a space.

Stan> +multi_client_states&

"multi_client_states &".

Stan> +get_client_states (void)

No more "(void)" now that we're using C++.  I think there's a few spots
for this.

Stan> +client_state&

"client_state &".

Stan> +multi_client_states::set_client_state (gdb_fildes_t fd)
Stan> +{
Stan> +  client_state &cs = get_client_state ();

Can get_client_state be made static now?
If not, why not?

Stan> +  int remote_desc = get_remote_desc();

"get_remote_desc ()".  But why does this use int while other spots use
gdb_fildes_t?

Stan> --- a/gdb/gdbserver/server.h
Stan> +++ b/gdb/gdbserver/server.h
Stan> @@ -142,6 +142,8 @@ struct client_state
Stan>      own_buf ((char *) xmalloc (PBUFSIZ + 1))
Stan>    {}

Stan> +  gdb_fildes_t file_desc;

Needs a comment.

Stan> -client_state &get_client_state ();
Stan> +struct multi_client_states
Stan> +{
Stan> +public:

Needs a comment.
Probably should say "class" and not "struct", particularly since it's
using "public:".

Stan> +  client_state & set_client_state (gdb_fildes_t);

"&set_client_state".  Needs a comment.

Stan> +client_state & get_client_state ();
Stan> +struct multi_client_states & get_client_states (void);

Extra spaces and lack of comments here too.

Tom

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

* Re: [PATCH] add file desc to gdbserver client_state
  2019-12-13 23:13   ` Tom Tromey
@ 2019-12-24  4:42     ` Stan Cox
  2020-01-24 18:21       ` Tom Tromey
  2020-02-06 19:41       ` Stan Cox
  0 siblings, 2 replies; 9+ messages in thread
From: Stan Cox @ 2019-12-24  4:42 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

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

This patch adds gdb_fildes_t support to gdbserver for later use by the
gdbserver multi-client patch.  That patch enables multiple clients
with each client associated with its corresponding file desc.   This
patch adds the corresponding file desc to the client_state and adds a
file desc parameter to the gdbserver I/O ops.  It also adds a minimal
multi_client_states which currently just manages a single client_state
but later will be expanded to be a collection of multiple
client_states where each client_state is the state for an individual
client.  In contrast to the previous patch, this patch
  1) uses client_state.remote_desc in remote-utils.c instead of the 
static version
  2) no longer uses the function get_client_state but instead defines 
get_client_state
     using multi_client_states
  The gdbserver multi-client patch is mentioned in:
  https://sourceware.org/ml/gdb-patches/2019-08/msg00577.html


> Will remote_desc be going away?  And hopefully get_remote_desc is also
> just a stepping stone?

remote_desc is now gone as mentioned in 1 above


> Can get_client_state be made static now?
> If not, why not?

get_client_state is now redefined as mentioned in 2) above

Thanks Tom!



[-- Attachment #2: gdbs-fd.patch --]
[-- Type: text/x-patch, Size: 13185 bytes --]

commit bac0aa21ec (HEAD -> master)
Author: Stan Cox <scox@redhat.com>
Date:   Fri Dec 20 15:54:22 2019 -0500

    Add struct multi_client_states
    
    * remote-utils.c (remote_desc):  Move to struct client_state
      (gdb_connected, handle_accept_event, remote_open, putpkt_binary_1)
      (putpkt_notif, input_interrupt, block_unblock_async_io)
      (nto_conctrl, getpkt): Change remote_desc reference.
    * server.c (captured_main): Move remote_desc initialization to
      remote_prepare.
---
 gdb/gdbserver/ChangeLog      | 12 ++++++++++++
 gdb/gdbserver/remote-utils.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------
 gdb/gdbserver/remote-utils.h |  2 ++
 gdb/gdbserver/server.c       | 15 ++++++++++++++-
 gdb/gdbserver/server.h       | 25 ++++++++++++++++++++++++-
 5 files changed, 100 insertions(+), 35 deletions(-)

diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 02414385b7..bf43b232f5 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,15 @@
+2019-12-23  Stan Cox  <scox@redhat.com>
+
+	* server.h (struct multi_client_states):  New.
+	* server.c (g_client_states)
+	(get_client_states, multi_client_sates::set_client_state):  New.
+	(captured_main):  Set the initial multi_client_states
+	* remote-utils.c (gdb_connected, remote_prepare, remote_close)
+	(putpkt_binary_1, getpkt)  Use client_state.remote_desc.
+	(handle_accept_event):  Set the client state for this remote file desc.
+	(write_prim, read_prim, readchar): Add gdb_fildes_t parameter.
+	Change all callers.
+
 2019-11-22  Tom Tromey  <tromey@adacore.com>
 
 	* gdb.ada/tasks.exp: Add -ada-task-info regression test.
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index d7da4b7aed..09095d9bbc 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -25,6 +25,7 @@
 #include "tdesc.h"
 #include "debug.h"
 #include "dll.h"
+#include "server.h"
 #include "gdbsupport/rsp-low.h"
 #include "gdbsupport/netstuff.h"
 #include "gdbsupport/filestuff.h"
@@ -93,7 +94,7 @@ enum {
    Either NOT_SCHEDULED or the callback id.  */
 static int readchar_callback = NOT_SCHEDULED;
 
-static int readchar (void);
+static int readchar (gdb_fildes_t);
 static void reset_readchar (void);
 static void reschedule (void);
 
@@ -107,7 +108,6 @@ struct sym_cache
 
 static int remote_is_stdio = 0;
 
-static gdb_fildes_t remote_desc = INVALID_DESCRIPTOR;
 static gdb_fildes_t listen_desc = INVALID_DESCRIPTOR;
 
 #ifdef USE_WIN32API
@@ -115,10 +115,12 @@ static gdb_fildes_t listen_desc = INVALID_DESCRIPTOR;
 # define write(fd, buf, len) send (fd, (char *) buf, len, 0)
 #endif
 
+
 int
 gdb_connected (void)
 {
-  return remote_desc != INVALID_DESCRIPTOR;
+  client_state &cs = get_client_state ();
+  return cs.remote_desc != INVALID_DESCRIPTOR;
 }
 
 /* Return true if the remote connection is over stdio.  */
@@ -148,6 +150,7 @@ handle_accept_event (int err, gdb_client_data client_data)
 {
   struct sockaddr_storage sockaddr;
   socklen_t len = sizeof (sockaddr);
+  gdb_fildes_t remote_desc;
 
   if (debug_threads)
     debug_printf ("handling possible accept event\n");
@@ -156,6 +159,9 @@ handle_accept_event (int err, gdb_client_data client_data)
   if (remote_desc == -1)
     perror_with_name ("Accept failed");
 
+  struct multi_client_states &client_states = get_client_states();
+  client_states.set_client_state (remote_desc);
+
   /* Enable TCP keep alive process. */
   socklen_t tmp = 1;
   setsockopt (remote_desc, SOL_SOCKET, SO_KEEPALIVE,
@@ -268,6 +274,8 @@ remote_prepare (const char *name)
     }
 #endif
 
+  cs.remote_desc = INVALID_DESCRIPTOR;
+
   int r = getaddrinfo (parsed.host_str.c_str (), parsed.port_str.c_str (),
 		       &hint, &ainfo);
 
@@ -324,6 +332,7 @@ void
 remote_open (const char *name)
 {
   const char *port_str;
+  gdb_fildes_t remote_desc;
 
   port_str = strchr (name, ':');
 #ifdef USE_WIN32API
@@ -416,17 +425,19 @@ remote_open (const char *name)
 void
 remote_close (void)
 {
-  delete_file_handler (remote_desc);
+  client_state &cs = get_client_state ();
+
+  delete_file_handler (cs.remote_desc);
 
   disable_async_io ();
 
 #ifdef USE_WIN32API
-  closesocket (remote_desc);
+  closesocket (cs.remote_desc);
 #else
   if (! remote_connection_is_stdio ())
-    close (remote_desc);
+    close (cs.remote_desc);
 #endif
-  remote_desc = INVALID_DESCRIPTOR;
+  cs.remote_desc = INVALID_DESCRIPTOR;
 
   reset_readchar ();
 }
@@ -607,12 +618,12 @@ read_ptid (const char *buf, const char **obuf)
    This may return less than COUNT.  */
 
 static int
-write_prim (const void *buf, int count)
+write_prim (gdb_fildes_t fd, const void *buf, int count)
 {
   if (remote_connection_is_stdio ())
     return write (fileno (stdout), buf, count);
   else
-    return write (remote_desc, buf, count);
+    return write (fd, buf, count);
 }
 
 /* Read COUNT bytes from the client and store in BUF.
@@ -620,12 +631,12 @@ write_prim (const void *buf, int count)
    This may return less than COUNT.  */
 
 static int
-read_prim (void *buf, int count)
+read_prim (gdb_fildes_t fd, void *buf, int count)
 {
   if (remote_connection_is_stdio ())
     return read (fileno (stdin), buf, count);
   else
-    return read (remote_desc, buf, count);
+    return read (fd, buf, count);
 }
 
 /* Send a packet to the remote machine, with error checking.
@@ -666,7 +677,7 @@ putpkt_binary_1 (char *buf, int cnt, int is_notif)
 
   do
     {
-      if (write_prim (buf2, p - buf2) != p - buf2)
+      if (write_prim (cs.remote_desc, buf2, p - buf2) != p - buf2)
 	{
 	  perror ("putpkt(write)");
 	  free (buf2);
@@ -679,9 +690,9 @@ putpkt_binary_1 (char *buf, int cnt, int is_notif)
 	  if (remote_debug)
 	    {
 	      if (is_notif)
-		debug_printf ("putpkt (\"%s\"); [notif]\n", buf2);
+	  	debug_printf ("putpkt (%d, \"%s\"); [notif]\n", cs.remote_desc, buf2);
 	      else
-		debug_printf ("putpkt (\"%s\"); [noack mode]\n", buf2);
+		debug_printf ("putpkt (%d, \"%s\"); [noack mode]\n", cs.remote_desc, buf2);
 	      debug_flush ();
 	    }
 	  break;
@@ -689,11 +700,11 @@ putpkt_binary_1 (char *buf, int cnt, int is_notif)
 
       if (remote_debug)
 	{
-	  debug_printf ("putpkt (\"%s\"); [looking for ack]\n", buf2);
+	  debug_printf ("putpkt (%d, \"%s\"); [looking for ack]\n", cs.remote_desc, buf2);
 	  debug_flush ();
 	}
 
-      cc = readchar ();
+      cc = readchar (cs.remote_desc);
 
       if (cc < 0)
 	{
@@ -748,6 +759,7 @@ putpkt_notif (char *buf)
 static void
 input_interrupt (int unused)
 {
+  client_state &cs = get_client_state ();
   fd_set readset;
   struct timeval immediate = { 0, 0 };
 
@@ -755,13 +767,13 @@ input_interrupt (int unused)
      be a problem under NetBSD 1.4 and 1.5.  */
 
   FD_ZERO (&readset);
-  FD_SET (remote_desc, &readset);
-  if (select (remote_desc + 1, &readset, 0, 0, &immediate) > 0)
+  FD_SET (cs.remote_desc, &readset);
+  if (select (cs.remote_desc + 1, &readset, 0, 0, &immediate) > 0)
     {
       int cc;
       char c = 0;
 
-      cc = read_prim (&c, 1);
+      cc = read_prim (cs.remote_desc, &c, 1);
 
       if (cc == 0)
 	{
@@ -786,10 +798,11 @@ input_interrupt (int unused)
 void
 check_remote_input_interrupt_request (void)
 {
+  client_state &cs = get_client_state ();
   /* This function may be called before establishing communications,
      therefore we need to validate the remote descriptor.  */
 
-  if (remote_desc == INVALID_DESCRIPTOR)
+  if (cs.remote_desc == INVALID_DESCRIPTOR)
     return;
 
   input_interrupt (0);
@@ -815,6 +828,7 @@ block_unblock_async_io (int block)
 static void
 nto_comctrl (int enable)
 {
+  client_state &cs = get_client_state ();
   struct sigevent event;
 
   if (enable)
@@ -824,11 +838,11 @@ nto_comctrl (int enable)
       event.sigev_code = 0;
       event.sigev_value.sival_ptr = NULL;
       event.sigev_priority = -1;
-      ionotify (remote_desc, _NOTIFY_ACTION_POLLARM, _NOTIFY_COND_INPUT,
+      ionotify (cs.remote_desc, _NOTIFY_ACTION_POLLARM, _NOTIFY_COND_INPUT,
 		&event);
     }
   else
-    ionotify (remote_desc, _NOTIFY_ACTION_POLL, _NOTIFY_COND_INPUT, NULL);
+    ionotify (cs.remote_desc, _NOTIFY_ACTION_POLL, _NOTIFY_COND_INPUT, NULL);
 }
 #endif /* __QNX__ */
 
@@ -891,13 +905,13 @@ static unsigned char *readchar_bufp;
 /* Returns next char from remote GDB.  -1 if error.  */
 
 static int
-readchar (void)
+readchar (gdb_fildes_t fd)
 {
   int ch;
 
   if (readchar_bufcnt == 0)
     {
-      readchar_bufcnt = read_prim (readchar_buf, sizeof (readchar_buf));
+      readchar_bufcnt = read_prim (fd, readchar_buf, sizeof (readchar_buf));
 
       if (readchar_bufcnt <= 0)
 	{
@@ -972,6 +986,7 @@ getpkt (char *buf)
   char *bp;
   unsigned char csum, c1, c2;
   int c;
+  gdb_fildes_t fd = cs.remote_desc;
 
   while (1)
     {
@@ -979,7 +994,7 @@ getpkt (char *buf)
 
       while (1)
 	{
-	  c = readchar ();
+	  c = readchar (fd);
 
 	  /* The '\003' may appear before or after each packet, so
 	     check for an input interrupt.  */
@@ -1004,7 +1019,7 @@ getpkt (char *buf)
       bp = buf;
       while (1)
 	{
-	  c = readchar ();
+	  c = readchar (fd);
 	  if (c < 0)
 	    return -1;
 	  if (c == '#')
@@ -1014,8 +1029,8 @@ getpkt (char *buf)
 	}
       *bp = 0;
 
-      c1 = fromhex (readchar ());
-      c2 = fromhex (readchar ());
+      c1 = fromhex (readchar (fd));
+      c2 = fromhex (readchar (fd));
 
       if (csum == (c1 << 4) + c2)
 	break;
@@ -1032,7 +1047,7 @@ getpkt (char *buf)
 
       fprintf (stderr, "Bad checksum, sentsum=0x%x, csum=0x%x, buf=%s\n",
 	       (c1 << 4) + c2, csum, buf);
-      if (write_prim ("-", 1) != 1)
+      if (write_prim (fd, "-", 1) != 1)
 	return -1;
     }
 
@@ -1040,11 +1055,11 @@ getpkt (char *buf)
     {
       if (remote_debug)
 	{
-	  debug_printf ("getpkt (\"%s\");  [sending ack] \n", buf);
+	  debug_printf ("getpkt (%d, \"%s\");  [sending ack] \n", fd, buf);
 	  debug_flush ();
 	}
 
-      if (write_prim ("+", 1) != 1)
+      if (write_prim (fd, "+", 1) != 1)
 	return -1;
 
       if (remote_debug)
@@ -1057,7 +1072,7 @@ getpkt (char *buf)
     {
       if (remote_debug)
 	{
-	  debug_printf ("getpkt (\"%s\");  [no ack sent] \n", buf);
+	  debug_printf ("getpkt (%d, \"%s\");  [no ack sent] \n", fd, buf);
 	  debug_flush ();
 	}
     }
@@ -1074,7 +1089,7 @@ getpkt (char *buf)
   while (readchar_bufcnt > 0 && *readchar_bufp == '\003')
     {
       /* Consume the interrupt character in the buffer.  */
-      readchar ();
+      readchar (fd);
       (*the_target->request_interrupt) ();
     }
 
diff --git a/gdb/gdbserver/remote-utils.h b/gdb/gdbserver/remote-utils.h
index 4ca5d9435e..606592852c 100644
--- a/gdb/gdbserver/remote-utils.h
+++ b/gdb/gdbserver/remote-utils.h
@@ -19,6 +19,8 @@
 #ifndef GDBSERVER_REMOTE_UTILS_H
 #define GDBSERVER_REMOTE_UTILS_H
 
+int get_remote_desc (void);
+
 int gdb_connected (void);
 
 #define STDIO_CONNECTION_NAME "stdio"
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index c5f7176cff..40b8ea1b80 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -153,11 +153,21 @@ static struct btrace_config current_btrace_conf;
 /* The client remote protocol state. */
 
 static client_state g_client_state;
+static struct multi_client_states g_client_states;
+
+multi_client_states &
+get_client_states ()
+{
+  return g_client_states;
+}
 
 client_state &
-get_client_state ()
+multi_client_states::set_client_state (gdb_fildes_t fd)
 {
   client_state &cs = g_client_state;
+  
+  cs.remote_desc = fd;
+  get_client_states().set_current_client (cs);
   return cs;
 }
 
@@ -3556,6 +3566,9 @@ captured_main (int argc, char *argv[])
 #endif
 
   current_directory = getcwd (NULL, 0);
+
+  multi_client_states &client_states = get_client_states ();
+  client_states.set_client_state (-1);
   client_state &cs = get_client_state ();
 
   if (current_directory == NULL)
diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
index e01c4f146e..027adbfe63 100644
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -142,6 +142,8 @@ struct client_state
     own_buf ((char *) xmalloc (PBUFSIZ + 1)) 
   {}
 
+  gdb_fildes_t remote_desc;
+
   /* The thread set with an `Hc' packet.  `Hc' is deprecated in favor of
      `vCont'.  Note the multi-process extensions made `vCont' a
      requirement, so `Hc pPID.TID' is pretty much undefined.  So
@@ -200,7 +202,28 @@ struct client_state
 
 };
 
-client_state &get_client_state ();
+/* Container of client remote protocol states for all the currently
+   connected clients.  */
+
+#define get_client_state() get_client_states().get_current_client()
+
+class multi_client_states
+{
+private:
+  client_state *current_cs;
+
+public:
+  /* Return the current client we are focused on. */
+  client_state &get_current_client () { return *current_cs; }
+
+  /* Set the current client we wish to focus on. */
+  void set_current_client (client_state &cs) { current_cs = &cs; }
+
+  /* Set, or create if necessary, the current client we wish to focus on */
+  client_state &set_client_state (gdb_fildes_t);
+};
+
+struct multi_client_states &get_client_states ();
 
 #include "gdbthread.h"
 #include "inferiors.h"

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

* Re: [PATCH] add file desc to gdbserver client_state
  2019-12-24  4:42     ` Stan Cox
@ 2020-01-24 18:21       ` Tom Tromey
  2020-02-06 19:41       ` Stan Cox
  1 sibling, 0 replies; 9+ messages in thread
From: Tom Tromey @ 2020-01-24 18:21 UTC (permalink / raw)
  To: Stan Cox; +Cc: Tom Tromey, gdb-patches

>>>>> "Stan" == Stan Cox <scox@redhat.com> writes:

Stan>     Add struct multi_client_states

Stan>     * remote-utils.c (remote_desc):  Move to struct client_state
Stan>       (gdb_connected, handle_accept_event, remote_open, putpkt_binary_1)
Stan>       (putpkt_notif, input_interrupt, block_unblock_async_io)
Stan>       (nto_conctrl, getpkt): Change remote_desc reference.
Stan>     * server.c (captured_main): Move remote_desc initialization to
Stan>       remote_prepare.

Thanks for the patch.

Stan> diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
Stan> index d7da4b7aed..09095d9bbc 100644
Stan> --- a/gdb/gdbserver/remote-utils.c
Stan> +++ b/gdb/gdbserver/remote-utils.c
Stan> @@ -25,6 +25,7 @@
Stan>  #include "tdesc.h"
Stan>  #include "debug.h"
Stan>  #include "dll.h"
Stan> +#include "server.h"

I think this is already included near the top.
Normally, .c files in gdbserver include this first.

Stan> +
Stan>  int
Stan>  gdb_connected (void)

Spurious newline addition.

Stan> +  struct multi_client_states &client_states = get_client_states();

Needs a space before the "(".

Stan> +  get_client_states().set_current_client (cs);

Ditto.

Stan> +/* Container of client remote protocol states for all the currently
Stan> +   connected clients.  */
Stan> +
Stan> +#define get_client_state() get_client_states().get_current_client()
Stan> +
Stan> +class multi_client_states
Stan> +{
Stan> +private:
Stan> +  client_state *current_cs;
Stan> +
Stan> +public:
Stan> +  /* Return the current client we are focused on. */
Stan> +  client_state &get_current_client () { return *current_cs; }
Stan> +
Stan> +  /* Set the current client we wish to focus on. */
Stan> +  void set_current_client (client_state &cs) { current_cs = &cs; }

I think this class is a bit strange.
It stores a pointer but the set_ method takes a reference.

If this class is going to own multiple clients in the future, then I
think it makes sense to design the API that way from the start.

thanks,
Tom

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

* Re: [PATCH] add file desc to gdbserver client_state
  2019-12-24  4:42     ` Stan Cox
  2020-01-24 18:21       ` Tom Tromey
@ 2020-02-06 19:41       ` Stan Cox
  1 sibling, 0 replies; 9+ messages in thread
From: Stan Cox @ 2020-02-06 19:41 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

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

This patch adds struct multi_client_states for managing multiple clients
and adds gdb_fildes_t support to routines write_prim, read_prim, readchar
for handling client specific io,with each client associated with its
corresponding file desc.   Thispatch adds the corresponding file desc to
the client_state and adds afile desc parameter to the gdbserver I/O ops.
It also adds structmulti_client_states which currently just manages a 
primary
client_statebut will later allow managing  a collection of client_states
where each client_state is the state for an individualclient.  This patch
defines get_client_state using multi_client_statesdefines.  Concurrent with
this patch is an strace patch that adds a gdbserver backend to strace.

Add struct multi_client_states

* server.h (struct multi_client_states):  New.
* server.c (g_client_states)
   (get_client_states, multi_client_sates::add_client_state):  New.
   (captured_main):  Set multi_client_states.client_states
* remote-utils.c (gdb_connected, remote_prepare, remote_close)
   (putpkt_binary_1, getpkt)  Use client_state.remote_desc.
   (handle_accept_event):  Set the client state for this remote file  desc.
   (write_prim, read_prim, readchar): Add gdb_fildes_t parameter.
    Change all callers.


[-- Attachment #2: gdbs-mcs.patch --]
[-- Type: text/x-patch, Size: 12718 bytes --]

    Add struct multi_client_states
    
    * server.h (struct multi_client_states):  New.
    * server.c (g_client_states)
      (get_client_states, multi_client_sates::add_client_state):  New.
      (captured_main):  Set multi_client_states.client_states
    * remote-utils.c (gdb_connected, remote_prepare, remote_close)
      (putpkt_binary_1, getpkt)  Use client_state.remote_desc.
      (handle_accept_event):  Set the client state for this remote file desc.
      (write_prim, read_prim, readchar): Add gdb_fildes_t parameter.
      Change all callers.
---
 gdb/gdbserver/ChangeLog      | 12 ++++++++++++
 gdb/gdbserver/remote-utils.c | 78 +++++++++++++++++++++++++++++++++++++++++++++---------------------------------
 gdb/gdbserver/server.c       | 28 +++++++++++++++++++++++-----
 gdb/gdbserver/server.h       | 31 +++++++++++++++++++++++++++++--
 4 files changed, 109 insertions(+), 40 deletions(-)

diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index b8a8c6576f9..63d3cad9597 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -94,7 +94,7 @@ enum {
    Either NOT_SCHEDULED or the callback id.  */
 static int readchar_callback = NOT_SCHEDULED;
 
-static int readchar (void);
+static int readchar (gdb_fildes_t);
 static void reset_readchar (void);
 static void reschedule (void);
 
@@ -108,7 +108,6 @@ struct sym_cache
 
 static int remote_is_stdio = 0;
 
-static gdb_fildes_t remote_desc = INVALID_DESCRIPTOR;
 static gdb_fildes_t listen_desc = INVALID_DESCRIPTOR;
 
 #ifdef USE_WIN32API
@@ -119,7 +118,8 @@ static gdb_fildes_t listen_desc = INVALID_DESCRIPTOR;
 int
 gdb_connected (void)
 {
-  return remote_desc != INVALID_DESCRIPTOR;
+  client_state &cs = get_client_state ();
+  return cs.remote_desc != INVALID_DESCRIPTOR;
 }
 
 /* Return true if the remote connection is over stdio.  */
@@ -149,6 +149,7 @@ handle_accept_event (int err, gdb_client_data client_data)
 {
   struct sockaddr_storage sockaddr;
   socklen_t len = sizeof (sockaddr);
+  gdb_fildes_t remote_desc;
 
   if (debug_threads)
     debug_printf ("handling possible accept event\n");
@@ -157,6 +158,8 @@ handle_accept_event (int err, gdb_client_data client_data)
   if (remote_desc == -1)
     perror_with_name ("Accept failed");
 
+  get_client_states ().add_client_state (remote_desc);
+
   /* Enable TCP keep alive process. */
   socklen_t tmp = 1;
   setsockopt (remote_desc, SOL_SOCKET, SO_KEEPALIVE,
@@ -269,6 +272,8 @@ remote_prepare (const char *name)
     }
 #endif
 
+  cs.remote_desc = INVALID_DESCRIPTOR;
+
   int r = getaddrinfo (parsed.host_str.c_str (), parsed.port_str.c_str (),
 		       &hint, &ainfo);
 
@@ -325,6 +330,7 @@ void
 remote_open (const char *name)
 {
   const char *port_str;
+  gdb_fildes_t remote_desc;
 
   port_str = strchr (name, ':');
 #ifdef USE_WIN32API
@@ -417,17 +423,19 @@ remote_open (const char *name)
 void
 remote_close (void)
 {
-  delete_file_handler (remote_desc);
+  client_state &cs = get_client_state ();
+
+  delete_file_handler (cs.remote_desc);
 
   disable_async_io ();
 
 #ifdef USE_WIN32API
-  closesocket (remote_desc);
+  closesocket (cs.remote_desc);
 #else
   if (! remote_connection_is_stdio ())
-    close (remote_desc);
+    close (cs.remote_desc);
 #endif
-  remote_desc = INVALID_DESCRIPTOR;
+  cs.remote_desc = INVALID_DESCRIPTOR;
 
   reset_readchar ();
 }
@@ -608,12 +616,12 @@ read_ptid (const char *buf, const char **obuf)
    This may return less than COUNT.  */
 
 static int
-write_prim (const void *buf, int count)
+write_prim (gdb_fildes_t fd, const void *buf, int count)
 {
   if (remote_connection_is_stdio ())
     return write (fileno (stdout), buf, count);
   else
-    return write (remote_desc, buf, count);
+    return write (fd, buf, count);
 }
 
 /* Read COUNT bytes from the client and store in BUF.
@@ -621,12 +629,12 @@ write_prim (const void *buf, int count)
    This may return less than COUNT.  */
 
 static int
-read_prim (void *buf, int count)
+read_prim (gdb_fildes_t fd, void *buf, int count)
 {
   if (remote_connection_is_stdio ())
     return read (fileno (stdin), buf, count);
   else
-    return read (remote_desc, buf, count);
+    return read (fd, buf, count);
 }
 
 /* Send a packet to the remote machine, with error checking.
@@ -667,7 +675,7 @@ putpkt_binary_1 (char *buf, int cnt, int is_notif)
 
   do
     {
-      if (write_prim (buf2, p - buf2) != p - buf2)
+      if (write_prim (cs.remote_desc, buf2, p - buf2) != p - buf2)
 	{
 	  perror ("putpkt(write)");
 	  free (buf2);
@@ -680,9 +688,9 @@ putpkt_binary_1 (char *buf, int cnt, int is_notif)
 	  if (remote_debug)
 	    {
 	      if (is_notif)
-		debug_printf ("putpkt (\"%s\"); [notif]\n", buf2);
+	  	debug_printf ("putpkt (%d, \"%s\"); [notif]\n", cs.remote_desc, buf2);
 	      else
-		debug_printf ("putpkt (\"%s\"); [noack mode]\n", buf2);
+		debug_printf ("putpkt (%d, \"%s\"); [noack mode]\n", cs.remote_desc, buf2);
 	      debug_flush ();
 	    }
 	  break;
@@ -690,11 +698,11 @@ putpkt_binary_1 (char *buf, int cnt, int is_notif)
 
       if (remote_debug)
 	{
-	  debug_printf ("putpkt (\"%s\"); [looking for ack]\n", buf2);
+	  debug_printf ("putpkt (%d, \"%s\"); [looking for ack]\n", cs.remote_desc, buf2);
 	  debug_flush ();
 	}
 
-      cc = readchar ();
+      cc = readchar (cs.remote_desc);
 
       if (cc < 0)
 	{
@@ -749,6 +757,7 @@ putpkt_notif (char *buf)
 static void
 input_interrupt (int unused)
 {
+  client_state &cs = get_client_state ();
   fd_set readset;
   struct timeval immediate = { 0, 0 };
 
@@ -756,13 +765,13 @@ input_interrupt (int unused)
      be a problem under NetBSD 1.4 and 1.5.  */
 
   FD_ZERO (&readset);
-  FD_SET (remote_desc, &readset);
-  if (select (remote_desc + 1, &readset, 0, 0, &immediate) > 0)
+  FD_SET (cs.remote_desc, &readset);
+  if (select (cs.remote_desc + 1, &readset, 0, 0, &immediate) > 0)
     {
       int cc;
       char c = 0;
 
-      cc = read_prim (&c, 1);
+      cc = read_prim (cs.remote_desc, &c, 1);
 
       if (cc == 0)
 	{
@@ -787,10 +796,11 @@ input_interrupt (int unused)
 void
 check_remote_input_interrupt_request (void)
 {
+  client_state &cs = get_client_state ();
   /* This function may be called before establishing communications,
      therefore we need to validate the remote descriptor.  */
 
-  if (remote_desc == INVALID_DESCRIPTOR)
+  if (cs.remote_desc == INVALID_DESCRIPTOR)
     return;
 
   input_interrupt (0);
@@ -816,6 +826,7 @@ block_unblock_async_io (int block)
 static void
 nto_comctrl (int enable)
 {
+  client_state &cs = get_client_state ();
   struct sigevent event;
 
   if (enable)
@@ -825,11 +836,11 @@ nto_comctrl (int enable)
       event.sigev_code = 0;
       event.sigev_value.sival_ptr = NULL;
       event.sigev_priority = -1;
-      ionotify (remote_desc, _NOTIFY_ACTION_POLLARM, _NOTIFY_COND_INPUT,
+      ionotify (cs.remote_desc, _NOTIFY_ACTION_POLLARM, _NOTIFY_COND_INPUT,
 		&event);
     }
   else
-    ionotify (remote_desc, _NOTIFY_ACTION_POLL, _NOTIFY_COND_INPUT, NULL);
+    ionotify (cs.remote_desc, _NOTIFY_ACTION_POLL, _NOTIFY_COND_INPUT, NULL);
 }
 #endif /* __QNX__ */
 
@@ -892,13 +903,13 @@ static unsigned char *readchar_bufp;
 /* Returns next char from remote GDB.  -1 if error.  */
 
 static int
-readchar (void)
+readchar (gdb_fildes_t fd)
 {
   int ch;
 
   if (readchar_bufcnt == 0)
     {
-      readchar_bufcnt = read_prim (readchar_buf, sizeof (readchar_buf));
+      readchar_bufcnt = read_prim (fd, readchar_buf, sizeof (readchar_buf));
 
       if (readchar_bufcnt <= 0)
 	{
@@ -973,6 +984,7 @@ getpkt (char *buf)
   char *bp;
   unsigned char csum, c1, c2;
   int c;
+  gdb_fildes_t fd = cs.remote_desc;
 
   while (1)
     {
@@ -980,7 +992,7 @@ getpkt (char *buf)
 
       while (1)
 	{
-	  c = readchar ();
+	  c = readchar (fd);
 
 	  /* The '\003' may appear before or after each packet, so
 	     check for an input interrupt.  */
@@ -1005,7 +1017,7 @@ getpkt (char *buf)
       bp = buf;
       while (1)
 	{
-	  c = readchar ();
+	  c = readchar (fd);
 	  if (c < 0)
 	    return -1;
 	  if (c == '#')
@@ -1015,8 +1027,8 @@ getpkt (char *buf)
 	}
       *bp = 0;
 
-      c1 = fromhex (readchar ());
-      c2 = fromhex (readchar ());
+      c1 = fromhex (readchar (fd));
+      c2 = fromhex (readchar (fd));
 
       if (csum == (c1 << 4) + c2)
 	break;
@@ -1033,7 +1045,7 @@ getpkt (char *buf)
 
       fprintf (stderr, "Bad checksum, sentsum=0x%x, csum=0x%x, buf=%s\n",
 	       (c1 << 4) + c2, csum, buf);
-      if (write_prim ("-", 1) != 1)
+      if (write_prim (fd, "-", 1) != 1)
 	return -1;
     }
 
@@ -1041,11 +1053,11 @@ getpkt (char *buf)
     {
       if (remote_debug)
 	{
-	  debug_printf ("getpkt (\"%s\");  [sending ack] \n", buf);
+	  debug_printf ("getpkt (%d, \"%s\");  [sending ack] \n", fd, buf);
 	  debug_flush ();
 	}
 
-      if (write_prim ("+", 1) != 1)
+      if (write_prim (fd, "+", 1) != 1)
 	return -1;
 
       if (remote_debug)
@@ -1058,7 +1070,7 @@ getpkt (char *buf)
     {
       if (remote_debug)
 	{
-	  debug_printf ("getpkt (\"%s\");  [no ack sent] \n", buf);
+	  debug_printf ("getpkt (%d, \"%s\");  [no ack sent] \n", fd, buf);
 	  debug_flush ();
 	}
     }
@@ -1075,7 +1087,7 @@ getpkt (char *buf)
   while (readchar_bufcnt > 0 && *readchar_bufp == '\003')
     {
       /* Consume the interrupt character in the buffer.  */
-      readchar ();
+      readchar (fd);
       (*the_target->request_interrupt) ();
     }
 
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 3fc026f78eb..8a5f23f9824 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -152,13 +152,30 @@ static struct btrace_config current_btrace_conf;
 
 /* The client remote protocol state. */
 
-static client_state g_client_state;
+static struct multi_client_states g_client_states;
+
+multi_client_states &
+get_client_states ()
+{
+  return g_client_states;
+}
+
+
+/* Add a new client state for a corresponding file descriptor fd */
 
 client_state &
-get_client_state ()
+multi_client_states::add_client_state (gdb_fildes_t fd)
 {
-  client_state &cs = g_client_state;
-  return cs;
+  if (fd == -1)
+    /* This is an initial client state setup in captured main */
+      client_states[fd] = client_state ();
+  else
+    /* Initialize this client state from the initial client state.
+     * If fd was disconnected / reconnected then it is reloaded.
+     */
+    client_states[fd] = client_states[-1];
+  client_states[fd].remote_desc = fd;
+  return set_current_client (fd);
 }
 
 
@@ -3556,7 +3573,8 @@ captured_main (int argc, char *argv[])
 #endif
 
   current_directory = getcwd (NULL, 0);
-  client_state &cs = get_client_state ();
+
+  client_state &cs = get_client_states ().add_client_state (-1);
 
   if (current_directory == NULL)
     {
diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
index 3c286862349..6c2b70e8004 100644
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -107,6 +107,7 @@ extern int in_queued_stop_replies (ptid_t ptid);
 #include "utils.h"
 #include "debug.h"
 #include "gdbsupport/gdb_vecs.h"
+#include <map>
 
 /* 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).  */
@@ -141,9 +142,11 @@ extern unsigned long signal_pid;
 struct client_state
 {
   client_state ():
-    own_buf ((char *) xmalloc (PBUFSIZ + 1)) 
+    own_buf ((char *) xmalloc (PBUFSIZ + 1))
   {}
 
+  gdb_fildes_t remote_desc;
+
   /* The thread set with an `Hc' packet.  `Hc' is deprecated in favor of
      `vCont'.  Note the multi-process extensions made `vCont' a
      requirement, so `Hc pPID.TID' is pretty much undefined.  So
@@ -202,7 +205,31 @@ struct client_state
 
 };
 
-client_state &get_client_state ();
+/* Container of client remote protocol states for all the currently
+   connected clients.  */
+
+#define get_client_state() get_client_states ().get_current_client ()
+
+class multi_client_states
+{
+private:
+  /*  Mapping from the file descriptor to the associated client state */
+  std::map<gdb_fildes_t, client_state> client_states;
+  /*  The file descriptor of the current client we are focused on */
+  gdb_fildes_t current_fd;
+
+public:
+  /* Return the current client we are focused on. */
+  client_state &get_current_client () { return client_states[current_fd]; }
+
+  /* Set the current client we wish to focus on. */
+  client_state &set_current_client (gdb_fildes_t fd) { current_fd = fd; return client_states[current_fd]; }
+
+  /* Add the current client we wish to focus on */
+  client_state &add_client_state (gdb_fildes_t fd);
+};
+
+struct multi_client_states &get_client_states ();
 
 #include "gdbthread.h"
 #include "inferiors.h"

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

* Re: [PATCH] Don't override various Makefile variables for gnulib et al
  2019-11-22 23:30 [PATCH] Don't override various Makefile variables for gnulib et al Christian Biesinger via gdb-patches
  2019-11-26 18:01 ` [PATCH] add file desc to gdbserver client_state Stan Cox
@ 2020-01-29 14:07 ` Christian Biesinger via gdb-patches
  2020-01-29 14:10 ` Christian Biesinger via gdb-patches
  2 siblings, 0 replies; 9+ messages in thread
From: Christian Biesinger via gdb-patches @ 2020-01-29 14:07 UTC (permalink / raw)
  To: gdb-patches

Ping

On Sat, Nov 23, 2019 at 12:30 AM Christian Biesinger
<cbiesinger@google.com> wrote:
>
> Normally the toplevel Makefile will pass various CC=foo and other
> flags down to subdir Makefiles. However, for Gnulib this is a problem
> because Gnulib's configure specifically sets CC to something that
> includes a -std=gnu11 flag on some systems, and this override would
> set it back to CC=gcc, leading to compile errors in a GDB build
> with an updated Gnulib.
>
> I don't believe this is needed outside of GCC, so this patch changes
> Gnulib and other non-GCC modules to just not override any flags --
> the values set during configure time should be fine. If a user
> overrides them manually when invoking make, those will still work.
>
> Under the same condition, I also removed the host_exports. I don't
> understand why this is ever necessary (this is only after configure
> has run).
>
> The other option is to clear MAKEOVERRIDES in gnulib/Makefile.am, but
> that means the user can't override any variables for this subdirectory.
>
> ChangeLog:
>
> 2019-11-22  Christian Biesinger  <cbiesinger@google.com>
>
>         * Makefile.def: Pass no_exports_and_flags to various non-GCC
>         modules.
>         * Makefile.in: Allow passing a no_exports_and_flags argument to
>         "all" to suppress emitting exports and make flags. Useful when
>         invoked via host_modules from Makefile.def.
>         * Makefile.tpl: Regenerate.
>
> Change-Id: I7d80328cf81c133ba6157eec7d10c422b6790723
> ---
>  Makefile.def | 12 ++++++------
>  Makefile.in  | 30 ++++++++++++------------------
>  Makefile.tpl |  9 ++++++---
>  3 files changed, 24 insertions(+), 27 deletions(-)
>
> diff --git a/Makefile.def b/Makefile.def
> index 311feb9de3..e1ff065202 100644
> --- a/Makefile.def
> +++ b/Makefile.def
> @@ -33,7 +33,7 @@ build_modules= { module= fixincludes; };
>  build_modules= { module= libcpp;
>                   extra_configure_flags='--disable-nls am_cv_func_iconv=no';};
>
> -host_modules= { module= bfd; bootstrap=true; };
> +host_modules= { module= bfd; bootstrap=true; no_exports_and_flags=true; };
>  host_modules= { module= opcodes; bootstrap=true; };
>  host_modules= { module= binutils; bootstrap=true; };
>  host_modules= { module= bison; no_check_cross= true; };
> @@ -105,15 +105,15 @@ host_modules= { module= libiconv;
>                 missing= install-html;
>                 missing= install-info; };
>  host_modules= { module= m4; };
> -host_modules= { module= readline; };
> +host_modules= { module= readline; no_exports_and_flags=true; };
>  host_modules= { module= sid; };
> -host_modules= { module= sim; };
> +host_modules= { module= sim; no_exports_and_flags=true; };
>  host_modules= { module= texinfo; no_install= true; };
>  host_modules= { module= zlib; no_install=true; no_check=true;
>                 bootstrap=true;
>                 extra_configure_flags='@extra_host_zlib_configure_flags@';};
> -host_modules= { module= gnulib; };
> -host_modules= { module= gdb; };
> +host_modules= { module= gnulib; no_exports_and_flags=true; };
> +host_modules= { module= gdb; no_exports_and_flags=true; };
>  host_modules= { module= expect; };
>  host_modules= { module= guile; };
>  host_modules= { module= tk; };
> @@ -129,7 +129,7 @@ host_modules= { module= lto-plugin; bootstrap=true;
>                 extra_make_flags='@extra_linker_plugin_flags@'; };
>  host_modules= { module= libcc1; extra_configure_flags=--enable-shared; };
>  host_modules= { module= gotools; };
> -host_modules= { module= libctf; no_check=true;
> +host_modules= { module= libctf; no_check=true; no_exports_and_flags=true;
>                 bootstrap=true; };
>
>  target_modules = { module= libstdc++-v3;
> diff --git a/Makefile.in b/Makefile.in
> index 1aabf6ede4..bd41753543 100644
> --- a/Makefile.in
> +++ b/Makefile.in
> @@ -3414,10 +3414,9 @@ maybe-all-bfd: all-bfd
>  all-bfd: configure-bfd
>         @r=`${PWD_COMMAND}`; export r; \
>         s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> -       $(HOST_EXPORTS)  \
> +        \
>         (cd $(HOST_SUBDIR)/bfd && \
> -         $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
> -               $(TARGET-bfd))
> +         $(MAKE) $(TARGET-bfd))
>  @endif bfd
>
>
> @@ -25530,10 +25529,9 @@ all-readline: configure-readline
>         @: $(MAKE); $(unstage)
>         @r=`${PWD_COMMAND}`; export r; \
>         s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> -       $(HOST_EXPORTS)  \
> +        \
>         (cd $(HOST_SUBDIR)/readline && \
> -         $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
> -               $(TARGET-readline))
> +         $(MAKE) $(TARGET-readline))
>  @endif readline
>
>
> @@ -26412,10 +26410,9 @@ all-sim: configure-sim
>         @: $(MAKE); $(unstage)
>         @r=`${PWD_COMMAND}`; export r; \
>         s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> -       $(HOST_EXPORTS)  \
> +        \
>         (cd $(HOST_SUBDIR)/sim && \
> -         $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
> -               $(TARGET-sim))
> +         $(MAKE) $(TARGET-sim))
>  @endif sim
>
>
> @@ -28150,10 +28147,9 @@ all-gnulib: configure-gnulib
>         @: $(MAKE); $(unstage)
>         @r=`${PWD_COMMAND}`; export r; \
>         s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> -       $(HOST_EXPORTS)  \
> +        \
>         (cd $(HOST_SUBDIR)/gnulib && \
> -         $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
> -               $(TARGET-gnulib))
> +         $(MAKE) $(TARGET-gnulib))
>  @endif gnulib
>
>
> @@ -28591,10 +28587,9 @@ all-gdb: configure-gdb
>         @: $(MAKE); $(unstage)
>         @r=`${PWD_COMMAND}`; export r; \
>         s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> -       $(HOST_EXPORTS)  \
> +        \
>         (cd $(HOST_SUBDIR)/gdb && \
> -         $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
> -               $(TARGET-gdb))
> +         $(MAKE) $(TARGET-gdb))
>  @endif gdb
>
>
> @@ -33571,10 +33566,9 @@ maybe-all-libctf: all-libctf
>  all-libctf: configure-libctf
>         @r=`${PWD_COMMAND}`; export r; \
>         s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> -       $(HOST_EXPORTS)  \
> +        \
>         (cd $(HOST_SUBDIR)/libctf && \
> -         $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
> -               $(TARGET-libctf))
> +         $(MAKE) $(TARGET-libctf))
>  @endif libctf
>
>
> diff --git a/Makefile.tpl b/Makefile.tpl
> index 5b118a8ba4..876ecf8dbd 100644
> --- a/Makefile.tpl
> +++ b/Makefile.tpl
> @@ -1126,10 +1126,13 @@ all-[+prefix+][+module+]: configure-[+prefix+][+module+][+ IF bootstrap +][+ ELS
>         @: $(MAKE); $(unstage)[+ ENDIF bootstrap +]
>         @r=`${PWD_COMMAND}`; export r; \
>         s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> -       [+exports+] [+extra_exports+] \
> +       [+ IF no_exports_and_flags +][+ ELSE
> +       +][+exports+] [+extra_exports+][+
> +       ENDIF no_exports_and_flags +] \
>         (cd [+subdir+]/[+module+] && \
> -         $(MAKE) $(BASE_FLAGS_TO_PASS) [+args+] [+stage1_args+] [+extra_make_flags+] \
> -               $(TARGET-[+prefix+][+module+]))
> +         $(MAKE) [+ IF no_exports_and_flags +][+ ELSE
> +         +]$(BASE_FLAGS_TO_PASS) [+args+] [+stage1_args+] [+extra_make_flags+] \
> +               [+ ENDIF no_exports_and_flags +]$(TARGET-[+prefix+][+module+]))
>  @endif [+prefix+][+module+]
>
>  [+ IF bootstrap +]
>
> base-commit: 987012b89bce7f6385ed88585547f852a8005a3f
> --
> 2.24.0.432.g9d3f5f5b63-goog
>


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

* Re: [PATCH] Don't override various Makefile variables for gnulib et al
  2019-11-22 23:30 [PATCH] Don't override various Makefile variables for gnulib et al Christian Biesinger via gdb-patches
  2019-11-26 18:01 ` [PATCH] add file desc to gdbserver client_state Stan Cox
  2020-01-29 14:07 ` [PATCH] Don't override various Makefile variables for gnulib et al Christian Biesinger via gdb-patches
@ 2020-01-29 14:10 ` Christian Biesinger via gdb-patches
  2020-02-12 21:16   ` Christian Biesinger via gdb-patches
  2 siblings, 1 reply; 9+ messages in thread
From: Christian Biesinger via gdb-patches @ 2020-01-29 14:10 UTC (permalink / raw)
  To: gdb-patches, Christian Biesinger via gcc-patches

Ping

On Sat, Nov 23, 2019 at 12:30 AM Christian Biesinger
<cbiesinger@google.com> wrote:
>
> Normally the toplevel Makefile will pass various CC=foo and other
> flags down to subdir Makefiles. However, for Gnulib this is a problem
> because Gnulib's configure specifically sets CC to something that
> includes a -std=gnu11 flag on some systems, and this override would
> set it back to CC=gcc, leading to compile errors in a GDB build
> with an updated Gnulib.
>
> I don't believe this is needed outside of GCC, so this patch changes
> Gnulib and other non-GCC modules to just not override any flags --
> the values set during configure time should be fine. If a user
> overrides them manually when invoking make, those will still work.
>
> Under the same condition, I also removed the host_exports. I don't
> understand why this is ever necessary (this is only after configure
> has run).
>
> The other option is to clear MAKEOVERRIDES in gnulib/Makefile.am, but
> that means the user can't override any variables for this subdirectory.
>
> ChangeLog:
>
> 2019-11-22  Christian Biesinger  <cbiesinger@google.com>
>
>         * Makefile.def: Pass no_exports_and_flags to various non-GCC
>         modules.
>         * Makefile.in: Allow passing a no_exports_and_flags argument to
>         "all" to suppress emitting exports and make flags. Useful when
>         invoked via host_modules from Makefile.def.
>         * Makefile.tpl: Regenerate.
>
> Change-Id: I7d80328cf81c133ba6157eec7d10c422b6790723
> ---
>  Makefile.def | 12 ++++++------
>  Makefile.in  | 30 ++++++++++++------------------
>  Makefile.tpl |  9 ++++++---
>  3 files changed, 24 insertions(+), 27 deletions(-)
>
> diff --git a/Makefile.def b/Makefile.def
> index 311feb9de3..e1ff065202 100644
> --- a/Makefile.def
> +++ b/Makefile.def
> @@ -33,7 +33,7 @@ build_modules= { module= fixincludes; };
>  build_modules= { module= libcpp;
>                   extra_configure_flags='--disable-nls am_cv_func_iconv=no';};
>
> -host_modules= { module= bfd; bootstrap=true; };
> +host_modules= { module= bfd; bootstrap=true; no_exports_and_flags=true; };
>  host_modules= { module= opcodes; bootstrap=true; };
>  host_modules= { module= binutils; bootstrap=true; };
>  host_modules= { module= bison; no_check_cross= true; };
> @@ -105,15 +105,15 @@ host_modules= { module= libiconv;
>                 missing= install-html;
>                 missing= install-info; };
>  host_modules= { module= m4; };
> -host_modules= { module= readline; };
> +host_modules= { module= readline; no_exports_and_flags=true; };
>  host_modules= { module= sid; };
> -host_modules= { module= sim; };
> +host_modules= { module= sim; no_exports_and_flags=true; };
>  host_modules= { module= texinfo; no_install= true; };
>  host_modules= { module= zlib; no_install=true; no_check=true;
>                 bootstrap=true;
>                 extra_configure_flags='@extra_host_zlib_configure_flags@';};
> -host_modules= { module= gnulib; };
> -host_modules= { module= gdb; };
> +host_modules= { module= gnulib; no_exports_and_flags=true; };
> +host_modules= { module= gdb; no_exports_and_flags=true; };
>  host_modules= { module= expect; };
>  host_modules= { module= guile; };
>  host_modules= { module= tk; };
> @@ -129,7 +129,7 @@ host_modules= { module= lto-plugin; bootstrap=true;
>                 extra_make_flags='@extra_linker_plugin_flags@'; };
>  host_modules= { module= libcc1; extra_configure_flags=--enable-shared; };
>  host_modules= { module= gotools; };
> -host_modules= { module= libctf; no_check=true;
> +host_modules= { module= libctf; no_check=true; no_exports_and_flags=true;
>                 bootstrap=true; };
>
>  target_modules = { module= libstdc++-v3;
> diff --git a/Makefile.in b/Makefile.in
> index 1aabf6ede4..bd41753543 100644
> --- a/Makefile.in
> +++ b/Makefile.in
> @@ -3414,10 +3414,9 @@ maybe-all-bfd: all-bfd
>  all-bfd: configure-bfd
>         @r=`${PWD_COMMAND}`; export r; \
>         s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> -       $(HOST_EXPORTS)  \
> +        \
>         (cd $(HOST_SUBDIR)/bfd && \
> -         $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
> -               $(TARGET-bfd))
> +         $(MAKE) $(TARGET-bfd))
>  @endif bfd
>
>
> @@ -25530,10 +25529,9 @@ all-readline: configure-readline
>         @: $(MAKE); $(unstage)
>         @r=`${PWD_COMMAND}`; export r; \
>         s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> -       $(HOST_EXPORTS)  \
> +        \
>         (cd $(HOST_SUBDIR)/readline && \
> -         $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
> -               $(TARGET-readline))
> +         $(MAKE) $(TARGET-readline))
>  @endif readline
>
>
> @@ -26412,10 +26410,9 @@ all-sim: configure-sim
>         @: $(MAKE); $(unstage)
>         @r=`${PWD_COMMAND}`; export r; \
>         s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> -       $(HOST_EXPORTS)  \
> +        \
>         (cd $(HOST_SUBDIR)/sim && \
> -         $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
> -               $(TARGET-sim))
> +         $(MAKE) $(TARGET-sim))
>  @endif sim
>
>
> @@ -28150,10 +28147,9 @@ all-gnulib: configure-gnulib
>         @: $(MAKE); $(unstage)
>         @r=`${PWD_COMMAND}`; export r; \
>         s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> -       $(HOST_EXPORTS)  \
> +        \
>         (cd $(HOST_SUBDIR)/gnulib && \
> -         $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
> -               $(TARGET-gnulib))
> +         $(MAKE) $(TARGET-gnulib))
>  @endif gnulib
>
>
> @@ -28591,10 +28587,9 @@ all-gdb: configure-gdb
>         @: $(MAKE); $(unstage)
>         @r=`${PWD_COMMAND}`; export r; \
>         s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> -       $(HOST_EXPORTS)  \
> +        \
>         (cd $(HOST_SUBDIR)/gdb && \
> -         $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
> -               $(TARGET-gdb))
> +         $(MAKE) $(TARGET-gdb))
>  @endif gdb
>
>
> @@ -33571,10 +33566,9 @@ maybe-all-libctf: all-libctf
>  all-libctf: configure-libctf
>         @r=`${PWD_COMMAND}`; export r; \
>         s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> -       $(HOST_EXPORTS)  \
> +        \
>         (cd $(HOST_SUBDIR)/libctf && \
> -         $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
> -               $(TARGET-libctf))
> +         $(MAKE) $(TARGET-libctf))
>  @endif libctf
>
>
> diff --git a/Makefile.tpl b/Makefile.tpl
> index 5b118a8ba4..876ecf8dbd 100644
> --- a/Makefile.tpl
> +++ b/Makefile.tpl
> @@ -1126,10 +1126,13 @@ all-[+prefix+][+module+]: configure-[+prefix+][+module+][+ IF bootstrap +][+ ELS
>         @: $(MAKE); $(unstage)[+ ENDIF bootstrap +]
>         @r=`${PWD_COMMAND}`; export r; \
>         s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> -       [+exports+] [+extra_exports+] \
> +       [+ IF no_exports_and_flags +][+ ELSE
> +       +][+exports+] [+extra_exports+][+
> +       ENDIF no_exports_and_flags +] \
>         (cd [+subdir+]/[+module+] && \
> -         $(MAKE) $(BASE_FLAGS_TO_PASS) [+args+] [+stage1_args+] [+extra_make_flags+] \
> -               $(TARGET-[+prefix+][+module+]))
> +         $(MAKE) [+ IF no_exports_and_flags +][+ ELSE
> +         +]$(BASE_FLAGS_TO_PASS) [+args+] [+stage1_args+] [+extra_make_flags+] \
> +               [+ ENDIF no_exports_and_flags +]$(TARGET-[+prefix+][+module+]))
>  @endif [+prefix+][+module+]
>
>  [+ IF bootstrap +]
>
> base-commit: 987012b89bce7f6385ed88585547f852a8005a3f
> --
> 2.24.0.432.g9d3f5f5b63-goog
>


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

* Re: [PATCH] Don't override various Makefile variables for gnulib et al
  2020-01-29 14:10 ` Christian Biesinger via gdb-patches
@ 2020-02-12 21:16   ` Christian Biesinger via gdb-patches
  0 siblings, 0 replies; 9+ messages in thread
From: Christian Biesinger via gdb-patches @ 2020-02-12 21:16 UTC (permalink / raw)
  To: gdb-patches, Christian Biesinger via gcc-patches

Ping

On Wed, Jan 29, 2020 at 8:07 AM Christian Biesinger
<cbiesinger@google.com> wrote:
>
> Ping
>
> On Sat, Nov 23, 2019 at 12:30 AM Christian Biesinger
> <cbiesinger@google.com> wrote:
> >
> > Normally the toplevel Makefile will pass various CC=foo and other
> > flags down to subdir Makefiles. However, for Gnulib this is a problem
> > because Gnulib's configure specifically sets CC to something that
> > includes a -std=gnu11 flag on some systems, and this override would
> > set it back to CC=gcc, leading to compile errors in a GDB build
> > with an updated Gnulib.
> >
> > I don't believe this is needed outside of GCC, so this patch changes
> > Gnulib and other non-GCC modules to just not override any flags --
> > the values set during configure time should be fine. If a user
> > overrides them manually when invoking make, those will still work.
> >
> > Under the same condition, I also removed the host_exports. I don't
> > understand why this is ever necessary (this is only after configure
> > has run).
> >
> > The other option is to clear MAKEOVERRIDES in gnulib/Makefile.am, but
> > that means the user can't override any variables for this subdirectory.
> >
> > ChangeLog:
> >
> > 2019-11-22  Christian Biesinger  <cbiesinger@google.com>
> >
> >         * Makefile.def: Pass no_exports_and_flags to various non-GCC
> >         modules.
> >         * Makefile.in: Allow passing a no_exports_and_flags argument to
> >         "all" to suppress emitting exports and make flags. Useful when
> >         invoked via host_modules from Makefile.def.
> >         * Makefile.tpl: Regenerate.
> >
> > Change-Id: I7d80328cf81c133ba6157eec7d10c422b6790723
> > ---
> >  Makefile.def | 12 ++++++------
> >  Makefile.in  | 30 ++++++++++++------------------
> >  Makefile.tpl |  9 ++++++---
> >  3 files changed, 24 insertions(+), 27 deletions(-)
> >
> > diff --git a/Makefile.def b/Makefile.def
> > index 311feb9de3..e1ff065202 100644
> > --- a/Makefile.def
> > +++ b/Makefile.def
> > @@ -33,7 +33,7 @@ build_modules= { module= fixincludes; };
> >  build_modules= { module= libcpp;
> >                   extra_configure_flags='--disable-nls am_cv_func_iconv=no';};
> >
> > -host_modules= { module= bfd; bootstrap=true; };
> > +host_modules= { module= bfd; bootstrap=true; no_exports_and_flags=true; };
> >  host_modules= { module= opcodes; bootstrap=true; };
> >  host_modules= { module= binutils; bootstrap=true; };
> >  host_modules= { module= bison; no_check_cross= true; };
> > @@ -105,15 +105,15 @@ host_modules= { module= libiconv;
> >                 missing= install-html;
> >                 missing= install-info; };
> >  host_modules= { module= m4; };
> > -host_modules= { module= readline; };
> > +host_modules= { module= readline; no_exports_and_flags=true; };
> >  host_modules= { module= sid; };
> > -host_modules= { module= sim; };
> > +host_modules= { module= sim; no_exports_and_flags=true; };
> >  host_modules= { module= texinfo; no_install= true; };
> >  host_modules= { module= zlib; no_install=true; no_check=true;
> >                 bootstrap=true;
> >                 extra_configure_flags='@extra_host_zlib_configure_flags@';};
> > -host_modules= { module= gnulib; };
> > -host_modules= { module= gdb; };
> > +host_modules= { module= gnulib; no_exports_and_flags=true; };
> > +host_modules= { module= gdb; no_exports_and_flags=true; };
> >  host_modules= { module= expect; };
> >  host_modules= { module= guile; };
> >  host_modules= { module= tk; };
> > @@ -129,7 +129,7 @@ host_modules= { module= lto-plugin; bootstrap=true;
> >                 extra_make_flags='@extra_linker_plugin_flags@'; };
> >  host_modules= { module= libcc1; extra_configure_flags=--enable-shared; };
> >  host_modules= { module= gotools; };
> > -host_modules= { module= libctf; no_check=true;
> > +host_modules= { module= libctf; no_check=true; no_exports_and_flags=true;
> >                 bootstrap=true; };
> >
> >  target_modules = { module= libstdc++-v3;
> > diff --git a/Makefile.in b/Makefile.in
> > index 1aabf6ede4..bd41753543 100644
> > --- a/Makefile.in
> > +++ b/Makefile.in
> > @@ -3414,10 +3414,9 @@ maybe-all-bfd: all-bfd
> >  all-bfd: configure-bfd
> >         @r=`${PWD_COMMAND}`; export r; \
> >         s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> > -       $(HOST_EXPORTS)  \
> > +        \
> >         (cd $(HOST_SUBDIR)/bfd && \
> > -         $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
> > -               $(TARGET-bfd))
> > +         $(MAKE) $(TARGET-bfd))
> >  @endif bfd
> >
> >
> > @@ -25530,10 +25529,9 @@ all-readline: configure-readline
> >         @: $(MAKE); $(unstage)
> >         @r=`${PWD_COMMAND}`; export r; \
> >         s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> > -       $(HOST_EXPORTS)  \
> > +        \
> >         (cd $(HOST_SUBDIR)/readline && \
> > -         $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
> > -               $(TARGET-readline))
> > +         $(MAKE) $(TARGET-readline))
> >  @endif readline
> >
> >
> > @@ -26412,10 +26410,9 @@ all-sim: configure-sim
> >         @: $(MAKE); $(unstage)
> >         @r=`${PWD_COMMAND}`; export r; \
> >         s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> > -       $(HOST_EXPORTS)  \
> > +        \
> >         (cd $(HOST_SUBDIR)/sim && \
> > -         $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
> > -               $(TARGET-sim))
> > +         $(MAKE) $(TARGET-sim))
> >  @endif sim
> >
> >
> > @@ -28150,10 +28147,9 @@ all-gnulib: configure-gnulib
> >         @: $(MAKE); $(unstage)
> >         @r=`${PWD_COMMAND}`; export r; \
> >         s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> > -       $(HOST_EXPORTS)  \
> > +        \
> >         (cd $(HOST_SUBDIR)/gnulib && \
> > -         $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
> > -               $(TARGET-gnulib))
> > +         $(MAKE) $(TARGET-gnulib))
> >  @endif gnulib
> >
> >
> > @@ -28591,10 +28587,9 @@ all-gdb: configure-gdb
> >         @: $(MAKE); $(unstage)
> >         @r=`${PWD_COMMAND}`; export r; \
> >         s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> > -       $(HOST_EXPORTS)  \
> > +        \
> >         (cd $(HOST_SUBDIR)/gdb && \
> > -         $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
> > -               $(TARGET-gdb))
> > +         $(MAKE) $(TARGET-gdb))
> >  @endif gdb
> >
> >
> > @@ -33571,10 +33566,9 @@ maybe-all-libctf: all-libctf
> >  all-libctf: configure-libctf
> >         @r=`${PWD_COMMAND}`; export r; \
> >         s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> > -       $(HOST_EXPORTS)  \
> > +        \
> >         (cd $(HOST_SUBDIR)/libctf && \
> > -         $(MAKE) $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(STAGE1_FLAGS_TO_PASS)  \
> > -               $(TARGET-libctf))
> > +         $(MAKE) $(TARGET-libctf))
> >  @endif libctf
> >
> >
> > diff --git a/Makefile.tpl b/Makefile.tpl
> > index 5b118a8ba4..876ecf8dbd 100644
> > --- a/Makefile.tpl
> > +++ b/Makefile.tpl
> > @@ -1126,10 +1126,13 @@ all-[+prefix+][+module+]: configure-[+prefix+][+module+][+ IF bootstrap +][+ ELS
> >         @: $(MAKE); $(unstage)[+ ENDIF bootstrap +]
> >         @r=`${PWD_COMMAND}`; export r; \
> >         s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> > -       [+exports+] [+extra_exports+] \
> > +       [+ IF no_exports_and_flags +][+ ELSE
> > +       +][+exports+] [+extra_exports+][+
> > +       ENDIF no_exports_and_flags +] \
> >         (cd [+subdir+]/[+module+] && \
> > -         $(MAKE) $(BASE_FLAGS_TO_PASS) [+args+] [+stage1_args+] [+extra_make_flags+] \
> > -               $(TARGET-[+prefix+][+module+]))
> > +         $(MAKE) [+ IF no_exports_and_flags +][+ ELSE
> > +         +]$(BASE_FLAGS_TO_PASS) [+args+] [+stage1_args+] [+extra_make_flags+] \
> > +               [+ ENDIF no_exports_and_flags +]$(TARGET-[+prefix+][+module+]))
> >  @endif [+prefix+][+module+]
> >
> >  [+ IF bootstrap +]
> >
> > base-commit: 987012b89bce7f6385ed88585547f852a8005a3f
> > --
> > 2.24.0.432.g9d3f5f5b63-goog
> >


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

end of thread, other threads:[~2020-02-12 21:16 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-22 23:30 [PATCH] Don't override various Makefile variables for gnulib et al Christian Biesinger via gdb-patches
2019-11-26 18:01 ` [PATCH] add file desc to gdbserver client_state Stan Cox
2019-12-13 23:13   ` Tom Tromey
2019-12-24  4:42     ` Stan Cox
2020-01-24 18:21       ` Tom Tromey
2020-02-06 19:41       ` Stan Cox
2020-01-29 14:07 ` [PATCH] Don't override various Makefile variables for gnulib et al Christian Biesinger via gdb-patches
2020-01-29 14:10 ` Christian Biesinger via gdb-patches
2020-02-12 21:16   ` Christian Biesinger via gdb-patches

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