remove pop_target

This patch fixes the target double-close problem (PR remote/15266),
and in the process removes pop_target entire (PR remote/15256).

The first issue is that pop_target calls target_close.  However, it
then calls unpush_target, which also calls target_close.  This means
targets must be able to be closed twice.  Not only is this strange,
but it also directly contradicts the contract of to_xclose targets.
(We currently have just a single such target, and it is never pushed;
but I plan to add more, and so this latent bug is triggered.)

The second issue is that it seems to me that calling pop_target is
often unsafe.  This is what cropped up in 15256, where the remote
target assumed that it could pop_target -- but there was another
target higher on the stack, leading to confusion.

But, it is always just as easy to call unpush_target as it is to call
pop_target; and it is also safer.  So, removing pop_target seemed like
an improvement.

Finally, this adds an assertion to target_close to ensure that no
currently-pushed target can be closed.

Built and regtested on x86-64 Fedora 18; both natively and using the
native-gdbserver board file.

	PR remote/15256, PR remote/15266:
	* bfd-target.c (target_bfd_reopen): Initialize to_magic.
	* monitor.c (monitor_detach): Use unpush_target.
	* remote-m32r-sdi.c (m32r_detach): Use unpush_target.
	* remote-mips.c (mips_detach): Use unpush_target.  Don't
	call mips_close.
	* remote-sim.c (gdbsim_detach): Use unpush_target.
	* target.c (pop_target): Remove.
	(pop_all_targets_above): Don't call target_close.
	(target_close): Assert that the target is unpushed.
	* target.h (pop_target): Don't declare.
	* tracepoint.c (tfile_open): Use unpush_target.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 782a2627a87a064258b97808ccb29a0760aefdec..ff9ff8708a94cba10bc9d447322db3574485d495 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,18 @@
+2013-07-25  Tom Tromey  <tromey@redhat.com>
+
+       PR remote/15256, PR remote/15266:
+       * bfd-target.c (target_bfd_reopen): Initialize to_magic.
+       * monitor.c (monitor_detach): Use unpush_target.
+       * remote-m32r-sdi.c (m32r_detach): Use unpush_target.
+       * remote-mips.c (mips_detach): Use unpush_target.  Don't
+       call mips_close.
+       * remote-sim.c (gdbsim_detach): Use unpush_target.
+       * target.c (pop_target): Remove.
+       (pop_all_targets_above): Don't call target_close.
+       (target_close): Assert that the target is unpushed.
+       * target.h (pop_target): Don't declare.
+       * tracepoint.c (tfile_open): Use unpush_target.
+
 2013-07-25  Tom Tromey  <tromey@redhat.com>
 
        * linux-thread-db.c (init_thread_db_ops): Call

diff --git a/gdb/bfd-target.c b/gdb/bfd-target.c
index a7dee242008e7ab4b82aab24e5a8321e77ea4d28..8d09831c8182f9d51edccd78506e1f84713869c1 100644 (file)
--- a/gdb/bfd-target.c
+++ b/gdb/bfd-target.c
@@ -96,6 +96,7 @@ target_bfd_reopen (struct bfd *abfd)
   t->to_xfer_partial = target_bfd_xfer_partial;
   t->to_xclose = target_bfd_xclose;
   t->to_data = data;
+  t->to_magic = OPS_MAGIC;
 
   return t;
 }

diff --git a/gdb/monitor.c b/gdb/monitor.c
index beca4e413e65365107b201bf8bebbeeca17346cd..3eb22493b7622a21c51fab0cc413c5610625ea33 100644 (file)
--- a/gdb/monitor.c
+++ b/gdb/monitor.c
@@ -877,7 +877,7 @@ monitor_close (void)
 static void
 monitor_detach (struct target_ops *ops, char *args, int from_tty)
 {
-  pop_target ();               /* calls monitor_close to do the real work.  */
+  unpush_target (ops);         /* calls monitor_close to do the real work.  */
   if (from_tty)
     printf_unfiltered (_("Ending remote %s debugging\n"), target_shortname);
 }

diff --git a/gdb/remote-m32r-sdi.c b/gdb/remote-m32r-sdi.c
index 2f910e609aa2d615bacea160203f4a0acab3f37c..81fea53955d5e773926e3cc56580813b19304296 100644 (file)
--- a/gdb/remote-m32r-sdi.c
+++ b/gdb/remote-m32r-sdi.c
@@ -886,7 +886,7 @@ m32r_detach (struct target_ops *ops, char *args, int from_tty)
   m32r_resume (ops, inferior_ptid, 0, GDB_SIGNAL_0);
 
   /* Calls m32r_close to do the real work.  */
