Re: [PATCH] Fix the crash at the end of the runtest

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Bernd Edlinger <bernd.edlinger@hotmail.de> [2021-07-22 13:10:11 +0200]:

> due to unknown reasons the runtest process crashes
> at the end of the testrun when this script is
> executed.
> 
>     #0  0x00007fe176cd8b69 in exp_close (interp=interp@entry=0x5595b70269d0, esPtr=0x5595bb6968b0) at exp_command.c:370
>     #1  0x00007fe176ceef9a in exp_close_all (interp=0x5595b70269d0) at exp_chan.c:582
>     #2  0x00007fe176bd5b72 in InvokeExitHandlers () at ./generic/tclEvent.c:911
>     #3  0x00007fe176bd5c0a in Tcl_Exit (status=1) at ./generic/tclEvent.c:976
>     #4  0x00007fe176cdb959 in Exp_ExitObjCmd (clientData=<optimized out>, interp=0x5595b70269d0, objc=<optimized out>, objv=<optimized out>) at exp_command.c:2531
>     #5  0x00007fe176b4d5f2 in TclNRRunCallbacks (interp=interp@entry=0x5595b70269d0, result=0, rootPtr=0x0) at ./generic/tclBasic.c:4492
>     #6  0x00007fe176b4cfc5 in Tcl_EvalObjv (interp=interp@entry=0x5595b70269d0, objc=objc@entry=1, objv=objv@entry=0x5595b70342d0, flags=flags@entry=2097168) at ./generic/tclBasic.c:4215
>     #7  0x00007fe176b4e924 in TclEvalEx (interp=interp@entry=0x5595b70269d0, script=0x5595b70ade40 "# runtest.exp -- Test framework driver\n# Copyright (C) 1992-2016 Free Software Foundation, Inc.\n#\n# This file is part of DejaGnu.\n#\n# DejaGnu is free software; you can redistribute it and/or modify it"..., numBytes=<optimized out>, flags=flags@entry=0, line=1908, line@entry=1, clNextOuter=clNextOuter@entry=0x0, outerScript=0x5595b70ade40 "# runtest.exp -- Test framework driver\n# Copyright (C) 1992-2016 Free Software Foundation, Inc.\n#\n# This file is part of DejaGnu.\n#\n# DejaGnu is free software; you can redistribute it and/or modify it"...) at ./generic/tclBasic.c:5361
>     #8  0x00007fe176c08f79 in Tcl_FSEvalFileEx (encodingName=<optimized out>, pathPtr=0x5595b7063510, interp=0x5595b70269d0) at ./generic/tclIOUtil.c:1824
>     #9  Tcl_FSEvalFileEx (interp=0x5595b70269d0, pathPtr=0x5595b7063510, encodingName=<optimized out>) at ./generic/tclIOUtil.c:1724
>     #10 0x00007fe176c0784c in Tcl_EvalFile (interp=0x5595b70269d0, fileName=<optimized out>) at ./generic/tclIOUtil.c:424
>     #11 0x00007fe176ce856e in exp_interpret_cmdfilename (interp=0x5595b70269d0, filename=0x7fff61eef573 "/usr/share/dejagnu/runtest.exp") at exp_main_sub.c:953
>     #12 0x00005595b6cad2e9 in main (argc=4, argv=0x7fff61eeee38) at exp_main_exp.c:48
> 
> This started after
> commit 9edb1e0191e ("gdb/testsuite: capture GDB tty name in default_gdb_spawn")
> 
> Temporarily renaming builtin_spawn to spawn fixes the issue.
> 
> 2021-07-22  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* gdb.base/gnu-debugdata.exp (pipeline): Temporarily undo the effect of
> 	commit 9edb1e0191e in this function.

Weirdly, when I try to run this test (without your patch) I see this
error (passing -v in RUNTESTFLAGS):

  spawn -open file11 failed, 1 can't unset "::last_spawn_tty_name": no such variable, can't unset "::last_spawn_tty_name": no such variable
      while executing
  "unset ::last_spawn_tty_name"
      (procedure "spawn" line 7)
      invoked from within
  "spawn -ignore SIGHUP -leaveopen file11"
      invoked from within
  "catch "spawn -ignore SIGHUP -leaveopen $id" result2"
  result is -1 {spawn failed}
  run_on_host failed: spawn failed

It appears to be choking on the call to unset in:

  proc spawn_capture_tty_name { args } {
      set result [uplevel builtin_spawn $args]
      upvar spawn_out spawn_out
      if { [info exists spawn_out] } {
  	set ::last_spawn_tty_name $spawn_out(slave,name)
      } else {
  	unset ::last_spawn_tty_name
      }
      return $result
  }

as ::last_spawn_tty_name has never previous been set.

And indeed, I am seeing runtest crash as you say.

However, I applied this patch:

----- PATCH START -----

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index e79e0622f9d..deab2420075 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2031,7 +2031,7 @@ proc spawn_capture_tty_name { args } {
     upvar spawn_out spawn_out
     if { [info exists spawn_out] } {
 	set ::last_spawn_tty_name $spawn_out(slave,name)
-    } else {
+    } elseif { [info exists ::last_spawn_tty_name] } {
 	unset ::last_spawn_tty_name
     }
     return $result

----- PATCH END -----

And I can now run the test successfully, and I'm not seeing runtest
crash any more.

Can you confirm you see the same behaviour?

Thanks,
Andrew



> ---
> Hi,
> 
> Is this OK for trunk, and the gdb-11-branch?
> 
> 
> Thanks
> Bernd.
> 
>  gdb/testsuite/gdb.base/gnu-debugdata.exp | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/gdb/testsuite/gdb.base/gnu-debugdata.exp b/gdb/testsuite/gdb.base/gnu-debugdata.exp
> index f6d0cd7..0dc8191 100644
> --- a/gdb/testsuite/gdb.base/gnu-debugdata.exp
> +++ b/gdb/testsuite/gdb.base/gnu-debugdata.exp
> @@ -52,11 +52,20 @@ proc pipeline {test args} {
>  	}
>  	verbose "cooked args are [list $program $arguments $input $output]"
>  
> +	# undo the effect of spawn_capture_tty_name
> +	# this avoids a crash at the end of the test run
> +	rename spawn dummy_spawn
> +	rename builtin_spawn spawn
> +
>  	if {[run_on_host "$test - invoke $program" $program $arguments \
>  		 $input $output]} {
>  	    return -1
>  	}
>  
> +	# restore the original spawn script
> +	rename spawn builtin_spawn
> +	rename dummy_spawn spawn
> +
>  	set input_file $output
>      }
>      return 0
> -- 
> 1.9.1