diff mbox

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

Message ID 7eed7ae3-6f2a-285b-1270-6006d524e87a@ericsson.com
State New
Headers show

Commit Message

Simon Marchi Feb. 19, 2019, 11:24 p.m. UTC
On 2019-02-19 17:37, Kevin Buettner wrote:
> 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).


Woops, yes.

>> 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.)


Good idea, I added a comment.

> 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?


Yes, many tests have a gdb_exit at the end.  I believe this is not necessary (though it doesn't hurt)
because it's done anyway after the execution of the test.  So it has become an automatism to remove
it if I am modifying that area of the tests.  I can leave it there if you prefer.

Thanks for the review, here's the updated version.  I also fixed the title,
where it should read "sourced" instead of "source".

From aa38855ad14c85cc1061082f83d5dd4b91b28d98 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>

Date: Mon, 18 Feb 2019 18:46:40 -0500
Subject: [PATCH] Fix error message and use-after-free on errors in nested
 sourced files

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.gdb"
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
-- 
2.20.1

Comments

Kevin Buettner Feb. 19, 2019, 11:51 p.m. UTC | #1
On Tue, 19 Feb 2019 23:24:01 +0000
Simon Marchi <simon.marchi@ericsson.com> wrote:

> > Did you mean to remove gdb_exit above?  If so, why?  
> 
> Yes, many tests have a gdb_exit at the end.  I believe this is not necessary (though it doesn't hurt)
> because it's done anyway after the execution of the test.  So it has become an automatism to remove
> it if I am modifying that area of the tests.  I can leave it there if you prefer.

Thanks for the explanation.

> Thanks for the review, here's the updated version.  I also fixed the title,
> where it should read "sourced" instead of "source".

LGTM.

Kevin
Simon Marchi Feb. 20, 2019, 2:16 a.m. UTC | #2
On 2019-02-19 6:51 p.m., Kevin Buettner wrote:
> On Tue, 19 Feb 2019 23:24:01 +0000
> Simon Marchi <simon.marchi@ericsson.com> wrote:
> 
>>> Did you mean to remove gdb_exit above?  If so, why?  
>>
>> Yes, many tests have a gdb_exit at the end.  I believe this is not necessary (though it doesn't hurt)
>> because it's done anyway after the execution of the test.  So it has become an automatism to remove
>> it if I am modifying that area of the tests.  I can leave it there if you prefer.
> 
> Thanks for the explanation.
> 
>> Thanks for the review, here's the updated version.  I also fixed the title,
>> where it should read "sourced" instead of "source".
> 
> LGTM.
> 
> Kevin
> 

Thanks, I pushed it.

Simon
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         | 16 +++++++++++-----
 gdb/top.c                                 |  4 ++--
 gdb/top.h                                 |  2 +-
 6 files changed, 41 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 aaead00a101..ba3cdd566cb 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 00000000000..c879fd2af6f
--- /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 c879fd2af6f..85e0d9f8cf1 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 c6ff65783da..a8fef098f10 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,14 @@  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
+
+# Test commands that error out in sourced files, including in nested sourced
+# files.
+#
+# This needs to come after the "dir" command tested above for source-error.gdb
+# to find source-error-1.gdb.
+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 c756a6978e8..22e6f7e29ab 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 1d860188c6d..025d9389d60 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;