From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 24731385B18D for ; Wed, 14 Dec 2022 22:01:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 24731385B18D Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca Received: from [10.0.0.11] (unknown [217.28.27.60]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 7FDC01E0CD; Wed, 14 Dec 2022 17:01:10 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1671055270; bh=0eUR+kqYSk699AuNcVcURnfZblcFHYukaQyKQvHpuik=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=Jo+d4HzG8mViXWPxVjVkoICEuqkzgqmwYUp3kKPGi/LwfzXW9X+IaIrOD3ZVfqm+I V0MFoSZCiFkrJ0VALKQ/EHoumAzy2n71/cQnplRAOD+HITEcekqkj//ixjJaTubDGE rX0VUlRI3iu4p/65tXubUY5NYHh/n9NJgItPWh7A= Message-ID: <965bb084-6f78-8ac9-3fc7-03cae7ee6011@simark.ca> Date: Wed, 14 Dec 2022 17:01:09 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.5.1 Subject: Re: [PATCH] gdb: fix command lookup in execute_command () commands" Content-Language: en-US To: Tom Tromey , Simon Marchi via Gdb-patches Cc: Jan Vrany , luis.machado@arm.com References: <4dc13e01-2fd8-a63e-24f2-a1f7c7650d3b@simark.ca> <20221214110747.1150349-1-jan.vrany@labware.com> <87lenadlhr.fsf@tromey.com> <603b56b4-5983-832d-89ad-7e1f7669065c@simark.ca> <87v8mddfne.fsf@tromey.com> From: Simon Marchi In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 12/14/22 13:30, Simon Marchi via Gdb-patches wrote: > > > On 12/14/22 13:05, Tom Tromey wrote: >> Simon> Do you have ASan enabled in that build? >> >> Nope, but I tried it and I was able to reproduce the problem. >> I should have thought of that earlier :{ >> >> Tom > > I think the problem is the static buffer used in command_line_input. > > The "define set xxx_yyy" command is read into the static buffer here: > > #5 0x564a50c93555 in command_line_input(char const*, char const*) /home/smarchi/src/binutils-gdb/gdb/top.c:1421 > #6 0x564a50c8c1b2 in read_command_file(_IO_FILE*) /home/smarchi/src/binutils-gdb/gdb/top.c:458 > #7 0x564a4ed1ea61 in script_from_file(_IO_FILE*, char const*) /home/smarchi/src/binutils-gdb/gdb/cli/cli-script.c:1638 > #8 0x564a4ec4cff3 in source_script_from_stream /home/smarchi/src/binutils-gdb/gdb/cli/cli-cmds.c:728 > #9 0x564a4ec4d43b in source_script_with_search /home/smarchi/src/binutils-gdb/gdb/cli/cli-cmds.c:773 > #10 0x564a4ec4da65 in source_command /home/smarchi/src/binutils-gdb/gdb/cli/cli-cmds.c:831 > > That same buffer is used for reading the lines inside the define, here: > > #5 0x564a50c93555 in command_line_input(char const*, char const*) /home/smarchi/src/binutils-gdb/gdb/top.c:1421 > #6 0x564a4ed0cc32 in read_next_line /home/smarchi/src/binutils-gdb/gdb/cli/cli-script.c:920 > #7 0x564a4ed286f6 in gdb::function_view::bind(char const* (*)())::{lambda(gdb::fv_detail::erased_callable)#1}::operator()(gdb::fv_detail::erased_callable) const /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/function-view.h:326 > #8 0x564a4ed28772 in gdb::function_view::bind(char const* (*)())::{lambda(gdb::fv_detail::erased_callable)#1}::_FUN(gdb::fv_detail::erased_callable) /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/function-view.h:320 > #9 0x564a4ed25288 in gdb::function_view::operator()() const /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/function-view.h:289 > #10 0x564a4ed1adf2 in read_command_lines_1(gdb::function_view, int, gdb::function_view) /home/smarchi/src/binutils-gdb/gdb/cli/cli-script.c:1234 > #11 0x564a4ed1a997 in read_command_lines(char const*, int, int, gdb::function_view) /home/smarchi/src/binutils-gdb/gdb/cli/cli-script.c:1202 > #12 0x564a4ed1c8e6 in do_define_command /home/smarchi/src/binutils-gdb/gdb/cli/cli-script.c:1450 > #13 0x564a4ed1d2bc in define_command /home/smarchi/src/binutils-gdb/gdb/cli/cli-script.c:1500 > #14 0x564a4ec82cbc in do_simple_func /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:95 > #15 0x564a4ec99059 in cmd_func(cmd_list_element*, char const*, int) /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:2543 > #16 0x564a50c8e30d in execute_command(char const*, int) /home/smarchi/src/binutils-gdb/gdb/top.c:692 > #17 0x564a4f67dc89 in command_handler(char const*) /home/smarchi/src/binutils-gdb/gdb/event-top.c:616 > #18 0x564a50c8c1d6 in read_command_file(_IO_FILE*) /home/smarchi/src/binutils-gdb/gdb/top.c:461 > #19 0x564a4ed1ea61 in script_from_file(_IO_FILE*, char const*) /home/smarchi/src/binutils-gdb/gdb/cli/cli-script.c:1638 > #20 0x564a4ec4cff3 in source_script_from_stream /home/smarchi/src/binutils-gdb/gdb/cli/cli-cmds.c:728 > #21 0x564a4ec4d43b in source_script_with_search /home/smarchi/src/binutils-gdb/gdb/cli/cli-cmds.c:773 > #22 0x564a4ec4da65 in source_command /home/smarchi/src/binutils-gdb/gdb/cli/cli-cmds.c:831 > > It then crashes when trying to look up again the command, for the > post-hook, as the pointer to the command text (pointing to the string > that used to be owned by the static buffer) is now stale: > > #0 0x564a4ec92209 in lookup_cmd_1(char const**, cmd_list_element*, cmd_list_element**, std::__cxx11::basic_string, std::allocator >*, int, bool) /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:1990 > #1 0x564a4ec9338e in lookup_cmd(char const**, cmd_list_element*, char const*, std::__cxx11::basic_string, std::allocator >*, int, int) /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:2147 > #2 0x564a50c8e3b2 in execute_command(char const*, int) /home/smarchi/src/binutils-gdb/gdb/top.c:701 > #3 0x564a4f67dc89 in command_handler(char const*) /home/smarchi/src/binutils-gdb/gdb/event-top.c:616 > #4 0x564a50c8c1d6 in read_command_file(_IO_FILE*) /home/smarchi/src/binutils-gdb/gdb/top.c:461 > #5 0x564a4ed1ea61 in script_from_file(_IO_FILE*, char const*) /home/smarchi/src/binutils-gdb/gdb/cli/cli-script.c:1638 > #6 0x564a4ec4cff3 in source_script_from_stream /home/smarchi/src/binutils-gdb/gdb/cli/cli-cmds.c:728 > #7 0x564a4ec4d43b in source_script_with_search /home/smarchi/src/binutils-gdb/gdb/cli/cli-cmds.c:773 > #8 0x564a4ec4da65 in source_command /home/smarchi/src/binutils-gdb/gdb/cli/cli-cmds.c:831 > > From the point of view of execute_command, its `p` argument unexpectedly > becomes stale after calling cmd_func. A quick fix would be to make > execute_command duplicate `p` on entry and use that. However, I think > the right fix would be to get rid of the static buffer, have > command_line_input return an allocated buffer / string. Otherwise, it's > just adding complexity to an area that is difficult to understand fully. > > Simon Just FYI and to avoid duplicate work, I have a patch that looks promising, that removes the static buffer and uses std::string throughout these functions (making managing memory easier than with struct buffer): https://review.lttng.org/c/binutils-gdb/+/9175 I'm currently waiting for my CI task to run, and I will also need to write the commit message (tomorrow). Simon