[patch] regression: Quit should not ask with core (PR 12071)

[patch] regression: Quit should not ask with core (PR 12071)

[Jan Kratochvil]

Hi,

my former patch
	Make core files the process_stratum.
	commit aa2614659d7863f1afe4da2251312f7da9236ea5
	http://sourceware.org/ml/gdb-cvs/2010-07/msg00107.html

has a regression as it asks while it did not in GDB-7.2:
	Regression: just a core file started to ask on quit
	http://sourceware.org/bugzilla/show_bug.cgi?id=12071

	./gdb -nx -c core -ex q
	->
	A debugging session is active.

		Inferior 1 [process 27943] will be killed.

	Quit anyway? (y or n) _

No regressions on {x86_64,x86_64-m32,i686}-fedora14snapshot-linux-gnu.


Thanks,
Jan


gdb/
2010-09-29  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* inferior.c (have_live_inferiors): Test also TO_HAS_EXECUTION.

gdb/testsuite/
2010-09-29  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/corefile.exp (quit with a process, no question: load core)
	(quit with a core file): New tests.

--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -467,7 +467,7 @@ have_live_inferiors (void)
      multiple target interfaces.  */
   if (have_inferiors ())
     for (t = current_target.beneath; t != NULL; t = t->beneath)
-      if (t->to_stratum == process_stratum)
+      if (t->to_stratum == process_stratum && t->to_has_execution (t))
 	return 1;
 
   return 0;
--- a/gdb/testsuite/gdb.base/corefile.exp
+++ b/gdb/testsuite/gdb.base/corefile.exp
@@ -201,8 +201,33 @@ gdb_test_multiple "info files" $test {
     }
 }
 
+set test "quit with a process"
+gdb_test_multiple "quit" $test {
+    -re "A debugging session is active.\r\n.*\r\nQuit anyway\\? \\(y or n\\) $" {
+	pass $test
+	gdb_test "n" {Not confirmed\.} "quit with processes: n"
+    }
+}
+
 gdb_exit
 
+# Verify there is no question if only a core file is loaded.
+
+gdb_start
+gdb_test "core-file $corefile" "Core was generated by .*" "no question: load core"
+
+set test "quit with a core file"
+gdb_test_multiple "quit" $test {
+    -re "A debugging session is active.\r\n.*\r\nQuit anyway\\? \\(y or n\\) $" {
+	fail $test
+	gdb_test "n" {Not confirmed\.} "quit with processes: n"
+    }
+    eof {
+	pass $test
+    }
+}
+
+gdb_exit
 
 # Test an attach command will clear any loaded core file.
 

Re: [patch] regression: Quit should not ask with core (PR 12071)

[Pedro Alves]

On Wednesday 29 September 2010 15:43:16, Jan Kratochvil wrote:
> @@ -467,7 +467,7 @@ have_live_inferiors (void)
>       multiple target interfaces.  */
>    if (have_inferiors ())
>      for (t = current_target.beneath; t != NULL; t = t->beneath)
> -      if (t->to_stratum == process_stratum)
> +      if (t->to_stratum == process_stratum && t->to_has_execution (t))
>         return 1;

This is about the same as removing the loop and calling "target_has_execution".
But, that wouldn't be correct either, because the to_has_execution depends
on the inferior selected (see default_child_has_execution).  So, for example,
if you have two inferiors in your gdb session, one of them is running as
a process, while the other is still just a pseudo-inferior / file_stratum
inferior yet, and you have the latter selected as current, your patch
will make that have-live-inferiors check still return 0, while there _is_
one live inferior.

The quickest fix is to iterate over all inferiors, and in turn, switch to
the iterated inferior, and check target_has_execution.  In pseudo-C:

int
have_live_inferiors (void)
{
  struct inferior *inf;

  foreach (inf in inferiors)
    {
      thread = any_thread_of_inferior (inf);
      if (thread)
        {
          switch_to_thread (thread->ptid);
          if (target_has_execution)
            return 1;
        }
    }

  return 0;
}

Want to give that a try?

-- 
Pedro Alves

Re: [patch] regression: Quit should not ask with core (PR 12071)

[Jan Kratochvil]

On Wed, 29 Sep 2010 17:04:17 +0200, Pedro Alves wrote:
> So, for example, if you have two inferiors in your gdb session, one of them
> is running as a process, while the other is still just a pseudo-inferior
> / file_stratum inferior yet, and you have the latter selected as current,
> your patch will make that have-live-inferiors check still return 0, while
> there _is_ one live inferior.

Confirming it had this problem.

No regressions on {x86_64,x86_64-m32,i686}-fedora14snapshot-linux-gnu.


Thanks,
Jan


gdb/
2010-09-29  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* inferior.c (have_live_inferiors): New variables old_chain, inf and
	tp.  Iterate INFERIOR_LIST and call target_has_execution.

Gdb/testsuite/
2010-09-29  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/corefile.exp (quit with a process, no question: load core)
	(quit with a core file): New tests.

