Make monitor targets always have a thread

Make monitor targets always have a thread

[Pedro Alves]

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

This patch makes the monitor targets always register an execution control
task in GDB's thread list.  Needed to clean up inferior control, and
get rid of context switching.

I confirmed that it builds with x86_64-unknown-linux-gnu x h8300-elf cross,
and that it doesn't introduce any new warnings (this file is built without
-Werror).

As I said in my previous email, I don't even know how to test this, although
I don't except to be any fallout.

Does it look OK?  Can anyone instruct me how to test this; or do a test
spin for me; or claim that it looks ok, and we'll fix problems as
they arise?  :-)

-- 
Pedro Alves

[-- Attachment #2: monitor_always_a_thread.diff --]
[-- Type: text/x-diff, Size: 5477 bytes --]

2008-07-03  Pedro Alves  <pedro@codesourcery.com>

	* monitor (monitor_ptid): New global.
	(monitor_open): Silently add the main task to GDB's thread list.
	(monitor_close, monitor_mourn_inferior): Silently delete the main
	task from GDB's thread list.
	(monitor_thread_alive, monitor_pid_to_str): New.
	(init_base_monitor_ops): Register monitor_thread_alive and
	monitor_pid_to_str.
	(_initialize_remote_monitors): Initialize monitor_ptid.

	* gdbthread.h (delete_thread_silent): Declare.
	* thread.c (delete_thread): Rename to ...
	(delete_thread_1): ... this.  Add "silent" parameter.  If silent,
	don't do exit notifications.
	(delete_thread, delete_thread_silent): New, as wrappers to
	delete_thread_1.

---
 gdb/gdbthread.h |    5 +++++
 gdb/monitor.c   |   49 ++++++++++++++++++++++++++++++++++++++++++++++++-
 gdb/thread.c    |   21 ++++++++++++++++++---
 3 files changed, 71 insertions(+), 4 deletions(-)

Index: src/gdb/monitor.c
===================================================================
--- src.orig/gdb/monitor.c	2008-07-03 16:15:04.000000000 +0100
+++ src/gdb/monitor.c	2008-07-03 16:21:04.000000000 +0100
@@ -106,6 +106,13 @@ static int dump_reg_flag;	/* Non-zero me
 static int first_time = 0;	/* is this the first time we're executing after 
 				   gaving created the child proccess? */
 
+
+/* This is the ptid we use while we're connected to a monitor.  It's
+   value is arbitrary, as monitor targets don't have a notion or
+   processes or thread, but we need something non-null to place in
+   inferior_ptid.  */
+static ptid_t monitor_ptid;
+
 #define TARGET_BUF_SIZE 2048
 
 /* Monitor specific debugging information.  Typically only useful to
@@ -808,7 +815,9 @@ monitor_open (char *args, struct monitor
   /* Start afresh.  */
   init_thread_list ();
 
-  inferior_ptid = pid_to_ptid (42000);	/* Make run command think we are busy... */
+  /* Make run command think we are busy...  */
+  inferior_ptid = monitor_ptid;
+  add_thread_silent (inferior_ptid);
 
   /* Give monitor_wait something to read */
 
@@ -834,6 +843,8 @@ monitor_close (int quitting)
     }
 
   monitor_desc = NULL;
+
+  delete_thread_silent (monitor_ptid);
 }
 
 /* Terminate the open connection to the remote debugger.  Use this
@@ -2002,6 +2013,7 @@ static void
 monitor_mourn_inferior (void)
 {
   unpush_target (targ_ops);
+  delete_thread_silent (monitor_ptid);
   generic_mourn_inferior ();	/* Do all the proper things now */
 }
 
@@ -2215,6 +2227,35 @@ monitor_get_dev_name (void)
   return dev_name;
 }
 
+/* Check to see if a thread is still alive.  */
+
+static int
+monitor_thread_alive (ptid_t ptid)
+{
+  if (ptid_equal (ptid, monitor_ptid))
+    /* The monitor's task is always alive.  */
+    return 1;
+
+  return 0;
+}
+
+/* Convert a thread ID to a string.  Returns the string in a static
+   buffer.  */
+
+static char *
+monitor_pid_to_str (ptid_t ptid)
+{
+  static char buf[64];
+
+  if (ptid_equal (monitor_ptid, ptid))
+    {
+      xsnprintf (buf, sizeof buf, "Thread <main>");
+      return buf;
+    }
+
+  return normal_pid_to_str (ptid);
+}
+
 static struct target_ops monitor_ops;
 
 static void
@@ -2238,6 +2279,8 @@ init_base_monitor_ops (void)
   monitor_ops.to_stop = monitor_stop;
   monitor_ops.to_rcmd = monitor_rcmd;
   monitor_ops.to_log_command = serial_log_command;
