diff mbox

Fix error message and use-after-free on errors in nested source files

Message ID 20190218234640.25664-1-simon.marchi@polymtl.ca
State New
Headers show

Commit Message

Simon Marchi Feb. 18, 2019, 11:46 p.m. UTC 
Errors that happen in nested sourced files (when a sourced file sources
another file) lead to a wrong error message, or use-after-free.

For example, if I put this in "a.gdb":

    command_that_doesnt_exist

and this in "b.gdb":

   source a.gdb

and try to "source b.gdb" in GDB, the result may look like this:

    (gdb) source b.gdb
    b.gdb:1: Error in sourced command file:
    _that_doesnt_exist:1: Error in sourced command file:
    Undefined command: "command_that_doesnt_exist".  Try "help".

Notice the wrong file name where "a.gdb" should be.  The exact result
may differ, depending on the feelings of the memory allocator.

What happens is:

- The "source a.gdb" command is saved by command_line_append_input_line
  in command_line_input's static buffer.
- Since we are sourcing a file, the script_from_file function stores the
  script name (a.gdb) in the source_file_name global.  However, it doesn't
  do a copy, it just saves a pointer to command_line_input's static buffer.
- The "command_that_doesnt_exist" command is saved by
  command_line_append_input_line in command_line_input's static buffer.
  Depending on what xrealloc does, source_file_name may now point to
  freed memory, or at the minimum the data it was pointing to was
  overwritten.
- When the error is handled in script_from_file, we dererence
  source_file_name to print the name of the file in which the error
  occured.

To fix it, I made source_file_name an std::string, so that keeps a copy of
the file name instead of pointing to a buffer with a too small
lifetime.

With this patch, the expected filename is printed, and no use-after-free
occurs:

    (gdb) source b.gdb
    b.gdb:1: Error in sourced command file:
    a.gdb:1: Error in sourced command file:
    Undefined command: "command_that_doesnt_exist".  Try "help".

I passed explicit template parameters to make_scoped_restore
(<std::string, const std::string &>), so that the second parameter is
passed by reference and avoid a copy.

It was not as obvious as I first thought to change gdb.base/source.exp
to test this, because source commands inside sourced files are
interpreted relative to GDB's current working directory, not the
directory of the currently sourced file.  As a workaround, I moved the
snippet that tests errors after the snippet that adds the source
directory to the search path.  This way, the "source source-error-1.exp"
line in source-error.exp manages to find the file.

For reference, here is what ASAN reports when use-after-free occurs:

(gdb) source b.gdb

Comments

Kevin Buettner Feb. 19, 2019, 10:37 p.m. UTC | #1 
On Mon, 18 Feb 2019 18:46:40 -0500
Simon Marchi <simon.marchi@polymtl.ca> wrote:

> Errors that happen in nested sourced files (when a sourced file sources
> another file) lead to a wrong error message, or use-after-free.
> 
> For example, if I put this in "a.gdb":
> 
>     command_that_doesnt_exist
> 
> and this in "b.gdb":
> 
>    source a.gdb
> 
> and try to "source b.gdb" in GDB, the result may look like this:
> 
>     (gdb) source b.gdb
>     b.gdb:1: Error in sourced command file:
>     _that_doesnt_exist:1: Error in sourced command file:
>     Undefined command: "command_that_doesnt_exist".  Try "help".
> 
> Notice the wrong file name where "a.gdb" should be.  The exact result
> may differ, depending on the feelings of the memory allocator.
> 
> What happens is:
> 
> - The "source a.gdb" command is saved by command_line_append_input_line
>   in command_line_input's static buffer.
> - Since we are sourcing a file, the script_from_file function stores the
>   script name (a.gdb) in the source_file_name global.  However, it doesn't
>   do a copy, it just saves a pointer to command_line_input's static buffer.
> - The "command_that_doesnt_exist" command is saved by
>   command_line_append_input_line in command_line_input's static buffer.
>   Depending on what xrealloc does, source_file_name may now point to
>   freed memory, or at the minimum the data it was pointing to was
>   overwritten.
> - When the error is handled in script_from_file, we dererence
>   source_file_name to print the name of the file in which the error
>   occured.
> 
> To fix it, I made source_file_name an std::string, so that keeps a copy of
> the file name instead of pointing to a buffer with a too small
> lifetime.
> 
> With this patch, the expected filename is printed, and no use-after-free
> occurs:
> 
>     (gdb) source b.gdb
>     b.gdb:1: Error in sourced command file:
>     a.gdb:1: Error in sourced command file:
>     Undefined command: "command_that_doesnt_exist".  Try "help".
> 
> I passed explicit template parameters to make_scoped_restore
> (<std::string, const std::string &>), so that the second parameter is
> passed by reference and avoid a copy.
> 
> It was not as obvious as I first thought to change gdb.base/source.exp
> to test this, because source commands inside sourced files are
> interpreted relative to GDB's current working directory, not the
> directory of the currently sourced file.  As a workaround, I moved the
> snippet that tests errors after the snippet that adds the source
> directory to the search path.  This way, the "source source-error-1.exp"