-  pop_target ();
+  unpush_target (ops);
   if (from_tty)
     fprintf_unfiltered (gdb_stdlog, "Ending remote %s debugging\n",
                        target_shortname);

diff --git a/gdb/remote-mips.c b/gdb/remote-mips.c
index 1619622b992eadd777eb3f646ea731b4e7550ab2..13ea1467bc45b540e9d1fa21244aa9c910fc4a23 100644 (file)
--- a/gdb/remote-mips.c
+++ b/gdb/remote-mips.c
@@ -1755,9 +1755,7 @@ mips_detach (struct target_ops *ops, char *args, int from_tty)
   if (args)
     error (_("Argument given to \"detach\" when remotely debugging."));
 
-  pop_target ();
-
-  mips_close ();
+  unpush_target (ops);
 
   if (from_tty)
     printf_unfiltered ("Ending remote MIPS debugging.\n");

diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index fda373577eecc5f46978fb8973356bec9521da35..d534e566be8af1bae3d7b5d7527e28580023bb3a 100644 (file)
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -821,7 +821,7 @@ gdbsim_detach (struct target_ops *ops, char *args, int from_tty)
   if (remote_debug)
     printf_filtered ("gdbsim_detach: args \"%s\"\n", args);
 
-  pop_target ();               /* calls gdbsim_close to do the real work */
+  unpush_target (ops);         /* calls gdbsim_close to do the real work */
   if (from_tty)
     printf_filtered ("Ending simulator %s debugging\n", target_shortname);
 }

diff --git a/gdb/target.c b/gdb/target.c
index 37b0a94b0cf17fa3d01342c4caed158fa14db108..e3dcb47f514cde025d5b0c5398989c7bd969be88 100644 (file)
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1093,26 +1093,11 @@ unpush_target (struct target_ops *t)
   return 1;
 }
 
-void
-pop_target (void)
-{
-  target_close (target_stack);         /* Let it clean up.  */
-  if (unpush_target (target_stack) == 1)
-    return;
-
-  fprintf_unfiltered (gdb_stderr,
-                     "pop_target couldn't find target %s\n",
-                     current_target.to_shortname);
-  internal_error (__FILE__, __LINE__,
-                 _("failed internal consistency check"));
-}
-
 void
 pop_all_targets_above (enum strata above_stratum)
 {
   while ((int) (current_target.to_stratum) > (int) above_stratum)
     {
-      target_close (target_stack);
       if (!unpush_target (target_stack))
        {
          fprintf_unfiltered (gdb_stderr,
@@ -3781,6 +3766,8 @@ debug_to_open (char *args, int from_tty)
 void
 target_close (struct target_ops *targ)
 {
+  gdb_assert (!target_is_pushed (targ));
+
   if (targ->to_xclose != NULL)
     targ->to_xclose (targ);
   else if (targ->to_close != NULL)

diff --git a/gdb/target.h b/gdb/target.h
index 1cf198f343e0ff8ff8106422257cc49aac25c549..1d73bcd07f8758c06818447e2c2eaf0a011e14e0 100644 (file)
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1759,9 +1759,7 @@ int target_verify_memory (const gdb_byte *data,
 
    unpush_target: Remove this from the stack of currently used targets,
    no matter where it is on the list.  Returns 0 if no
-   change, 1 if removed from stack.
-
-   pop_target:   Remove the top thing on the stack of current targets.  */
+   change, 1 if removed from stack.  */
 
 extern void add_target (struct target_ops *);
 
@@ -1783,8 +1781,6 @@ extern void target_pre_inferior (int);
 
 extern void target_preopen (int);
 
-extern void pop_target (void);
-
 /* Does whatever cleanup is required to get rid of all pushed targets.  */
 extern void pop_all_targets (void);
 

diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 054372a20dfe0f408af40de5ac30be2f90f73501..8b70bd3eaf333057161554d9199c08591a519a4c 100644 (file)
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -4366,8 +4366,8 @@ tfile_open (char *filename, int from_tty)
     }
   if (ex.reason < 0)
     {
-      /* Pop the partially set up target.  */
-      pop_target ();
+      /* Remove the partially set up target.  */
+      unpush_target (&tfile_ops);
       throw_exception (ex);
     }