+  monitor_ops.to_thread_alive = monitor_thread_alive;
+  monitor_ops.to_pid_to_str = monitor_pid_to_str;
   monitor_ops.to_stratum = process_stratum;
   monitor_ops.to_has_all_memory = 1;
   monitor_ops.to_has_memory = 1;
@@ -2282,4 +2325,8 @@ is displayed."),
 			    NULL,
 			    NULL, /* FIXME: i18n: */
 			    &setdebuglist, &showdebuglist);
+
+  /* Yes, 42000 is arbitrary.  The only sense out of it, is that it
+     isn't 0.  */
+  monitor_ptid = ptid_build (42000, 0, 42000);
 }
Index: src/gdb/gdbthread.h
===================================================================
--- src.orig/gdb/gdbthread.h	2008-07-03 16:15:04.000000000 +0100
+++ src/gdb/gdbthread.h	2008-07-03 16:15:06.000000000 +0100
@@ -91,6 +91,11 @@ extern struct thread_info *add_thread_wi
 /* Delete an existing thread list entry.  */
 extern void delete_thread (ptid_t);
 
+/* Delete an existing thread list entry, and be quiet about it.  Used
+   after the process this thread having belonged to having already
+   exited, for example.  */
+extern void delete_thread_silent (ptid_t);
+
 /* Delete a step_resume_breakpoint from the thread database. */
 extern void delete_step_resume_breakpoint (void *);
 
Index: src/gdb/thread.c
===================================================================
--- src.orig/gdb/thread.c	2008-07-03 16:15:04.000000000 +0100
+++ src/gdb/thread.c	2008-07-03 16:15:06.000000000 +0100
@@ -153,8 +153,10 @@ add_thread (ptid_t ptid)
   return add_thread_with_info (ptid, NULL);
 }
 
-void
-delete_thread (ptid_t ptid)
+/* Delete thread PTID.  If SILENT, don't notify the observer of this
+   exit.  */
+static void
+delete_thread_1 (ptid_t ptid, int silent)
 {
   struct thread_info *tp, *tpprev;
 
@@ -172,11 +174,24 @@ delete_thread (ptid_t ptid)
   else
     thread_list = tp->next;
 
-  observer_notify_thread_exit (tp);
+  if (!silent)
+    observer_notify_thread_exit (tp);
 
   free_thread (tp);
 }
 
+void
+delete_thread (ptid_t ptid)
+{
+  delete_thread_1 (ptid, 0 /* not silent */);
+}
+
+void
+delete_thread_silent (ptid_t ptid)
+{
+  delete_thread_1 (ptid, 1 /* silent */);
+}
+
 static struct thread_info *
 find_thread_id (int num)
 {

Make remote-sim target always have a thread

[Pedro Alves]

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

A Thursday 03 July 2008 16:23:29, Pedro Alves wrote:
> This patch makes the monitor targets always register an execution control
> task in GDB's thread list.  Needed to clean up inferior control, and
> get rid of context switching.
>
> I confirmed that it builds with x86_64-unknown-linux-gnu x h8300-elf cross,
> and that it doesn't introduce any new warnings (this file is built without
> -Werror).
>
> As I said in my previous email, I don't even know how to test this,
> although I don't except to be any fallout.
>
> Does it look OK?  Can anyone instruct me how to test this; or do a test
> spin for me; or claim that it looks ok, and we'll fix problems as
> they arise?  :-)

This patch does the same to remote-sim.

(applies on top of the monitor patch)

Managed to get myself an arm-elf toolchain and tested with a
x86_64-unknow-linux-gnu x arm-elf, --target_board=arm-sim,
without regressions.  Does it look OK?

Default results look much better than I was expecting without
tweaking the boardfile.