I think you meant source-error-1.gdb (instead of .exp).

> line in source-error.exp manages to find the file.

Thanks for this detailed explanation!

Might it be possible to (additionally) place some semblance of that
last paragraph into the .exp file?  (I'm thinking that it might be useful
to explain the pitfalls of moving that test from where you have it
now to earlier in the file.)

The patch looks (mostly) good to me, though I do have a question...

> diff --git a/gdb/testsuite/gdb.base/source.exp b/gdb/testsuite/gdb.base/source.exp
> index c6ff65783da0..5dd4decf6a5e 100644
> --- a/gdb/testsuite/gdb.base/source.exp
> +++ b/gdb/testsuite/gdb.base/source.exp
> @@ -23,10 +23,6 @@ standard_testfile structs.c
>  
>  gdb_start
>  
> -gdb_test "source ${srcdir}/${subdir}/source-error.gdb" \
> -    "source-error.gdb:21: Error in sourced command file:\[\r\n\]*Cannot access memory at address 0x0.*" \
> -    "script contains error"
> -
>  gdb_test "source -v ${srcdir}/${subdir}/source-test.gdb" \
>      "echo test source options.*" \
>      "source -v"
> @@ -66,4 +62,9 @@ gdb_test "source for-sure-nonexistant-file" \
>  gdb_test "source source-nofile.gdb" \
>           "warning: for-sure-nonexistant-file: No such file or directory\.\[\r\n\]*source error not fatal"
>  
> -gdb_exit
> +
> +gdb_test "source ${srcdir}/${subdir}/source-error.gdb" \
> +    [multi_line ".*source-error.gdb:20: Error in sourced command file:" \
> +		"source-error-1.gdb:21: Error in sourced command file:" \
> +		"Cannot access memory at address 0x0" ] \
> +    "script contains error"

Did you mean to remove gdb_exit above?  If so, why?

Kevin
diff mbox

Patch

