[PATCH] PR/12691 Add the inferior to Python exited event

[PATCH] PR/12691 Add the inferior to Python exited event

[Kevin Pouget]

Following Tom's advise
(http://sourceware.org/ml/gdb/2011-04/msg00116.html), here is a patch
which hooks the inferior to the `exited' event object
let me know what you think about it


Cordially,

Kevin

2011-04-21  Kevin Pouget  <kevin.pouget@st.com>

        PR python/12691: Add the inferior to Python exited event
        * python/py-exitedevent.c (create_exited_event_object): Add inferior
        to exited_event.
        * python/py-event.h (emit_exited_event): Likewise
        * python/-inferior.c (python_inferior_exit): Likewise

2011-04-21  Kevin Pouget  <kevin.pouget@st.com>

        PR python/12691: Add the inferior to Python exited event
        * gdb.python/py-events.exp: Test the inferior attribute of exited
        event with a fork.
        * gdb.python/py-events.py: Print inferior number on exit.
        * gdb.python/py-events.c: Fork the inferior.

2011-04-21  Kevin Pouget  <kevin.pouget@st.com>

        PR python/12691: Add the inferior to Python exited event
        * gdb.texinfo (Events In Python): Describe exited inferior attribute.

--

From 181089a16134e1c6a900e42bb542adb165158ec2 Mon Sep 17 00:00:00 2001
From: Kevin Pouget <kevin.pouget@st.com>
Date: Thu, 21 Apr 2011 10:27:03 -0400
Subject: [PATCH] Add the inferior to Python exited event

---
 gdb/doc/gdb.texinfo                    |    5 ++++-
 gdb/python/py-event.h                  |    2 +-
 gdb/python/py-exitedevent.c            |   10 +++++++---
 gdb/python/py-inferior.c               |    2 +-
 gdb/testsuite/gdb.python/py-events.c   |    1 +
 gdb/testsuite/gdb.python/py-events.exp |   11 ++++++++++-
 gdb/testsuite/gdb.python/py-events.py  |    1 +
 7 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index edcf5c2..6e06bde 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22056,11 +22056,14 @@ inherited attribute refer to
@code{gdb.ThreadEvent} above.

 @item events.exited
 Emits @code{events.ExitedEvent} which indicates that the inferior has exited.
-@code{events.ExitedEvent} has one attribute:
+@code{events.ExitedEvent} has two attributes:
 @table @code
 @defivar ExitedEvent exit_code
 An integer representing the exit code which the inferior has returned.
 @end defivar
+@defivar ExitedEvent inferior
+A reference to the inferior which triggered the exited event.
+@end defivar
 @end table

 @item events.stop
diff --git a/gdb/python/py-event.h b/gdb/python/py-event.h
index bc95521..5ac73b4 100644
--- a/gdb/python/py-event.h
+++ b/gdb/python/py-event.h
@@ -104,7 +104,7 @@ typedef struct
 } event_object;

 extern int emit_continue_event (ptid_t ptid);
-extern int emit_exited_event (LONGEST exit_code);
+extern int emit_exited_event (LONGEST exit_code, struct inferior *inf);

 extern int evpy_emit_event (PyObject *event,
                             eventregistry_object *registry);
diff --git a/gdb/python/py-exitedevent.c b/gdb/python/py-exitedevent.c
index 457a4fe..7d2a512 100644
--- a/gdb/python/py-exitedevent.c
+++ b/gdb/python/py-exitedevent.c
@@ -22,7 +22,7 @@
 static PyTypeObject exited_event_object_type;

 PyObject *
-create_exited_event_object (LONGEST exit_code)
+create_exited_event_object (LONGEST exit_code, struct inferior *inf)
 {
   PyObject *exited_event;

@@ -35,6 +35,10 @@ create_exited_event_object (LONGEST exit_code)
                           "exit_code",
                           PyLong_FromLongLong (exit_code)) < 0)
     goto fail;
+  if (evpy_add_attribute (exited_event,
+                          "inferior",
+                          inferior_to_inferior_object (inf)) < 0)
+    goto fail;

   return exited_event;

@@ -47,14 +51,14 @@ create_exited_event_object (LONGEST exit_code)
    will create a new Python exited event object.  */

 int
-emit_exited_event (LONGEST exit_code)
+emit_exited_event (LONGEST exit_code, struct inferior *inf)
 {
   PyObject *event;

   if (evregpy_no_listeners_p (gdb_py_events.exited))
     return 0;

-  event = create_exited_event_object (exit_code);
+  event = create_exited_event_object (exit_code, inf);

   if (event)
     return evpy_emit_event (event, gdb_py_events.exited);
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index b9df394..19d45d7 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -123,7 +123,7 @@ python_inferior_exit (struct inferior *inf)
   exit_code = status.value.integer;

   if (exit_code >= 0
-      && emit_exited_event (exit_code) < 0)
+      && emit_exited_event (exit_code, inf) < 0)
     gdbpy_print_stack ();

   do_cleanups (cleanup);
diff --git a/gdb/testsuite/gdb.python/py-events.c
b/gdb/testsuite/gdb.python/py-events.c
index ceb697e..665ca51 100644
--- a/gdb/testsuite/gdb.python/py-events.c
+++ b/gdb/testsuite/gdb.python/py-events.c
@@ -17,6 +17,7 @@
 */

 int second(){
+  fork() ;
   return 12;
 }

diff --git a/gdb/testsuite/gdb.python/py-events.exp
b/gdb/testsuite/gdb.python/py-events.exp
index e5d6daf..1dd7e9f 100644
--- a/gdb/testsuite/gdb.python/py-events.exp
+++ b/gdb/testsuite/gdb.python/py-events.exp
@@ -42,6 +42,8 @@ if ![runto_main ] then {
     return -1
 }

+gdb_test_no_output "set detach-on-fork off" ""
+
 gdb_test "Test_Events" "Event testers registered."

 gdb_breakpoint "first"
@@ -56,4 +58,11 @@ all threads stopped"
 #test exited event.
 gdb_test "continue" ".*event type: continue.*
 .*event type: exit.*
-.*exit code: 12.*"
+.*exit code: 12.*
+.*exit inf: 1.*"
+
+gdb_test "inferior 2" ".*Switching to inferior 2.*"
+gdb_test "continue" ".*event type: continue.*
+.*event type: exit.*
+.*exit code: 12.*
+.*exit inf: 2.*"
diff --git a/gdb/testsuite/gdb.python/py-events.py
b/gdb/testsuite/gdb.python/py-events.py
index 9f05b9f..b40f074 100644
--- a/gdb/testsuite/gdb.python/py-events.py
+++ b/gdb/testsuite/gdb.python/py-events.py
@@ -41,6 +41,7 @@ def exit_handler (event):
     if (isinstance (event, gdb.ExitedEvent)):
         print "event type: exit"
     print "exit code: %d" % (event.exit_code)
+    print "exit inf: %d" % (event.inferior.num)

 def continue_handler (event):
     if (isinstance (event, gdb.ContinueEvent)):
-- 
1.7.4.4

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Eli Zaretskii]

> From: Kevin Pouget <kevin.pouget@gmail.com>
> Date: Thu, 21 Apr 2011 05:18:39 -0400
> 
> 2011-04-21  Kevin Pouget  <kevin.pouget@st.com>
> 
>         PR python/12691: Add the inferior to Python exited event
>         * gdb.texinfo (Events In Python): Describe exited inferior attribute.

This part is okay.

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Tom Tromey]

>>>>> "Kevin" == Kevin Pouget <kevin.pouget@gmail.com> writes:

Kevin> Following Tom's advise
Kevin> (http://sourceware.org/ml/gdb/2011-04/msg00116.html), here is a patch
Kevin> which hooks the inferior to the `exited' event object
Kevin> let me know what you think about it

It looks pretty good, but I think it needs a couple small changes.

Kevin> +  if (evpy_add_attribute (exited_event,
Kevin> +                          "inferior",
Kevin> +                          inferior_to_inferior_object (inf)) < 0)
Kevin> +    goto fail;

This does not account for the possibility that
inferior_to_inferior_object could fail.  I think there has
to be a temporary object that is explicitly checked against NULL.

Kevin> +gdb_test "continue" ".*event type: continue.*
Kevin> +.*event type: exit.*
Kevin> +.*exit code: 12.*
Kevin> +.*exit inf: 2.*"

Pass a test name to gdb_test.

Tom

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Kevin Pouget]

On Mon, Apr 25, 2011 at 2:19 PM, Tom Tromey <tromey@redhat.com> wrote:
>
> >>>>> "Kevin" == Kevin Pouget <kevin.pouget@gmail.com> writes:
>
> Kevin> Following Tom's advise
> Kevin> (http://sourceware.org/ml/gdb/2011-04/msg00116.html), here is a patch
> Kevin> which hooks the inferior to the `exited' event object
> Kevin> let me know what you think about it
>
> It looks pretty good, but I think it needs a couple small changes.
>
> Kevin> +  if (evpy_add_attribute (exited_event,
> Kevin> +                          "inferior",
> Kevin> +                          inferior_to_inferior_object (inf)) < 0)
> Kevin> +    goto fail;
>
> This does not account for the possibility that
> inferior_to_inferior_object could fail.  I think there has
> to be a temporary object that is explicitly checked against NULL.


done, it should only fail when the process runs out of space, so GDB
must handle it somewhere else

> Kevin> +gdb_test "continue" ".*event type: continue.*
> Kevin> +.*event type: exit.*
> Kevin> +.*exit code: 12.*
> Kevin> +.*exit inf: 2.*"
>
> Pass a test name to gdb_test.

right


thanks,

Kevin

--

2011-04-21  Kevin Pouget  <kevin.pouget@st.com>

       PR python/12691: Add the inferior to Python exited event
       * python/py-exitedevent.c (create_exited_event_object): Add inferior
       to exited_event.
       * python/py-event.h (emit_exited_event): Likewise
       * python/-inferior.c (python_inferior_exit): Likewise

2011-04-21  Kevin Pouget  <kevin.pouget@st.com>

       PR python/12691: Add the inferior to Python exited event
       * gdb.python/py-events.exp: Test the inferior attribute of exited
       event with a fork.
       * gdb.python/py-events.py: Print inferior number on exit.
       * gdb.python/py-events.c: Fork the inferior.

2011-04-21  Kevin Pouget  <kevin.pouget@st.com>

       PR python/12691: Add the inferior to Python exited event
       * gdb.texinfo (Events In Python): Describe exited inferior attribute.

--

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index edcf5c2..6e06bde 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22056,11 +22056,14 @@ inherited attribute refer to
@code{gdb.ThreadEvent} above.

 @item events.exited
 Emits @code{events.ExitedEvent} which indicates that the inferior has exited.
-@code{events.ExitedEvent} has one attribute:
+@code{events.ExitedEvent} has two attributes:
 @table @code
 @defivar ExitedEvent exit_code
 An integer representing the exit code which the inferior has returned.
 @end defivar
+@defivar ExitedEvent inferior
+A reference to the inferior which triggered the exited event.
+@end defivar
 @end table

 @item events.stop
diff --git a/gdb/python/py-event.h b/gdb/python/py-event.h
index bc95521..5ac73b4 100644
--- a/gdb/python/py-event.h
+++ b/gdb/python/py-event.h
@@ -104,7 +104,7 @@ typedef struct
 } event_object;

 extern int emit_continue_event (ptid_t ptid);
-extern int emit_exited_event (LONGEST exit_code);
+extern int emit_exited_event (LONGEST exit_code, struct inferior *inf);

 extern int evpy_emit_event (PyObject *event,
                             eventregistry_object *registry);
diff --git a/gdb/python/py-exitedevent.c b/gdb/python/py-exitedevent.c
index 457a4fe..323bdd0 100644
--- a/gdb/python/py-exitedevent.c
+++ b/gdb/python/py-exitedevent.c
@@ -22,9 +22,10 @@
 static PyTypeObject exited_event_object_type;

 PyObject *
-create_exited_event_object (LONGEST exit_code)
+create_exited_event_object (LONGEST exit_code, struct inferior *inf)
 {
   PyObject *exited_event;
+  PyObject *inf_obj;

   exited_event = create_event_object (&exited_event_object_type);

@@ -35,6 +36,12 @@ create_exited_event_object (LONGEST exit_code)
                           "exit_code",
                           PyLong_FromLongLong (exit_code)) < 0)
     goto fail;
+
+  inf_obj = inferior_to_inferior_object (inf);
+  if (!inf_obj || evpy_add_attribute (exited_event,
+                                      "inferior",
+                                      inf_obj) < 0)
+    goto fail;

   return exited_event;

@@ -47,14 +54,14 @@ create_exited_event_object (LONGEST exit_code)
    will create a new Python exited event object.  */

 int
-emit_exited_event (LONGEST exit_code)
+emit_exited_event (LONGEST exit_code, struct inferior *inf)
 {
   PyObject *event;

   if (evregpy_no_listeners_p (gdb_py_events.exited))
     return 0;

-  event = create_exited_event_object (exit_code);
+  event = create_exited_event_object (exit_code, inf);

   if (event)
     return evpy_emit_event (event, gdb_py_events.exited);
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index b9df394..19d45d7 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -123,7 +123,7 @@ python_inferior_exit (struct inferior *inf)
   exit_code = status.value.integer;

   if (exit_code >= 0
-      && emit_exited_event (exit_code) < 0)
+      && emit_exited_event (exit_code, inf) < 0)
     gdbpy_print_stack ();

   do_cleanups (cleanup);
diff --git a/gdb/testsuite/gdb.python/py-events.c
b/gdb/testsuite/gdb.python/py-events.c
index ceb697e..665ca51 100644
--- a/gdb/testsuite/gdb.python/py-events.c
+++ b/gdb/testsuite/gdb.python/py-events.c
@@ -17,6 +17,7 @@
 */

 int second(){
+  fork() ;
   return 12;
 }

diff --git a/gdb/testsuite/gdb.python/py-events.exp
b/gdb/testsuite/gdb.python/py-events.exp
index e5d6daf..cdf4ae6 100644
--- a/gdb/testsuite/gdb.python/py-events.exp
+++ b/gdb/testsuite/gdb.python/py-events.exp
@@ -42,6 +42,8 @@ if ![runto_main ] then {
     return -1
 }

+gdb_test_no_output "set detach-on-fork off" "Don't detach on fork"
+
 gdb_test "Test_Events" "Event testers registered."

 gdb_breakpoint "first"
@@ -56,4 +58,11 @@ all threads stopped"
 #test exited event.
 gdb_test "continue" ".*event type: continue.*
 .*event type: exit.*
-.*exit code: 12.*"
+.*exit code: 12.*
+.*exit inf: 1.*" "Inferior 1 terminated."
+
+gdb_test "inferior 2" ".*Switching to inferior 2.*"
+gdb_test "continue" ".*event type: continue.*
+.*event type: exit.*
+.*exit code: 12.*
+.*exit inf: 2.*" "Inferior 1 terminated."
diff --git a/gdb/testsuite/gdb.python/py-events.py
b/gdb/testsuite/gdb.python/py-events.py
index 9f05b9f..b40f074 100644
--- a/gdb/testsuite/gdb.python/py-events.py
+++ b/gdb/testsuite/gdb.python/py-events.py
@@ -41,6 +41,7 @@ def exit_handler (event):
     if (isinstance (event, gdb.ExitedEvent)):
         print "event type: exit"
     print "exit code: %d" % (event.exit_code)
+    print "exit inf: %d" % (event.inferior.num)

 def continue_handler (event):
     if (isinstance (event, gdb.ContinueEvent)):
--

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Kevin Pouget]

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

On Tue, Apr 26, 2011 at 10:22 AM, Kevin Pouget <kevin.pouget@gmail.com> wrote:
> On Mon, Apr 25, 2011 at 2:19 PM, Tom Tromey <tromey@redhat.com> wrote:
>>
>> >>>>> "Kevin" == Kevin Pouget <kevin.pouget@gmail.com> writes:
>>
>> Kevin> Following Tom's advise
>> Kevin> (http://sourceware.org/ml/gdb/2011-04/msg00116.html), here is a patch
>> Kevin> which hooks the inferior to the `exited' event object
>> Kevin> let me know what you think about it
>>
>> It looks pretty good, but I think it needs a couple small changes.
>>
>> Kevin> +  if (evpy_add_attribute (exited_event,
>> Kevin> +                          "inferior",
>> Kevin> +                          inferior_to_inferior_object (inf)) < 0)
>> Kevin> +    goto fail;
>>
>> This does not account for the possibility that
>> inferior_to_inferior_object could fail.  I think there has
>> to be a temporary object that is explicitly checked against NULL.
>
>
> done, it should only fail when the process runs out of space, so GDB
> must handle it somewhere else
>
>> Kevin> +gdb_test "continue" ".*event type: continue.*
>> Kevin> +.*event type: exit.*
>> Kevin> +.*exit code: 12.*
>> Kevin> +.*exit inf: 2.*"
>>
>> Pass a test name to gdb_test.
>
> right


Hello,

this patch was not explicitly accepted, could you please review this
version updated against the last trunk?
The doc was accepted, but I had to reshape the paragraph during the
update, Eli, could you please tell me what you think about it?

Cordially,

Kevin

2011-04-21  Kevin Pouget  <kevin.pouget@st.com>

	PR python/12691: Add the inferior to Python exited event
	* python/py-exitedevent.c (create_exited_event_object): Add inferior
	to exited_event.
	* python/py-event.h (emit_exited_event): Likewise
	* python/-inferior.c (python_inferior_exit): Likewise

 2011-04-21  Kevin Pouget  <kevin.pouget@st.com>

	PR python/12691: Add the inferior to Python exited event
	* gdb.python/py-events.exp: Test the inferior attribute of exited
	event with a fork.
	* gdb.python/py-events.py: Print inferior number on exit.
	* gdb.python/py-events.c: Fork the inferior.

 2011-04-21  Kevin Pouget  <kevin.pouget@st.com>

	PR python/12691: Add the inferior to Python exited event
	* gdb.texinfo (Events In Python): Describe exited inferior attribute.

[-- Attachment #2: 0001-PR-12691-Add-the-inferior-to-Python-exited-event --]
[-- Type: application/octet-stream, Size: 3653 bytes --]

From 9caa3edc65ec1bef5ac54d676365c5595674ce7e Mon Sep 17 00:00:00 2001
From: Kevin Pouget <kevin.pouget@st.com>
Date: Wed, 31 Aug 2011 16:32:45 +0200
Subject: [PATCH] PR/12691 Add the inferior to Python exited event

---
 gdb/doc/gdb.texinfo                    |   10 ++++++----
 gdb/python/py-exitedevent.c            |    6 ++++++
 gdb/testsuite/gdb.python/py-events.c   |    1 +
 gdb/testsuite/gdb.python/py-events.exp |   11 ++++++++++-
 gdb/testsuite/gdb.python/py-events.py  |    1 +
 5 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 23b2a98..bbc7a7d 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22285,12 +22285,14 @@ inherited attribute refer to @code{gdb.ThreadEvent} above.
 
 @item events.exited
 Emits @code{events.ExitedEvent} which indicates that the inferior has exited.
-@code{events.ExitedEvent} has one optional attribute.  This attribute
-will exist only in the case that the inferior exited with some
-status.
+@code{events.ExitedEvent} has two attributes:
 @table @code
 @defivar ExitedEvent exit_code
-An integer representing the exit code which the inferior has returned.
+An integer representing the exit code which the inferior has returned. (Optional,
+will exist only in the case that the inferior exited with some status.)
+@end defivar
+@defivar ExitedEvent inferior
+A reference to the inferior which triggered the exited event.
 @end defivar
 @end table
 
diff --git a/gdb/python/py-exitedevent.c b/gdb/python/py-exitedevent.c
index 08150e5..5ee6eb8 100644
--- a/gdb/python/py-exitedevent.c
+++ b/gdb/python/py-exitedevent.c
@@ -37,6 +37,12 @@ create_exited_event_object (const LONGEST *exit_code)
 			     PyLong_FromLongLong (*exit_code)) < 0)
     goto fail;
 
+  inf_obj = inferior_to_inferior_object (inf);
+  if (!inf_obj || evpy_add_attribute (exited_event,
+                                      "inferior",
+                                      inf_obj) < 0)
+    goto fail;
+
   return exited_event;
 
   fail:
