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


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

[committed] : [PATCH 0/4] PR14161: a partial fix


On Saturday, June 09, 2012 08:46:38 PM Yao Qi wrote:
> Hi,
> PR14161 reports that agent socket '/tmp/gdb_ustXXXX' is not removed when
> program is done.  This patch series is to address this problem partially
> only in GDBserver side (in patch 3/4) and kfail it temporarily in GDB
> side (in patch 4/4) due to some limitations on threads control.
> Patch 1/4 is a new test case to show this problem.

This is what I applied to HEAD.

-- 
Yao (éå)

gdb/gdbserver/
	PR remote/14161.

	* server.h: Declare gdb_agent_about_to_close.
	* target.c (kill_inferior): Include "agent.h".
	New.  Send command 'kill'.
    	* target.h (kill_inferior): Removed macro.
    	* tracepoint.c (gdb_agent_about_to_close): New.
    	(gdb_agent_helper_thread): Handle command 'close'.
    	Wait endlessly until the inferior stops.
    	Install gdb_agent_remove_socket to atexit hook.
    	(agent_socket_name): New static variable.
    	(gdb_agent_socket_init): Replace local variable 'name' with
    	'agent_socket_name'.
    	(gdb_agent_remove_socket): New.

gdb/doc/
    	* gdb.texinfo (IPA Protocol Commands): Document new command
    	'close'.

gdb/testsuite/
	KFAIL for PR remote/14161.
	* gdb.trace/strace.exp (strace_remove_socket): kfail for native.
	Cleanup socket files.
	(strace_info_marker): Detach inferior.

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index facca8f..a4503bf 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -33673,6 +33673,10 @@ for an error
 
 @end table
 
+@item close
+Closes the in-process agent.  This command is sent when @value{GDBN} or GDBserver
+is about to kill inferiors.
+
 @item qTfSTM
 @xref{qTfSTM}.
 @item qTsSTM
diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
index 02dfa29..752bc3b 100644
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -467,6 +467,7 @@ void stop_tracing (void);
 
 int claim_trampoline_space (ULONGEST used, CORE_ADDR *trampoline);
 int have_fast_tracepoint_trampoline_buffer (char *msgbuf);
+void gdb_agent_about_to_close (int pid);
 #endif
 
 struct traceframe;
diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index 7539476..e8b2b08 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -182,3 +182,11 @@ target_waitstatus_to_string (const struct target_waitstatus *ws)
 
   return buf;
 }
+
+int
+kill_inferior (int pid)
+{
+  gdb_agent_about_to_close (pid);
+
+  return (*the_target->kill) (pid);
+}
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index 0cf5687..9f96e04 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -409,8 +409,7 @@ void set_target_ops (struct target_ops *);
 #define myattach(pid) \
   (*the_target->attach) (pid)
 
-#define kill_inferior(pid) \
-  (*the_target->kill) (pid)
+int kill_inferior (int);
 
 #define detach_inferior(pid) \
   (*the_target->detach) (pid)
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index f103dfc..7ad77f0 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -3923,6 +3923,38 @@ cmd_qtstmat (char *packet)
     run_inferior_command (packet, strlen (packet) + 1);
 }
 
+/* Sent the agent a command to close it.  */
+
+void
+gdb_agent_about_to_close (int pid)
+{
+  char buf[IPA_CMD_BUF_SIZE];
+
+  if (!maybe_write_ipa_not_loaded (buf))
+    {
+      struct thread_info *save_inferior;
+      struct inferior_list_entry *inf = all_threads.head;
+
+      save_inferior = current_inferior;
+
+      /* Find a certain thread which belongs to process PID.  */
+      while (inf != NULL)
+	{
+	  if (ptid_get_pid (inf->id) == pid)
+	    break;
+	  inf = inf->next;
+	}
+
+      current_inferior = (struct thread_info *) inf;
+
+      strcpy (buf, "close");
+
+      run_inferior_command (buf, strlen (buf) + 1);
+
+      current_inferior = save_inferior;
+    }
+}
+
 /* Return the minimum instruction size needed for fast tracepoints as a
    hexadecimal number.  */
 
@@ -6748,13 +6780,14 @@ init_named_socket (const char *name)
   return fd;
 }
 