=================================================================
==18498==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c000019847 at pc 0x7f1d3645de8e bp 0x7ffdcb892e50 sp 0x7ffdcb8925c8
READ of size 6 at 0x60c000019847 thread T0
    #0 0x7f1d3645de8d in printf_common /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors_format.inc:546
    #1 0x7f1d36477175 in __interceptor_vasprintf /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1525
    #2 0x5632eaffa277 in xstrvprintf(char const*, __va_list_tag*) /home/simark/src/binutils-gdb/gdb/common/common-utils.c:122
    #3 0x5632eaff96d1 in throw_it /home/simark/src/binutils-gdb/gdb/common/common-exceptions.c:351
    #4 0x5632eaff98df in throw_verror(errors, char const*, __va_list_tag*) /home/simark/src/binutils-gdb/gdb/common/common-exceptions.c:379
    #5 0x5632eaff9a2a in throw_error(errors, char const*, ...) /home/simark/src/binutils-gdb/gdb/common/common-exceptions.c:394
    #6 0x5632eafca21a in script_from_file(_IO_FILE*, char const*) /home/simark/src/binutils-gdb/gdb/cli/cli-script.c:1553
    #7 0x5632eaf8a500 in source_script_from_stream /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:569
    #8 0x5632eaf8a735 in source_script_with_search /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:605
    #9 0x5632eaf8ab20 in source_command /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:664
    #10 0x5632eafa8b4a in do_const_cfunc /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:106
    #11 0x5632eafb0687 in cmd_func(cmd_list_element*, char const*, int) /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:1892
    #12 0x5632ebf3dd87 in execute_command(char const*, int) /home/simark/src/binutils-gdb/gdb/top.c:630
    #13 0x5632eb3b25d3 in command_handler(char const*) /home/simark/src/binutils-gdb/gdb/event-top.c:583
    #14 0x5632ebf3cf09 in read_command_file(_IO_FILE*) /home/simark/src/binutils-gdb/gdb/top.c:425
    #15 0x5632eafca054 in script_from_file(_IO_FILE*, char const*) /home/simark/src/binutils-gdb/gdb/cli/cli-script.c:1547
    #16 0x5632eaf8a500 in source_script_from_stream /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:569
    #17 0x5632eaf8a735 in source_script_with_search /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:605
    #18 0x5632eaf8ab20 in source_command /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:664
    #19 0x5632eafa8b4a in do_const_cfunc /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:106
    #20 0x5632eafb0687 in cmd_func(cmd_list_element*, char const*, int) /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:1892
    #21 0x5632ebf3dd87 in execute_command(char const*, int) /home/simark/src/binutils-gdb/gdb/top.c:630
    #22 0x5632eb3b25d3 in command_handler(char const*) /home/simark/src/binutils-gdb/gdb/event-top.c:583
    #23 0x5632eb3b2f87 in command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) /home/simark/src/binutils-gdb/gdb/event-top.c:770
    #24 0x5632eb3b0fe1 in gdb_rl_callback_handler /home/simark/src/binutils-gdb/gdb/event-top.c:213
    #25 0x5632ec1c8729 in rl_callback_read_char /home/simark/src/binutils-gdb/readline/callback.c:220
    #26 0x5632eb3b0b8f in gdb_rl_callback_read_char_wrapper_noexcept /home/simark/src/binutils-gdb/gdb/event-top.c:175
    #27 0x5632eb3b0da1 in gdb_rl_callback_read_char_wrapper /home/simark/src/binutils-gdb/gdb/event-top.c:192
    #28 0x5632eb3b2186 in stdin_event_handler(int, void*) /home/simark/src/binutils-gdb/gdb/event-top.c:511
    #29 0x5632eb3aa6a9 in handle_file_event /home/simark/src/binutils-gdb/gdb/event-loop.c:733
    #30 0x5632eb3aaf41 in gdb_wait_for_event /home/simark/src/binutils-gdb/gdb/event-loop.c:859
    #31 0x5632eb3a88ea in gdb_do_one_event() /home/simark/src/binutils-gdb/gdb/event-loop.c:347
    #32 0x5632eb3a89bf in start_event_loop() /home/simark/src/binutils-gdb/gdb/event-loop.c:371
    #33 0x5632eb76fbfc in captured_command_loop /home/simark/src/binutils-gdb/gdb/main.c:330
    #34 0x5632eb772ea8 in captured_main /home/simark/src/binutils-gdb/gdb/main.c:1176
    #35 0x5632eb773071 in gdb_main(captured_main_args*) /home/simark/src/binutils-gdb/gdb/main.c:1192
    #36 0x5632eabfe7f9 in main /home/simark/src/binutils-gdb/gdb/gdb.c:32
    #37 0x7f1d3554f222 in __libc_start_main (/usr/lib/libc.so.6+0x24222)
    #38 0x5632eabfe5dd in _start (/home/simark/build/binutils-gdb/gdb/gdb+0x195d5dd)

