aboutsummaryrefslogtreecommitdiff
path: root/gdb/breakpoint.c
diff options
context:
space:
mode:
authorSimon Marchi <simon.marchi@polymtl.ca>2022-12-15 14:06:25 -0500
committerSimon Marchi <simon.marchi@polymtl.ca>2022-12-15 21:49:29 -0500
commitf8631e5e04dbef678323e9be6b7329f39049d2c4 (patch)
treeaaed5236c5d78d30cb74fd0109cf60fec6c08a83 /gdb/breakpoint.c
parentffd894b51dc68c87f88ae0b09fc90e9bb272aa08 (diff)
downloadfsf-binutils-gdb-f8631e5e04dbef678323e9be6b7329f39049d2c4.zip
fsf-binutils-gdb-f8631e5e04dbef678323e9be6b7329f39049d2c4.tar.gz
fsf-binutils-gdb-f8631e5e04dbef678323e9be6b7329f39049d2c4.tar.bz2
gdb: remove static buffer in command_line_input
[I sent this earlier today, but I don't see it in the archives. Resending it through a different computer / SMTP.] The use of the static buffer in command_line_input is becoming problematic, as explained here [1]. In short, with this patch [2] that attempt to fix a post-hook bug, when running gdb.base/commands.exp, we hit a case where we read a "define" command line from a script file using command_command_line_input. The command line is stored in command_line_input's static buffer. Inside the define command's execution, we read the lines inside the define using command_line_input, which overwrites the define command, in command_line_input's static buffer. After the execution of the define command, execute_command does a command look up to see if a post-hook is registered. For that, it uses a now stale pointer that used to point to the define command, in the static buffer, causing a use-after-free. Note that the pointer in execute_command points to the dynamically-allocated buffer help by the static buffer in command_line_input, not to the static object itself, hence why we see a use-after-free. Fix that by removing the static buffer. I initially changed command_line_input and other related functions to return an std::string, which is the obvious but naive solution. The thing is that some callees don't need to return an allocated string, so this this an unnecessary pessimization. I changed it to passing in a reference to an std::string buffer, which the callee can use if it needs to return dynamically-allocated content. It fills the buffer and returns a pointers to the C string inside. The callees that don't need to return dynamically-allocated content simply don't use it. So, it started with modifying command_line_input as described above, all the other changes derive directly from that. One slightly shady thing is in handle_line_of_input, where we now pass a pointer to an std::string's internal buffer to readline's history_value function, which takes a `char *`. I'm pretty sure that this function does not modify the input string, because I was able to change it (with enough massaging) to take a `const char *`. A subtle change is that we now clear a UI's line buffer using a SCOPE_EXIT in command_line_handler, after executing the command. This was previously done by this line in handle_line_of_input: /* We have a complete command line now. Prepare for the next command, but leave ownership of memory to the buffer . */ cmd_line_buffer->used_size = 0; I think the new way is clearer. [1] https://inbox.sourceware.org/gdb-patches/becb8438-81ef-8ad8-cc42-fcbfaea8cddd@simark.ca/ [2] https://inbox.sourceware.org/gdb-patches/20221213112241.621889-1-jan.vrany@labware.com/ Change-Id: I8fc89b1c69870c7fc7ad9c1705724bd493596300 Reviewed-By: Tom Tromey <tom@tromey.com>
Diffstat (limited to 'gdb/breakpoint.c')
-rw-r--r--gdb/breakpoint.c4
1 files changed, 2 insertions, 2 deletions
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index f0276a9..7a245b6 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -13782,8 +13782,8 @@ strace_command (const char *arg, int from_tty)
static struct uploaded_tp *this_utp;
static int next_cmd;
-static char *
-read_uploaded_action (void)
+static const char *
+read_uploaded_action (std::string &buffer)
{
char *rslt = nullptr;