FAIL: gdb.base/args.exp: argc for one empty
FAIL: gdb.base/args.exp: argv[2] for one empty
FAIL: gdb.base/args.exp: argv[3] for one empty
FAIL: gdb.base/args.exp: argc for two empty
FAIL: gdb.base/args.exp: argv[2] for two empty
FAIL: gdb.base/args.exp: argv[3] for two empty
FAIL: gdb.base/args.exp: argv[4] for two empty
FAIL: gdb.base/args.exp: argv[2] for one empty (with single quotes)
FAIL: gdb.base/args.exp: argv[2] for two empty (with single quotes)
FAIL: gdb.base/args.exp: argv[3] for two empty (with single quotes)
FAIL: gdb.base/chng-syms.exp: running with invalidated bpt condition after 
executable changes
FAIL: gdb.base/fileio.exp: Creating already existing file returns EEXIST
FAIL: gdb.base/fileio.exp: Open directory for writing returns EISDIR
FAIL: gdb.base/fileio.exp: Open for write but no write permission returns 
EACCES
FAIL: gdb.base/fileio.exp: Writing to a file
FAIL: gdb.base/fileio.exp: Write using invalid file descriptor returns EBADF
FAIL: gdb.base/fileio.exp: Reading from a file
FAIL: gdb.base/fileio.exp: Read using invalid file descriptor returns EBADF
FAIL: gdb.base/fileio.exp: Lseeking END a file
FAIL: gdb.base/fileio.exp: Closing an invalid file descriptor returns EBADF
FAIL: gdb.base/fileio.exp: Stat a file
FAIL: gdb.base/fileio.exp: Fstat an open file
FAIL: gdb.base/fileio.exp: Fstat an invalid file descriptor returns EBADF
FAIL: gdb.base/fileio.exp: Isatty (invalid fd)
FAIL: gdb.base/fileio.exp: Isatty (open file)
FAIL: gdb.base/fileio.exp: System says shell is available
FAIL: gdb.base/fileio.exp: System(3) call
FAIL: gdb.base/fileio.exp: System with invalid command returns 127
FAIL: gdb.base/fileio.exp: Rename a file
FAIL: gdb.base/fileio.exp: Renaming a file to existing directory returns 
EISDIR
FAIL: gdb.base/fileio.exp: Renaming a directory to a non-empty directory 
returns ENOTEMPTY or EEXIST
FAIL: gdb.base/fileio.exp: Renaming a directory to a subdir of itself returns 
EINVAL
FAIL: gdb.base/fileio.exp: Renaming a nonexistant file returns ENOENT
FAIL: gdb.base/fileio.exp: Unlinking a file in a directory w/o write access 
returns EACCES
FAIL: gdb.base/fileio.exp: Unlinking a nonexistant file returns ENOENT

# of expected passes            2493
# of unexpected failures        35
# of expected failures          3
# of untested testcases         3
# of unsupported tests          39

(only board tweakable failures, cool)

-- 
Pedro Alves