0x60c000019847 is located 7 bytes inside of 128-byte region [0x60c000019840,0x60c0000198c0)
freed by thread T0 here:
    #0 0x7f1d36502491 in __interceptor_realloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:105
    #1 0x5632eaff9f47 in xrealloc /home/simark/src/binutils-gdb/gdb/common/common-utils.c:62
    #2 0x5632eaff6b44 in buffer_grow(buffer*, char const*, unsigned long) /home/simark/src/binutils-gdb/gdb/common/buffer.c:40
    #3 0x5632eb3b271d in command_line_append_input_line /home/simark/src/binutils-gdb/gdb/event-top.c:614
    #4 0x5632eb3b28c6 in handle_line_of_input(buffer*, char const*, int, char const*) /home/simark/src/binutils-gdb/gdb/event-top.c:654
    #5 0x5632ebf402a6 in command_line_input(char const*, char const*) /home/simark/src/binutils-gdb/gdb/top.c:1252
    #6 0x5632ebf3cee9 in read_command_file(_IO_FILE*) /home/simark/src/binutils-gdb/gdb/top.c:422
    #7 0x5632eafca054 in script_from_file(_IO_FILE*, char const*) /home/simark/src/binutils-gdb/gdb/cli/cli-script.c:1547
    #8 0x5632eaf8a500 in source_script_from_stream /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:569
    #9 0x5632eaf8a735 in source_script_with_search /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:605
    #10 0x5632eaf8ab20 in source_command /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:664
    #11 0x5632eafa8b4a in do_const_cfunc /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:106
    #12 0x5632eafb0687 in cmd_func(cmd_list_element*, char const*, int) /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:1892
    #13 0x5632ebf3dd87 in execute_command(char const*, int) /home/simark/src/binutils-gdb/gdb/top.c:630
    #14 0x5632eb3b25d3 in command_handler(char const*) /home/simark/src/binutils-gdb/gdb/event-top.c:583
    #15 0x5632ebf3cf09 in read_command_file(_IO_FILE*) /home/simark/src/binutils-gdb/gdb/top.c:425
    #16 0x5632eafca054 in script_from_file(_IO_FILE*, char const*) /home/simark/src/binutils-gdb/gdb/cli/cli-script.c:1547
    #17 0x5632eaf8a500 in source_script_from_stream /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:569
    #18 0x5632eaf8a735 in source_script_with_search /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:605
    #19 0x5632eaf8ab20 in source_command /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:664
    #20 0x5632eafa8b4a in do_const_cfunc /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:106
    #21 0x5632eafb0687 in cmd_func(cmd_list_element*, char const*, int) /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:1892
    #22 0x5632ebf3dd87 in execute_command(char const*, int) /home/simark/src/binutils-gdb/gdb/top.c:630
    #23 0x5632eb3b25d3 in command_handler(char const*) /home/simark/src/binutils-gdb/gdb/event-top.c:583
    #24 0x5632eb3b2f87 in command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) /home/simark/src/binutils-gdb/gdb/event-top.c:770
    #25 0x5632eb3b0fe1 in gdb_rl_callback_handler /home/simark/src/binutils-gdb/gdb/event-top.c:213
    #26 0x5632ec1c8729 in rl_callback_read_char /home/simark/src/binutils-gdb/readline/callback.c:220
    #27 0x5632eb3b0b8f in gdb_rl_callback_read_char_wrapper_noexcept /home/simark/src/binutils-gdb/gdb/event-top.c:175
    #28 0x5632eb3b0da1 in gdb_rl_callback_read_char_wrapper /home/simark/src/binutils-gdb/gdb/event-top.c:192
    #29 0x5632eb3b2186 in stdin_event_handler(int, void*) /home/simark/src/binutils-gdb/gdb/event-top.c:511