+static char agent_socket_name[UNIX_PATH_MAX];
+
 static int
 gdb_agent_socket_init (void)
 {
   int result, fd;
-  char name[UNIX_PATH_MAX];
 
-  result = xsnprintf (name, UNIX_PATH_MAX, "%s/gdb_ust%d",
+  result = xsnprintf (agent_socket_name, UNIX_PATH_MAX, "%s/gdb_ust%d",
 		      SOCK_DIR, getpid ());
   if (result >= UNIX_PATH_MAX)
     {
@@ -6762,11 +6795,11 @@ gdb_agent_socket_init (void)
       return -1;
     }
 
-  fd = init_named_socket (name);
+  fd = init_named_socket (agent_socket_name);
   if (fd < 0)
     warning ("Error initializing named socket (%s) for communication with the "
 	     "ust helper thread. Check that directory exists and that it "
-	     "is writable.", name);
+	     "is writable.", agent_socket_name);
 
   return fd;
 }
@@ -6995,6 +7028,13 @@ gdb_ust_init (void)
 #endif /* HAVE_UST */
 
 #include <sys/syscall.h>
+#include <stdlib.h>
+
+static void
+gdb_agent_remove_socket (void)
+{
+  unlink (agent_socket_name);
+}
 
 /* Helper thread of agent.  */
 