[-- Attachment #2: remote_sim_always_a_thread.diff --]
[-- Type: text/x-diff, Size: 5105 bytes --]

2008-07-03  Pedro Alves  <pedro@codesourcery.com>

	* remote-sim.c: Include gdbthread.h.
	(remote_sim_ptid): New global.
	(gdbsim_create_inferior): Silently add the main task to GDB's
	thread list.
	(gdbsim_close, gdbsim_mourn_inferior): Silently delete the main
	task from GDB's thread list.
	(gdbsim_resume): Adjust to use remote_sim_ptid.
	(gdbsim_thread_alive, gdbsim_pid_to_str): New.
	(init_gdbsim_ops): Register gdbsim_thread_alive and
	gdbsim_pid_to_str.
	(_initialize_remote_sim): Initialize remote_sim_ptid.
	* Makefile.in (remote-sim.o): Depend on $(gdbthread_h).

---
 gdb/Makefile.in  |    2 +-
 gdb/remote-sim.c |   54 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 52 insertions(+), 4 deletions(-)

Index: src/gdb/remote-sim.c
===================================================================
--- src.orig/gdb/remote-sim.c	2008-07-03 18:21:37.000000000 +0100
+++ src/gdb/remote-sim.c	2008-07-03 18:47:41.000000000 +0100
@@ -41,6 +41,7 @@
 #include "sim-regno.h"
 #include "arch-utils.h"
 #include "readline/readline.h"
+#include "gdbthread.h"
 
 /* Prototypes */
 
@@ -115,6 +116,12 @@ static int program_loaded = 0;
    back to the other sim_foo routines.  */
 static SIM_DESC gdbsim_desc = 0;
 
+/* This is the ptid we use while we're connected to the simulator.
+   It's value is arbitrary, as the simulator target don't have a
+   notion or processes or threads, but we need something non-null to
+   place in inferior_ptid.  */
+static ptid_t remote_sim_ptid;
+
 static void
 dump_mem (char *buf, int len)
 {
@@ -452,7 +459,8 @@ gdbsim_create_inferior (char *exec_file,
 		     (exec_file ? exec_file : "(NULL)"),
 		     args);
 
-  gdbsim_kill ();
+  if (ptid_equal (inferior_ptid, remote_sim_ptid))
+    gdbsim_kill ();
   remove_breakpoints ();
   init_wait_for_inferior ();
 
@@ -471,7 +479,9 @@ gdbsim_create_inferior (char *exec_file,
     argv = NULL;
   sim_create_inferior (gdbsim_desc, exec_bfd, argv, env);
 
-  inferior_ptid = pid_to_ptid (42);
+  inferior_ptid = remote_sim_ptid;
+  add_thread_silent (inferior_ptid);
+
   target_mark_running (&gdbsim_ops);
   insert_breakpoints ();	/* Needed to get correct instruction in cache */
 
@@ -579,6 +589,8 @@ gdbsim_close (int quitting)
     }
 
   end_callbacks ();
+  if (!ptid_equal (inferior_ptid, null_ptid))
+    delete_thread_silent (inferior_ptid);
   generic_mourn_inferior ();
 }
 
@@ -612,7 +624,7 @@ static int resume_step;
 static void
 gdbsim_resume (ptid_t ptid, int step, enum target_signal siggnal)
 {
-  if (PIDGET (inferior_ptid) != 42)
+  if (!ptid_equal (inferior_ptid, remote_sim_ptid))
     error (_("The program is not being run."));
 
   if (remote_debug)
@@ -815,6 +827,7 @@ gdbsim_mourn_inferior (void)
   if (remote_debug)
     printf_filtered ("gdbsim_mourn_inferior:\n");
 
+  delete_thread_silent (inferior_ptid);
   remove_breakpoints ();
   target_mark_exited (&gdbsim_ops);
   generic_mourn_inferior ();
@@ -849,6 +862,35 @@ simulator_command (char *args, int from_
   registers_changed ();
 }
 
+/* Check to see if a thread is still alive.  */
+
+static int
+gdbsim_thread_alive (ptid_t ptid)
+{
+  if (ptid_equal (ptid, remote_sim_ptid))
+    /* The simulators' task is always alive.  */
+    return 1;
+
+  return 0;
+}
+
+/* Convert a thread ID to a string.  Returns the string in a static
+   buffer.  */
+
+static char *
+gdbsim_pid_to_str (ptid_t ptid)
+{
+  static char buf[64];
+
+  if (ptid_equal (remote_sim_ptid, ptid))
+    {
+      xsnprintf (buf, sizeof buf, "Thread <main>");
+      return buf;
+    }
+
+  return normal_pid_to_str (ptid);
+}
+
 /* Define the target subroutine names */
 
 struct target_ops gdbsim_ops;
@@ -876,6 +918,8 @@ init_gdbsim_ops (void)
   gdbsim_ops.to_create_inferior = gdbsim_create_inferior;
   gdbsim_ops.to_mourn_inferior = gdbsim_mourn_inferior;
   gdbsim_ops.to_stop = gdbsim_stop;
+  gdbsim_ops.to_thread_alive = gdbsim_thread_alive;
+  gdbsim_ops.to_pid_to_str = gdbsim_pid_to_str;
   gdbsim_ops.to_stratum = process_stratum;
   gdbsim_ops.to_has_all_memory = 1;
   gdbsim_ops.to_has_memory = 1;
@@ -897,4 +941,8 @@ _initialize_remote_sim (void)
 
   add_com ("sim", class_obscure, simulator_command,
 	   _("Send a command to the simulator."));
+
+  /* Yes, 42000 is arbitrary.  The only sense out of it, is that it
+     isn't 0.  */
+  remote_sim_ptid = ptid_build (42000, 0, 42000);
 }
Index: src/gdb/Makefile.in
===================================================================
--- src.orig/gdb/Makefile.in	2008-07-03 18:21:37.000000000 +0100
+++ src/gdb/Makefile.in	2008-07-03 18:24:21.000000000 +0100
@@ -2671,7 +2671,7 @@ remote-sim.o: remote-sim.c $(defs_h) $(i
 	$(gdb_string_h) $(terminal_h) $(target_h) $(gdbcore_h) \
 	$(gdb_callback_h) $(gdb_remote_sim_h) $(command_h) \
 	$(regcache_h) $(gdb_assert_h) $(sim_regno_h) $(arch_utils_h) \
-	$(readline_h)
+	$(readline_h) $(gdbthread_h)
 rs6000-nat.o: rs6000-nat.c $(defs_h) $(inferior_h) $(target_h) $(gdbcore_h) \
 	$(xcoffsolib_h) $(symfile_h) $(objfiles_h) $(libbfd_h) $(bfd_h) \
 	$(exceptions_h) $(gdb_stabs_h) $(regcache_h) $(arch_utils_h) \

fix load command bug [was : Re: Make remote-sim target always have a thread]

[Pedro Alves]

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

A Thursday 03 July 2008 19:04:20, Pedro Alves wrote:

> Managed to get myself an arm-elf toolchain and tested with a
> x86_64-unknow-linux-gnu x arm-elf, --target_board=arm-sim,
> without regressions.  Does it look OK?
>
> Default results look much better than I was expecting without
> tweaking the boardfile.
>

> FAIL: gdb.base/chng-syms.exp: running with invalidated bpt condition after
> executable changes

Actually, looking back, this one was hiding in the middle.
It's a real bug.

1708    static void
1709    load_command (char *arg, int from_tty)
1710    {
1711      if (arg == NULL)
1712        {
1713          char *parg;
1714          int count = 0;
1715
1716          parg = arg = get_exec_file (1);
1717
1718          /* Count how many \ " ' tab space there are in the name.  */
1719          while ((parg = strpbrk (parg, "\\\"'\t ")))
1720            {
1721              parg++;
1722              count++;
1723            }
1724
1725          if (count)
1726            {
1727              /* We need to quote this string so buildargv can pull it apart.  */
1728              char *temp = xmalloc (strlen (arg) + count + 1 );
1729              char *ptemp = temp;
1730              char *prev;
1731
1732              make_cleanup (xfree, temp);
1733
1734              prev = parg = arg;
1735              while ((parg = strpbrk (parg, "\\\"'\t ")))
1736                {
1737                  strncpy (ptemp, prev, parg - prev);
1738                  ptemp += parg - prev;
1739                  prev = parg++;
1740                  *ptemp++ = '\\';
1741                }
1742              strcpy (ptemp, prev);
1743
1744              arg = temp;
1745            }
1746        }
1747
1748      /* The user might be reloading because the binary has changed.  Take
1749         this opportunity to check.  */
1750      reopen_exec_file ();
1751      reread_symbols ();
1752
1753      target_load (arg, from_tty);
1754
1755      /* After re-loading the executable, we don't really know which
1756         overlays are mapped any more.  */
1757      overlay_cache_invalid = 1;
1758    }


 Executing on host: arm-elf-gcc ../../../src/gdb/testsuite/gdb.base/chng-syms.c 
gdb_tg.o  -DVARIABLE=var2      -g  -Wl,-wrap,exit -Wl,-wrap,_exit -Wl,-wrap,main -Wl,-wrap,abort -lm   -o /home/pedro/gdb/monitor_always_a_thread/build_arm-elf/gdb/testsuite/gdb.base/chng-syms   
(timeout = 300)
 target sim
 Connected to the simulator.
 (gdb) load
 `/home/pedro/gdb/monitor_always_a_thread/build_arm-elf/gdb/testsuite/gdb.base/chng-syms' has changed; re-reading symbols.
 warning: failed to reevaluate condition for breakpoint 1: No symbol "var1" in current context.
 gdbsim: can't open "": No such file or directory
 unable to load program
 (gdb) run
 Starting program: /home/pedro/gdb/monitor_always_a_thread/build_arm-elf/gdb/testsuite/gdb.base/chng-syms
 warning: No program loaded.

 *** EXIT code 0

 Program exited normally.
 (gdb) FAIL: gdb.base/chng-syms.exp: running with invalidated bpt condition after executable changes

Notice the 'can't open ""' part.

Sometimes I'd get "#33sfads" kind of garbage instead of "".
Surprisingly, I can't seem to reproduce it that easilly now that I
fixed the issue.  The same heap address is probably being reused,
and the file name didn't change, so it goes unnoticed many times.

The issue is that reopen_exec_file destroys the previous exec_file
memory, which invalidates the arg that is passed into target_load.

The fix it to move the reopening of the exec file to the top of load_command.

The other problem is that on the sym target, the 
warning: failed to reevaluate condition for breakpoint 1: No symbol "var1" in current context.

notice is printed on load, but not after a run, as after a run is issued,
the inferior doesn't stop until it exits.

So, I've included a tweak to the test file to allow this.  If the inferior runs
to completion, it means the breakpoint wasn't sucessfully reset, and that's
what is expected in this test to happen.

Test on arm-elf, and x86_64-unknown-linux-gnu

OK?

-- 
Pedro Alves

[-- Attachment #2: fix_load_reload_syms.diff --]
[-- Type: text/x-diff, Size: 2131 bytes --]

gdb/
2008-07-04  Pedro Alves  <pedro@codesourcery.com>

	* symfile.c (load_command): Reopen the exec file and reread
	symbols before anything else.

gdb/testsuite/
2008-07-04  Pedro Alves  <pedro@codesourcery.com>

	* gdb.base/chng-syms.exp: Don't expect "No symbol ...".

---
 gdb/symfile.c                        |   10 +++++-----
 gdb/testsuite/gdb.base/chng-syms.exp |    4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

Index: src/gdb/symfile.c
===================================================================
--- src.orig/gdb/symfile.c	2008-07-04 15:53:26.000000000 +0100
+++ src/gdb/symfile.c	2008-07-04 15:53:48.000000000 +0100
@@ -1708,6 +1708,11 @@ find_sym_fns (bfd *abfd)
 static void
 load_command (char *arg, int from_tty)
 {
+  /* The user might be reloading because the binary has changed.  Take
+     this opportunity to check.  */
+  reopen_exec_file ();
+  reread_symbols ();
+
   if (arg == NULL)
     {
       char *parg;
@@ -1745,11 +1750,6 @@ load_command (char *arg, int from_tty)
 	}
     }
 
-  /* The user might be reloading because the binary has changed.  Take
-     this opportunity to check.  */
-  reopen_exec_file ();
-  reread_symbols ();
-
   target_load (arg, from_tty);
 
   /* After re-loading the executable, we don't really know which
Index: src/gdb/testsuite/gdb.base/chng-syms.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/chng-syms.exp	2008-07-04 15:53:26.000000000 +0100
+++ src/gdb/testsuite/gdb.base/chng-syms.exp	2008-07-04 15:53:48.000000000 +0100
@@ -105,10 +105,10 @@ if  { [gdb_compile "${srcdir}/${subdir}/
 
     gdb_run_cmd
     gdb_expect {
-	-re ".*No symbol .var1..*Program exited normally.*$gdb_prompt $" {
+	-re ".*Program exited normally.*$gdb_prompt $" {
 	    pass "running with invalidated bpt condition after executable changes" 
 	}
-	-re "No symbol .var1..*Breakpoint .*,( 0x.* in)? (\[^ \]*)exit .*$gdb_prompt $" {
+	-re ".*Breakpoint .*,( 0x.* in)? (\[^ \]*)exit .*$gdb_prompt $" {
 	    pass "running with invalidated bpt condition after executable changes" 
 	}
 	-re "$gdb_prompt $" { 

Re: Make monitor targets always have a thread

[Daniel Jacobowitz]

On Thu, Jul 03, 2008 at 04:23:29PM +0100, Pedro Alves wrote:
> Does it look OK?  Can anyone instruct me how to test this; or do a test
> spin for me; or claim that it looks ok, and we'll fix problems as
> they arise?  :-)

It's OK.  I don't have any way to test monitor either.
> +/* This is the ptid we use while we're connected to a monitor.  It's

Its

> +   value is arbitrary, as monitor targets don't have a notion or

notion of

> +   processes or thread, but we need something non-null to place in

threads

-- 
Daniel Jacobowitz
CodeSourcery

Re: Make remote-sim target always have a thread

[Daniel Jacobowitz]

On Thu, Jul 03, 2008 at 07:04:20PM +0100, Pedro Alves wrote:
> Default results look much better than I was expecting without
> tweaking the boardfile.

If you'd like, I believe arm*-*-eabi and arm*-*-elf should XFAIL the
args.exp and fileio.exp failures.  I believe they're inherent; ARM
defined the semihosting interface (long ago), and it does not include
the necessary bits for some of this.  For instance, the command line
is passed as a string.  It's possible some of the fileio.exp failures
are bugs in the simulator though - I haven't checked them exhaustively.

> +/* This is the ptid we use while we're connected to the simulator.
> +   It's value is arbitrary, as the simulator target don't have a
> +   notion or processes or threads, but we need something non-null to
> +   place in inferior_ptid.  */
> +static ptid_t remote_sim_ptid;

Same spelling errors as last patch.
> +    /* The simulators' task is always alive.  */

simulator's

OK.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Make monitor targets always have a thread

[Stan Shebs]

Daniel Jacobowitz wrote:
> On Thu, Jul 03, 2008 at 04:23:29PM +0100, Pedro Alves wrote:
>   
>> Does it look OK?  Can anyone instruct me how to test this; or do a test
>> spin for me; or claim that it looks ok, and we'll fix problems as
>> they arise?  :-)
>>     
>
> It's OK.  I don't have any way to test monitor either.
>   
Yeah, I wonder how long it's been since any of those monitor targets 
have been tested, they seem to be bordering on obsolescence.

Stan

Re: fix load command bug [was : Re: Make remote-sim target always have a thread]

[Daniel Jacobowitz]

On Fri, Jul 04, 2008 at 03:59:41PM +0100, Pedro Alves wrote:
> gdb/
> 2008-07-04  Pedro Alves  <pedro@codesourcery.com>
> 
> 	* symfile.c (load_command): Reopen the exec file and reread
> 	symbols before anything else.

This part is OK.

> gdb/testsuite/
> 2008-07-04  Pedro Alves  <pedro@codesourcery.com>
> 
> 	* gdb.base/chng-syms.exp: Don't expect "No symbol ...".

This is unfortunate, since we want to verify that the user was
informed.  But I can't think of another way :-(  OK.


-- 
Daniel Jacobowitz
CodeSourcery

Re: Make monitor targets always have a thread

[Daniel Jacobowitz]

On Mon, Jul 07, 2008 at 12:16:33PM -0700, Stan Shebs wrote:
> Daniel Jacobowitz wrote:
>> On Thu, Jul 03, 2008 at 04:23:29PM +0100, Pedro Alves wrote:
>>   
>>> Does it look OK?  Can anyone instruct me how to test this; or do a test
>>> spin for me; or claim that it looks ok, and we'll fix problems as
>>> they arise?  :-)
>>>     
>>
>> It's OK.  I don't have any way to test monitor either.
>>   
> Yeah, I wonder how long it's been since any of those monitor targets have 
> been tested, they seem to be bordering on obsolescence.

In theory, someone reported success during my round of code deletion
(~ 18 months ago).  I think it was Maciej, but I don't remember.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Make monitor targets always have a thread

[Stan Shebs]

Daniel Jacobowitz wrote:
> On Mon, Jul 07, 2008 at 12:16:33PM -0700, Stan Shebs wrote:
>   
>> Daniel Jacobowitz wrote:
>>     
>>> On Thu, Jul 03, 2008 at 04:23:29PM +0100, Pedro Alves wrote:
>>>   
>>>       
>>>> Does it look OK?  Can anyone instruct me how to test this; or do a test
>>>> spin for me; or claim that it looks ok, and we'll fix problems as
>>>> they arise?  :-)
>>>>     
>>>>         
>>> It's OK.  I don't have any way to test monitor either.
>>>   
>>>       
>> Yeah, I wonder how long it's been since any of those monitor targets have 
>> been tested, they seem to be bordering on obsolescence.
>>     
>
> In theory, someone reported success during my round of code deletion
> (~ 18 months ago).  I think it was Maciej, but I don't remember.
>   
It occurs to me that one could construct a testing harness by loading a 
ROM monitor binary into an appropriately configured simulator, but 
that's way more work and trouble than these bits are worth. :-)

Stan

[ob] don't skip most of the testsuite... [was: Re: Make remote-sim target always have a thread]

[Pedro Alves]

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

A Monday 07 July 2008 20:15:18, Daniel Jacobowitz wrote:
> On Thu, Jul 03, 2008 at 07:04:20PM +0100, Pedro Alves wrote:
> > Default results look much better than I was expecting without
> > tweaking the boardfile.

Damn, I keep losing my bonus points.

I looked back as I was going to commit the patches, and noticed
that every test that ran after fullname.exp was being skipped, like
so...

 Running ../../../src/gdb/testsuite/gdb.base/fullname.exp ...
 gdb compile failed, arm-elf-gcc: gdb_tg.o: No such file or directory
 Running ../../../src/gdb/testsuite/gdb.base/funcargs.exp ...
 ERROR: ../../../src/gdb/testsuite/gdb.base/funcargs.exp does not exist.
 ERROR: ...
 ...

No wonder I'd got such a clean testsuite run.

The compile failure seems to happen because the gluefile is passed
a relative path to the testsuite/ dir, but this test explicitly
changed dirs:

 Executing on host: arm-elf-gcc tmp-fullname.c       -g  -Wl,-wrap,exit -Wl,-wrap,_exit -Wl,-wrap,main -Wl,-wrap,abort gdb_tg.o -lm   -o fullname    (timeout = 300)
 arm-elf-gcc: gdb_tg.o: No such file or directory
 compiler exited with status 1
 output is:
 arm-elf-gcc: gdb_tg.o: No such file or directory

Fixing this bit will be for a rainy day, but, skipping most of
the testsuite has an obvious fix.  See attached, checked in.

I get to keep my bonus, since no one noticed how it must have
been too good to be true to have no other failures :-)

-- 
Pedro Alves

[-- Attachment #2: fullname.diff --]
[-- Type: text/x-diff, Size: 700 bytes --]

2008-07-09  Pedro Alves  <pedro@codesourcery.com>

	* gdb.base/fullname.exp: Restore pwd if compiling failed.

---
 gdb/testsuite/gdb.base/fullname.exp |    1 +
 1 file changed, 1 insertion(+)

Index: src/gdb/testsuite/gdb.base/fullname.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/fullname.exp	2008-07-09 00:05:42.000000000 +0100
+++ src/gdb/testsuite/gdb.base/fullname.exp	2008-07-09 00:10:42.000000000 +0100
@@ -104,6 +104,7 @@ if { [gdb_breakpoint ${objdir}/${subdir}
 set save_pwd [pwd]
 cd ${subdir}
 if  { [gdb_compile "tmp-${srcfile}" "${testfile}" executable {debug}] != "" } {
+    cd $save_pwd
     return -1
 }
 cd $save_pwd

Re: Make remote-sim target always have a thread

[Pedro Alves]

A Monday 07 July 2008 20:15:18, Daniel Jacobowitz wrote:
> On Thu, Jul 03, 2008 at 07:04:20PM +0100, Pedro Alves wrote:
> > Default results look much better than I was expecting without
> > tweaking the boardfile.
>
> If you'd like, I believe arm*-*-eabi and arm*-*-elf should XFAIL the
> args.exp and fileio.exp failures.  I believe they're inherent; ARM
> defined the semihosting interface (long ago), and it does not include
> the necessary bits for some of this.  For instance, the command line
> is passed as a string.  It's possible some of the fileio.exp failures
> are bugs in the simulator though - I haven't checked them exhaustively.

I'd prefer to investigate better the failures than XFAILing them
blindly.  Not all the args.exp tests fail for instance, and I just
noticed that there are more failures in other tests than I
originally reported.  I'll have to pass for now, and defer to someone
who's more caring for the sim target.

> > +/* This is the ptid we use while we're connected to the simulator.
> > +   It's value is arbitrary, as the simulator target don't have a
> > +   notion or processes or threads, but we need something non-null to
> > +   place in inferior_ptid.  */
> > +static ptid_t remote_sim_ptid;
>
> Same spelling errors as last patch.
>
> > +    /* The simulators' task is always alive.  */
>
> simulator's
>
> OK.

Thanks, I've checked it in with those changed, after rerunning
the testsuite (without skipping most tests...).  There were
no regressions (actually fixed some MI issues than I posted about
a couple of emails ago).

-- 
Pedro Alves

Re: fix load command bug [was : Re: Make remote-sim target always have a thread]

[Pedro Alves]

A Monday 07 July 2008 20:19:49, Daniel Jacobowitz wrote:
> On Fri, Jul 04, 2008 at 03:59:41PM +0100, Pedro Alves wrote:
> > gdb/
> > 2008-07-04  Pedro Alves  <pedro@codesourcery.com>
> >
> > 	* symfile.c (load_command): Reopen the exec file and reread
> > 	symbols before anything else.
>
> This part is OK.
>
> > gdb/testsuite/
> > 2008-07-04  Pedro Alves  <pedro@codesourcery.com>
> >
> > 	* gdb.base/chng-syms.exp: Don't expect "No symbol ...".
>
> This is unfortunate, since we want to verify that the user was
> informed.  But I can't think of another way :-(  OK.

Yeah, same sentiment here :-(

I've checked it in.

-- 
Pedro Alves

Re: [ob] don't skip most of the testsuite... [was: Re: Make remote-sim target always have a thread]

[Daniel Jacobowitz]

On Wed, Jul 09, 2008 at 11:52:43AM +0100, Pedro Alves wrote:
> @@ -104,6 +104,7 @@ if { [gdb_breakpoint ${objdir}/${subdir}
>  set save_pwd [pwd]
>  cd ${subdir}
>  if  { [gdb_compile "tmp-${srcfile}" "${testfile}" executable {debug}] != "" } {
> +    cd $save_pwd
>      return -1
>  }
>  cd $save_pwd

Thanks.  If you want those bonus points back, you can get them by only
compiling to object instead of executable in this command; link in a
separate gdb_compile invocation.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Make remote-sim target always have a thread

[Daniel Jacobowitz]

On Wed, Jul 09, 2008 at 12:11:15PM +0100, Pedro Alves wrote:
> A Monday 07 July 2008 20:15:18, Daniel Jacobowitz wrote:
> > If you'd like, I believe arm*-*-eabi and arm*-*-elf should XFAIL the
> > args.exp and fileio.exp failures.  I believe they're inherent; ARM
> > defined the semihosting interface (long ago), and it does not include
> > the necessary bits for some of this.  For instance, the command line
> > is passed as a string.  It's possible some of the fileio.exp failures
> > are bugs in the simulator though - I haven't checked them exhaustively.
> 
> I'd prefer to investigate better the failures than XFAILing them
> blindly.  Not all the args.exp tests fail for instance, and I just
> noticed that there are more failures in other tests than I
> originally reported.  I'll have to pass for now, and defer to someone
> who's more caring for the sim target.

It shows up in hardware ARM testing too.  The args.exp failures are
because the command-line-as-string bits do not handle single and
double quotes in the way GDB expects (i.e. the way a Bourne shell
does); so tests for empty arguments in particular do not work.

The fileio tests, yes, those should be checked more precisely.

Thanks for fixing up sim, anyway :-)

-- 
Daniel Jacobowitz
CodeSourcery