diff --git a/gdb/testsuite/gdb.python/py-events.c b/gdb/testsuite/gdb.python/py-events.c
index ceb697e..665ca51 100644
--- a/gdb/testsuite/gdb.python/py-events.c
+++ b/gdb/testsuite/gdb.python/py-events.c
@@ -17,6 +17,7 @@
 */
 
 int second(){
+  fork() ;
   return 12;
 }
 
diff --git a/gdb/testsuite/gdb.python/py-events.exp b/gdb/testsuite/gdb.python/py-events.exp
index e5d6daf..cdf4ae6 100644
--- a/gdb/testsuite/gdb.python/py-events.exp
+++ b/gdb/testsuite/gdb.python/py-events.exp
@@ -42,6 +42,8 @@ if ![runto_main ] then {
     return -1
 }
 
+gdb_test_no_output "set detach-on-fork off" "Don't detach on fork"
+
 gdb_test "Test_Events" "Event testers registered."
 
 gdb_breakpoint "first"
@@ -56,4 +58,11 @@ all threads stopped"
 #test exited event.
 gdb_test "continue" ".*event type: continue.*
 .*event type: exit.*
-.*exit code: 12.*"
+.*exit code: 12.*
+.*exit inf: 1.*" "Inferior 1 terminated."
+
+gdb_test "inferior 2" ".*Switching to inferior 2.*"
+gdb_test "continue" ".*event type: continue.*
+.*event type: exit.*
+.*exit code: 12.*
+.*exit inf: 2.*" "Inferior 1 terminated."
diff --git a/gdb/testsuite/gdb.python/py-events.py b/gdb/testsuite/gdb.python/py-events.py
index 9f05b9f..b40f074 100644
--- a/gdb/testsuite/gdb.python/py-events.py
+++ b/gdb/testsuite/gdb.python/py-events.py
@@ -41,6 +41,7 @@ def exit_handler (event):
     if (isinstance (event, gdb.ExitedEvent)):
         print "event type: exit"
     print "exit code: %d" % (event.exit_code)
+    print "exit inf: %d" % (event.inferior.num)
 
 def continue_handler (event):
     if (isinstance (event, gdb.ContinueEvent)):
-- 
1.7.6

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Eli Zaretskii]

> From: Kevin Pouget <kevin.pouget@gmail.com>
> Date: Wed, 31 Aug 2011 16:33:55 +0200
> Cc: gdb-patches@sourceware.org
> 
> The doc was accepted, but I had to reshape the paragraph during the
> update, Eli, could you please tell me what you think about it?

I have a couple of minor comments:

>  @defivar ExitedEvent exit_code
> -An integer representing the exit code which the inferior has returned.
> +An integer representing the exit code which the inferior has returned.
                                                            ^^^
I think we would be better off without that "has" word.

>                                                                 (Optional,
> +will exist only in the case that the inferior exited with some status.)

I understand you want to tell that the exit_code attribute is
optional?  This calls for some rewording, but I actually don't
understand how can it be that an inferior doesn't return any exit code
at all.  AFAIK, the exit code could be random garbage, but it's always
there.  Can you explain?

> +@defivar ExitedEvent inferior
> +A reference to the inferior which triggered the exited event.

I'd suggest to use @code{exited}, since this is a Python symbol.

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Kevin Pouget]

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

On Wed, Aug 31, 2011 at 4:46 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Kevin Pouget <kevin.pouget@gmail.com>
>> Date: Wed, 31 Aug 2011 16:33:55 +0200
>> Cc: gdb-patches@sourceware.org
>>
>> The doc was accepted, but I had to reshape the paragraph during the
>> update, Eli, could you please tell me what you think about it?
>
> I have a couple of minor comments:

>> +@defivar ExitedEvent inferior
>> +A reference to the inferior which triggered the exited event.
>
> I'd suggest to use @code{exited}, since this is a Python symbol.

this one is fine,

>>  @defivar ExitedEvent exit_code
>> -An integer representing the exit code which the inferior has returned.
>> +An integer representing the exit code which the inferior has returned.
>                                                            ^^^
> I think we would be better off without that "has" word.
>
>>                                                                 (Optional,
>> +will exist only in the case that the inferior exited with some status.)
>
> I understand you want to tell that the exit_code attribute is
> optional?  This calls for some rewording, but I actually don't
> understand how can it be that an inferior doesn't return any exit code
> at all.  AFAIK, the exit code could be random garbage, but it's always
> there.  Can you explain?

but for these two, I didn't write it myself: as you can read in the patch,
I only rearranged the sentence to include my information, the points
you mention were not changed.
(Let me know if you want me to change something anyway)


Thanks,

Kevin

[-- Attachment #2: 0001-PR-12691-Add-the-inferior-to-Python-exited-event --]
[-- Type: application/octet-stream, Size: 3656 bytes --]

From f75be1eba12e32a0ebcecede2fcd697d3b4f672b Mon Sep 17 00:00:00 2001
From: Kevin Pouget <kevin.pouget@st.com>
Date: Wed, 31 Aug 2011 16:32:45 +0200
Subject: [PATCH] PR/12691 Add the inferior to Python exited event

---
 gdb/doc/gdb.texinfo                    |   10 ++++++----
 gdb/python/py-exitedevent.c            |    6 ++++++
 gdb/testsuite/gdb.python/py-events.c   |    1 +
 gdb/testsuite/gdb.python/py-events.exp |   11 ++++++++++-
 gdb/testsuite/gdb.python/py-events.py  |    1 +
 5 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 23b2a98..51aaad4 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22285,12 +22285,14 @@ inherited attribute refer to @code{gdb.ThreadEvent} above.
 
 @item events.exited
 Emits @code{events.ExitedEvent} which indicates that the inferior has exited.
-@code{events.ExitedEvent} has one optional attribute.  This attribute
-will exist only in the case that the inferior exited with some
-status.
+@code{events.ExitedEvent} has two attributes:
 @table @code
 @defivar ExitedEvent exit_code
-An integer representing the exit code which the inferior has returned.
+An integer representing the exit code which the inferior has returned. (Optional,
+will exist only in the case that the inferior exited with some status.)
+@end defivar
+@defivar ExitedEvent inferior
+A reference to the inferior which triggered the @{exited} event.
 @end defivar
 @end table
 
diff --git a/gdb/python/py-exitedevent.c b/gdb/python/py-exitedevent.c
index 08150e5..5ee6eb8 100644
--- a/gdb/python/py-exitedevent.c
+++ b/gdb/python/py-exitedevent.c
@@ -37,6 +37,12 @@ create_exited_event_object (const LONGEST *exit_code)
 			     PyLong_FromLongLong (*exit_code)) < 0)
     goto fail;
 
+  inf_obj = inferior_to_inferior_object (inf);
+  if (!inf_obj || evpy_add_attribute (exited_event,
+                                      "inferior",
+                                      inf_obj) < 0)
+    goto fail;
+
   return exited_event;
 
   fail:
diff --git a/gdb/testsuite/gdb.python/py-events.c b/gdb/testsuite/gdb.python/py-events.c
index ceb697e..665ca51 100644
--- a/gdb/testsuite/gdb.python/py-events.c
+++ b/gdb/testsuite/gdb.python/py-events.c
@@ -17,6 +17,7 @@
 */
 
 int second(){
+  fork() ;
   return 12;
 }
 
diff --git a/gdb/testsuite/gdb.python/py-events.exp b/gdb/testsuite/gdb.python/py-events.exp
index e5d6daf..cdf4ae6 100644
--- a/gdb/testsuite/gdb.python/py-events.exp
+++ b/gdb/testsuite/gdb.python/py-events.exp
@@ -42,6 +42,8 @@ if ![runto_main ] then {
     return -1
 }
 
+gdb_test_no_output "set detach-on-fork off" "Don't detach on fork"
+
 gdb_test "Test_Events" "Event testers registered."
 
 gdb_breakpoint "first"
@@ -56,4 +58,11 @@ all threads stopped"
 #test exited event.
 gdb_test "continue" ".*event type: continue.*
 .*event type: exit.*
-.*exit code: 12.*"
+.*exit code: 12.*
+.*exit inf: 1.*" "Inferior 1 terminated."
+
+gdb_test "inferior 2" ".*Switching to inferior 2.*"
+gdb_test "continue" ".*event type: continue.*
+.*event type: exit.*
+.*exit code: 12.*
+.*exit inf: 2.*" "Inferior 1 terminated."
diff --git a/gdb/testsuite/gdb.python/py-events.py b/gdb/testsuite/gdb.python/py-events.py
index 9f05b9f..b40f074 100644
--- a/gdb/testsuite/gdb.python/py-events.py
+++ b/gdb/testsuite/gdb.python/py-events.py
@@ -41,6 +41,7 @@ def exit_handler (event):
     if (isinstance (event, gdb.ExitedEvent)):
         print "event type: exit"
     print "exit code: %d" % (event.exit_code)
+    print "exit inf: %d" % (event.inferior.num)
 
 def continue_handler (event):
     if (isinstance (event, gdb.ContinueEvent)):
-- 
1.7.6

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Tom Tromey]

>>>>> "Kevin" == Kevin Pouget <kevin.pouget@gmail.com> writes:

Kevin> this patch was not explicitly accepted, could you please review this
Kevin> version updated against the last trunk?

Code bits are ok.  Thanks.

Tom

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Eli Zaretskii]

> From: Kevin Pouget <kevin.pouget@gmail.com>
> Date: Wed, 31 Aug 2011 16:57:48 +0200
> Cc: tromey@redhat.com, gdb-patches@sourceware.org
> 
> [1:text/plain Hide]
> On Wed, Aug 31, 2011 at 4:46 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> From: Kevin Pouget <kevin.pouget@gmail.com>
> >> Date: Wed, 31 Aug 2011 16:33:55 +0200
> >> Cc: gdb-patches@sourceware.org
> >>
> >> The doc was accepted, but I had to reshape the paragraph during the
> >> update, Eli, could you please tell me what you think about it?
> >
> > I have a couple of minor comments:
> 
> >> +@defivar ExitedEvent inferior
> >> +A reference to the inferior which triggered the exited event.
> >
> > I'd suggest to use @code{exited}, since this is a Python symbol.
> 
> this one is fine,
> 
> >>  @defivar ExitedEvent exit_code
> >> -An integer representing the exit code which the inferior has returned.
> >> +An integer representing the exit code which the inferior has returned.
> >                                                            ^^^
> > I think we would be better off without that "has" word.
> >
> >>                                                                 (Optional,
> >> +will exist only in the case that the inferior exited with some status.)
> >
> > I understand you want to tell that the exit_code attribute is
> > optional?  This calls for some rewording, but I actually don't
> > understand how can it be that an inferior doesn't return any exit code
> > at all.  AFAIK, the exit code could be random garbage, but it's always
> > there.  Can you explain?
> 
> but for these two, I didn't write it myself: as you can read in the patch,
> I only rearranged the sentence to include my information, the points
> you mention were not changed.
> (Let me know if you want me to change something anyway)

I would rephrase like this:

  An integer representing the exit code, if any, that the inferior
  returned.

I'd still like to hear from someone "in the know" how is it possible
that the exit code won't be available, but let's not block the commit
on that behalf.

Thanks.

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Eli> I'd still like to hear from someone "in the know" how is it possible
Eli> that the exit code won't be available, but let's not block the commit
Eli> on that behalf.

It can happen at least on detach.  See the docs for inferior_exit in
observer.texi.  I thought it could also happen if the inferior exec'd,
but today that doesn't make sense to me.

Tom

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Cc: Kevin Pouget <kevin.pouget@gmail.com>, gdb-patches@sourceware.org
> Date: Wed, 31 Aug 2011 11:53:21 -0600
> 
> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
> 
> Eli> I'd still like to hear from someone "in the know" how is it possible
> Eli> that the exit code won't be available, but let's not block the commit
> Eli> on that behalf.
> 
> It can happen at least on detach.

Right, thanks.  Perhaps we should mention that in parentheses.

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Kevin Pouget]

On Wed, Aug 31, 2011 at 7:56 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Tom Tromey <tromey@redhat.com>
>> Cc: Kevin Pouget <kevin.pouget@gmail.com>, gdb-patches@sourceware.org
>> Date: Wed, 31 Aug 2011 11:53:21 -0600
>>
>> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
>>
>> Eli> I'd still like to hear from someone "in the know" how is it possible
>> Eli> that the exit code won't be available, but let's not block the commit
>> Eli> on that behalf.
>>
>> It can happen at least on detach.
>
> Right, thanks.  Perhaps we should mention that in parentheses.
>

do you want it to appear in my patch, something like

> Optional, will exist only in the case that the inferior exited with some status---i.e., not detached)

along with

>> +An integer representing the exit code which the inferior has returned.
> I think we would be better off without that "has" word.

?


thanks,

Kevin

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Kevin Pouget]

On Wed, Aug 31, 2011 at 7:56 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Tom Tromey <tromey@redhat.com>
>> Cc: Kevin Pouget <kevin.pouget@gmail.com>, gdb-patches@sourceware.org
>> Date: Wed, 31 Aug 2011 11:53:21 -0600
>>
>> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
>>
>> Eli> I'd still like to hear from someone "in the know" how is it possible
>> Eli> that the exit code won't be available, but let's not block the commit
>> Eli> on that behalf.
>>
>> It can happen at least on detach.
>
> Right, thanks.  Perhaps we should mention that in parentheses.
>

do you want it to appear in my patch, something like

> Optional, will exist only in the case that the inferior exited with some status---i.e., not detached)

along with

>> +An integer representing the exit code which the inferior has returned.
> I think we would be better off without that "has" word.

?


thanks,

Kevin

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Eli Zaretskii]

> From: Kevin Pouget <kevin.pouget@gmail.com>
> Date: Thu, 1 Sep 2011 10:44:39 +0200
> Cc: Tom Tromey <tromey@redhat.com>, gdb-patches@sourceware.org
> 
> On Wed, Aug 31, 2011 at 7:56 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> From: Tom Tromey <tromey@redhat.com>
> >> Cc: Kevin Pouget <kevin.pouget@gmail.com>, gdb-patches@sourceware.org
> >> Date: Wed, 31 Aug 2011 11:53:21 -0600
> >>
> >> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
> >>
> >> Eli> I'd still like to hear from someone "in the know" how is it possible
> >> Eli> that the exit code won't be available, but let's not block the commit
> >> Eli> on that behalf.
> >>
> >> It can happen at least on detach.
> >
> > Right, thanks.  Perhaps we should mention that in parentheses.
> >
> 
> do you want it to appear in my patch, something like
> 
> > Optional, will exist only in the case that the inferior exited with some status---i.e., not detached)
> 
> along with
> 
> >> +An integer representing the exit code which the inferior has returned.
> > I think we would be better off without that "has" word.
> 
> ?

No, I meant something like

  An integer representing the exit code, if available, which the
  inferior has returned.  (The exit code could be unavailable if, for
  example, @value{GDBN} detaches from the inferior.)

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Kevin Pouget]

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

On Thu, Sep 1, 2011 at 12:07 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Kevin Pouget <kevin.pouget@gmail.com>
>> Date: Thu, 1 Sep 2011 10:44:39 +0200
>> Cc: Tom Tromey <tromey@redhat.com>, gdb-patches@sourceware.org
>>
>> On Wed, Aug 31, 2011 at 7:56 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> >> From: Tom Tromey <tromey@redhat.com>
>> >> Cc: Kevin Pouget <kevin.pouget@gmail.com>, gdb-patches@sourceware.org
>> >> Date: Wed, 31 Aug 2011 11:53:21 -0600
>> >>
>> >> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
>> >>
>> >> Eli> I'd still like to hear from someone "in the know" how is it possible
>> >> Eli> that the exit code won't be available, but let's not block the commit
>> >> Eli> on that behalf.
>> >>
>> >> It can happen at least on detach.
>> >
>> > Right, thanks.  Perhaps we should mention that in parentheses.
>> >
>>
>> do you want it to appear in my patch, something like
>>
>> > Optional, will exist only in the case that the inferior exited with some status---i.e., not detached)
>>
>> along with
>>
>> >> +An integer representing the exit code which the inferior has returned.
>> > I think we would be better off without that "has" word.
>>
>> ?
>
> No, I meant something like
>
>  An integer representing the exit code, if available, which the
>  inferior has returned.  (The exit code could be unavailable if, for
>  example, @value{GDBN} detaches from the inferior.)
>

right, I've updated the patch, is it fine for you ?


thanks,

Kevin

2011-09-20  Kevin Pouget  <kevin.pouget@st.com>

	PR python/12691: Add the inferior to Python exited event
	* python/py-exitedevent.c (create_exited_event_object): Add inferior
	to exited_event.
	* python/py-event.h (emit_exited_event): Likewise
	* python/-inferior.c (python_inferior_exit): Likewise

 2011-09-01  Kevin Pouget  <kevin.pouget@st.com>

	PR python/12691: Add the inferior to Python exited event
	* gdb.python/py-events.exp: Test the inferior attribute of exited
	event with a fork.
	* gdb.python/py-events.py: Print inferior number on exit.
	* gdb.python/py-events.c: Fork the inferior.

 2011-09-01  Kevin Pouget  <kevin.pouget@st.com>

	PR python/12691: Add the inferior to Python exited event
	* gdb.texinfo (Events In Python): Describe exited inferior attribute.

[-- Attachment #2: 0001-PR-12691-Add-the-inferior-to-Python-exited-event.patch --]
[-- Type: text/x-patch, Size: 3687 bytes --]

From 4d6bbef65231e37144b6b17ecd4c76bf629cf111 Mon Sep 17 00:00:00 2001
From: Kevin Pouget <kevin.pouget@st.com>
Date: Wed, 31 Aug 2011 16:32:45 +0200
Subject: [PATCH] PR/12691 Add the inferior to Python exited event

---
 gdb/doc/gdb.texinfo                    |   11 +++++++----
 gdb/python/py-exitedevent.c            |    6 ++++++
 gdb/testsuite/gdb.python/py-events.c   |    1 +
 gdb/testsuite/gdb.python/py-events.exp |   11 ++++++++++-
 gdb/testsuite/gdb.python/py-events.py  |    1 +
 5 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 23b2a98..795334d 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22285,12 +22285,15 @@ inherited attribute refer to @code{gdb.ThreadEvent} above.
 
 @item events.exited
 Emits @code{events.ExitedEvent} which indicates that the inferior has exited.
-@code{events.ExitedEvent} has one optional attribute.  This attribute
-will exist only in the case that the inferior exited with some
-status.
+@code{events.ExitedEvent} has two attributes:
 @table @code
 @defivar ExitedEvent exit_code
-An integer representing the exit code which the inferior has returned.
+An integer representing the exit code, if available, which the inferior 
+has returned.  (The exit code could be unavailable if, for example,
+@value{GDBN} detaches from the inferior.)
+@end defivar
+@defivar ExitedEvent inferior
+A reference to the inferior which triggered the @{exited} event.
 @end defivar
 @end table
 
diff --git a/gdb/python/py-exitedevent.c b/gdb/python/py-exitedevent.c
index 08150e5..5ee6eb8 100644
--- a/gdb/python/py-exitedevent.c
+++ b/gdb/python/py-exitedevent.c
@@ -37,6 +37,12 @@ create_exited_event_object (const LONGEST *exit_code)
 			     PyLong_FromLongLong (*exit_code)) < 0)
     goto fail;
 
+  inf_obj = inferior_to_inferior_object (inf);
+  if (!inf_obj || evpy_add_attribute (exited_event,
+                                      "inferior",
+                                      inf_obj) < 0)
+    goto fail;
+
   return exited_event;
 
   fail:
diff --git a/gdb/testsuite/gdb.python/py-events.c b/gdb/testsuite/gdb.python/py-events.c
index ceb697e..665ca51 100644
--- a/gdb/testsuite/gdb.python/py-events.c
+++ b/gdb/testsuite/gdb.python/py-events.c
@@ -17,6 +17,7 @@
 */
 
 int second(){
+  fork() ;
   return 12;
 }
 
diff --git a/gdb/testsuite/gdb.python/py-events.exp b/gdb/testsuite/gdb.python/py-events.exp
index e5d6daf..cdf4ae6 100644
--- a/gdb/testsuite/gdb.python/py-events.exp
+++ b/gdb/testsuite/gdb.python/py-events.exp
@@ -42,6 +42,8 @@ if ![runto_main ] then {
     return -1
 }
 
+gdb_test_no_output "set detach-on-fork off" "Don't detach on fork"
+
 gdb_test "Test_Events" "Event testers registered."
 
 gdb_breakpoint "first"
