[commit] problem sourcing GDB script in interactive-mode on

Doug Evans dje@google.com
Sat Jan 15 11:09:00 GMT 2011


On Thu, Jan 13, 2011 at 3:05 PM, Joel Brobecker <brobecker@adacore.com> wrote:
> When interactive-mode is not auto, GDB always uses that setting to
> determine whether to act as if the input stream is a terminal or not.
> However, this setting should only be honored if the input stream is
> the standard input stream.  Otherwise, we run into trouble while
> source-ing a GDB script, as shown below (on x86_64-linux):
>
>        % cat script
>        print 1
>        print 2
>        % gdb  -q
>        (gdb) set interactive-mode on
>        (gdb) source script
>        (gdb) print 3
>        $1 = 3
>
> The lack of output and the fact that the "print 3" command returned
> a value saved in $1 (as opposed to $3) indicates that the script was
> not even evaluated at all.

For completeness sake, I think(!) it's a bit more subtle than this.
A backtrace after the "source script" command shows gdb is reading
commands from the keyboard while inside source_command.
Yikes!

Some uses of input_from_terminal_p also test from_tty.
E.g. if (from_tty && input_from_terminal_p ())
But top.c:command_line_input doesn't do this.
It does test instream == stdin in a few places, but it doesn't do this
test when calling input_from_terminal_p.
Instead it has:

    if (deprecated_readline_hook && input_from_terminal_p ())
      ...
    else if (command_editing_p && input_from_terminal_p ())
      ...

I'd like to be consistent.
Either we require callers of input_from_terminal_p to also test
from_tty (or instream == stdin), or we do the test in
input_from_terminal_p and remove the from_tty test in callers that do
that.

An alternative is doing this in command_line_input:

    if (deprecated_readline_hook && instream == stdin &&
input_from_terminal_p ())
      ...
    else if (command_editing_p && instream == stdin && input_from_terminal_p ())
      ...

There's also the calls to input_from_terminal_p in utils.c to consider.
What should happen to paging, for example, if interactive-mode = on
and we're sourcing a script?
Before your patch, set interactive-mode = on means paging is on even
when sourcing a script.
After your patch, paging is off regardless.
I'm kinda leaning towards patching command_line_input instead of
input_from_terminal_p, but I don't actually know when one would set
interactive-mode, so I don't have a strong opinion other than the
desire for consistency (i.e. if we keep your patch, callers shouldn't
have to test from_tty - I think(!)).

Comments?

P.S. Another comment towards the end of the patch.

> gdb/ChangeLog:
>
>        * top.c (input_from_terminal_p): Restrict the use of interactive_mode
>        to the case where instream is stdin.
>
> gdb/testsuite/ChangeLog:
>
>        * gdb.base/interact.exp: New testcase.
>
> Tested on x86_64-linux.  Checked in.
>
>
> ---
>  gdb/ChangeLog                       |    5 +++
>  gdb/testsuite/ChangeLog             |    4 +++
>  gdb/testsuite/gdb.base/interact.exp |   48 +++++++++++++++++++++++++++++++++++
>  gdb/top.c                           |    2 +-
>  4 files changed, 58 insertions(+), 1 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/interact.exp
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index a23aab9..e8ba178 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,10 @@
>  2011-01-13  Joel Brobecker  <brobecker@adacore.com>
>
> +       * top.c (input_from_terminal_p): Restrict the use of interactive_mode
> +       to the case where instream is stdin.
> +
> +2011-01-13  Joel Brobecker  <brobecker@adacore.com>
> +
>        * ia64-tdep.h (struct regcache): Forward declare.
>        (struct ia64_infcall_ops): New struct type.
>        (struct gdbarch_tdep): New fields "find_global_pointer_from_solib"
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index f6577c7..8926527 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,7 @@
> +2011-01-13  Joel Brobecker  <brobecker@adacore.com>
> +
> +       * gdb.base/interact.exp: New testcase.
> +
>  2011-01-12  Tom Tromey  <tromey@redhat.com>
>
>        * gdb.mi/gdb2549.exp: Update for error message changes.
> diff --git a/gdb/testsuite/gdb.base/interact.exp b/gdb/testsuite/gdb.base/interact.exp
> new file mode 100644
> index 0000000..1f15fd8
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/interact.exp
> @@ -0,0 +1,48 @@
> +# Copyright 2011 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Create a GDB script that we can source.  The script needs to generate
> +# some output, to allow us to verify that it is executed properly.
> +set fd [open "zzz-gdbscript" "w"]
> +puts $fd "print 1"
> +puts $fd "print 2"
> +close $fd
> +
> +# The expected output from the script...
> +set script_output "\\$\[0-9\]+ = 1\[\r\n\]+\\$\[0-9\]+ = 2.*"
> +
> +# Start a fresh GDB.  We don't need an executable for this test, so
> +# nothing else to do in terms of testcase setup.
> +gdb_exit
> +gdb_start
> +
> +# Test sourcing of the script with interactive mode `auto'
> +gdb_test_no_output "set interactive-mode auto"
> +gdb_test "source zzz-gdbscript" "$script_output" \
> +         "source script with interactive-mode auto"
> +gdb_test "print 3" "= 3" "sanity check with interactive-mode auto"
> +
> +# Test sourcing of the script with interactive mode `on'
> +gdb_test_no_output "set interactive-mode on"
> +gdb_test "source zzz-gdbscript" "$script_output" \
> +         "source script with interactive-mode on"
> +gdb_test "print 4" "= 4" "sanity check with interactive-mode on"
> +
> +# Test sourcing of the script with interactive mode `of'
> +gdb_test_no_output "set interactive-mode off"
> +gdb_test "source zzz-gdbscript" "$script_output" \
> +         "source script with interactive-mode off"
> +gdb_test "print 5" "= 5" "sanity check with interactive-mode off"
> +
> diff --git a/gdb/top.c b/gdb/top.c
> index d974897..bba1a2d 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -1292,7 +1292,7 @@ show_interactive_mode (struct ui_file *file, int from_tty,
>  int
>  input_from_terminal_p (void)
>  {
> -  if (interactive_mode != AUTO_BOOLEAN_AUTO)
> +  if (interactive_mode != AUTO_BOOLEAN_AUTO && instream == stdin)
>     return interactive_mode == AUTO_BOOLEAN_TRUE;

It's kinda weird to always fall through to the AUTO case when instream
!= stdin, but maybe it's ok.

>   if (batch_flag)
> --
> 1.7.1
>
>



More information about the Gdb-patches mailing list