@@ -7003,6 +7043,8 @@ gdb_agent_helper_thread (void *arg)
 {
   int listen_fd;
 
+  atexit (gdb_agent_remove_socket);
+
   while (1)
     {
       listen_fd = gdb_agent_socket_init ();
@@ -7023,6 +7065,7 @@ gdb_agent_helper_thread (void *arg)
 	  int fd;
 	  char buf[1];
 	  int ret;
+	  int stop_loop = 0;
 
 	  tmp = sizeof (sockaddr);
 
@@ -7055,8 +7098,12 @@ gdb_agent_helper_thread (void *arg)
 
 	  if (cmd_buf[0])
 	    {
+	      if (strncmp ("close", cmd_buf, 5) == 0)
+		{
+		  stop_loop = 1;
+		}
 #ifdef HAVE_UST
-	      if (strcmp ("qTfSTM", cmd_buf) == 0)
+	      else if (strcmp ("qTfSTM", cmd_buf) == 0)
 		{
 		  cmd_qtfstm (cmd_buf);
 		}
@@ -7088,6 +7135,20 @@ gdb_agent_helper_thread (void *arg)
 	  /* Fix compiler's warning: ignoring return value of 'write'.  */
 	  ret = write (fd, buf, 1);
 	  close (fd);
+
+	  if (stop_loop)
+	    {
+	      close (listen_fd);
+	      unlink (agent_socket_name);
+
+	      /* Sleep endlessly to wait the whole inferior stops.  This
+		 thread can not exit because GDB or GDBserver may still need
+		 'current_inferior' (representing this thread) to access
+		 inferior memory.  Otherwise, this thread exits earlier than
+		 other threads, and 'current_inferior' is set to NULL.  */
+	      while (1)
+		sleep (10);
+	    }
 	}
     }
 
diff --git a/gdb/testsuite/gdb.trace/strace.exp b/gdb/testsuite/gdb.trace/strace.exp
index d9e90b6..f09eeb3 100644
--- a/gdb/testsuite/gdb.trace/strace.exp
+++ b/gdb/testsuite/gdb.trace/strace.exp
@@ -37,12 +37,103 @@ if { [gdb_compile $srcdir/$subdir/$srcfile $binfile executable $additional_flags
     return -1
 }
 
+# Test that the socket file is removed when GDB quits, detaches or
+# resumes the inferior until it exits.
+
+proc strace_remove_socket { action } {
+    with_test_prefix "remove_socket_after_${action}" {
+
+    global executable
+    global gdb_prompt
+    global libipa
+
+    # Restart with a fresh gdb.
+    clean_restart $executable
+    gdb_load_shlibs $libipa
+    if ![runto_main] {
+	fail "Can't run to main"
+	return -1
+    }
+
+    # List the markers in program.
+    gdb_test "info static-tracepoint-markers" \
+	".*ust/bar\[\t \]+n\[\t \]+.*ust/bar2\[\t \]+n\[\t \]+.*"
+
+    set pid ""
+    set test "collect pid"
+    gdb_test_multiple "info inferiors" $test {
+	-re "process (\[-0-9a-fx\]+) \[^\n\]*\n.*${gdb_prompt} $" {
+	    set pid $expect_out(1,string)
+	    pass $test
+	}
+	-re ".*${gdb_prompt} $" {
+	    fail $test
+	}
+    }
+
+    set test "socket file exists"
+    set socket_file "/tmp/gdb_ust${pid}"
+    set status [remote_exec target "sh -c { \[ -S $socket_file \] }"]
+
+    if { [lindex $status 0] == 0 } {
+	pass $test
+    } else {
+	fail $test
+    }
+
+    send_gdb "${action}\n"
+    gdb_expect {
+	-re "A debugging session is active.\r\n.*\r\nQuit anyway\\? \\(y or n\\) $" {
+	    send_gdb "y\n"
+	}
+	-re "Detaching .*, process .*$" {
+	}
+	-re "Continuing.*$" {
+	}
+    }
+
+    set exists 1
+
+    for {set i 1} {$i <= 5} {incr i} {
+	set status [remote_exec target "sh -c { \[ -S $socket_file \] }"]
+	if { [lindex $status 0] != 0 } {
+	    set exists 0
+	    break
+	}
+	sleep 1
+    }
+
+    if { ![is_remote target] && ![string equal $action "detach"] } {
+	setup_kfail gdb/14161 *-*-*
+    }
+
+    set test "socket file removed"
+
+    if { $exists } {
+	fail $test
+	# Since $socket_file is a socket file instead of a regular file, we
+	# can't use 'remote_file target delete $socket_file' here.
+	remote_exec target "sh -c \"rm -r $socket_file\""
+    } else {
+	pass $test
+    }
+
+    if { [string equal $action "quit"] && [is_remote host] } {
+	global gdb_spawn_id
+	# unset gdb_spawn_id here to avoid sending command 'quit' to GDB
+	# later in default_gdb_exit.
+	unset gdb_spawn_id
+    }
+}}
+
 proc strace_info_marker { } { with_test_prefix "info_marker" {
     global executable
     global gdb_prompt
+    global libipa
 
     # Restart with a fresh gdb.
     clean_restart $executable
+    gdb_load_shlibs $libipa
     if ![runto_main] {
 	fail "Can't run to main"
 	return -1
@@ -59,6 +150,13 @@ proc strace_info_marker { } { with_test_prefix "info_marker" {
 	    pass "info threads"
 	}
     }
+
+    # GDB detaches inferior so that the socket file can be removed.
+    gdb_test_multiple "detach" "detach" {
+	-re "Detaching .*, process .*${gdb_prompt} $" {
+	    pass "detach"
+	}
+    }
 }}
 
 proc strace_probe_marker { } { with_test_prefix "probe_marker" {
@@ -66,9 +164,11 @@ proc strace_probe_marker { } { with_test_prefix "probe_marker" {
     global expect_out
     global gdb_prompt
     global hex
+    global libipa
 
     # Restart with a fresh gdb.
     clean_restart $executable
+    gdb_load_shlibs $libipa
     if ![runto_main] {
 	fail "Can't run to main"
 	return -1
@@ -97,9 +197,11 @@ with_test_prefix "trace_same_addr $type" {
     global expect_out
     global gdb_prompt
     global hex
+    global libipa
 
     # Restart with a fresh gdb.
     clean_restart $executable
+    gdb_load_shlibs $libipa
     if ![runto_main] {
 	fail "Can't run to main"
 	return -1
@@ -192,9 +294,11 @@ proc strace_trace_on_diff_addr { } { with_test_prefix "trace_diff_addr" {
     global expect_out
     global gdb_prompt
     global hex
+    global libipa
 
     # Restart with a fresh gdb.
     clean_restart $executable
+    gdb_load_shlibs $libipa
     if ![runto_main] {
 	fail "Can't run to main"
 	return -1
@@ -240,28 +344,30 @@ proc strace_trace_on_diff_addr { } { with_test_prefix "trace_diff_addr" {
     gdb_test "tfind" "Target failed to find requested trace frame\\..*"
 }}
 
-clean_restart $executable
+# Run it on x86/x86_64 linux.
+if { [istarget "x86_64-*-linux*"] || [istarget "i\[34567\]86-*-linux*"] } {
+    strace_info_marker
+    strace_remove_socket "quit"
+    strace_remove_socket "detach"
+    strace_remove_socket "continue"
+}
 
+clean_restart $executable
+gdb_load_shlibs $libipa
 if ![runto_main] {
     fail "Can't run to main to check for trace support"
     return -1
 }
-
-# Run it on native x86/x86_64 linux.
-if { ![is_remote target]
-     && ([istarget "x86_64-*-linux*"] || [istarget "i\[34567\]86-*-linux*"]) } {
-    strace_info_marker
-    return
-}
-
 if { ![gdb_target_supports_trace] } then {
+    # At this point, the socket file has been created.  We must make sure it is
+    # removed when we return here.  GDB detaches inferior so that the socket
+    # file can be removed.  Note that GDB simply kill inferior doesn't remove
+    # the socket file.
+    gdb_test "detach" "Detaching .*, process .*"
     unsupported "Current target does not support trace"
     return -1;
 }
 
-gdb_load_shlibs $libipa
-
-strace_info_marker
 strace_probe_marker
 
 strace_trace_on_same_addr "trace"


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