@@ -56,4 +58,11 @@ all threads stopped"
 #test exited event.
 gdb_test "continue" ".*event type: continue.*
 .*event type: exit.*
-.*exit code: 12.*"
+.*exit code: 12.*
+.*exit inf: 1.*" "Inferior 1 terminated."
+
+gdb_test "inferior 2" ".*Switching to inferior 2.*"
+gdb_test "continue" ".*event type: continue.*
+.*event type: exit.*
+.*exit code: 12.*
+.*exit inf: 2.*" "Inferior 1 terminated."
diff --git a/gdb/testsuite/gdb.python/py-events.py b/gdb/testsuite/gdb.python/py-events.py
index 9f05b9f..b40f074 100644
--- a/gdb/testsuite/gdb.python/py-events.py
+++ b/gdb/testsuite/gdb.python/py-events.py
@@ -41,6 +41,7 @@ def exit_handler (event):
     if (isinstance (event, gdb.ExitedEvent)):
         print "event type: exit"
     print "exit code: %d" % (event.exit_code)
+    print "exit inf: %d" % (event.inferior.num)
 
 def continue_handler (event):
     if (isinstance (event, gdb.ContinueEvent)):
-- 
1.7.6

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Kevin Pouget]

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

On Thu, Sep 1, 2011 at 1:08 PM, Kevin Pouget <kevin.pouget@gmail.com> wrote:
> On Thu, Sep 1, 2011 at 12:07 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>>> From: Kevin Pouget <kevin.pouget@gmail.com>
>>> Date: Thu, 1 Sep 2011 10:44:39 +0200
>>> Cc: Tom Tromey <tromey@redhat.com>, gdb-patches@sourceware.org
>>>
>>> On Wed, Aug 31, 2011 at 7:56 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>>> >> From: Tom Tromey <tromey@redhat.com>
>>> >> Cc: Kevin Pouget <kevin.pouget@gmail.com>, gdb-patches@sourceware.org
>>> >> Date: Wed, 31 Aug 2011 11:53:21 -0600
>>> >>
>>> >> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
>>> >>
>>> >> Eli> I'd still like to hear from someone "in the know" how is it possible
>>> >> Eli> that the exit code won't be available, but let's not block the commit
>>> >> Eli> on that behalf.
>>> >>
>>> >> It can happen at least on detach.
>>> >
>>> > Right, thanks.  Perhaps we should mention that in parentheses.
>>> >
>>>
>>> do you want it to appear in my patch, something like
>>>
>>> > Optional, will exist only in the case that the inferior exited with some status---i.e., not detached)
>>>
>>> along with
>>>
>>> >> +An integer representing the exit code which the inferior has returned.
>>> > I think we would be better off without that "has" word.
>>>
>>> ?
>>
>> No, I meant something like
>>
>>  An integer representing the exit code, if available, which the
>>  inferior has returned.  (The exit code could be unavailable if, for
>>  example, @value{GDBN} detaches from the inferior.)
>>
>
> right, I've updated the patch, is it fine for you ?
>
>
> thanks,
>
> Kevin
>
> 2011-09-20  Kevin Pouget  <kevin.pouget@st.com>
>
>        PR python/12691: Add the inferior to Python exited event
>        * python/py-exitedevent.c (create_exited_event_object): Add inferior
>        to exited_event.
>        * python/py-event.h (emit_exited_event): Likewise
>        * python/-inferior.c (python_inferior_exit): Likewise
>
>  2011-09-01  Kevin Pouget  <kevin.pouget@st.com>
>
>        PR python/12691: Add the inferior to Python exited event
>        * gdb.python/py-events.exp: Test the inferior attribute of exited
>        event with a fork.
>        * gdb.python/py-events.py: Print inferior number on exit.
>        * gdb.python/py-events.c: Fork the inferior.
>
>  2011-09-01  Kevin Pouget  <kevin.pouget@st.com>
>
>        PR python/12691: Add the inferior to Python exited event
>        * gdb.texinfo (Events In Python): Describe exited inferior attribute.
>

ping

Eli, could you confirm that you agree with the last version of the
documentation?

Thanks,

Kevin

[-- Attachment #2: 0001-PR-12691-Add-the-inferior-to-Python-exited-event.patch --]
[-- Type: text/x-patch, Size: 3687 bytes --]

From 4d6bbef65231e37144b6b17ecd4c76bf629cf111 Mon Sep 17 00:00:00 2001
From: Kevin Pouget <kevin.pouget@st.com>
Date: Wed, 31 Aug 2011 16:32:45 +0200
Subject: [PATCH] PR/12691 Add the inferior to Python exited event

---
 gdb/doc/gdb.texinfo                    |   11 +++++++----
 gdb/python/py-exitedevent.c            |    6 ++++++
 gdb/testsuite/gdb.python/py-events.c   |    1 +
 gdb/testsuite/gdb.python/py-events.exp |   11 ++++++++++-
 gdb/testsuite/gdb.python/py-events.py  |    1 +
 5 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 23b2a98..795334d 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22285,12 +22285,15 @@ inherited attribute refer to @code{gdb.ThreadEvent} above.
 
 @item events.exited
 Emits @code{events.ExitedEvent} which indicates that the inferior has exited.
-@code{events.ExitedEvent} has one optional attribute.  This attribute
-will exist only in the case that the inferior exited with some
-status.
+@code{events.ExitedEvent} has two attributes:
 @table @code
 @defivar ExitedEvent exit_code
-An integer representing the exit code which the inferior has returned.
+An integer representing the exit code, if available, which the inferior 
+has returned.  (The exit code could be unavailable if, for example,
+@value{GDBN} detaches from the inferior.)
+@end defivar
+@defivar ExitedEvent inferior
+A reference to the inferior which triggered the @{exited} event.
 @end defivar
 @end table
 
diff --git a/gdb/python/py-exitedevent.c b/gdb/python/py-exitedevent.c
index 08150e5..5ee6eb8 100644
--- a/gdb/python/py-exitedevent.c
+++ b/gdb/python/py-exitedevent.c
@@ -37,6 +37,12 @@ create_exited_event_object (const LONGEST *exit_code)
 			     PyLong_FromLongLong (*exit_code)) < 0)
     goto fail;
 
+  inf_obj = inferior_to_inferior_object (inf);
+  if (!inf_obj || evpy_add_attribute (exited_event,
+                                      "inferior",
+                                      inf_obj) < 0)
+    goto fail;
+
   return exited_event;
 
   fail:
diff --git a/gdb/testsuite/gdb.python/py-events.c b/gdb/testsuite/gdb.python/py-events.c
index ceb697e..665ca51 100644
--- a/gdb/testsuite/gdb.python/py-events.c
+++ b/gdb/testsuite/gdb.python/py-events.c
@@ -17,6 +17,7 @@
 */
 
 int second(){
+  fork() ;
   return 12;
 }
 
diff --git a/gdb/testsuite/gdb.python/py-events.exp b/gdb/testsuite/gdb.python/py-events.exp
index e5d6daf..cdf4ae6 100644
--- a/gdb/testsuite/gdb.python/py-events.exp
+++ b/gdb/testsuite/gdb.python/py-events.exp
@@ -42,6 +42,8 @@ if ![runto_main ] then {
     return -1
 }
 
+gdb_test_no_output "set detach-on-fork off" "Don't detach on fork"
+
 gdb_test "Test_Events" "Event testers registered."
 
 gdb_breakpoint "first"
@@ -56,4 +58,11 @@ all threads stopped"
 #test exited event.
 gdb_test "continue" ".*event type: continue.*
 .*event type: exit.*
-.*exit code: 12.*"
+.*exit code: 12.*
+.*exit inf: 1.*" "Inferior 1 terminated."
+
+gdb_test "inferior 2" ".*Switching to inferior 2.*"
+gdb_test "continue" ".*event type: continue.*
+.*event type: exit.*
+.*exit code: 12.*
+.*exit inf: 2.*" "Inferior 1 terminated."
diff --git a/gdb/testsuite/gdb.python/py-events.py b/gdb/testsuite/gdb.python/py-events.py
index 9f05b9f..b40f074 100644
--- a/gdb/testsuite/gdb.python/py-events.py
+++ b/gdb/testsuite/gdb.python/py-events.py
@@ -41,6 +41,7 @@ def exit_handler (event):
     if (isinstance (event, gdb.ExitedEvent)):
         print "event type: exit"
     print "exit code: %d" % (event.exit_code)
+    print "exit inf: %d" % (event.inferior.num)
 
 def continue_handler (event):
     if (isinstance (event, gdb.ContinueEvent)):
-- 
1.7.6

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Eli Zaretskii]

> From: Kevin Pouget <kevin.pouget@gmail.com>
> Date: Thu, 15 Sep 2011 14:34:08 +0200
> Cc: gdb-patches@sourceware.org
> 
> ping
> 
> Eli, could you confirm that you agree with the last version of the
> documentation?

Sorry.  Yes, this is fine.

RE: [PATCH] PR/12691 Add the inferior to Python exited event

[Paul_Koning]

>  An integer representing the exit code, if available, which the
>>  inferior has returned.  (The exit code could be unavailable if, for
>>  example, @value{GDBN} detaches from the inferior.)
>>
>
> right, I've updated the patch, is it fine for you ?

The old text is more explicit about what you see if the exit code is not available.  It would be good to keep that -- i.e., the attribute isn't there if the exit code is not available.  The reason for being explicit is that a different way of handling that situation -- and arguably a more "Pythonic" one -- is for the attribute to exist in all cases but have the value None if not applicable.  So you might make it:

+An integer representing the exit code, if available, which the inferior 
+has returned.  (The exit code could be unavailable if, for example,
+@value{GDBN} detaches from the inferior.)  If the exit code is
+unavailable, the attribute does not exist.

	paul

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Kevin Pouget]

On Thu, Sep 15, 2011 at 3:57 PM,  <Paul_Koning@dell.com> wrote:
>>  An integer representing the exit code, if available, which the
>>>  inferior has returned.  (The exit code could be unavailable if, for
>>>  example, @value{GDBN} detaches from the inferior.)
>>>
>>
>> right, I've updated the patch, is it fine for you ?
>
> The old text is more explicit about what you see if the exit code is not available.  It would be good to keep that -- i.e., the attribute isn't there if the exit code is not available.  The reason for being explicit is that a different way of handling that situation -- and arguably a more "Pythonic" one -- is for the attribute to exist in all cases but have the value None if not applicable.  So you might make it:
>
> +An integer representing the exit code, if available, which the inferior
> +has returned.  (The exit code could be unavailable if, for example,
> +@value{GDBN} detaches from the inferior.)  If the exit code is
> +unavailable, the attribute does not exist.


I'm okay with this modification; I'll let Eli accept it before
applying this patch


Cordially,

Kevin

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Eli Zaretskii]

> From: <Paul_Koning@Dell.com>
> CC: <gdb-patches@sourceware.org>
> Date: Thu, 15 Sep 2011 08:57:59 -0500
> 
> +An integer representing the exit code, if available, which the inferior 
> +has returned.  (The exit code could be unavailable if, for example,
> +@value{GDBN} detaches from the inferior.)  If the exit code is
> +unavailable, the attribute does not exist.

Fine with me, if by "don't exist" you mean literally that.

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Kevin Pouget]

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

On Wed, Aug 31, 2011 at 5:50 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Kevin" == Kevin Pouget <kevin.pouget@gmail.com> writes:
>
> Kevin> this patch was not explicitly accepted, could you please review this
> Kevin> version updated against the last trunk?
>
> Code bits are ok.  Thanks.
>
> Tom

Hello,

There was a discrepancy between the version of the patch you approved
(http://sourceware.org/ml/gdb-patches/2011-08/msg00659/0001-PR-12691-Add-the-inferior-to-Python-exited-event)
and the one I originally submitted (I certainly missed a step during
the April->August update in the git tree):

 gdb/doc/gdb.texinfo                    |   10 ++++++----
 gdb/python/py-exitedevent.c            |    6 ++++++
 gdb/testsuite/gdb.python/py-events.c   |    1 +
 gdb/testsuite/gdb.python/py-events.exp |   11 ++++++++++-
 gdb/testsuite/gdb.python/py-events.py  |    1 +
 5 files changed, 24 insertions(+), 5 deletions(-)

and

> gdb/doc/gdb.texinfo                    |   12 ++++++++----
> gdb/python/py-event.h                  |    2 +-
> gdb/python/py-exitedevent.c            |   13 ++++++++++---
> gdb/python/py-inferior.c               |    2 +-

 gdb/testsuite/gdb.python/py-events.c   |    1 +
 gdb/testsuite/gdb.python/py-events.exp |   11 ++++++++++-
 gdb/testsuite/gdb.python/py-events.py  |    1 +

so I think it has to be re-approved.
There is no regression on x86_64 against the up-to-date GIT tree and
the generated PDF looks as expected.


Thanks,

Kevin

(I can't build the CVS tree because of missing ".deps/*.Po" files from
GDB/binutils, don't know where the problem comes from)


2011-09-20  Kevin Pouget  <kevin.pouget@st.com>

	PR python/12691: Add the inferior to Python exited event
	* python/py-exitedevent.c (create_exited_event_object): Add inferior
	to exited_event.
	* python/py-event.h (emit_exited_event): Likewise
	* python/-inferior.c (python_inferior_exit): Likewise

 2011-09-01  Kevin Pouget  <kevin.pouget@st.com>

	PR python/12691: Add the inferior to Python exited event
	* gdb.python/py-events.exp: Test the inferior attribute of exited
	event with a fork.
	* gdb.python/py-events.py: Print inferior number on exit.
	* gdb.python/py-events.c: Fork the inferior.

 2011-09-01  Kevin Pouget  <kevin.pouget@st.com>

	PR python/12691: Add the inferior to Python exited event
	* gdb.texinfo (Events In Python): Describe exited inferior attribute.

[-- Attachment #2: 0001-PR-12691-Add-the-inferior-to-Python-exited-event.patch --]
[-- Type: text/x-patch, Size: 5631 bytes --]

From e3a5558105530ba0c95ae0f5af1c98bc6f04e08a Mon Sep 17 00:00:00 2001
From: Kevin Pouget <kevin.pouget@st.com>
Date: Mon, 19 Sep 2011 11:31:02 +0200
Subject: [PATCH] PR/12691 Add the inferior to Python exited event

---
 gdb/doc/gdb.texinfo                    |   12 ++++++++----
 gdb/python/py-event.h                  |    2 +-
 gdb/python/py-exitedevent.c            |   13 ++++++++++---
 gdb/python/py-inferior.c               |    2 +-
 gdb/testsuite/gdb.python/py-events.c   |    1 +
 gdb/testsuite/gdb.python/py-events.exp |   11 ++++++++++-
 gdb/testsuite/gdb.python/py-events.py  |    1 +
 7 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 051377d..e55efb1 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22289,12 +22289,16 @@ inherited attribute refer to @code{gdb.ThreadEvent} above.
 
 @item events.exited
 Emits @code{events.ExitedEvent} which indicates that the inferior has exited.
-@code{events.ExitedEvent} has one optional attribute.  This attribute
-will exist only in the case that the inferior exited with some
-status.
+@code{events.ExitedEvent} has two attributes:
 @table @code
 @defvar ExitedEvent.exit_code
-An integer representing the exit code which the inferior has returned.
+An integer representing the exit code, if available, which the inferior 
+has returned.  (The exit code could be unavailable if, for example,
+@value{GDBN} detaches from the inferior.) If the exit code is unavailable,
+the attribute does not exist.
+@end defvar
+@defvar ExitedEvent inferior
+A reference to the inferior which triggered the @code{exited} event.
 @end defvar
 @end table
 
diff --git a/gdb/python/py-event.h b/gdb/python/py-event.h
index f4b3ee2..49d7b6a 100644
--- a/gdb/python/py-event.h
+++ b/gdb/python/py-event.h
@@ -104,7 +104,7 @@ typedef struct
 } event_object;
 
 extern int emit_continue_event (ptid_t ptid);
-extern int emit_exited_event (const LONGEST *exit_code);
+extern int emit_exited_event (const LONGEST *exit_code, struct inferior *inf);
 
 extern int evpy_emit_event (PyObject *event,
                             eventregistry_object *registry);
diff --git a/gdb/python/py-exitedevent.c b/gdb/python/py-exitedevent.c
index 08150e5..85ea2e2 100644
--- a/gdb/python/py-exitedevent.c
+++ b/gdb/python/py-exitedevent.c
@@ -22,9 +22,10 @@
 static PyTypeObject exited_event_object_type;
 
 static PyObject *
-create_exited_event_object (const LONGEST *exit_code)
+create_exited_event_object (const LONGEST *exit_code, struct inferior *inf)
 {
   PyObject *exited_event;
+  PyObject *inf_obj;
 
   exited_event = create_event_object (&exited_event_object_type);
 
@@ -37,6 +38,12 @@ create_exited_event_object (const LONGEST *exit_code)
 			     PyLong_FromLongLong (*exit_code)) < 0)
     goto fail;
 
+  inf_obj = inferior_to_inferior_object (inf);
+  if (!inf_obj || evpy_add_attribute (exited_event,
+                                      "inferior",
+                                      inf_obj) < 0)
+    goto fail;
+
   return exited_event;
 
   fail:
@@ -48,14 +55,14 @@ create_exited_event_object (const LONGEST *exit_code)
    will create a new Python exited event object.  */
 
 int
-emit_exited_event (const LONGEST *exit_code)
+emit_exited_event (const LONGEST *exit_code, struct inferior *inf)
 {
   PyObject *event;
 
   if (evregpy_no_listeners_p (gdb_py_events.exited))
     return 0;
 
-  event = create_exited_event_object (exit_code);
+  event = create_exited_event_object (exit_code, inf);
 
   if (event)
     return evpy_emit_event (event, gdb_py_events.exited);
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 8ed3ea5..275caea 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -119,7 +119,7 @@ python_inferior_exit (struct inferior *inf)
   if (inf->has_exit_code)
     exit_code = &inf->exit_code;
 
-  if (emit_exited_event (exit_code) < 0)
+  if (emit_exited_event (exit_code, inf) < 0)
     gdbpy_print_stack ();
 
   do_cleanups (cleanup);
diff --git a/gdb/testsuite/gdb.python/py-events.c b/gdb/testsuite/gdb.python/py-events.c
index ceb697e..665ca51 100644
--- a/gdb/testsuite/gdb.python/py-events.c
+++ b/gdb/testsuite/gdb.python/py-events.c
@@ -17,6 +17,7 @@
 */
 
 int second(){
+  fork() ;
   return 12;
 }
 
diff --git a/gdb/testsuite/gdb.python/py-events.exp b/gdb/testsuite/gdb.python/py-events.exp
index 2d2139d..18419fa 100644
--- a/gdb/testsuite/gdb.python/py-events.exp
+++ b/gdb/testsuite/gdb.python/py-events.exp
@@ -37,6 +37,8 @@ if ![runto_main ] then {
     return -1
 }
 
+gdb_test_no_output "set detach-on-fork off" ""
+
 gdb_test "Test_Events" "Event testers registered."
 
 gdb_breakpoint "first"
@@ -54,4 +56,11 @@ all threads stopped"
 #test exited event.
 gdb_test "continue" ".*event type: continue.*
 .*event type: exit.*
-.*exit code: 12.*"
+.*exit code: 12.*
+.*exit inf: 1.*" "Inferior 1 terminated."
+
+gdb_test "inferior 2" ".*Switching to inferior 2.*"
+gdb_test "continue" ".*event type: continue.*
+.*event type: exit.*
+.*exit code: 12.*
+.*exit inf: 2.*" "Inferior 2 terminated."
diff --git a/gdb/testsuite/gdb.python/py-events.py b/gdb/testsuite/gdb.python/py-events.py
index 10aea4f..6bdd935 100644
--- a/gdb/testsuite/gdb.python/py-events.py
+++ b/gdb/testsuite/gdb.python/py-events.py
@@ -43,6 +43,7 @@ def exit_handler (event):
     if (isinstance (event, gdb.ExitedEvent)):
         print "event type: exit"
     print "exit code: %d" % (event.exit_code)
+    print "exit inf: %d" % (event.inferior.num)
 
 def continue_handler (event):
     if (isinstance (event, gdb.ContinueEvent)):
-- 
1.7.6

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Eli Zaretskii]

> From: Kevin Pouget <kevin.pouget@gmail.com>
> Date: Mon, 19 Sep 2011 12:15:56 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>, gdb-patches@sourceware.org
> 
> +An integer representing the exit code, if available, which the inferior 
> +has returned.  (The exit code could be unavailable if, for example,
> +@value{GDBN} detaches from the inferior.) If the exit code is unavailable,
> +the attribute does not exist.           ^^

One more space there, please.

Otherwise, fine with me.

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Tom Tromey]