previously allocated by thread T0 here:
    #0 0x7f1d36502491 in __interceptor_realloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:105
    #1 0x5632eaff9f47 in xrealloc /home/simark/src/binutils-gdb/gdb/common/common-utils.c:62
    #2 0x5632eaff6b44 in buffer_grow(buffer*, char const*, unsigned long) /home/simark/src/binutils-gdb/gdb/common/buffer.c:40
    #3 0x5632eb3b271d in command_line_append_input_line /home/simark/src/binutils-gdb/gdb/event-top.c:614
    #4 0x5632eb3b28c6 in handle_line_of_input(buffer*, char const*, int, char const*) /home/simark/src/binutils-gdb/gdb/event-top.c:654
    #5 0x5632ebf402a6 in command_line_input(char const*, char const*) /home/simark/src/binutils-gdb/gdb/top.c:1252
    #6 0x5632ebf3cee9 in read_command_file(_IO_FILE*) /home/simark/src/binutils-gdb/gdb/top.c:422
    #7 0x5632eafca054 in script_from_file(_IO_FILE*, char const*) /home/simark/src/binutils-gdb/gdb/cli/cli-script.c:1547
    #8 0x5632eaf8a500 in source_script_from_stream /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:569
    #9 0x5632eaf8a735 in source_script_with_search /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:605
    #10 0x5632eaf8ab20 in source_command /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:664
    #11 0x5632eafa8b4a in do_const_cfunc /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:106
    #12 0x5632eafb0687 in cmd_func(cmd_list_element*, char const*, int) /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:1892
    #13 0x5632ebf3dd87 in execute_command(char const*, int) /home/simark/src/binutils-gdb/gdb/top.c:630
    #14 0x5632eb3b25d3 in command_handler(char const*) /home/simark/src/binutils-gdb/gdb/event-top.c:583
    #15 0x5632eb3b2f87 in command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) /home/simark/src/binutils-gdb/gdb/event-top.c:770
    #16 0x5632eb3b0fe1 in gdb_rl_callback_handler /home/simark/src/binutils-gdb/gdb/event-top.c:213
    #17 0x5632ec1c8729 in rl_callback_read_char /home/simark/src/binutils-gdb/readline/callback.c:220
    #18 0x5632eb3b0b8f in gdb_rl_callback_read_char_wrapper_noexcept /home/simark/src/binutils-gdb/gdb/event-top.c:175
    #19 0x5632eb3b0da1 in gdb_rl_callback_read_char_wrapper /home/simark/src/binutils-gdb/gdb/event-top.c:192
    #20 0x5632eb3b2186 in stdin_event_handler(int, void*) /home/simark/src/binutils-gdb/gdb/event-top.c:511
    #21 0x5632eb3aa6a9 in handle_file_event /home/simark/src/binutils-gdb/gdb/event-loop.c:733
    #22 0x5632eb3aaf41 in gdb_wait_for_event /home/simark/src/binutils-gdb/gdb/event-loop.c:859
    #23 0x5632eb3a88ea in gdb_do_one_event() /home/simark/src/binutils-gdb/gdb/event-loop.c:347
    #24 0x5632eb3a89bf in start_event_loop() /home/simark/src/binutils-gdb/gdb/event-loop.c:371
    #25 0x5632eb76fbfc in captured_command_loop /home/simark/src/binutils-gdb/gdb/main.c:330
    #26 0x5632eb772ea8 in captured_main /home/simark/src/binutils-gdb/gdb/main.c:1176
    #27 0x5632eb773071 in gdb_main(captured_main_args*) /home/simark/src/binutils-gdb/gdb/main.c:1192
    #28 0x5632eabfe7f9 in main /home/simark/src/binutils-gdb/gdb/gdb.c:32
    #29 0x7f1d3554f222 in __libc_start_main (/usr/lib/libc.so.6+0x24222)

SUMMARY: AddressSanitizer: heap-use-after-free /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors_format.inc:546 in printf_common

gdb/ChangeLog:

	* top.h (source_file_name): Change to std::string.
	* top.c (source_file_name): Likewise.
	(command_line_input): Adjust.
	* cli/cli-script.c (script_from_file): Adjust.

gdb/testsuite/ChangeLog:

	* gdb.base/source.exp: Move "error in sourced script" code to
	the end.
	* gdb.base/source-error.gdb: Move contents to
	source-error-1.gdb.  Add new code to source source-error-1.gdb.
	* gdb.base/source-error-1.gdb: New file, from previous
	source-error.gdb.
---
 gdb/cli/cli-script.c                      |  6 +++---
 gdb/testsuite/gdb.base/source-error-1.gdb | 22 ++++++++++++++++++++++
 gdb/testsuite/gdb.base/source-error.gdb   |  6 ++----
 gdb/testsuite/gdb.base/source.exp         | 11 ++++++-----
 gdb/top.c                                 |  4 ++--
 gdb/top.h                                 |  2 +-
 6 files changed, 36 insertions(+), 15 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/source-error-1.gdb

diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index aaead00a101c..ba3cdd566cb4 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -1537,8 +1537,8 @@  script_from_file (FILE *stream, const char *file)
 
   scoped_restore restore_line_number
     = make_scoped_restore (&source_line_number, 0);
-  scoped_restore resotre_file
-    = make_scoped_restore (&source_file_name, file);
+  scoped_restore restore_file
+    = make_scoped_restore<std::string, const std::string &> (&source_file_name, file);
 
   scoped_restore save_async = make_scoped_restore (&current_ui->async, 0);
 