--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -461,16 +461,29 @@ have_inferiors (void)
 int
 have_live_inferiors (void)
 {
-  struct target_ops *t;
+  struct cleanup *old_chain;
+  struct inferior *inf;
 
-  /* The check on stratum suffices, as GDB doesn't currently support
-     multiple target interfaces.  */
-  if (have_inferiors ())
-    for (t = current_target.beneath; t != NULL; t = t->beneath)
-      if (t->to_stratum == process_stratum)
-	return 1;
+  old_chain = make_cleanup_restore_current_thread ();
 
-  return 0;
+  for (inf = inferior_list; inf; inf = inf->next)
+    if (inf->pid != 0)
+      {
+	struct thread_info *tp;
+	
+	tp = any_thread_of_process (inf->pid);
+	if (tp)
+	  {
+	    switch_to_thread (tp->ptid);
+
+	    if (target_has_execution)
+	      break;
+	  }
+      }
+
+  do_cleanups (old_chain);
+
+  return inf != NULL;
 }
 
 /* Prune away automatically added program spaces that aren't required
--- a/gdb/testsuite/gdb.base/corefile.exp
+++ b/gdb/testsuite/gdb.base/corefile.exp
@@ -201,8 +201,33 @@ gdb_test_multiple "info files" $test {
     }
 }
 
+set test "quit with a process"
+gdb_test_multiple "quit" $test {
+    -re "A debugging session is active.\r\n.*\r\nQuit anyway\\? \\(y or n\\) $" {
+	pass $test
+	gdb_test "n" {Not confirmed\.} "quit with processes: n"
+    }
+}
+
 gdb_exit
 
+# Verify there is no question if only a core file is loaded.
+
+gdb_start
+gdb_test "core-file $corefile" "Core was generated by .*" "no question: load core"
+
+set test "quit with a core file"
+gdb_test_multiple "quit" $test {
+    -re "A debugging session is active.\r\n.*\r\nQuit anyway\\? \\(y or n\\) $" {
+	fail $test
+	gdb_test "n" {Not confirmed\.} "quit with processes: n"
+    }
+    eof {
+	pass $test
+    }
+}
+
+gdb_exit
 
 # Test an attach command will clear any loaded core file.
 

Re: [patch] regression: Quit should not ask with core (PR 12071)

[Pedro Alves]

On Wednesday 29 September 2010 23:11:38, Jan Kratochvil wrote:
> On Wed, 29 Sep 2010 17:04:17 +0200, Pedro Alves wrote:
> > So, for example, if you have two inferiors in your gdb session, one of them
> > is running as a process, while the other is still just a pseudo-inferior
> > / file_stratum inferior yet, and you have the latter selected as current,
> > your patch will make that have-live-inferiors check still return 0, while
> > there _is_ one live inferior.
> 
> Confirming it had this problem.
> 
> No regressions on {x86_64,x86_64-m32,i686}-fedora14snapshot-linux-gnu.

Okay, thanks.

-- 
Pedro Alves

> 
> 
> Thanks,
> Jan
> 
> 
> gdb/
> 2010-09-29  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* inferior.c (have_live_inferiors): New variables old_chain, inf and
> 	tp.  Iterate INFERIOR_LIST and call target_has_execution.
> 
> Gdb/testsuite/
> 2010-09-29  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.base/corefile.exp (quit with a process, no question: load core)
> 	(quit with a core file): New tests.
> 
> --- a/gdb/inferior.c
> +++ b/gdb/inferior.c
> @@ -461,16 +461,29 @@ have_inferiors (void)
>  int
>  have_live_inferiors (void)
>  {
> -  struct target_ops *t;
> +  struct cleanup *old_chain;
> +  struct inferior *inf;
>  
> -  /* The check on stratum suffices, as GDB doesn't currently support
> -     multiple target interfaces.  */
> -  if (have_inferiors ())
> -    for (t = current_target.beneath; t != NULL; t = t->beneath)
> -      if (t->to_stratum == process_stratum)
> -	return 1;
> +  old_chain = make_cleanup_restore_current_thread ();
>  
> -  return 0;
> +  for (inf = inferior_list; inf; inf = inf->next)
> +    if (inf->pid != 0)
> +      {
> +	struct thread_info *tp;
> +	
> +	tp = any_thread_of_process (inf->pid);
> +	if (tp)
> +	  {
> +	    switch_to_thread (tp->ptid);
> +
> +	    if (target_has_execution)
> +	      break;
> +	  }
> +      }
> +
> +  do_cleanups (old_chain);
> +
> +  return inf != NULL;
>  }
>  
>  /* Prune away automatically added program spaces that aren't required
> --- a/gdb/testsuite/gdb.base/corefile.exp
> +++ b/gdb/testsuite/gdb.base/corefile.exp
> @@ -201,8 +201,33 @@ gdb_test_multiple "info files" $test {
>      }
>  }
>  
> +set test "quit with a process"
> +gdb_test_multiple "quit" $test {
> +    -re "A debugging session is active.\r\n.*\r\nQuit anyway\\? \\(y or n\\) $" {
> +	pass $test
> +	gdb_test "n" {Not confirmed\.} "quit with processes: n"
> +    }
> +}
> +
>  gdb_exit
>  
> +# Verify there is no question if only a core file is loaded.
> +
> +gdb_start
> +gdb_test "core-file $corefile" "Core was generated by .*" "no question: load core"
> +
> +set test "quit with a core file"
> +gdb_test_multiple "quit" $test {
> +    -re "A debugging session is active.\r\n.*\r\nQuit anyway\\? \\(y or n\\) $" {
> +	fail $test
> +	gdb_test "n" {Not confirmed\.} "quit with processes: n"
> +    }
> +    eof {
> +	pass $test
> +    }
> +}
> +
> +gdb_exit
>  
>  # Test an attach command will clear any loaded core file.
>  
> 

Re: [patch] regression: Quit should not ask with core (PR 12071)

[Jan Kratochvil]

On Thu, 30 Sep 2010 11:11:48 +0200, Pedro Alves wrote:
> Okay, thanks.

Checked-in:
	http://sourceware.org/ml/gdb-cvs/2010-09/msg00181.html


Thanks,
Jan