>>>>> "Kevin" == Kevin Pouget <kevin.pouget@gmail.com> writes:

I didn't see whether this one went in yet or not.

Kevin> There was a discrepancy between the version of the patch you approved
Kevin> (http://sourceware.org/ml/gdb-patches/2011-08/msg00659/0001-PR-12691-Add-the-inferior-to-Python-exited-event)
Kevin> and the one I originally submitted (I certainly missed a step during
Kevin> the April->August update in the git tree):
[...]
Kevin> so I think it has to be re-approved.

For avoidance of doubt, this is ok.

thanks,
Tom

Re: [PATCH] PR/12691 Add the inferior to Python exited event

[Kevin Pouget]

On Mon, Oct 3, 2011 at 6:38 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Kevin" == Kevin Pouget <kevin.pouget@gmail.com> writes:
>
> I didn't see whether this one went in yet or not.
>
> Kevin> There was a discrepancy between the version of the patch you approved
> Kevin> (http://sourceware.org/ml/gdb-patches/2011-08/msg00659/0001-PR-12691-Add-the-inferior-to-Python-exited-event)
> Kevin> and the one I originally submitted (I certainly missed a step during
> Kevin> the April->August update in the git tree):
> [...]
> Kevin> so I think it has to be re-approved.
>
> For avoidance of doubt, this is ok.
>
> thanks,
> Tom


Thanks, commited

http://sourceware.org/ml/gdb-cvs/2011-10/msg00019.html

Regression (or a new FAIL?): gdb.python/py-events.exp [Re: [PATCH] PR/12691 Add the inferior to Python exited event]

[Jan Kratochvil]

On Tue, 04 Oct 2011 10:05:06 +0200, Kevin Pouget wrote:
> Thanks, commited
> 
> http://sourceware.org/ml/gdb-cvs/2011-10/msg00019.html

With gdbserver on {x86_64,x86_64-m32,i686}-fedora16pre-linux-gnu I have started
to get:

FAIL: gdb.python/py-events.exp: inferior 2
FAIL: gdb.python/py-events.exp: Inferior 2 terminated. (the program is no longer running)

6080d11494b683ff7c7aeb7141751110fd966187 is the first bad commit
commit 6080d11494b683ff7c7aeb7141751110fd966187
Author: kpouget <kpouget>
Date:   Tue Oct 4 08:04:08 2011 +0000

    2011-10-04  Kevin Pouget  <kevin.pouget@st.com>
    
           PR python/12691: Add the inferior to Python exited event


Thanks,
Jan

Re: Regression (or a new FAIL?): gdb.python/py-events.exp [Re: [PATCH] PR/12691 Add the inferior to Python exited event]

[Kevin Pouget]

On Wed, Oct 5, 2011 at 2:15 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Tue, 04 Oct 2011 10:05:06 +0200, Kevin Pouget wrote:
>> Thanks, commited
>>
>> http://sourceware.org/ml/gdb-cvs/2011-10/msg00019.html
>
> With gdbserver on {x86_64,x86_64-m32,i686}-fedora16pre-linux-gnu I have started
> to get:
>
> FAIL: gdb.python/py-events.exp: inferior 2
> FAIL: gdb.python/py-events.exp: Inferior 2 terminated. (the program is no longer running)
>
> 6080d11494b683ff7c7aeb7141751110fd966187 is the first bad commit
> commit 6080d11494b683ff7c7aeb7141751110fd966187
> Author: kpouget <kpouget>
> Date:   Tue Oct 4 08:04:08 2011 +0000
>
>    2011-10-04  Kevin Pouget  <kevin.pouget@st.com>
>
>           PR python/12691: Add the inferior to Python exited event
>
>
> Thanks,
> Jan

Thanks for noticing that,

it looks more like a failure than a regression since the "native"
testsuite runs correctly on my computer, but I'll investigate it
anyway.
How do you test gdbserver? I'm working on a Fedora 15/X86_64, I should
be able to reproduce it


Thanks,

Kevin

Re: Regression (or a new FAIL?): gdb.python/py-events.exp [Re: [PATCH] PR/12691 Add the inferior to Python exited event]

[Jan Kratochvil]

On Wed, 05 Oct 2011 14:36:36 +0200, Kevin Pouget wrote:
> How do you test gdbserver? I'm working on a Fedora 15/X86_64, I should
> be able to reproduce it

There is:
	http://sourceware.org/gdb/wiki/TestingGDB#Testing_gdbserver_in_a_native_configuration

I use it with my copy:
	git clone git://vps.jankratochvil.net/nethome
	mv nethome/src/runtest-gdbserver ~/src/runtest-gdbserver
	~/src/runtest-gdbserver/runtest-gdbserver gdb.python/py-events.exp

Reproducible with `CFLAGS=-O0 ./configure ;make' on f15-x86_64.


Thanks,
Jan

Re: Regression (or a new FAIL?): gdb.python/py-events.exp [Re: [PATCH] PR/12691 Add the inferior to Python exited event]

[Kevin Pouget]

On Wed, Oct 5, 2011 at 4:15 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Wed, 05 Oct 2011 14:36:36 +0200, Kevin Pouget wrote:
>> How do you test gdbserver? I'm working on a Fedora 15/X86_64, I should
>> be able to reproduce it
>
> There is:
>        http://sourceware.org/gdb/wiki/TestingGDB#Testing_gdbserver_in_a_native_configuration
>
> I use it with my copy:
>        git clone git://vps.jankratochvil.net/nethome
>        mv nethome/src/runtest-gdbserver ~/src/runtest-gdbserver
>        ~/src/runtest-gdbserver/runtest-gdbserver gdb.python/py-events.exp
>
> Reproducible with `CFLAGS=-O0 ./configure ;make' on f15-x86_64.
>
>
> Thanks,
> Jan
>

So I can confirm, it's not a regression but a gdbserver failure:

> $ cat fork.c
> int main (){
>  fork();
> }

> (gdb) target remote :2346
> 0x0000003c47a016b0 in _start () from /lib64/ld-linux-x86-64.so.2
> (gdb) b mainBreakpoint 1 at 0x4004c8: file fork.c, line 2.
> (gdb) cont
> Breakpoint 1, main () at fork.c:22  fork();
> (gdb) set detach-on-fork off
> (gdb) n
> -------------------> Expected: [New process 26420]
> 3 }
> (gdb) quit


what do you want me to do with that, certainly fill a bug report,
shall I change anything in my patch?


Cordially,

Kevin

[patch] gdb.python/py-events.exp: Disable multi-inferior for gdbserver [Re: Regression (or a new FAIL?): gdb.python/py-events.exp]

[Jan Kratochvil]

On Wed, 05 Oct 2011 16:55:28 +0200, Kevin Pouget wrote:
> what do you want me to do with that, certainly fill a bug report,
> shall I change anything in my patch?

I realized gdbserver supports multi-inferior by attaching each new inferior
but it does not yet support (=remote.c ignores) `set detach-on-fork off' at
all.

Therefore proposing the patch below.  I will check it in in some time.


Thanks,
Jan


gdb/testsuite/
2011-10-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.python/py-events.exp: New comment for `set detach-on-fork off'.
	(inferior 2, Inferior 2 terminated.): Run them only if not remote.

--- a/gdb/testsuite/gdb.python/py-events.exp
+++ b/gdb/testsuite/gdb.python/py-events.exp
@@ -53,6 +53,7 @@ gdb_breakpoint "main" {temporary}
 
 gdb_test "run" ".*event type: new_objfile.*new objfile name.*" "New objfile notification"
 
+# Ignored for [is_remote target].
 gdb_test_no_output "set detach-on-fork off" ""
 
 gdb_test "Test_Events" "Event testers registered."
@@ -75,8 +76,10 @@ gdb_test "continue" ".*event type: continue.*
 .*exit code: 12.*
 .*exit inf: 1.*" "Inferior 1 terminated."
 
-gdb_test "inferior 2" ".*Switching to inferior 2.*"
-gdb_test "continue" ".*event type: continue.*
+if ![is_remote target] {
+    gdb_test "inferior 2" ".*Switching to inferior 2.*"
+    gdb_test "continue" ".*event type: continue.*
 .*event type: exit.*
 .*exit code: 12.*
 .*exit inf: 2.*" "Inferior 2 terminated."
+}

Re: [patch] gdb.python/py-events.exp: Disable multi-inferior for gdbserver [Re: Regression (or a new FAIL?): gdb.python/py-events.exp]

[Pedro Alves]

On Sunday 09 October 2011 19:16:54, Jan Kratochvil wrote:
> On Wed, 05 Oct 2011 16:55:28 +0200, Kevin Pouget wrote:
> > what do you want me to do with that, certainly fill a bug report,
> > shall I change anything in my patch?
> 
> I realized gdbserver supports multi-inferior by attaching each new inferior
> but it does not yet support (=remote.c ignores) `set detach-on-fork off' at
> all.

Right, there's no support for following forks or execs in the RSP yet.

> Therefore proposing the patch below.  I will check it in in some time.

> gdb/testsuite/
> 2011-10-09  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.python/py-events.exp: New comment for `set detach-on-fork off'.
> 	(inferior 2, Inferior 2 terminated.): Run them only if not remote.
> 
> --- a/gdb/testsuite/gdb.python/py-events.exp
> +++ b/gdb/testsuite/gdb.python/py-events.exp
> @@ -53,6 +53,7 @@ gdb_breakpoint "main" {temporary}
>  
>  gdb_test "run" ".*event type: new_objfile.*new objfile name.*" "New objfile notification"

"run" doesn't work with "target remote" either.  If that is succeeding,
it's because it's actually running the default native target,
which is obviously bogus.  If that can't be fixed easily, we should skip
the test on remote targets.  But if you see failures related to fork
afterwards, maybe that because you have a customboard file that runs
in extended-remote?

>  
> +# Ignored for [is_remote target].
>  gdb_test_no_output "set detach-on-fork off" ""
>  
>  gdb_test "Test_Events" "Event testers registered."
> @@ -75,8 +76,10 @@ gdb_test "continue" ".*event type: continue.*
>  .*exit code: 12.*
>  .*exit inf: 1.*" "Inferior 1 terminated."
>  
> -gdb_test "inferior 2" ".*Switching to inferior 2.*"
> -gdb_test "continue" ".*event type: continue.*
> +if ![is_remote target] {
> +    gdb_test "inferior 2" ".*Switching to inferior 2.*"
> +    gdb_test "continue" ".*event type: continue.*
>  .*event type: exit.*
>  .*exit code: 12.*
>  .*exit inf: 2.*" "Inferior 2 terminated."
> +}
> 

-- 
Pedro Alves

[patch] Forbid "run" etc. for use_gdb_stub targets [Re: [patch] gdb.python/py-events.exp: Disable multi-inferior for gdbserver]

[Jan Kratochvil]

On Wed, 26 Oct 2011 15:48:58 +0200, Pedro Alves wrote:
> "run" doesn't work with "target remote" either.  If that is succeeding,
> it's because it's actually running the default native target,

Thanks for catching it.  I have seen these false runs already before,
therefore I find it unmanageable to always catch it at least myself.

Maybe some more testfiles could be fixed instead of skipped below, I fixed
one, still I find that as a possible different patch.

I am not sure what is the correct way to handle gdbserver_start_extended.
gdb.server/ext-*.exp run gdbserver even in the default native run.  They
either should not run in the default native or they should not run in the
non-extended gdbserver mode.  Currently they run in both modes the same way.

AFAIK there is currently no way to run the whole testsuite in extended
gdbserver mode.  (There were some attempts for it on the archer list IIRC.)

No regressions on {x86_64,x86_64-m32,i686}-fedora16pre-linux-gnu and with
gdbsever.


Thanks,
Jan


gdb/testsuite/
2011-10-27  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/async-shell.exp: Skip the testfile for is_remote target.
	* gdb.base/attach-pie-misread.exp: Skip the testfile for use_gdb_stub.
	* gdb.base/break-interp.exp: Likewise.
	* gdb.base/default.exp (attach, run "r" abbreviation, run): Skip these
	testcases for use_gdb_stub.
	* gdb.dwarf2/dw2-restore.exp: Fix the run for use_gdb_stub.
	(continue): New testcase.
	* gdb.python/py-events.exp: Skip the testfile for is_remote target.
	* gdb.threads/attach-into-signal.exp: Likewise.
	* gdb.threads/attach-stopped.exp: Likewise.
	* gdb.threads/attachstop-mt.exp: Likewise.
	* lib/gdb.exp (gdb_test_multiple): Forbid run, start or attach for
	use_gdb_stub without gdbserver_start_extended.
	* lib/gdbserver-support.exp (gdbserver_start_extended): Set global
	gdbserver_protocol and gdbserver_gdbport.

--- a/gdb/testsuite/gdb.base/async-shell.exp
+++ b/gdb/testsuite/gdb.base/async-shell.exp
@@ -20,6 +20,10 @@ if { ![support_displaced_stepping] } {
     return -1
 }
 
+if [is_remote target] { 
+    return -1
+}
+
 if { [prepare_for_testing ${testfile}.exp ${testfile}] } {
     return -1
 }
--- a/gdb/testsuite/gdb.base/attach-pie-misread.exp
+++ b/gdb/testsuite/gdb.base/attach-pie-misread.exp
@@ -14,7 +14,8 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 # This test only works on GNU/Linux.
-if { ![isnative] || [is_remote host] || ![istarget *-linux*] || [skip_shlib_tests]} {
+if { ![isnative] || [is_remote host] || [target_info exists use_gdb_stub]
+     || ![istarget *-linux*] || [skip_shlib_tests]} {
     continue
 }
 
--- a/gdb/testsuite/gdb.base/break-interp.exp
+++ b/gdb/testsuite/gdb.base/break-interp.exp
@@ -14,7 +14,8 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 # This test only works on GNU/Linux.
-if { ![isnative] || [is_remote host] || ![istarget *-linux*] || [skip_shlib_tests]} {
+if { ![isnative] || [is_remote host] || [target_info exists use_gdb_stub]
+     || ![istarget *-linux*] || [skip_shlib_tests]} {
     continue
 }
 
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -39,23 +39,23 @@ gdb_test "append value"  "Missing filename\."
 gdb_test "append binary memory" "Missing filename\." 
 gdb_test "append binary value"  "Missing filename\." 
 
-setup_xfail "mips-idt-*"
-gdb_test_multiple "attach" "attach" {
-    -re "Argument required .(process-id|program) to attach.*$gdb_prompt $"\
-	{ pass "attach" }
-    -re "You can't do that when your target is `None'.*$gdb_prompt $"\
-	{ pass "attach" }
-    -re "You can't do that without a process to debug.*$gdb_prompt $"\
-	{ pass "attach" }
-    -re "Don't know how to attach.  Try \"help target\"..*$gdb_prompt $"\
-	{ pass "attach" }
-    -re "Kill it. .y or n." {
-	send_gdb "y\n"
-	exp_continue
+if ![target_info exists use_gdb_stub] {
+    setup_xfail "mips-idt-*"
+    gdb_test_multiple "attach" "attach" {
+	-re "Argument required .(process-id|program) to attach.*$gdb_prompt $"\
+	    { pass "attach" }
+	-re "You can't do that when your target is `None'.*$gdb_prompt $"\
+	    { pass "attach" }
+	-re "You can't do that without a process to debug.*$gdb_prompt $"\
+	    { pass "attach" }
+	-re "Don't know how to attach.  Try \"help target\"..*$gdb_prompt $"\
+	    { pass "attach" }
+	-re "Kill it. .y or n." {
+	    send_gdb "y\n"
+	    exp_continue
+	}
     }
-}
 
-if ![target_info exists use_gdb_stub] {
     gdb_test "break" "No default breakpoint address now." "break"
     foreach i "b br bre brea" {
 	gdb_test $i "No default breakpoint address now." "break \"$i\" abbreviation"
@@ -420,7 +420,9 @@ gdb_test "ptype" "The history is empty." "ptype"
 gdb_test "pwd" "Working directory .*" "pwd"
 
 #test run "r" abbreviation
-if [istarget "*-*-vxworks*"] then {
+if [target_info exists use_gdb_stub] {
+    # Only extended-remote supports "run".
+} elseif [istarget "*-*-vxworks*"] then {
     gdb_test "set args" ".*" "" 
 
     gdb_test "r" "Starting program: .*
@@ -446,7 +448,9 @@ You must specify a function name to run, and arguments if any"\
 }
 
 #test run
-if [istarget "*-*-vxworks*"] then {
+if [target_info exists use_gdb_stub] {
+    # Only extended-remote supports "run".
+} elseif [istarget "*-*-vxworks*"] then {
     gdb_test "set args" ".*" "" 
 
     gdb_test "run" "Starting program: .*
--- a/gdb/testsuite/gdb.dwarf2/dw2-restore.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-restore.exp
@@ -33,5 +33,8 @@ gdb_start
 gdb_reinitialize_dir $srcdir/$subdir
 gdb_load ${binfile}
 
-gdb_test "run" ".*"
-gdb_test "where" ".*$hex in foo ().+$hex in _start ().*"
+if ![runto foo] {
+    return 0
+}
+gdb_test "continue" "$hex in foo \\(\\)"
+gdb_test "where" "#0 +$hex in foo \\(\\)\r\n#1 +$hex in _start \\(\\)"
--- a/gdb/testsuite/gdb.python/py-events.exp
+++ b/gdb/testsuite/gdb.python/py-events.exp
@@ -13,6 +13,10 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+if [target_info exists use_gdb_stub] {
+    return 0
+}
+
 if $tracelevel then {
     strace $tracelevel
 }
--- a/gdb/testsuite/gdb.threads/attach-into-signal.exp
+++ b/gdb/testsuite/gdb.threads/attach-into-signal.exp
@@ -17,7 +17,8 @@
 # This file was created by Jan Kratochvil <jan.kratochvil@redhat.com>.
 
 # This test only works on Linux
-if { ![isnative] || [is_remote host] || ![istarget *-linux*] } {
+if { ![isnative] || [is_remote host] || [target_info exists use_gdb_stub]
+     || ![istarget *-linux*] } {
     continue
 }
 
--- a/gdb/testsuite/gdb.threads/attach-stopped.exp
+++ b/gdb/testsuite/gdb.threads/attach-stopped.exp
@@ -18,7 +18,8 @@
 # This file was updated by Jan Kratochvil <jan.kratochvil@redhat.com>.
 
 # This test only works on Linux
-if { ![isnative] || [is_remote host] || ![istarget *-linux*] } {
+if { ![isnative] || [is_remote host] || [target_info exists use_gdb_stub]
+     || ![istarget *-linux*] } {
     continue
 }
 
--- a/gdb/testsuite/gdb.threads/attachstop-mt.exp
+++ b/gdb/testsuite/gdb.threads/attachstop-mt.exp
@@ -17,7 +17,8 @@
 # <jjohnstn@redhat.com> and Jan Kratochvil <jan.kratochvil@redhat.com>.
 
 # This test only works on Linux
-if { ![isnative] || [is_remote host] || ![istarget *-linux*] } {
+if { ![isnative] || [is_remote host] || [target_info exists use_gdb_stub]
+     || ![istarget *-linux*] } {
     continue
 }
 
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -601,6 +601,18 @@ proc gdb_test_multiple { command message user_code } {
 	error "Invalid newline in \"$message\" test"
     }
 
+    if {[regexp -nocase {^\s*(r|run|star|start|at|att|atta|attac|attach)\M} \
+	 $command]
+        && [target_info exists use_gdb_stub]} {
+	global gdbserver_protocol
+
+	# gdbserver_start_extended runs as a part of non-extended configuration.
+	if {![info exists gdbserver_protocol]
+	    || ![string match "extended-*" $gdbserver_protocol]} {
+	    error "gdbserver does not support $command without extended-remote"
+	}
+    }
+
     # TCL/EXPECT WART ALERT
     # Expect does something very strange when it receives a single braced
     # argument.  It splits it along word separators and performs substitutions.
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -337,6 +337,9 @@ proc gdbserver_reconnect { } {
 
 # Start and connect to a gdbserver in extended mode.
 proc gdbserver_start_extended { } {
+    global gdbserver_protocol
+    global gdbserver_gdbport
+
     set res [gdbserver_start "--multi" ""]
     set gdbserver_protocol "extended-[lindex $res 0]"
     set gdbserver_gdbport [lindex $res 1]

Re: [patch] Forbid "run" etc. for use_gdb_stub targets [Re: [patch] gdb.python/py-events.exp: Disable multi-inferior for gdbserver]

[Pedro Alves]

On Thursday 27 October 2011 11:17:00, Jan Kratochvil wrote:
> On Wed, 26 Oct 2011 15:48:58 +0200, Pedro Alves wrote:
> > "run" doesn't work with "target remote" either.  If that is succeeding,
> > it's because it's actually running the default native target,
> 
> Thanks for catching it.  I have seen these false runs already before,
> therefore I find it unmanageable to always catch it at least myself.

Funny, they tend to pop and ring alarm bells right at me.  :-)

> Maybe some more testfiles could be fixed instead of skipped below, I fixed
> one, still I find that as a possible different patch.
> 
> I am not sure what is the correct way to handle gdbserver_start_extended.
> gdb.server/ext-*.exp run gdbserver even in the default native run.  They
> either should not run in the default native or they should not run in the
> non-extended gdbserver mode.  Currently they run in both modes the same way.

These tests were written to smoke test gdbserver on native test
runs.  Not sure what you mean.  Is is that use_gdb_stub will be wrong?
Maybe we can override it?

> AFAIK there is currently no way to run the whole testsuite in extended
> gdbserver mode.  

Yeah.  Maybe it's possible today with a clever board file.  But it'd be
nice to have it integrated without hacks (even if it means a different
board file, which probabably does).

> (There were some attempts for it on the archer list IIRC.)

Missed those.

> No regressions on {x86_64,x86_64-m32,i686}-fedora16pre-linux-gnu and with
> gdbsever.
> 
> 
> Thanks,
> Jan
> 
> 
> gdb/testsuite/
> 2011-10-27  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.base/async-shell.exp: Skip the testfile for is_remote target.
> 	* gdb.base/attach-pie-misread.exp: Skip the testfile for use_gdb_stub.
> 	* gdb.base/break-interp.exp: Likewise.
> 	* gdb.base/default.exp (attach, run "r" abbreviation, run): Skip these
> 	testcases for use_gdb_stub.
> 	* gdb.dwarf2/dw2-restore.exp: Fix the run for use_gdb_stub.
> 	(continue): New testcase.
> 	* gdb.python/py-events.exp: Skip the testfile for is_remote target.
> 	* gdb.threads/attach-into-signal.exp: Likewise.
> 	* gdb.threads/attach-stopped.exp: Likewise.
> 	* gdb.threads/attachstop-mt.exp: Likewise.
> 	* lib/gdb.exp (gdb_test_multiple): Forbid run, start or attach for
> 	use_gdb_stub without gdbserver_start_extended.
> 	* lib/gdbserver-support.exp (gdbserver_start_extended): Set global
> 	gdbserver_protocol and gdbserver_gdbport.
> 
> --- a/gdb/testsuite/gdb.base/async-shell.exp
> +++ b/gdb/testsuite/gdb.base/async-shell.exp
> @@ -20,6 +20,10 @@ if { ![support_displaced_stepping] } {
>      return -1
>  }
>  
> +if [is_remote target] { 
> +    return -1
> +}

Shouldn't this be a check for use_gdb_stub instead?  (I know
the bug was originally linux native.)  Doesn't matter much though.

The test fixes look right to me.  IMO, you should put them
in upfront and separately from the new smart detection.

> +    if {[regexp -nocase {^\s*(r|run|star|start|at|att|atta|attac|attach)\M} \
> +	 $command]
> +        && [target_info exists use_gdb_stub]} {
> +	global gdbserver_protocol
> +
> +	# gdbserver_start_extended runs as a part of non-extended configuration.
> +	if {![info exists gdbserver_protocol]
> +	    || ![string match "extended-*" $gdbserver_protocol]} {
> +	    error "gdbserver does not support $command without extended-remote"
> +	}
> +    }

I think this should be:

   if {[info exists gdbserver_protocol]
	    && ![string match "extended-*" $gdbserver_protocol]} {
	    error "gdbserver does not support $command without extended-remote"

We don't want to throw an error if gdbserver_protocol does not exist?

-- 
Pedro Alves

[commit test fixes] [patch] Forbid "run" etc. for use_gdb_stub targets [Re: [patch] gdb.python/py-events.exp: Disable multi-inferior for gdbserver]

[Jan Kratochvil]

On Thu, 27 Oct 2011 20:09:36 +0200, Pedro Alves wrote:
> > I am not sure what is the correct way to handle gdbserver_start_extended.
> > gdb.server/ext-*.exp run gdbserver even in the default native run.  They
> > either should not run in the default native or they should not run in the
> > non-extended gdbserver mode.  Currently they run in both modes the same way.
> 
> These tests were written to smoke test gdbserver on native test runs.

Therefore shouldn't be the gdb.server/ext-*.exp testfiles skipped if the
target board defined use_gdb_stub?  (ext-attach.exp already is skipped, for
the [is_remote target] reason)

I have left it as is as the same duplicate run with different boards does not
harm anything.


> > --- a/gdb/testsuite/gdb.base/async-shell.exp
> > +++ b/gdb/testsuite/gdb.base/async-shell.exp
> > @@ -20,6 +20,10 @@ if { ![support_displaced_stepping] } {
> >      return -1
> >  }
> >  
> > +if [is_remote target] { 
> > +    return -1
> > +}
> 
> Shouldn't this be a check for use_gdb_stub instead?  (I know
> the bug was originally linux native.)  Doesn't matter much though.

Used now:
# The testfile uses "run".  The real bug happened only for ![is_remote target].

You are right the test can run everywhere !use_gdb_stub.  But with remote.c in
use the bug being tested cannot happen (there are no two child processes
conflicting for wait(PID) result), so it just gives PASS not testing much.


> The test fixes look right to me.  IMO, you should put them
> in upfront and separately from the new smart detection.

Checked in those.


Thanks,
Jan



http://sourceware.org/ml/gdb-cvs/2011-10/msg00208.html

--- src/gdb/testsuite/ChangeLog	2011/10/28 14:49:18	1.2913
+++ src/gdb/testsuite/ChangeLog	2011/10/28 17:32:28	1.2914
@@ -1,15 +1,27 @@
+2011-10-28  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* gdb.base/async-shell.exp: Skip the testfile for use_gdb_stub.
+	* gdb.base/attach-pie-misread.exp: Likewise.
+	* gdb.base/break-interp.exp: Likewise.
+	* gdb.base/default.exp (attach, run "r" abbreviation, run): Skip these
+	testcases for use_gdb_stub.
+	* gdb.dwarf2/dw2-restore.exp: Fix the run for use_gdb_stub.
+	(continue): New testcase.
+	* gdb.python/py-events.exp: Skip the testfile for use_gdb_stub.
+	* gdb.threads/attach-into-signal.exp: Likewise.
+	* gdb.threads/attach-stopped.exp: Likewise.
+	* gdb.threads/attachstop-mt.exp: Likewise.
+
 2011-10-28  Paul Koning  <paul_koning@dell.com>
 
 	* gdb.python/lib-types.exp (deep_items): Rename from deepitems.
 
-
 2011-10-27  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	* lib/mi-support.exp (breakpoint_re): Suppress match reporting.
 	(mi_gdb_test): Import globals thread_selected_re
 	and breakpoint_re.  Expect them optionally at the regex start.
 
-
 2011-10-27  Doug Evans  <dje@google.com>
 
 	* gdb.python/python.exp: Test source -s.
--- src/gdb/testsuite/gdb.base/async-shell.exp	2011/07/18 04:14:21	1.2
+++ src/gdb/testsuite/gdb.base/async-shell.exp	2011/10/28 17:32:29	1.3
@@ -20,6 +20,11 @@
     return -1
 }
 
+# The testfile uses "run".  The real bug happened only for ![is_remote target].
+if [target_info exists use_gdb_stub] {
+    return 0
+}
+
 if { [prepare_for_testing ${testfile}.exp ${testfile}] } {
     return -1
 }
--- src/gdb/testsuite/gdb.base/attach-pie-misread.exp	2011/01/01 15:33:40	1.3
+++ src/gdb/testsuite/gdb.base/attach-pie-misread.exp	2011/10/28 17:32:30	1.4
@@ -14,7 +14,8 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 # This test only works on GNU/Linux.
-if { ![isnative] || [is_remote host] || ![istarget *-linux*] || [skip_shlib_tests]} {
+if { ![isnative] || [is_remote host] || [target_info exists use_gdb_stub]
+     || ![istarget *-linux*] || [skip_shlib_tests]} {
     continue
 }
 
--- src/gdb/testsuite/gdb.base/break-interp.exp	2011/07/01 19:12:12	1.30
+++ src/gdb/testsuite/gdb.base/break-interp.exp	2011/10/28 17:32:30	1.31
@@ -14,7 +14,8 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 # This test only works on GNU/Linux.
-if { ![isnative] || [is_remote host] || ![istarget *-linux*] || [skip_shlib_tests]} {
+if { ![isnative] || [is_remote host] || [target_info exists use_gdb_stub]
+     || ![istarget *-linux*] || [skip_shlib_tests]} {
     continue
 }
 
--- src/gdb/testsuite/gdb.base/default.exp	2011/05/20 14:37:20	1.49
+++ src/gdb/testsuite/gdb.base/default.exp	2011/10/28 17:32:30	1.50
@@ -39,23 +39,23 @@
 gdb_test "append binary memory" "Missing filename\." 
 gdb_test "append binary value"  "Missing filename\." 
 
-setup_xfail "mips-idt-*"
-gdb_test_multiple "attach" "attach" {
-    -re "Argument required .(process-id|program) to attach.*$gdb_prompt $"\
-	{ pass "attach" }
-    -re "You can't do that when your target is `None'.*$gdb_prompt $"\
-	{ pass "attach" }
-    -re "You can't do that without a process to debug.*$gdb_prompt $"\
-	{ pass "attach" }
-    -re "Don't know how to attach.  Try \"help target\"..*$gdb_prompt $"\
-	{ pass "attach" }
-    -re "Kill it. .y or n." {
-	send_gdb "y\n"
-	exp_continue
+if ![target_info exists use_gdb_stub] {
+    setup_xfail "mips-idt-*"
+    gdb_test_multiple "attach" "attach" {
+	-re "Argument required .(process-id|program) to attach.*$gdb_prompt $"\
+	    { pass "attach" }
+	-re "You can't do that when your target is `None'.*$gdb_prompt $"\
+	    { pass "attach" }
+	-re "You can't do that without a process to debug.*$gdb_prompt $"\
+	    { pass "attach" }
+	-re "Don't know how to attach.  Try \"help target\"..*$gdb_prompt $"\
+	    { pass "attach" }
+	-re "Kill it. .y or n." {
+	    send_gdb "y\n"
+	    exp_continue
+	}
     }
-}
 
-if ![target_info exists use_gdb_stub] {
     gdb_test "break" "No default breakpoint address now." "break"
     foreach i "b br bre brea" {
 	gdb_test $i "No default breakpoint address now." "break \"$i\" abbreviation"
@@ -420,7 +420,9 @@
 gdb_test "pwd" "Working directory .*" "pwd"
 
 #test run "r" abbreviation
-if [istarget "*-*-vxworks*"] then {
+if [target_info exists use_gdb_stub] {
+    # Only extended-remote supports "run".
+} elseif [istarget "*-*-vxworks*"] then {
     gdb_test "set args" ".*" "" 
 
     gdb_test "r" "Starting program: .*
@@ -446,7 +448,9 @@
 }
 
 #test run
-if [istarget "*-*-vxworks*"] then {
+if [target_info exists use_gdb_stub] {
+    # Only extended-remote supports "run".
+} elseif [istarget "*-*-vxworks*"] then {
     gdb_test "set args" ".*" "" 
 
     gdb_test "run" "Starting program: .*
--- src/gdb/testsuite/gdb.dwarf2/dw2-restore.exp	2011/05/13 17:17:22	1.5
+++ src/gdb/testsuite/gdb.dwarf2/dw2-restore.exp	2011/10/28 17:32:30	1.6
@@ -33,5 +33,8 @@
 gdb_reinitialize_dir $srcdir/$subdir
 gdb_load ${binfile}
 
-gdb_test "run" ".*"
-gdb_test "where" ".*$hex in foo ().+$hex in _start ().*"
+if ![runto foo] {
+    return 0
+}
+gdb_test "continue" "$hex in foo \\(\\)"
+gdb_test "where" "#0 +$hex in foo \\(\\)\r\n#1 +$hex in _start \\(\\)"
--- src/gdb/testsuite/gdb.python/py-events.exp	2011/10/07 07:38:30	1.5
+++ src/gdb/testsuite/gdb.python/py-events.exp	2011/10/28 17:32:30	1.6
@@ -13,6 +13,10 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+if [target_info exists use_gdb_stub] {
+    return 0
+}
+
 if $tracelevel then {
     strace $tracelevel
 }
--- src/gdb/testsuite/gdb.threads/attach-into-signal.exp	2011/01/01 15:33:50	1.6
+++ src/gdb/testsuite/gdb.threads/attach-into-signal.exp	2011/10/28 17:32:30	1.7
@@ -17,7 +17,8 @@
 # This file was created by Jan Kratochvil <jan.kratochvil@redhat.com>.
 
 # This test only works on Linux
-if { ![isnative] || [is_remote host] || ![istarget *-linux*] } {
+if { ![isnative] || [is_remote host] || [target_info exists use_gdb_stub]
+     || ![istarget *-linux*] } {
     continue
 }
 
--- src/gdb/testsuite/gdb.threads/attach-stopped.exp	2011/10/14 16:05:05	1.7
+++ src/gdb/testsuite/gdb.threads/attach-stopped.exp	2011/10/28 17:32:30	1.8
@@ -18,7 +18,8 @@
 # This file was updated by Jan Kratochvil <jan.kratochvil@redhat.com>.
 
 # This test only works on Linux
-if { ![isnative] || [is_remote host] || ![istarget *-linux*] } {
+if { ![isnative] || [is_remote host] || [target_info exists use_gdb_stub]
+     || ![istarget *-linux*] } {
     continue
 }
 
--- src/gdb/testsuite/gdb.threads/attachstop-mt.exp	2011/10/14 16:05:05	1.8
+++ src/gdb/testsuite/gdb.threads/attachstop-mt.exp	2011/10/28 17:32:30	1.9
@@ -17,7 +17,8 @@
 # <jjohnstn@redhat.com> and Jan Kratochvil <jan.kratochvil@redhat.com>.
 
 # This test only works on Linux
-if { ![isnative] || [is_remote host] || ![istarget *-linux*] } {
+if { ![isnative] || [is_remote host] || [target_info exists use_gdb_stub]
+     || ![istarget *-linux*] } {
     continue
 }
 

Re: [patch] Forbid "run" etc. for use_gdb_stub targets [Re: [patch] gdb.python/py-events.exp: Disable multi-inferior for gdbserver]

[Jan Kratochvil]

On Thu, 27 Oct 2011 20:09:36 +0200, Pedro Alves wrote:
> Not sure what you mean.  Is is that use_gdb_stub will be wrong?
> Maybe we can override it?

Done overriding both ways, to have use_gdb_stub always right.

Do you agree with it this way?


> > AFAIK there is currently no way to run the whole testsuite in extended
> > gdbserver mode.  
[...]
> Missed those.

It was for kernel-based ugdb using extended-remote protocol, but I see now it
is not worth of a reusal for extended-remote FSF gdbserver run.
	Re: ugdb for the gdb testsuite (failed)
	http://sourceware.org/ml/archer/2010-q3/msg00147.html


> > +    if {[regexp -nocase {^\s*(r|run|star|start|at|att|atta|attac|attach)\M} \
> > +	 $command]
> > +        && [target_info exists use_gdb_stub]} {
> > +	global gdbserver_protocol
> > +
> > +	# gdbserver_start_extended runs as a part of non-extended configuration.
> > +	if {![info exists gdbserver_protocol]
> > +	    || ![string match "extended-*" $gdbserver_protocol]} {
> > +	    error "gdbserver does not support $command without extended-remote"
> > +	}
> > +    }
> 
> I think this should be:
> 
>    if {[info exists gdbserver_protocol]
> 	    && ![string match "extended-*" $gdbserver_protocol]} {
> 	    error "gdbserver does not support $command without extended-remote"
> 
> We don't want to throw an error if gdbserver_protocol does not exist?

It was a leftover, thanks; anyway dropped this conditional now when
use_gdb_stub should be always correct now.


Thanks,
Jan


gdb/testsuite/
2011-10-28  Jan Kratochvil  <jan.kratochvil@redhat.com>

        * lib/gdb.exp (gdb_test_multiple): Forbid run, start or attach for
        use_gdb_stub.
        * lib/gdbserver-support.exp (gdbserver_start): Set use_gdb_stub for
	gdb_protocol unset from the target board.
	(gdbserver_start_extended): Set global gdbserver_protocol and
	gdbserver_gdbport.  Unset use_gdb_stub.

--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -601,6 +601,12 @@ proc gdb_test_multiple { command message user_code } {
 	error "Invalid newline in \"$message\" test"
     }
 
+    if {[regexp -nocase {^\s*(r|run|star|start|at|att|atta|attac|attach)\M} \
+	 $command]
+        && [target_info exists use_gdb_stub]} {
+	error "gdbserver does not support $command without extended-remote"
+    }
+
     # TCL/EXPECT WART ALERT
     # Expect does something very strange when it receives a single braced
     # argument.  It splits it along word separators and performs substitutions.
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -206,6 +206,7 @@ proc gdbserver_start { options arguments } {
     if [target_info exists gdb_protocol] {
 	set protocol [target_info gdb_protocol]
     } else {
+	set_currtarget_info use_gdb_stub 1
 	set protocol "remote"
     }
 
@@ -337,9 +338,13 @@ proc gdbserver_reconnect { } {
 
 # Start and connect to a gdbserver in extended mode.
 proc gdbserver_start_extended { } {
+    global gdbserver_protocol
+    global gdbserver_gdbport
+
     set res [gdbserver_start "--multi" ""]
     set gdbserver_protocol "extended-[lindex $res 0]"
     set gdbserver_gdbport [lindex $res 1]
+    unset_currtarget_info use_gdb_stub
 
     return [gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport]
 }

Re: [patch] Forbid "run" etc. for use_gdb_stub targets [Re: [patch] gdb.python/py-events.exp: Disable multi-inferior for gdbserver]

[Pedro Alves]

On Friday 28 October 2011 18:40:14, Jan Kratochvil wrote:
> On Thu, 27 Oct 2011 20:09:36 +0200, Pedro Alves wrote:
> > Not sure what you mean.  Is is that use_gdb_stub will be wrong?
> > Maybe we can override it?
> 
> Done overriding both ways, to have use_gdb_stub always right.
> 
> Do you agree with it this way?

Yes, but see below.

> It was a leftover, thanks; anyway dropped this conditional now when
> use_gdb_stub should be always correct now.

Ah, much better.

> +
>      # TCL/EXPECT WART ALERT
>      # Expect does something very strange when it receives a single braced
>      # argument.  It splits it along word separators and performs substitutions.
> --- a/gdb/testsuite/lib/gdbserver-support.exp
> +++ b/gdb/testsuite/lib/gdbserver-support.exp
> @@ -206,6 +206,7 @@ proc gdbserver_start { options arguments } {
>      if [target_info exists gdb_protocol] {
>  	set protocol [target_info gdb_protocol]
>      } else {
> +	set_currtarget_info use_gdb_stub 1
>  	set protocol "remote"
>      }

Isn't this too late though?  We do the [target_info exists use_gdb_stub]
check at the top of the test files, before gdbserver_start has had a chance
of running.  So I'm guessing that in the first test that runs right
after the gdb.server/ tests complete, use_gdb_stub will be wrong at the top of
the test, but it'll get fixed by that test, so the following ones are right.
If that test file happen to have a use_gdb_stub check, it'll lose.
We may need to do this closer to gdb_init/default_gdb_init (or an override).

>  
> @@ -337,9 +338,13 @@ proc gdbserver_reconnect { } {
>  
>  # Start and connect to a gdbserver in extended mode.
>  proc gdbserver_start_extended { } {
> +    global gdbserver_protocol
> +    global gdbserver_gdbport
> +
>      set res [gdbserver_start "--multi" ""]
>      set gdbserver_protocol "extended-[lindex $res 0]"
>      set gdbserver_gdbport [lindex $res 1]
> +    unset_currtarget_info use_gdb_stub
>  
>      return [gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport]
>  }
> 

-- 
Pedro Alves

Re: [patch] Forbid "run" etc. for use_gdb_stub targets [Re: [patch] gdb.python/py-events.exp: Disable multi-inferior for gdbserver]

[Jan Kratochvil]

On Fri, 28 Oct 2011 20:17:44 +0200, Pedro Alves wrote:
> Isn't this too late though?  We do the [target_info exists use_gdb_stub]
> check at the top of the test files, before gdbserver_start has had a chance
> of running.  So I'm guessing that in the first test that runs right
> after the gdb.server/ tests complete, use_gdb_stub will be wrong at the top of
> the test, but it'll get fixed by that test, so the following ones are right.
> If that test file happen to have a use_gdb_stub check, it'll lose.
> We may need to do this closer to gdb_init/default_gdb_init (or an override).

I have tested it and you are right, I had some wrong dejagnu assumptions.

Modifying target_info's use_gdb_stub is probably not much correct, therefore
there is created global $use_gdb_stub instead.

Global $use_gdb_stub is intentionally unset before gdb_start to cause errors
trying to interpret the value before.  Some problem is interpreting
[target_info exists use_gdb_stub] after gdb_start is incorrect but it does not
cause any sanity-check error.

No regressions on {x86_64,x86_64-m32,i686}-fedora16pre-linux-gnu and with
gdbserver.


Thanks,
Jan


gdb/testsuite/
2011-10-29  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/break-entry.exp: Move the target use_gdb_stub test before
	starting GDB.
	* gdb.base/default.exp: Replace target use_gdb_stub checks by global
	$use_gdb_stub.
	* gdb.base/display.exp: Likewise.
	* gdb.base/ending-run.exp: Likewise.
	* gdb.base/list.exp (test_listsize): Likewise.
	* gdb.base/setshow.exp: Likewise.
	* gdb.base/valgrind-db-attach.exp: Set global use_gdb_stub to 1.
	* lib/gdb.exp (gdb_run_cmd, gdb_start_cmd): Replace target use_gdb_stub
	check by global $use_gdb_stub.
	(gdb_test_multiple): Forbid run, start or attach for !$use_gdb_stub.
	(default_gdb_start): Set global use_gdb_stub from target use_gdb_stub.
	(default_gdb_init): Unset global $use_gdb_stub.
	(gdb_continue_to_end, rerun_to_main): Replace target use_gdb_stub check
	by global $use_gdb_stub.
	* lib/gdbserver-support.exp: Extend comments for set_board_info
	gdb_protocol and gdb,socketport, new comment for set_board_info
	use_gdb_stub.
	(gdbserver_start_extended): Set global gdbserver_protocol and
	gdbserver_gdbport.  Clear global use_gdb_stub.
	* lib/mi-support.exp (default_mi_gdb_start): Set global use_gdb_stub
	from target use_gdb_stub.
	(mi_run_cmd): Replace target use_gdb_stub check by global $use_gdb_stub.
--- a/gdb/testsuite/gdb.base/break-entry.exp
+++ b/gdb/testsuite/gdb.base/break-entry.exp
@@ -16,11 +16,6 @@
 # Test inferior can stop at its very first instruction, usually "_start".
 # Dynamic executables have first instruction in ld.so.
 
-set testfile break-entry
-if { [prepare_for_testing ${testfile}.exp ${testfile} start.c {additional_flags=-static}] } {
-    return -1
-}
-
 # If we're using a stub, we'll already be debugging a live program and
 # stopped at the entry point when we connect, and so the runto below
 # will issue a "continue", which always skips any breakpoint at PC.
@@ -30,11 +25,17 @@ if { [prepare_for_testing ${testfile}.exp ${testfile} start.c {additional_flags=
 # continues the process with the equivalent of "jump *$PC", which
 # triggers any breakpoint at $PC.  The latter is what we want to test.
 
+set testfile break-entry
+
 if [target_info exists use_gdb_stub] {
     untested ${testfile}.exp
     return
 }
 
+if { [prepare_for_testing ${testfile}.exp ${testfile} start.c {additional_flags=-static}] } {
+    return -1
+}
+
 set test "info files"
 set entry ""
 gdb_test_multiple $test $test {
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -39,7 +39,7 @@ gdb_test "append value"  "Missing filename\."
 gdb_test "append binary memory" "Missing filename\." 
 gdb_test "append binary value"  "Missing filename\." 
 
-if ![target_info exists use_gdb_stub] {
+if !$use_gdb_stub {
     setup_xfail "mips-idt-*"
     gdb_test_multiple "attach" "attach" {
 	-re "Argument required .(process-id|program) to attach.*$gdb_prompt $"\
@@ -420,7 +420,7 @@ gdb_test "ptype" "The history is empty." "ptype"
 gdb_test "pwd" "Working directory .*" "pwd"
 
 #test run "r" abbreviation
-if [target_info exists use_gdb_stub] {
+if $use_gdb_stub {
     # Only extended-remote supports "run".
 } elseif [istarget "*-*-vxworks*"] then {
     gdb_test "set args" ".*" "" 
@@ -448,7 +448,7 @@ You must specify a function name to run, and arguments if any"\
 }
 
 #test run
-if [target_info exists use_gdb_stub] {
+if $use_gdb_stub {
     # Only extended-remote supports "run".
 } elseif [istarget "*-*-vxworks*"] then {
     gdb_test "set args" ".*" "" 
--- a/gdb/testsuite/gdb.base/display.exp
+++ b/gdb/testsuite/gdb.base/display.exp
@@ -51,7 +51,7 @@ gdb_load ${binfile}
 
 # Some coverage stuff
 #
-if ![target_info exists use_gdb_stub] {
+if !$use_gdb_stub {
     gdb_test "kill" ".*The program is not being run.*"
     gdb_test "detach" ".*"
     gdb_test "run" ".*"
--- a/gdb/testsuite/gdb.base/ending-run.exp
+++ b/gdb/testsuite/gdb.base/ending-run.exp
@@ -208,7 +208,7 @@ gdb_test_multiple "next" "step out of main" {
 set program_exited_normally 0
 set program_not_exited 0
 set program_in_exit 0
-if {! [target_info exists use_gdb_stub]
+if {!$use_gdb_stub
     && (! [target_info exists use_cygmon] || ! [target_info use_cygmon])} {
     global program_exited;
     if {[eval expr $program_exited == 0]} {
--- a/gdb/testsuite/gdb.base/list.exp
+++ b/gdb/testsuite/gdb.base/list.exp
@@ -81,7 +81,7 @@ proc set_listsize { arg } {
 #
 
 proc test_listsize {} {
-    global gdb_prompt
+    global gdb_prompt use_gdb_stub
     global hp_cc_compiler
     global hp_aCC_compiler
 
@@ -94,7 +94,7 @@ proc test_listsize {} {
     # list the lines there instead of main, so we skip this test for remote targets.
     # The second case is for optimized code, it is still correct.
     
-    if [target_info exists use_gdb_stub] {
+    if $use_gdb_stub {
 	runto_main;
 	unsupported "list default lines around main";
     } else {
--- a/gdb/testsuite/gdb.base/setshow.exp
+++ b/gdb/testsuite/gdb.base/setshow.exp
@@ -97,7 +97,7 @@ gdb_test_no_output "set args foo bar blup baz bubble" "set args"
 gdb_test "show args" "Argument list to give program being debugged when it is started is \"foo bar blup baz bubble\"..*" "show args"
 
 # Don't test if we can't pass args or if we're using a stub.
-if { ![target_info exists use_gdb_stub] && ![target_info exists noargs] } {
+if { !$use_gdb_stub && ![target_info exists noargs] } {
     #test passing args
     gdb_test "cont" "Continuing.*" "continuing"
     delete_breakpoints
--- a/gdb/testsuite/gdb.base/valgrind-db-attach.exp
+++ b/gdb/testsuite/gdb.base/valgrind-db-attach.exp
@@ -41,6 +41,9 @@ pass $test
 # Declare GDB now as running.
 set gdb_spawn_id -1
 
+# This value applies to GDB spawned by valgrind --db-attach=yes.
+set use_gdb_stub 1
+
 set test "valgrind started"
 # The trailing '.' differs for different memcheck versions.
 gdb_test_multiple "" $test {
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -196,7 +196,7 @@ proc delete_breakpoints {} {
 # elsewhere.
 #
 proc gdb_run_cmd {args} {
-    global gdb_prompt
+    global gdb_prompt use_gdb_stub
 
     if [target_info exists gdb_init_command] {
 	send_gdb "[target_info gdb_init_command]\n";
@@ -209,7 +209,7 @@ proc gdb_run_cmd {args} {
 	}
     }
 
-    if [target_info exists use_gdb_stub] {
+    if $use_gdb_stub {
 	if [target_info exists gdb,do_reload_on_run] {
 	    if { [gdb_reload] != 0 } {
 		return;
@@ -302,7 +302,7 @@ proc gdb_run_cmd {args} {
 # if we could not.
 
 proc gdb_start_cmd {args} {
-    global gdb_prompt
+    global gdb_prompt use_gdb_stub
 
     if [target_info exists gdb_init_command] {
 	send_gdb "[target_info gdb_init_command]\n";
@@ -315,7 +315,7 @@ proc gdb_start_cmd {args} {
 	}
     }
 
-    if [target_info exists use_gdb_stub] {
+    if $use_gdb_stub {
 	return -1
     }
 
@@ -582,7 +582,7 @@ proc gdb_internal_error_resync {} {
 # ...", all being implicitly appended to that list.
 #
 proc gdb_test_multiple { command message user_code } {
-    global verbose
+    global verbose use_gdb_stub
     global gdb_prompt
     global GDB
     global inferior_exited_re
@@ -601,6 +601,12 @@ proc gdb_test_multiple { command message user_code } {
 	error "Invalid newline in \"$message\" test"
     }
 
+    if {$use_gdb_stub
+        && [regexp -nocase {^\s*(r|run|star|start|at|att|atta|attac|attach)\M} \
+	    $command]} {
+	error "gdbserver does not support $command without extended-remote"
+    }
+
     # TCL/EXPECT WART ALERT
     # Expect does something very strange when it receives a single braced
     # argument.  It splits it along word separators and performs substitutions.
@@ -1316,7 +1322,7 @@ proc gdb_file_cmd { arg } {
 # get really slow.  Give gdb at least 3 minutes to start up.
 #
 proc default_gdb_start { } {
-    global verbose
+    global verbose use_gdb_stub
     global GDB
     global INTERNAL_GDBFLAGS GDBFLAGS
     global gdb_prompt
@@ -1325,6 +1331,9 @@ proc default_gdb_start { } {
 
     gdb_stop_suppressing_tests;
 
+    # Set the default value, it may be overriden later by specific testfile.
+    set use_gdb_stub [target_info exists use_gdb_stub]
+
     verbose "Spawning $GDB $INTERNAL_GDBFLAGS $GDBFLAGS"
 
     if [info exists gdb_spawn_id] {
@@ -2951,6 +2960,10 @@ proc default_gdb_init { args } {
     } else {
 	set gdb_prompt "\\(gdb\\)"
     }
+    global use_gdb_stub
+    if [info exists use_gdb_stub] {
+	unset use_gdb_stub
+    }
 }
 
 # The default timeout used when testing GDB commands.  We want to use
@@ -3297,7 +3310,7 @@ proc gdb_get_line_number { text { file "" } } {
 #	is accepted.
 
 proc gdb_continue_to_end {{mssg ""} {command continue} {allow_extra 0}} {
-  global inferior_exited_re
+  global inferior_exited_re use_gdb_stub
 
   if {$mssg == ""} {
       set text "continue until exit"
@@ -3309,7 +3322,7 @@ proc gdb_continue_to_end {{mssg ""} {command continue} {allow_extra 0}} {
   } else {
       set extra ""
   }
-  if [target_info exists use_gdb_stub] {
+  if $use_gdb_stub {
     if {![gdb_breakpoint "exit"]} {
       return 0
     }
@@ -3326,9 +3339,9 @@ proc gdb_continue_to_end {{mssg ""} {command continue} {allow_extra 0}} {
 }
 
 proc rerun_to_main {} {
-  global gdb_prompt
+  global gdb_prompt use_gdb_stub
 
-  if [target_info exists use_gdb_stub] {
+  if $use_gdb_stub {
     gdb_run_cmd
     gdb_expect {
       -re ".*Breakpoint .*main .*$gdb_prompt $"\
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -22,6 +22,14 @@
 #
 #   set_board_info gdb_protocol "remote"
 #	Unles you have a gdbserver that uses a different protocol...
+#	After GDB starts you should check global $gdbserver_protocol instead as
+#	the testfile may force a specific different target protocol itself.
+#
+#   set_board_info use_gdb_stub
+#	Flag the inferior is already started after connecting and run/attach
+#	are not supported.  This is used for the "remote" protocol.
+#	After GDB starts you should check global $use_gdb_stub instead as the
+#	testfile may force a specific different target protocol itself.
 #
 #   set_board_info gdb_server_prog
 #	This will be the path to the gdbserver program you want to test.
@@ -35,6 +43,8 @@
 #   set_board_info gdb,socketport
 #	Port id to use for socket connection.  If not set explicitly,
 #	it will start at "2345" and increment for each use.
+#	After GDB starts you should check global $gdbserver_gdbport for the
+#	real port used.  It is not useful if $gdbserver_reconnect_p was not set.
 #
 
 #
@@ -337,9 +347,17 @@ proc gdbserver_reconnect { } {
 
 # Start and connect to a gdbserver in extended mode.
 proc gdbserver_start_extended { } {
+    global gdbserver_protocol
+    global gdbserver_gdbport
+    global use_gdb_stub
+
     set res [gdbserver_start "--multi" ""]
     set gdbserver_protocol "extended-[lindex $res 0]"
     set gdbserver_gdbport [lindex $res 1]
 
+    # For both native mode and for stub-using gdbserver target board we run
+    # a new gdbserver in extended-remote mode, therefore without stub.
+    set use_gdb_stub 0
+
     return [gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport]
 }
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -96,7 +96,7 @@ proc mi_uncatched_gdb_exit {} {
 # get really slow.  Give gdb at least 3 minutes to start up.
 #
 proc default_mi_gdb_start { args } {
-    global verbose
+    global verbose use_gdb_stub
     global GDB
     global INTERNAL_GDBFLAGS GDBFLAGS
     global gdb_prompt
@@ -108,6 +108,9 @@ proc default_mi_gdb_start { args } {
     gdb_stop_suppressing_tests;
     set inferior_pty no-tty
 
+    # Set the default value, it may be overriden later by specific testfile.
+    set use_gdb_stub [target_info exists use_gdb_stub]
+
     if { [llength $args] == 1} {
 	set inferior_pty [lindex $args 0]
     }
@@ -788,7 +791,7 @@ proc mi_run_cmd {args} {
     if { $suppress_flag } {
 	return -1
     }
-    global mi_gdb_prompt
+    global mi_gdb_prompt use_gdb_stub
     global thread_selected_re
     global library_loaded_re
 
@@ -807,7 +810,7 @@ proc mi_run_cmd {args} {
 	return -1
     }
 
-    if [target_info exists use_gdb_stub] {
+    if $use_gdb_stub {
 	if [target_info exists gdb,do_reload_on_run] {
 	    send_gdb "220-exec-continue\n";
 	    gdb_expect 60 {

ping: Re: [patch] Forbid "run" etc. for use_gdb_stub targets

[Jan Kratochvil]

I am a bit reluctant to check it on my own as I already made there some
mistakes before.  I see there is more work going on with this code.


On Sat, 29 Oct 2011 21:47:45 +0200, Jan Kratochvil wrote:

On Fri, 28 Oct 2011 20:17:44 +0200, Pedro Alves wrote:
> Isn't this too late though?  We do the [target_info exists use_gdb_stub]
> check at the top of the test files, before gdbserver_start has had a chance
> of running.  So I'm guessing that in the first test that runs right
> after the gdb.server/ tests complete, use_gdb_stub will be wrong at the top of
> the test, but it'll get fixed by that test, so the following ones are right.
> If that test file happen to have a use_gdb_stub check, it'll lose.
> We may need to do this closer to gdb_init/default_gdb_init (or an override).

I have tested it and you are right, I had some wrong dejagnu assumptions.

Modifying target_info's use_gdb_stub is probably not much correct, therefore
there is created global $use_gdb_stub instead.

Global $use_gdb_stub is intentionally unset before gdb_start to cause errors
trying to interpret the value before.  Some problem is interpreting
[target_info exists use_gdb_stub] after gdb_start is incorrect but it does not
cause any sanity-check error.

No regressions on {x86_64,x86_64-m32,i686}-fedora16pre-linux-gnu and with
gdbserver.


Thanks,
Jan


gdb/testsuite/
2011-10-29  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/break-entry.exp: Move the target use_gdb_stub test before
	starting GDB.
	* gdb.base/default.exp: Replace target use_gdb_stub checks by global
	$use_gdb_stub.
	* gdb.base/display.exp: Likewise.
	* gdb.base/ending-run.exp: Likewise.
	* gdb.base/list.exp (test_listsize): Likewise.
	* gdb.base/setshow.exp: Likewise.
	* gdb.base/valgrind-db-attach.exp: Set global use_gdb_stub to 1.
	* lib/gdb.exp (gdb_run_cmd, gdb_start_cmd): Replace target use_gdb_stub
	check by global $use_gdb_stub.
	(gdb_test_multiple): Forbid run, start or attach for !$use_gdb_stub.
	(default_gdb_start): Set global use_gdb_stub from target use_gdb_stub.
	(default_gdb_init): Unset global $use_gdb_stub.
	(gdb_continue_to_end, rerun_to_main): Replace target use_gdb_stub check
	by global $use_gdb_stub.
	* lib/gdbserver-support.exp: Extend comments for set_board_info
	gdb_protocol and gdb,socketport, new comment for set_board_info
	use_gdb_stub.
	(gdbserver_start_extended): Set global gdbserver_protocol and
	gdbserver_gdbport.  Clear global use_gdb_stub.
	* lib/mi-support.exp (default_mi_gdb_start): Set global use_gdb_stub
	from target use_gdb_stub.
	(mi_run_cmd): Replace target use_gdb_stub check by global $use_gdb_stub.

--- a/gdb/testsuite/gdb.base/break-entry.exp
+++ b/gdb/testsuite/gdb.base/break-entry.exp
@@ -16,11 +16,6 @@
 # Test inferior can stop at its very first instruction, usually "_start".
 # Dynamic executables have first instruction in ld.so.
 
-set testfile break-entry
-if { [prepare_for_testing ${testfile}.exp ${testfile} start.c {additional_flags=-static}] } {
-    return -1
-}
-
 # If we're using a stub, we'll already be debugging a live program and
 # stopped at the entry point when we connect, and so the runto below
 # will issue a "continue", which always skips any breakpoint at PC.
@@ -30,11 +25,17 @@ if { [prepare_for_testing ${testfile}.exp ${testfile} start.c {additional_flags=
 # continues the process with the equivalent of "jump *$PC", which
 # triggers any breakpoint at $PC.  The latter is what we want to test.
 
+set testfile break-entry
+
 if [target_info exists use_gdb_stub] {
     untested ${testfile}.exp
     return
 }
 
+if { [prepare_for_testing ${testfile}.exp ${testfile} start.c {additional_flags=-static}] } {
+    return -1
+}
+
 set test "info files"
 set entry ""
 gdb_test_multiple $test $test {
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -39,7 +39,7 @@ gdb_test "append value"  "Missing filename\."
 gdb_test "append binary memory" "Missing filename\." 
 gdb_test "append binary value"  "Missing filename\." 
 
-if ![target_info exists use_gdb_stub] {
+if !$use_gdb_stub {
     setup_xfail "mips-idt-*"
     gdb_test_multiple "attach" "attach" {
 	-re "Argument required .(process-id|program) to attach.*$gdb_prompt $"\
@@ -420,7 +420,7 @@ gdb_test "ptype" "The history is empty." "ptype"
 gdb_test "pwd" "Working directory .*" "pwd"
 
 #test run "r" abbreviation
-if [target_info exists use_gdb_stub] {
+if $use_gdb_stub {
     # Only extended-remote supports "run".
 } elseif [istarget "*-*-vxworks*"] then {
     gdb_test "set args" ".*" "" 
@@ -448,7 +448,7 @@ You must specify a function name to run, and arguments if any"\
 }
 
 #test run
-if [target_info exists use_gdb_stub] {
+if $use_gdb_stub {
     # Only extended-remote supports "run".
 } elseif [istarget "*-*-vxworks*"] then {
     gdb_test "set args" ".*" "" 
--- a/gdb/testsuite/gdb.base/display.exp
+++ b/gdb/testsuite/gdb.base/display.exp
@@ -51,7 +51,7 @@ gdb_load ${binfile}
 
 # Some coverage stuff
 #
-if ![target_info exists use_gdb_stub] {
+if !$use_gdb_stub {
     gdb_test "kill" ".*The program is not being run.*"
     gdb_test "detach" ".*"
     gdb_test "run" ".*"
--- a/gdb/testsuite/gdb.base/ending-run.exp
+++ b/gdb/testsuite/gdb.base/ending-run.exp
@@ -208,7 +208,7 @@ gdb_test_multiple "next" "step out of main" {
 set program_exited_normally 0
 set program_not_exited 0
 set program_in_exit 0
-if {! [target_info exists use_gdb_stub]
+if {!$use_gdb_stub
     && (! [target_info exists use_cygmon] || ! [target_info use_cygmon])} {
     global program_exited;
     if {[eval expr $program_exited == 0]} {
--- a/gdb/testsuite/gdb.base/list.exp
+++ b/gdb/testsuite/gdb.base/list.exp
@@ -81,7 +81,7 @@ proc set_listsize { arg } {
 #
 
 proc test_listsize {} {
-    global gdb_prompt
+    global gdb_prompt use_gdb_stub
     global hp_cc_compiler
     global hp_aCC_compiler
 
@@ -94,7 +94,7 @@ proc test_listsize {} {
     # list the lines there instead of main, so we skip this test for remote targets.
     # The second case is for optimized code, it is still correct.
     
-    if [target_info exists use_gdb_stub] {
+    if $use_gdb_stub {
 	runto_main;
 	unsupported "list default lines around main";
     } else {
--- a/gdb/testsuite/gdb.base/setshow.exp
+++ b/gdb/testsuite/gdb.base/setshow.exp
@@ -97,7 +97,7 @@ gdb_test_no_output "set args foo bar blup baz bubble" "set args"
 gdb_test "show args" "Argument list to give program being debugged when it is started is \"foo bar blup baz bubble\"..*" "show args"
 
 # Don't test if we can't pass args or if we're using a stub.
-if { ![target_info exists use_gdb_stub] && ![target_info exists noargs] } {
+if { !$use_gdb_stub && ![target_info exists noargs] } {
     #test passing args
     gdb_test "cont" "Continuing.*" "continuing"
     delete_breakpoints
--- a/gdb/testsuite/gdb.base/valgrind-db-attach.exp
+++ b/gdb/testsuite/gdb.base/valgrind-db-attach.exp
@@ -41,6 +41,9 @@ pass $test
 # Declare GDB now as running.
 set gdb_spawn_id -1
 
+# This value applies to GDB spawned by valgrind --db-attach=yes.
+set use_gdb_stub 1
+
 set test "valgrind started"
 # The trailing '.' differs for different memcheck versions.
 gdb_test_multiple "" $test {
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -196,7 +196,7 @@ proc delete_breakpoints {} {
 # elsewhere.
 #
 proc gdb_run_cmd {args} {
-    global gdb_prompt
+    global gdb_prompt use_gdb_stub
 
     if [target_info exists gdb_init_command] {
 	send_gdb "[target_info gdb_init_command]\n";
@@ -209,7 +209,7 @@ proc gdb_run_cmd {args} {
 	}
     }
 
-    if [target_info exists use_gdb_stub] {
+    if $use_gdb_stub {
 	if [target_info exists gdb,do_reload_on_run] {
 	    if { [gdb_reload] != 0 } {
 		return;
@@ -302,7 +302,7 @@ proc gdb_run_cmd {args} {
 # if we could not.
 
 proc gdb_start_cmd {args} {
-    global gdb_prompt
+    global gdb_prompt use_gdb_stub
 
     if [target_info exists gdb_init_command] {
 	send_gdb "[target_info gdb_init_command]\n";
@@ -315,7 +315,7 @@ proc gdb_start_cmd {args} {
 	}
     }
 
-    if [target_info exists use_gdb_stub] {
+    if $use_gdb_stub {
 	return -1
     }
 
@@ -582,7 +582,7 @@ proc gdb_internal_error_resync {} {
 # ...", all being implicitly appended to that list.
 #
 proc gdb_test_multiple { command message user_code } {
-    global verbose
+    global verbose use_gdb_stub
     global gdb_prompt
     global GDB
     global inferior_exited_re
@@ -601,6 +601,12 @@ proc gdb_test_multiple { command message user_code } {
 	error "Invalid newline in \"$message\" test"
     }
 
+    if {$use_gdb_stub
+        && [regexp -nocase {^\s*(r|run|star|start|at|att|atta|attac|attach)\M} \
+	    $command]} {
+	error "gdbserver does not support $command without extended-remote"
+    }
+
     # TCL/EXPECT WART ALERT
     # Expect does something very strange when it receives a single braced
     # argument.  It splits it along word separators and performs substitutions.
@@ -1316,7 +1322,7 @@ proc gdb_file_cmd { arg } {
 # get really slow.  Give gdb at least 3 minutes to start up.
 #
 proc default_gdb_start { } {
-    global verbose
+    global verbose use_gdb_stub
     global GDB
     global INTERNAL_GDBFLAGS GDBFLAGS
     global gdb_prompt
@@ -1325,6 +1331,9 @@ proc default_gdb_start { } {
 
     gdb_stop_suppressing_tests;
 
+    # Set the default value, it may be overriden later by specific testfile.
+    set use_gdb_stub [target_info exists use_gdb_stub]
+
     verbose "Spawning $GDB $INTERNAL_GDBFLAGS $GDBFLAGS"
 
     if [info exists gdb_spawn_id] {
@@ -2951,6 +2960,10 @@ proc default_gdb_init { args } {
     } else {
 	set gdb_prompt "\\(gdb\\)"
     }
+    global use_gdb_stub
+    if [info exists use_gdb_stub] {
+	unset use_gdb_stub
+    }
 }
 
 # The default timeout used when testing GDB commands.  We want to use
@@ -3297,7 +3310,7 @@ proc gdb_get_line_number { text { file "" } } {
 #	is accepted.
 
 proc gdb_continue_to_end {{mssg ""} {command continue} {allow_extra 0}} {
-  global inferior_exited_re
+  global inferior_exited_re use_gdb_stub
 
   if {$mssg == ""} {
       set text "continue until exit"
@@ -3309,7 +3322,7 @@ proc gdb_continue_to_end {{mssg ""} {command continue} {allow_extra 0}} {
   } else {
       set extra ""
   }
-  if [target_info exists use_gdb_stub] {
+  if $use_gdb_stub {
     if {![gdb_breakpoint "exit"]} {
       return 0
     }
@@ -3326,9 +3339,9 @@ proc gdb_continue_to_end {{mssg ""} {command continue} {allow_extra 0}} {
 }
 
 proc rerun_to_main {} {
-  global gdb_prompt
+  global gdb_prompt use_gdb_stub
 
-  if [target_info exists use_gdb_stub] {
+  if $use_gdb_stub {
     gdb_run_cmd
     gdb_expect {
       -re ".*Breakpoint .*main .*$gdb_prompt $"\
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -22,6 +22,14 @@
 #
 #   set_board_info gdb_protocol "remote"
 #	Unles you have a gdbserver that uses a different protocol...
+#	After GDB starts you should check global $gdbserver_protocol instead as
+#	the testfile may force a specific different target protocol itself.
+#
+#   set_board_info use_gdb_stub
+#	Flag the inferior is already started after connecting and run/attach
+#	are not supported.  This is used for the "remote" protocol.
+#	After GDB starts you should check global $use_gdb_stub instead as the
+#	testfile may force a specific different target protocol itself.
 #
 #   set_board_info gdb_server_prog
 #	This will be the path to the gdbserver program you want to test.
@@ -35,6 +43,8 @@
 #   set_board_info gdb,socketport
 #	Port id to use for socket connection.  If not set explicitly,
 #	it will start at "2345" and increment for each use.
+#	After GDB starts you should check global $gdbserver_gdbport for the
+#	real port used.  It is not useful if $gdbserver_reconnect_p was not set.
 #
 
 #
@@ -337,9 +347,17 @@ proc gdbserver_reconnect { } {
 
 # Start and connect to a gdbserver in extended mode.
 proc gdbserver_start_extended { } {
+    global gdbserver_protocol
+    global gdbserver_gdbport
+    global use_gdb_stub
+
     set res [gdbserver_start "--multi" ""]
     set gdbserver_protocol "extended-[lindex $res 0]"
     set gdbserver_gdbport [lindex $res 1]
 
+    # For both native mode and for stub-using gdbserver target board we run
+    # a new gdbserver in extended-remote mode, therefore without stub.
+    set use_gdb_stub 0
+
     return [gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport]
 }
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -96,7 +96,7 @@ proc mi_uncatched_gdb_exit {} {
 # get really slow.  Give gdb at least 3 minutes to start up.
 #
 proc default_mi_gdb_start { args } {
-    global verbose
+    global verbose use_gdb_stub
     global GDB
     global INTERNAL_GDBFLAGS GDBFLAGS
     global gdb_prompt
@@ -108,6 +108,9 @@ proc default_mi_gdb_start { args } {
     gdb_stop_suppressing_tests;
     set inferior_pty no-tty
 
+    # Set the default value, it may be overriden later by specific testfile.
+    set use_gdb_stub [target_info exists use_gdb_stub]
+
     if { [llength $args] == 1} {
 	set inferior_pty [lindex $args 0]
     }
@@ -788,7 +791,7 @@ proc mi_run_cmd {args} {
     if { $suppress_flag } {
 	return -1
     }
-    global mi_gdb_prompt
+    global mi_gdb_prompt use_gdb_stub
     global thread_selected_re
     global library_loaded_re
 
@@ -807,7 +810,7 @@ proc mi_run_cmd {args} {
 	return -1
     }
 
-    if [target_info exists use_gdb_stub] {
+    if $use_gdb_stub {
 	if [target_info exists gdb,do_reload_on_run] {
 	    send_gdb "220-exec-continue\n";
 	    gdb_expect 60 {

Re: ping: Re: [patch] Forbid "run" etc. for use_gdb_stub targets

[Pedro Alves]

On Saturday 03 December 2011 18:36:13, Jan Kratochvil wrote:
> I am a bit reluctant to check it on my own as I already made there some
> mistakes before.  I see there is more work going on with this code.

Sorry, and thanks.  I completely forgot about this until I saw Doug's patch.

> --- a/gdb/testsuite/gdb.base/valgrind-db-attach.exp
> +++ b/gdb/testsuite/gdb.base/valgrind-db-attach.exp
> @@ -41,6 +41,9 @@ pass $test
>  # Declare GDB now as running.
>  set gdb_spawn_id -1
>  
> +# This value applies to GDB spawned by valgrind --db-attach=yes.
> +set use_gdb_stub 1

Why?

> --- a/gdb/testsuite/lib/gdbserver-support.exp
> +++ b/gdb/testsuite/lib/gdbserver-support.exp
> @@ -22,6 +22,14 @@
>  #
>  #   set_board_info gdb_protocol "remote"
>  #      Unles you have a gdbserver that uses a different protocol...
> +#      After GDB starts you should check global $gdbserver_protocol instead as
> +#      the testfile may force a specific different target protocol itself.
> +#
> +#   set_board_info use_gdb_stub
> +#      Flag the inferior is already started after connecting and run/attach
> +#      are not supported.  This is used for the "remote" protocol.
> +#      After GDB starts you should check global $use_gdb_stub instead as the
> +#      testfile may force a specific different target protocol itself.

This is the wrong place to document this.  use_gdb_stub is not
really gdbserver-specific.  Please document it in the common
framework, somewhere in gdb.exp?

>  # Start and connect to a gdbserver in extended mode.
>  proc gdbserver_start_extended { } {
> +    global gdbserver_protocol
> +    global gdbserver_gdbport
> +    global use_gdb_stub
> +
>      set res [gdbserver_start "--multi" ""]
>      set gdbserver_protocol "extended-[lindex $res 0]"
>      set gdbserver_gdbport [lindex $res 1]
>  
> +    # For both native mode and for stub-using gdbserver target board we run
> +    # a new gdbserver in extended-remote mode, therefore without stub.
> +    set use_gdb_stub 0

I suggest instead:

   # Even if the board file is testing with target remote, our caller
   # wants to test against gdbserver in extended-remote mode.  Make sure to
   # disable stub-like techniques.

> +
>      return [gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport]
>  }

Otherwise looks good to me.

-- 
Pedro Alves

[commit] [patch] Forbid "run" etc. for use_gdb_stub targets

[Jan Kratochvil]

On Sat, 03 Dec 2011 20:14:52 +0100, Pedro Alves wrote:
> > --- a/gdb/testsuite/gdb.base/valgrind-db-attach.exp
> > +++ b/gdb/testsuite/gdb.base/valgrind-db-attach.exp
> > +# This value applies to GDB spawned by valgrind --db-attach=yes.
> > +set use_gdb_stub 1
> 
> Why?

Used now:

+# GDB spawned by `valgrind --db-attach=yes' stops already after the startup
is
+# executed, like with non-extended gdbserver.  It is also not correct to
+# run/attach the inferior.
+set use_gdb_stub 1


> > --- a/gdb/testsuite/lib/gdbserver-support.exp
> > +++ b/gdb/testsuite/lib/gdbserver-support.exp
> > @@ -22,6 +22,14 @@
> > +#   set_board_info use_gdb_stub
> > +#      Flag the inferior is already started after connecting and run/attach
> > +#      are not supported.  This is used for the "remote" protocol.
> > +#      After GDB starts you should check global $use_gdb_stub instead as the
> > +#      testfile may force a specific different target protocol itself.
> 
> This is the wrong place to document this.  use_gdb_stub is not
> really gdbserver-specific.  Please document it in the common
> framework, somewhere in gdb.exp?

OK, moved to default_gdb_start:

+    # Set the default value, it may be overriden later by specific testfile.
+    #
+    # Use `set_board_info use_gdb_stub' for the board file to flag the inferior
+    # is already started after connecting and run/attach are not supported.
+    # This is used for the "remote" protocol.  After GDB starts you should
+    # check global $use_gdb_stub instead of the board as the testfile may force
+    # a specific different target protocol itself.
+    set use_gdb_stub [target_info exists use_gdb_stub]


> I suggest instead:

Used.


> Otherwise looks good to me.

And checked in.


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2011-12/msg00022.html

--- src/gdb/testsuite/ChangeLog	2011/12/03 18:01:49	1.2961
+++ src/gdb/testsuite/ChangeLog	2011/12/03 20:20:28	1.2962
@@ -1,3 +1,30 @@
+2011-12-03  Jan Kratochvil  <jan.kratochvil@redhat.com>
+	    Pedro Alves  <pedro@codesourcery.com>
+
+	* gdb.base/break-entry.exp: Move the target use_gdb_stub test before
+	starting GDB.
+	* gdb.base/default.exp: Replace target use_gdb_stub checks by global
+	$use_gdb_stub.
+	* gdb.base/display.exp: Likewise.
+	* gdb.base/ending-run.exp: Likewise.
+	* gdb.base/list.exp (test_listsize): Likewise.
+	* gdb.base/setshow.exp: Likewise.
+	* gdb.base/valgrind-db-attach.exp: Set global use_gdb_stub to 1.
+	* lib/gdb.exp (gdb_run_cmd, gdb_start_cmd): Replace target use_gdb_stub
+	check by global $use_gdb_stub.
+	(gdb_test_multiple): Forbid run, start or attach for !$use_gdb_stub.
+	(default_gdb_start): Set global use_gdb_stub from target use_gdb_stub.
+	(default_gdb_init): Unset global $use_gdb_stub.
+	(gdb_continue_to_end, rerun_to_main): Replace target use_gdb_stub check
+	by global $use_gdb_stub.
+	* lib/gdbserver-support.exp: Extend comments for set_board_info
+	gdb_protocol and gdb,socketport.
+	(gdbserver_start_extended): Set global gdbserver_protocol and
+	gdbserver_gdbport.  Clear global use_gdb_stub.
+	* lib/mi-support.exp (default_mi_gdb_start): Set global use_gdb_stub
+	from target use_gdb_stub.
+	(mi_run_cmd): Replace target use_gdb_stub check by global $use_gdb_stub.
+
 2011-12-03  Doug Evans  <dje@google.com>
 
 	* lib/gdb.exp (gdb_run_cmd, gdb_start_cmd, run_to_main): Add comments.
--- src/gdb/testsuite/gdb.base/break-entry.exp	2011/01/01 15:33:40	1.4
+++ src/gdb/testsuite/gdb.base/break-entry.exp	2011/12/03 20:20:28	1.5
@@ -16,11 +16,6 @@
 # Test inferior can stop at its very first instruction, usually "_start".
 # Dynamic executables have first instruction in ld.so.
 
-set testfile break-entry
-if { [prepare_for_testing ${testfile}.exp ${testfile} start.c {additional_flags=-static}] } {
-    return -1
-}
-
 # If we're using a stub, we'll already be debugging a live program and
 # stopped at the entry point when we connect, and so the runto below
 # will issue a "continue", which always skips any breakpoint at PC.
@@ -30,11 +25,17 @@
 # continues the process with the equivalent of "jump *$PC", which
 # triggers any breakpoint at $PC.  The latter is what we want to test.
 
+set testfile break-entry
+
 if [target_info exists use_gdb_stub] {
     untested ${testfile}.exp
     return
 }
 
+if { [prepare_for_testing ${testfile}.exp ${testfile} start.c {additional_flags=-static}] } {
+    return -1
+}
+
 set test "info files"
 set entry ""
 gdb_test_multiple $test $test {
--- src/gdb/testsuite/gdb.base/default.exp	2011/10/28 17:32:30	1.50
+++ src/gdb/testsuite/gdb.base/default.exp	2011/12/03 20:20:29	1.51
@@ -39,7 +39,7 @@
 gdb_test "append binary memory" "Missing filename\." 
 gdb_test "append binary value"  "Missing filename\." 
 
-if ![target_info exists use_gdb_stub] {
+if !$use_gdb_stub {
     setup_xfail "mips-idt-*"
     gdb_test_multiple "attach" "attach" {
 	-re "Argument required .(process-id|program) to attach.*$gdb_prompt $"\
@@ -420,7 +420,7 @@
 gdb_test "pwd" "Working directory .*" "pwd"
 
 #test run "r" abbreviation
-if [target_info exists use_gdb_stub] {
+if $use_gdb_stub {
     # Only extended-remote supports "run".
 } elseif [istarget "*-*-vxworks*"] then {
     gdb_test "set args" ".*" "" 
@@ -448,7 +448,7 @@
 }
 
 #test run
-if [target_info exists use_gdb_stub] {
+if $use_gdb_stub {
     # Only extended-remote supports "run".
 } elseif [istarget "*-*-vxworks*"] then {
     gdb_test "set args" ".*" "" 
--- src/gdb/testsuite/gdb.base/display.exp	2011/08/09 16:21:18	1.26
+++ src/gdb/testsuite/gdb.base/display.exp	2011/12/03 20:20:29	1.27
@@ -51,7 +51,7 @@
 
 # Some coverage stuff
 #
-if ![target_info exists use_gdb_stub] {
+if !$use_gdb_stub {
     gdb_test "kill" ".*The program is not being run.*"
     gdb_test "detach" ".*"
     gdb_test "run" ".*"
--- src/gdb/testsuite/gdb.base/ending-run.exp	2011/06/01 15:55:55	1.46
+++ src/gdb/testsuite/gdb.base/ending-run.exp	2011/12/03 20:20:29	1.47
@@ -208,7 +208,7 @@
 set program_exited_normally 0
 set program_not_exited 0
 set program_in_exit 0
-if {! [target_info exists use_gdb_stub]
+if {!$use_gdb_stub
     && (! [target_info exists use_cygmon] || ! [target_info use_cygmon])} {
     global program_exited;
     if {[eval expr $program_exited == 0]} {
--- src/gdb/testsuite/gdb.base/list.exp	2011/01/01 15:33:42	1.24
+++ src/gdb/testsuite/gdb.base/list.exp	2011/12/03 20:20:29	1.25
@@ -81,7 +81,7 @@
 #
 
 proc test_listsize {} {
-    global gdb_prompt
+    global gdb_prompt use_gdb_stub
     global hp_cc_compiler
     global hp_aCC_compiler
 
@@ -94,7 +94,7 @@
     # list the lines there instead of main, so we skip this test for remote targets.
     # The second case is for optimized code, it is still correct.
     
-    if [target_info exists use_gdb_stub] {
+    if $use_gdb_stub {
 	runto_main;
 	unsupported "list default lines around main";
     } else {
--- src/gdb/testsuite/gdb.base/setshow.exp	2011/04/20 14:56:49	1.21
+++ src/gdb/testsuite/gdb.base/setshow.exp	2011/12/03 20:20:29	1.22
@@ -97,7 +97,7 @@
 gdb_test "show args" "Argument list to give program being debugged when it is started is \"foo bar blup baz bubble\"..*" "show args"
 
 # Don't test if we can't pass args or if we're using a stub.
-if { ![target_info exists use_gdb_stub] && ![target_info exists noargs] } {
+if { !$use_gdb_stub && ![target_info exists noargs] } {
     #test passing args
     gdb_test "cont" "Continuing.*" "continuing"
     delete_breakpoints
--- src/gdb/testsuite/gdb.base/valgrind-db-attach.exp	2011/09/08 14:56:34	1.8
+++ src/gdb/testsuite/gdb.base/valgrind-db-attach.exp	2011/12/03 20:20:29	1.9
@@ -41,6 +41,11 @@
 # Declare GDB now as running.
 set gdb_spawn_id -1
 
+# GDB spawned by `valgrind --db-attach=yes' stops already after the startup is
+# executed, like with non-extended gdbserver.  It is also not correct to
+# run/attach the inferior.
+set use_gdb_stub 1
+
 set test "valgrind started"
 # The trailing '.' differs for different memcheck versions.
 gdb_test_multiple "" $test {
--- src/gdb/testsuite/lib/gdb.exp	2011/12/03 18:01:50	1.195
+++ src/gdb/testsuite/lib/gdb.exp	2011/12/03 20:20:29	1.196
@@ -197,7 +197,7 @@
 # that is the caller's responsibility.
 
 proc gdb_run_cmd {args} {
-    global gdb_prompt
+    global gdb_prompt use_gdb_stub
 
     if [target_info exists gdb_init_command] {
 	send_gdb "[target_info gdb_init_command]\n";
@@ -210,7 +210,7 @@
 	}
     }
 
-    if [target_info exists use_gdb_stub] {
+    if $use_gdb_stub {
 	if [target_info exists gdb,do_reload_on_run] {
 	    if { [gdb_reload] != 0 } {
 		return;
@@ -306,7 +306,7 @@
 # that is the caller's responsibility.
 
 proc gdb_start_cmd {args} {
-    global gdb_prompt
+    global gdb_prompt use_gdb_stub
 
     if [target_info exists gdb_init_command] {
 	send_gdb "[target_info gdb_init_command]\n";
@@ -319,7 +319,7 @@
 	}
     }
 
-    if [target_info exists use_gdb_stub] {
+    if $use_gdb_stub {
 	return -1
     }
 
@@ -587,7 +587,7 @@
 # ...", all being implicitly appended to that list.
 #
 proc gdb_test_multiple { command message user_code } {
-    global verbose
+    global verbose use_gdb_stub
     global gdb_prompt
     global GDB
     global inferior_exited_re
@@ -606,6 +606,12 @@
 	error "Invalid newline in \"$message\" test"
     }
 
+    if {$use_gdb_stub
+        && [regexp -nocase {^\s*(r|run|star|start|at|att|atta|attac|attach)\M} \
+	    $command]} {
+	error "gdbserver does not support $command without extended-remote"
+    }
+
     # TCL/EXPECT WART ALERT
     # Expect does something very strange when it receives a single braced
     # argument.  It splits it along word separators and performs substitutions.
@@ -1317,7 +1323,7 @@
 # get really slow.  Give gdb at least 3 minutes to start up.
 #
 proc default_gdb_start { } {
-    global verbose
+    global verbose use_gdb_stub
     global GDB
     global INTERNAL_GDBFLAGS GDBFLAGS
     global gdb_prompt
@@ -1326,6 +1332,15 @@
 
     gdb_stop_suppressing_tests;
 
+    # Set the default value, it may be overriden later by specific testfile.
+    #
+    # Use `set_board_info use_gdb_stub' for the board file to flag the inferior
+    # is already started after connecting and run/attach are not supported.
+    # This is used for the "remote" protocol.  After GDB starts you should
+    # check global $use_gdb_stub instead of the board as the testfile may force
+    # a specific different target protocol itself.
+    set use_gdb_stub [target_info exists use_gdb_stub]
+
     verbose "Spawning $GDB $INTERNAL_GDBFLAGS $GDBFLAGS"
 
     if [info exists gdb_spawn_id] {
@@ -2978,6 +2993,10 @@
     } else {
 	set gdb_prompt "\\(gdb\\)"
     }
+    global use_gdb_stub
+    if [info exists use_gdb_stub] {
+	unset use_gdb_stub
+    }
 }
 
 # The default timeout used when testing GDB commands.  We want to use
@@ -3350,7 +3369,7 @@
 #	is accepted.
 
 proc gdb_continue_to_end {{mssg ""} {command continue} {allow_extra 0}} {
-  global inferior_exited_re
+  global inferior_exited_re use_gdb_stub
 
   if {$mssg == ""} {
       set text "continue until exit"
@@ -3362,7 +3381,7 @@
   } else {
       set extra ""
   }
-  if [target_info exists use_gdb_stub] {
+  if $use_gdb_stub {
     if {![gdb_breakpoint "exit"]} {
       return 0
     }
@@ -3379,9 +3398,9 @@
 }
 
 proc rerun_to_main {} {
-  global gdb_prompt
+  global gdb_prompt use_gdb_stub
 
-  if [target_info exists use_gdb_stub] {
+  if $use_gdb_stub {
     gdb_run_cmd
     gdb_expect {
       -re ".*Breakpoint .*main .*$gdb_prompt $"\
--- src/gdb/testsuite/lib/gdbserver-support.exp	2011/11/20 23:59:49	1.20
+++ src/gdb/testsuite/lib/gdbserver-support.exp	2011/12/03 20:20:29	1.21
@@ -22,6 +22,8 @@
 #
 #   set_board_info gdb_protocol "remote"
 #	Unles you have a gdbserver that uses a different protocol...
+#	After GDB starts you should check global $gdbserver_protocol instead as
+#	the testfile may force a specific different target protocol itself.
 #
 #   set_board_info gdb_server_prog
 #	This will be the path to the gdbserver program you want to test.
@@ -35,6 +37,8 @@
 #   set_board_info gdb,socketport
 #	Port id to use for socket connection.  If not set explicitly,
 #	it will start at "2345" and increment for each use.
+#	After GDB starts you should check global $gdbserver_gdbport for the
+#	real port used.  It is not useful if $gdbserver_reconnect_p was not set.
 #
 
 #
@@ -338,9 +342,18 @@
 
 # Start and connect to a gdbserver in extended mode.
 proc gdbserver_start_extended { } {
+    global gdbserver_protocol
+    global gdbserver_gdbport
+    global use_gdb_stub
+
     set res [gdbserver_start "--multi" ""]
     set gdbserver_protocol "extended-[lindex $res 0]"
     set gdbserver_gdbport [lindex $res 1]
 
+    # Even if the board file is testing with target remote, our caller
+    # wants to test against gdbserver in extended-remote mode.  Make sure to
+    # disable stub-like techniques.
+    set use_gdb_stub 0
+
     return [gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport]
 }
--- src/gdb/testsuite/lib/mi-support.exp	2011/12/02 22:35:17	1.105
+++ src/gdb/testsuite/lib/mi-support.exp	2011/12/03 20:20:29	1.106
@@ -96,7 +96,7 @@
 # get really slow.  Give gdb at least 3 minutes to start up.
 #
 proc default_mi_gdb_start { args } {
-    global verbose
+    global verbose use_gdb_stub
     global GDB
     global INTERNAL_GDBFLAGS GDBFLAGS
     global gdb_prompt
@@ -108,6 +108,9 @@
     gdb_stop_suppressing_tests;
     set inferior_pty no-tty
 
+    # Set the default value, it may be overriden later by specific testfile.
+    set use_gdb_stub [target_info exists use_gdb_stub]
+
     if { [llength $args] == 1} {
 	set inferior_pty [lindex $args 0]
     }
@@ -793,7 +796,7 @@
     if { $suppress_flag } {
 	return -1
     }
-    global mi_gdb_prompt
+    global mi_gdb_prompt use_gdb_stub
     global thread_selected_re
     global library_loaded_re
 
@@ -820,7 +823,7 @@
 	return -1
     }
 
-    if [target_info exists use_gdb_stub] {
+    if $use_gdb_stub {
 	if [target_info exists gdb,do_reload_on_run] {
 	    send_gdb "${run_prefix}continue\n";
 	    gdb_expect 60 {

Re: ping: Re: [patch] Forbid "run" etc. for use_gdb_stub targets

[Doug Evans]

On Sat, Dec 3, 2011 at 10:36 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> I am a bit reluctant to check it on my own as I already made there some
> mistakes before.  I see there is more work going on with this code.
>
>
> On Sat, 29 Oct 2011 21:47:45 +0200, Jan Kratochvil wrote:
>
> On Fri, 28 Oct 2011 20:17:44 +0200, Pedro Alves wrote:
>> Isn't this too late though?  We do the [target_info exists use_gdb_stub]
>> check at the top of the test files, before gdbserver_start has had a chance
>> of running.  So I'm guessing that in the first test that runs right
>> after the gdb.server/ tests complete, use_gdb_stub will be wrong at the top of
>> the test, but it'll get fixed by that test, so the following ones are right.
>> If that test file happen to have a use_gdb_stub check, it'll lose.
>> We may need to do this closer to gdb_init/default_gdb_init (or an override).
>
> I have tested it and you are right, I had some wrong dejagnu assumptions.
>
> Modifying target_info's use_gdb_stub is probably not much correct, therefore
> there is created global $use_gdb_stub instead.
>
> Global $use_gdb_stub is intentionally unset before gdb_start to cause errors
> trying to interpret the value before.  Some problem is interpreting
> [target_info exists use_gdb_stub] after gdb_start is incorrect but it does not
> cause any sanity-check error.
>
> No regressions on {x86_64,x86_64-m32,i686}-fedora16pre-linux-gnu and with
> gdbserver.
>
>
> Thanks,
> Jan
>
>
> gdb/testsuite/
> 2011-10-29  Jan Kratochvil  <jan.kratochvil@redhat.com>
>
>        * gdb.base/break-entry.exp: Move the target use_gdb_stub test before
>        starting GDB.
>        * gdb.base/default.exp: Replace target use_gdb_stub checks by global
>        $use_gdb_stub.
>        * gdb.base/display.exp: Likewise.
>        * gdb.base/ending-run.exp: Likewise.
>        * gdb.base/list.exp (test_listsize): Likewise.
>        * gdb.base/setshow.exp: Likewise.
>        * gdb.base/valgrind-db-attach.exp: Set global use_gdb_stub to 1.
>        * lib/gdb.exp (gdb_run_cmd, gdb_start_cmd): Replace target use_gdb_stub
>        check by global $use_gdb_stub.
>        (gdb_test_multiple): Forbid run, start or attach for !$use_gdb_stub.
>        (default_gdb_start): Set global use_gdb_stub from target use_gdb_stub.
>        (default_gdb_init): Unset global $use_gdb_stub.
>        (gdb_continue_to_end, rerun_to_main): Replace target use_gdb_stub check
>        by global $use_gdb_stub.
>        * lib/gdbserver-support.exp: Extend comments for set_board_info
>        gdb_protocol and gdb,socketport, new comment for set_board_info
>        use_gdb_stub.
>        (gdbserver_start_extended): Set global gdbserver_protocol and
>        gdbserver_gdbport.  Clear global use_gdb_stub.
>        * lib/mi-support.exp (default_mi_gdb_start): Set global use_gdb_stub
>        from target use_gdb_stub.
>        (mi_run_cmd): Replace target use_gdb_stub check by global $use_gdb_stub.

I'd like to get away from conflating anything related to gdb stubs and
valgrind / gdbserver.

Re: ping: Re: [patch] Forbid "run" etc. for use_gdb_stub targets

[Jan Kratochvil]

On Sun, 04 Dec 2011 02:02:06 +0100, Doug Evans wrote:
> I'd like to get away from conflating anything related to gdb stubs and
> valgrind / gdbserver.

Currently $use_gdb_stub means the startup has been already executed when the
testsuite gets into GDB control.

IIUC in reality currently the real stubs are not in use but gdbserver in
non-extended mode behaves as native GDB with the stubs in the past.

Maybe $use_gdb_stub name is not fortunate for what it does, real stubs are
only subset of $use_gdb_stub cases.



wrt valgrind, you may be referring to:
http://sourceware.org/ml/gdb-patches/2011-10/msg00768.html
# Therefore shouldn't be the gdb.server/ext-*.exp testfiles skipped if the
# target board defined use_gdb_stub?  (ext-attach.exp already is skipped, for
# the [is_remote target] reason)
[unreplied]

Currently I did not change on the GDB testsuite behavior what was not clearly
wrong.  Therefore:
 * If a testcase can run only in some its specific mode/board of run:
   * Some testcases skip themselves if their mode/board is not satisfied.
     --- if [is_remote target] then { return 0 }
   * Some testcases enforce their mode/board no matter what mode has been
     specified by user for the testsuite.
     --- gdbserver_start_extended
     This creates duplicate runs of a testcase in the same mode/board if one
     runs the testsuite in multiple modes.
 * If a testcase is compatible with any mode/board it uses the mode/board
   specified by user for the testsuite.

So far I just fixed those cases there were clearly wrong - such as spawning a
gdbserver and then running new inferior locally in GDB.

In the case of gdb.base/valgrind-db-attach.exp it enforces its mode and always
runs.


Thanks,
Jan

Re: ping: Re: [patch] Forbid "run" etc. for use_gdb_stub targets

[Doug Evans]

On Sat, Dec 3, 2011 at 5:29 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Sun, 04 Dec 2011 02:02:06 +0100, Doug Evans wrote:
>> I'd like to get away from conflating anything related to gdb stubs and
>> valgrind / gdbserver.
>
> Currently $use_gdb_stub means the startup has been already executed when the
> testsuite gets into GDB control.
>
> IIUC in reality currently the real stubs are not in use but gdbserver in
> non-extended mode behaves as native GDB with the stubs in the past.

Not precisely.

> Maybe $use_gdb_stub name is not fortunate for what it does, real stubs are
> only subset of $use_gdb_stub cases.

Poorly chosen names lead to confusion, bugs, and wasted time.
[No claim is made that this is news. :-)]

Plus, there's no reason why, for example, gdb_start_cmd can't work
with target remote and gdbserver.

Re: ping: Re: [patch] Forbid "run" etc. for use_gdb_stub targets

[Jan Kratochvil]

On Sun, 04 Dec 2011 03:55:07 +0100, Doug Evans wrote:
> Poorly chosen names lead to confusion, bugs, and wasted time.
> [No claim is made that this is news. :-)]

Just $use_gdb_stub is a different form of [target_info exists use_gdb_stub] so
no matter what the name is IMO it is correct the name is the same.

And one cannot change use_gdb_stub in the board file for board files backward
compatibility.


> Plus, there's no reason why, for example, gdb_start_cmd can't work
> with target remote and gdbserver.

It does not work with non-extended gdbserver - which has use_gdb_stub == 1.
Also my change did not change it:
-    if [target_info exists use_gdb_stub] {
+    if $use_gdb_stub {
(gdb) target remote localhost:1234
Remote debugging using localhost:1234
[...]
(gdb) start
The "remote" target does not support "run".  Try "help target" or "continue".

It works with extended gdbserver - which has use_gdb_stub == 0.

I do not see the problem you describe.


Thanks,
Jan

Re: ping: Re: [patch] Forbid "run" etc. for use_gdb_stub targets

[Doug Evans]

On Mon, Dec 5, 2011 at 12:15 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Sun, 04 Dec 2011 03:55:07 +0100, Doug Evans wrote:
>> Poorly chosen names lead to confusion, bugs, and wasted time.
>> [No claim is made that this is news. :-)]
>
> Just $use_gdb_stub is a different form of [target_info exists use_gdb_stub] so
> no matter what the name is IMO it is correct the name is the same.
>
> And one cannot change use_gdb_stub in the board file for board files backward
> compatibility.

I realize that.  grep use_gdb_stub /usr/share/dejagnu/baseboards/*.exp.
ref: http://sourceware.org/ml/gdb-patches/2011-12/msg00048.html

I was suggesting either a layer of abstraction and/or have gdbserver
set something with a more meaningful name, and use the more meaningful
name in contexts where we currently use use_gdb_stubs.

[btw, once gdb_stubs is gone, I think we should also remove (at least
in name) use_gdb_stubs, at least within gdb.  Otherwise we're not
finishing the job of cleaning things up.]

>> Plus, there's no reason why, for example, gdb_start_cmd can't work
>> with target remote and gdbserver.
>
> It does not work with non-extended gdbserver - which has use_gdb_stub == 1.
> Also my change did not change it:
> -    if [target_info exists use_gdb_stub] {
> +    if $use_gdb_stub {
> (gdb) target remote localhost:1234
> Remote debugging using localhost:1234
> [...]
> (gdb) start
> The "remote" target does not support "run".  Try "help target" or "continue".

I realize "target remote" doesn't support "start".
What I meant was there's no reason why gdb_start_cmd couldn't do
something like the following for non-extended-remote gdbserver:

tbreak main
continue

>
> It works with extended gdbserver - which has use_gdb_stub == 0.
>
> I do not see the problem you describe.
>
>
> Thanks,
> Jan
>

Re: ping: Re: [patch] Forbid "run" etc. for use_gdb_stub targets

[Doug Evans]

On Mon, Dec 5, 2011 at 1:27 PM, Doug Evans <dje@google.com> wrote:
> On Mon, Dec 5, 2011 at 12:15 PM, Jan Kratochvil
> <jan.kratochvil@redhat.com> wrote:
>> On Sun, 04 Dec 2011 03:55:07 +0100, Doug Evans wrote:
>>> Poorly chosen names lead to confusion, bugs, and wasted time.
>>> [No claim is made that this is news. :-)]
>>
>> Just $use_gdb_stub is a different form of [target_info exists use_gdb_stub] so
>> no matter what the name is IMO it is correct the name is the same.
>>
>> And one cannot change use_gdb_stub in the board file for board files backward
>> compatibility.
>
> I realize that.  grep use_gdb_stub /usr/share/dejagnu/baseboards/*.exp.
> ref: http://sourceware.org/ml/gdb-patches/2011-12/msg00048.html
>
> I was suggesting either a layer of abstraction and/or have gdbserver
> set something with a more meaningful name, and use the more meaningful
> name in contexts where we currently use use_gdb_stubs.
>
> [btw, once gdb_stubs is gone, I think we should also remove (at least
> in name) use_gdb_stubs, at least within gdb.  Otherwise we're not
> finishing the job of cleaning things up.]
>
>>> Plus, there's no reason why, for example, gdb_start_cmd can't work
>>> with target remote and gdbserver.
>>
>> It does not work with non-extended gdbserver - which has use_gdb_stub == 1.
>> Also my change did not change it:
>> -    if [target_info exists use_gdb_stub] {
>> +    if $use_gdb_stub {
>> (gdb) target remote localhost:1234
>> Remote debugging using localhost:1234
>> [...]
>> (gdb) start
>> The "remote" target does not support "run".  Try "help target" or "continue".
>
> I realize "target remote" doesn't support "start".
> What I meant was there's no reason why gdb_start_cmd couldn't do
> something like the following for non-extended-remote gdbserver:
>
> tbreak main
> continue

btw ...

I realize gdb.{ada,base}/start.exp uses gdb_start_cmd to actually test
the start command.

There are two independent uses of gdb_start_cmd in the testsuite:
1) test the "start" command
2) runto main without the side-effects of runto_main (delete all breakpoints)

If you want to add a parameter to runto_main, or some such, to skip
the side-effects, that's another way to go.
I don't have strong feelings on actually splitting up gdb_start_cmd into two.