public inbox for gdb-cvs@sourceware.org
help / color / mirror / Atom feed
* [binutils-gdb] gdb: fix ASan failure after recent string changes
@ 2023-06-09 14:30 Andrew Burgess
  0 siblings, 0 replies; only message in thread
From: Andrew Burgess @ 2023-06-09 14:30 UTC (permalink / raw)
  To: gdb-cvs

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=ec5e9488c0a4adecd7502071ae585f045552051b

commit ec5e9488c0a4adecd7502071ae585f045552051b
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Tue Jun 6 16:34:35 2023 +0100

    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<char, std::char_traits<char>, std::allocator<char> >*, 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<char, std::char_traits<char>, std::allocator<char> >*, 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<char, std::char_traits<char>, std::allocator<char> >*, 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<char, gdb::xfree_deleter<char> >&&) /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<char, gdb::xfree_deleter<char> >&&) /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 <tom@tromey.com>

Diff:
---
 gdb/cli/cli-cmds.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

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)

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2023-06-09 14:30 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-09 14:30 [binutils-gdb] gdb: fix ASan failure after recent string changes Andrew Burgess

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).