[patch] testsuite: Fix "ERROR: no fileid for"

[patch] testsuite: Fix "ERROR: no fileid for"

[Jan Kratochvil]

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

Hi,

a35cfb4007cee8cb84106412cd17f4e12f13345b is the first bad commit
commit a35cfb4007cee8cb84106412cd17f4e12f13345b
Author: Maciej W. Rozycki <macro@codesourcery.com>
Date:   Thu Oct 24 23:32:30 2013 +0100

$ runtest gdb.base/solib-disc.exp
Running ./gdb.base/solib-disc.exp ...
ERROR: no fileid for host1
[...]


Jan

[-- Attachment #2: gdbfinish.patch --]
[-- Type: text/plain, Size: 715 bytes --]

gdb/testsuite/
2014-02-06  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix "ERROR: no fileid for" in the testsuite.
	* lib/gdb.exp (gdb_finish): Check gdb_spawn_id.

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 533b81b..5c53cdf 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -3708,7 +3708,8 @@ proc gdb_finish { } {
     global cleanfiles
 
     # Give persistent gdbserver a chance to terminate before GDB is killed.
-    if {[info exists gdbserver_reconnect_p] && $gdbserver_reconnect_p} {
+    if {[info exists gdbserver_reconnect_p] && $gdbserver_reconnect_p
+	&& [info exists gdb_spawn_id]} {
 	send_gdb "kill\n";
 	gdb_expect 10 {
 	    -re "y or n" {

Re: [patch] testsuite: Fix "ERROR: no fileid for"

[Maciej W. Rozycki]

On Thu, 6 Feb 2014, Jan Kratochvil wrote:

> a35cfb4007cee8cb84106412cd17f4e12f13345b is the first bad commit
> commit a35cfb4007cee8cb84106412cd17f4e12f13345b
> Author: Maciej W. Rozycki <macro@codesourcery.com>
> Date:   Thu Oct 24 23:32:30 2013 +0100
> 
> $ runtest gdb.base/solib-disc.exp
> Running ./gdb.base/solib-disc.exp ...
> ERROR: no fileid for host1

 Thanks for your report and proposed fix.  Can you provide a more 
elaborate log of your test session?  What are the exact conditions for 
this problem to trigger?

 I'm asking because I fear your change could defeat the purpose of the 
commit you referred to if there's a catastrophic failure causing GDB to 
crash while running gdb.base/solib-disc.exp -- in such a case an instance 
of gdbserver would stay behind running, ruining the remaining part of the 
test suite in environments where only a single TCP port is available for 
the RSP connection.

  Maciej

Re: [patch] testsuite: Fix "ERROR: no fileid for"

[Jan Kratochvil]

On Thu, 06 Feb 2014 23:09:25 +0100, Maciej W. Rozycki wrote:
> Can you provide a more elaborate log of your test session?

$ echo 'set gdbserver_reconnect_p 1' >test.exp;runtest test.exp

> What are the exact conditions for this problem to trigger?

If GDB does not run and the testsuite tries to close down it errors.


>  I'm asking because I fear your change could defeat the purpose of the 
> commit you referred to if there's a catastrophic failure causing GDB to 
> crash while running gdb.base/solib-disc.exp -- in such a case an instance 
> of gdbserver would stay behind running, ruining the remaining part of the 
> test suite in environments where only a single TCP port is available for 
> the RSP connection.

If GDB has crashed then gdb_spawn_id still exists (although it does not work).
So my patch does not change anything.  And also currently it will leave the
stale gdbserver running anyway.

In general if gdb_spawn_id does not exist then send_gdb + gdb_expect just do
not make sense anyway.  So this patch just prevents the error in such case.

The killing of stale gdbserver could be improved multiple ways (also as
suggested by Pedro in the original thread) but that is IMO outside of the
scope of this patch.  Apparently if there is no good response from GDB then
gdb_finish() should try to call gdb_start just to kill that gdbserver, IIUC.


Thanks,
Jan

Re: [patch] testsuite: Fix "ERROR: no fileid for"

[Jan Kratochvil]

Hi Maciej,

ping: I expect you are fine with this "incomplete" fix so I will check it in.


Jan


On Fri, 07 Feb 2014 16:10:18 +0100, Jan Kratochvil wrote:
> On Thu, 06 Feb 2014 23:09:25 +0100, Maciej W. Rozycki wrote:
> > Can you provide a more elaborate log of your test session?
> 
> $ echo 'set gdbserver_reconnect_p 1' >test.exp;runtest test.exp
> 
> > What are the exact conditions for this problem to trigger?
> 
> If GDB does not run and the testsuite tries to close down it errors.
> 
> 
> >  I'm asking because I fear your change could defeat the purpose of the 
> > commit you referred to if there's a catastrophic failure causing GDB to 
> > crash while running gdb.base/solib-disc.exp -- in such a case an instance 
> > of gdbserver would stay behind running, ruining the remaining part of the 
> > test suite in environments where only a single TCP port is available for 
> > the RSP connection.
> 
> If GDB has crashed then gdb_spawn_id still exists (although it does not work).
> So my patch does not change anything.  And also currently it will leave the
> stale gdbserver running anyway.
> 
> In general if gdb_spawn_id does not exist then send_gdb + gdb_expect just do
> not make sense anyway.  So this patch just prevents the error in such case.
> 
> The killing of stale gdbserver could be improved multiple ways (also as
> suggested by Pedro in the original thread) but that is IMO outside of the
> scope of this patch.  Apparently if there is no good response from GDB then
> gdb_finish() should try to call gdb_start just to kill that gdbserver, IIUC.
> 
> 
> Thanks,
> Jan

Re: [patch] testsuite: Fix "ERROR: no fileid for"

[Maciej W. Rozycki]

Hi Jan,

> ping: I expect you are fine with this "incomplete" fix so I will check it in.

 Yes, I am.

> > > Can you provide a more elaborate log of your test session?
> > 
> > $ echo 'set gdbserver_reconnect_p 1' >test.exp;runtest test.exp
> > 
> > > What are the exact conditions for this problem to trigger?
> > 
> > If GDB does not run and the testsuite tries to close down it errors.
> > 
> > 
> > >  I'm asking because I fear your change could defeat the purpose of the 
> > > commit you referred to if there's a catastrophic failure causing GDB to 
> > > crash while running gdb.base/solib-disc.exp -- in such a case an instance 
> > > of gdbserver would stay behind running, ruining the remaining part of the 
> > > test suite in environments where only a single TCP port is available for 
> > > the RSP connection.
> > 
> > If GDB has crashed then gdb_spawn_id still exists (although it does not work).
> > So my patch does not change anything.  And also currently it will leave the
> > stale gdbserver running anyway.

 Correct.  I wasn't sure of the spawn ID situation, thanks for the 
clarification.

> > In general if gdb_spawn_id does not exist then send_gdb + gdb_expect just do
> > not make sense anyway.  So this patch just prevents the error in such case.

 I'm still curious as to the actual use for such a scenario, but from the 
corectness POV your fix is valid indeed.

> > The killing of stale gdbserver could be improved multiple ways (also as
> > suggested by Pedro in the original thread) but that is IMO outside of the
> > scope of this patch.  Apparently if there is no good response from GDB then
> > gdb_finish() should try to call gdb_start just to kill that gdbserver, IIUC.

 So do I.  Thanks for your fix and apologies for the delay.

  Maciej

[commit] [patch] testsuite: Fix "ERROR: no fileid for"

[Jan Kratochvil]

On Sun, 16 Feb 2014 21:45:06 +0100, Maciej W. Rozycki wrote:
> > ping: I expect you are fine with this "incomplete" fix so I will check it in.
> 
>  Yes, I am.

Checked in: 0b10be4faf97396c88746f932b55bc3d2fb2d907


Jan