@@ -1552,7 +1552,7 @@  script_from_file (FILE *stream, const char *file)
 	 prepended.  */
       throw_error (e.error,
 		   _("%s:%d: Error in sourced command file:\n%s"),
-		   source_file_name, source_line_number, e.message);
+		   source_file_name.c_str (), source_line_number, e.message);
     }
   END_CATCH
 }
diff --git a/gdb/testsuite/gdb.base/source-error-1.gdb b/gdb/testsuite/gdb.base/source-error-1.gdb
new file mode 100644
index 000000000000..c879fd2af6f0
--- /dev/null
+++ b/gdb/testsuite/gdb.base/source-error-1.gdb
@@ -0,0 +1,22 @@ 
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2005-2019 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/>.
+
+# Test GDB's "source" command - reads in a GDB script.
+
+echo working line\n
+x 0
+echo should not reach this line\n
diff --git a/gdb/testsuite/gdb.base/source-error.gdb b/gdb/testsuite/gdb.base/source-error.gdb
index c879fd2af6f0..85e0d9f8cf11 100644
--- a/gdb/testsuite/gdb.base/source-error.gdb
+++ b/gdb/testsuite/gdb.base/source-error.gdb
@@ -1,6 +1,6 @@ 
 # This testcase is part of GDB, the GNU debugger.
 
-# Copyright 2005-2019 Free Software Foundation, Inc.
+# Copyright 2019 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
@@ -17,6 +17,4 @@ 
 
 # Test GDB's "source" command - reads in a GDB script.
 
-echo working line\n
-x 0
-echo should not reach this line\n
+source source-error-1.gdb
diff --git a/gdb/testsuite/gdb.base/source.exp b/gdb/testsuite/gdb.base/source.exp
index c6ff65783da0..5dd4decf6a5e 100644
--- a/gdb/testsuite/gdb.base/source.exp
+++ b/gdb/testsuite/gdb.base/source.exp
@@ -23,10 +23,6 @@  standard_testfile structs.c
 
 gdb_start
 
-gdb_test "source ${srcdir}/${subdir}/source-error.gdb" \
-    "source-error.gdb:21: Error in sourced command file:\[\r\n\]*Cannot access memory at address 0x0.*" \
-    "script contains error"
-
 gdb_test "source -v ${srcdir}/${subdir}/source-test.gdb" \
     "echo test source options.*" \
     "source -v"
@@ -66,4 +62,9 @@  gdb_test "source for-sure-nonexistant-file" \
 gdb_test "source source-nofile.gdb" \
          "warning: for-sure-nonexistant-file: No such file or directory\.\[\r\n\]*source error not fatal"
 
-gdb_exit
+
+gdb_test "source ${srcdir}/${subdir}/source-error.gdb" \
+    [multi_line ".*source-error.gdb:20: Error in sourced command file:" \
+		"source-error-1.gdb:21: Error in sourced command file:" \
+		"Cannot access memory at address 0x0" ] \
+    "script contains error"
diff --git a/gdb/top.c b/gdb/top.c
index c756a6978e8d..22e6f7e29ab9 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -400,7 +400,7 @@  quit_cover (void)
 /* NOTE 1999-04-29: This variable will be static again, once we modify
    gdb to use the event loop as the default command loop and we merge
    event-top.c into this file, top.c.  */
-/* static */ const char *source_file_name;
+/* static */ std::string source_file_name;
 
 /* Read commands from STREAM.  */
 void
@@ -1221,7 +1221,7 @@  command_line_input (const char *prompt_arg, const char *annotation_suffix)
       gdb_flush (gdb_stdout);
       gdb_flush (gdb_stderr);
 
-      if (source_file_name != NULL)
+      if (!source_file_name.empty ())
 	++source_line_number;
 
       if (from_tty && annotation_level > 1)
diff --git a/gdb/top.h b/gdb/top.h
index 1d860188c6da..025d9389d601 100644
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -281,7 +281,7 @@  extern void gdb_init (char *);
 /* For use by event-top.c.  */
 /* Variables from top.c.  */
 extern int source_line_number;
-extern const char *source_file_name;
+extern std::string source_file_name;
 extern int history_expansion_p;
 extern int server_command;
 extern char *lim_at_start;