From: Andrew Burgess Date: Tue, 6 Jun 2023 15:34:35 +0000 (+0100) Subject: gdb: fix ASan failure after recent string changes X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=ec5e9488c0a4adecd7502071ae585f045552051b;p=binutils-gdb.git gdb: fix ASan failure after recent string changes After this commit: commit baab375361c365afee2577c94cbbd3fdd443d6da Date: Tue Jul 13 14:44:27 2021 -0400 gdb: building inferior strings from within GDB It was pointed out that a new ASan failure had been introduced which was triggered by gdb.base/internal-string-values.exp: (gdb) PASS: gdb.base/internal-string-values.exp: test_setting: all langs: lang=ada: ptype "foo" print $_gdb_maint_setting("test-settings string") ================================================================= ==80377==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000068034 at pc 0x564785cba682 bp 0x7ffd20644620 sp 0x7ffd20644610 READ of size 1 at 0x603000068034 thread T0 #0 0x564785cba681 in find_command_name_length(char const*) /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:2129 #1 0x564785cbacb2 in lookup_cmd_1(char const**, cmd_list_element*, cmd_list_element**, std::__cxx11::basic_string, std::allocator >*, int, bool) /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:2186 #2 0x564785cbb539 in lookup_cmd_1(char const**, cmd_list_element*, cmd_list_element**, std::__cxx11::basic_string, std::allocator >*, int, bool) /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:2248 #3 0x564785cbbcf3 in lookup_cmd(char const**, cmd_list_element*, char const*, std::__cxx11::basic_string, std::allocator >*, int, int) /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:2339 #4 0x564785c82df2 in setting_cmd /tmp/src/binutils-gdb/gdb/cli/cli-cmds.c:2219 #5 0x564785c84274 in gdb_maint_setting_internal_fn /tmp/src/binutils-gdb/gdb/cli/cli-cmds.c:2348 #6 0x564788167b3b in call_internal_function(gdbarch*, language_defn const*, value*, int, value**) /tmp/src/binutils-gdb/gdb/value.c:2321 #7 0x5647854b6ebd in expr::ada_funcall_operation::evaluate(type*, expression*, noside) /tmp/src/binutils-gdb/gdb/ada-lang.c:11254 #8 0x564786658266 in expression::evaluate(type*, noside) /tmp/src/binutils-gdb/gdb/eval.c:111 #9 0x5647871242d6 in process_print_command_args /tmp/src/binutils-gdb/gdb/printcmd.c:1322 #10 0x5647871244b3 in print_command_1 /tmp/src/binutils-gdb/gdb/printcmd.c:1335 #11 0x564787125384 in print_command /tmp/src/binutils-gdb/gdb/printcmd.c:1468 #12 0x564785caac44 in do_simple_func /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:95 #13 0x564785cc18f0 in cmd_func(cmd_list_element*, char const*, int) /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:2735 #14 0x564787c70c68 in execute_command(char const*, int) /tmp/src/binutils-gdb/gdb/top.c:574 #15 0x564786686180 in command_handler(char const*) /tmp/src/binutils-gdb/gdb/event-top.c:543 #16 0x56478668752f in command_line_handler(std::unique_ptr >&&) /tmp/src/binutils-gdb/gdb/event-top.c:779 #17 0x564787dcb29a in tui_command_line_handler /tmp/src/binutils-gdb/gdb/tui/tui-interp.c:104 #18 0x56478668443d in gdb_rl_callback_handler /tmp/src/binutils-gdb/gdb/event-top.c:250 #19 0x7f4efd506246 in rl_callback_read_char (/usr/lib/libreadline.so.8+0x3b246) (BuildId: 092e91fc4361b0ef94561e3ae03a75f69398acbb) #20 0x564786683dea in gdb_rl_callback_read_char_wrapper_noexcept /tmp/src/binutils-gdb/gdb/event-top.c:192 #21 0x564786684042 in gdb_rl_callback_read_char_wrapper /tmp/src/binutils-gdb/gdb/event-top.c:225 #22 0x564787f1b119 in stdin_event_handler /tmp/src/binutils-gdb/gdb/ui.c:155 #23 0x56478862438d in handle_file_event /tmp/src/binutils-gdb/gdbsupport/event-loop.cc:573 #24 0x564788624d23 in gdb_wait_for_event /tmp/src/binutils-gdb/gdbsupport/event-loop.cc:694 #25 0x56478862297c in gdb_do_one_event(int) /tmp/src/binutils-gdb/gdbsupport/event-loop.cc:264 #26 0x564786df99f0 in start_event_loop /tmp/src/binutils-gdb/gdb/main.c:412 #27 0x564786dfa069 in captured_command_loop /tmp/src/binutils-gdb/gdb/main.c:476 #28 0x564786dff61f in captured_main /tmp/src/binutils-gdb/gdb/main.c:1320 #29 0x564786dff75c in gdb_main(captured_main_args*) /tmp/src/binutils-gdb/gdb/main.c:1339 #30 0x564785381b6d in main /tmp/src/binutils-gdb/gdb/gdb.c:32 #31 0x7f4efbc3984f (/usr/lib/libc.so.6+0x2384f) (BuildId: 2f005a79cd1a8e385972f5a102f16adba414d75e) #32 0x7f4efbc39909 in __libc_start_main (/usr/lib/libc.so.6+0x23909) (BuildId: 2f005a79cd1a8e385972f5a102f16adba414d75e) #33 0x564785381934 in _start (/tmp/build/binutils-gdb/gdb/gdb+0xabc5934) (BuildId: 90de353ac158646e7dab501b76a18a76628fca33) 0x603000068034 is located 0 bytes after 20-byte region [0x603000068020,0x603000068034) allocated by thread T0 here: #0 0x7f4efcee0cd1 in __interceptor_calloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:77 #1 0x5647856265d8 in xcalloc /tmp/src/binutils-gdb/gdb/alloc.c:97 #2 0x564788610c6b in xzalloc(unsigned long) /tmp/src/binutils-gdb/gdbsupport/common-utils.cc:29 #3 0x56478815721a in value::allocate_contents(bool) /tmp/src/binutils-gdb/gdb/value.c:929 #4 0x564788157285 in value::allocate(type*, bool) /tmp/src/binutils-gdb/gdb/value.c:941 #5 0x56478815733a in value::allocate(type*) /tmp/src/binutils-gdb/gdb/value.c:951 #6 0x5647854ae81c in expr::ada_string_operation::evaluate(type*, expression*, noside) /tmp/src/binutils-gdb/gdb/ada-lang.c:10675 #7 0x5647854b63b8 in expr::ada_funcall_operation::evaluate(type*, expression*, noside) /tmp/src/binutils-gdb/gdb/ada-lang.c:11184 #8 0x564786658266 in expression::evaluate(type*, noside) /tmp/src/binutils-gdb/gdb/eval.c:111 #9 0x5647871242d6 in process_print_command_args /tmp/src/binutils-gdb/gdb/printcmd.c:1322 #10 0x5647871244b3 in print_command_1 /tmp/src/binutils-gdb/gdb/printcmd.c:1335 #11 0x564787125384 in print_command /tmp/src/binutils-gdb/gdb/printcmd.c:1468 #12 0x564785caac44 in do_simple_func /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:95 #13 0x564785cc18f0 in cmd_func(cmd_list_element*, char const*, int) /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:2735 #14 0x564787c70c68 in execute_command(char const*, int) /tmp/src/binutils-gdb/gdb/top.c:574 #15 0x564786686180 in command_handler(char const*) /tmp/src/binutils-gdb/gdb/event-top.c:543 #16 0x56478668752f in command_line_handler(std::unique_ptr >&&) /tmp/src/binutils-gdb/gdb/event-top.c:779 #17 0x564787dcb29a in tui_command_line_handler /tmp/src/binutils-gdb/gdb/tui/tui-interp.c:104 #18 0x56478668443d in gdb_rl_callback_handler /tmp/src/binutils-gdb/gdb/event-top.c:250 #19 0x7f4efd506246 in rl_callback_read_char (/usr/lib/libreadline.so.8+0x3b246) (BuildId: 092e91fc4361b0ef94561e3ae03a75f69398acbb) The problem is in cli/cli-cmds.c, in the function setting_cmd, where we do this: const char *a0 = (const char *) argv[0]->contents ().data (); Here argv[0] is a value* which we know is either a TYPE_CODE_ARRAY or a TYPE_CODE_STRING. The problem is that the above line is casting the value contents directly to a C-string, i.e. one that is assumed to have a null-terminator at the end. After the above commit this can no longer be assumed to be true. A string value will be represented just as it would be in the current language, so for Ada and Fortran the string will be an array of characters with no null-terminator at the end. My proposed solution is to copy the string contents into a std::string object, and then use the std::string::c_str() value, this will ensure that a null-terminator has been added. I had a check through GDB at places TYPE_CODE_STRING was used and couldn't see any other obvious places where this type of assumption was being made, so hopefully this is the only offender. Running the above test with ASan compiled in no longer gives an error. Reviewed-By: Tom Tromey --- diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index 8994cc5ea3d..638c138e7cb 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -2215,7 +2215,13 @@ setting_cmd (const char *fnname, struct cmd_list_element *showlist, && type0->code () != TYPE_CODE_STRING) error (_("First argument of %s must be a string."), fnname); - const char *a0 = (const char *) argv[0]->contents ().data (); + /* Not all languages null-terminate their strings, by moving the string + content into a std::string we ensure that a null-terminator is added. + For languages that do add a null-terminator the std::string might end + up with two null characters at the end, but that's harmless. */ + const std::string setting ((const char *) argv[0]->contents ().data (), + type0->length ()); + const char *a0 = setting.c_str (); cmd_list_element *cmd = lookup_cmd (&a0, showlist, "", NULL, -1, 0); if (cmd == nullptr || cmd->type != show_cmd)