diff options
author | Simon Marchi <simon.marchi@polymtl.ca> | 2022-12-15 14:06:25 -0500 |
---|---|---|
committer | Simon Marchi <simon.marchi@polymtl.ca> | 2022-12-15 21:49:29 -0500 |
commit | f8631e5e04dbef678323e9be6b7329f39049d2c4 (patch) | |
tree | aaed5236c5d78d30cb74fd0109cf60fec6c08a83 /gdb/top.c | |
parent | ffd894b51dc68c87f88ae0b09fc90e9bb272aa08 (diff) | |
download | gdb-f8631e5e04dbef678323e9be6b7329f39049d2c4.zip gdb-f8631e5e04dbef678323e9be6b7329f39049d2c4.tar.gz 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/top.c')
-rw-r--r-- | gdb/top.c | 39 |
1 files changed, 13 insertions, 26 deletions
@@ -307,8 +307,6 @@ ui::ui (FILE *instream_, FILE *outstream_, FILE *errstream_) m_gdb_stderr (new stderr_file (errstream)), m_gdb_stdlog (m_gdb_stderr) { - buffer_init (&line_buffer); - unbuffer_stream (instream_); if (ui_list == NULL) @@ -343,8 +341,6 @@ ui::~ui () delete m_gdb_stdin; delete m_gdb_stdout; delete m_gdb_stderr; - - buffer_free (&line_buffer); } /* Open file named NAME for read/write, making sure not to make it the @@ -452,11 +448,11 @@ read_command_file (FILE *stream) while (ui->instream != NULL && !feof (ui->instream)) { - const char *command; - /* Get a command-line. This calls the readline package. */ - command = command_line_input (NULL, NULL); - if (command == NULL) + std::string command_buffer; + const char *command + = command_line_input (command_buffer, nullptr, nullptr); + if (command == nullptr) break; command_handler (command); } @@ -1333,23 +1329,23 @@ gdb_safe_append_history (void) } } -/* Read one line from the command input stream `instream' into a local - static buffer. The buffer is made bigger as necessary. Returns - the address of the start of the line. +/* Read one line from the command input stream `instream'. + + CMD_LINE_BUFFER is a buffer that the function may use to store the result, if + it needs to be dynamically-allocated. Otherwise, it is unused.string - NULL is returned for end of file. + Return nullptr for end of file. This routine either uses fancy command line editing or simple input as the user has requested. */ const char * -command_line_input (const char *prompt_arg, const char *annotation_suffix) +command_line_input (std::string &cmd_line_buffer, const char *prompt_arg, + const char *annotation_suffix) { - static struct buffer cmd_line_buffer; - static int cmd_line_buffer_initialized; struct ui *ui = current_ui; const char *prompt = prompt_arg; - char *cmd; + const char *cmd; int from_tty = ui->instream == ui->stdin_stream; /* The annotation suffix must be non-NULL. */ @@ -1374,15 +1370,6 @@ command_line_input (const char *prompt_arg, const char *annotation_suffix) prompt = local_prompt; } - if (!cmd_line_buffer_initialized) - { - buffer_init (&cmd_line_buffer); - cmd_line_buffer_initialized = 1; - } - - /* Starting a new command line. */ - cmd_line_buffer.used_size = 0; - #ifdef SIGTSTP if (job_control) signal (SIGTSTP, handle_sigtstp); @@ -1422,7 +1409,7 @@ command_line_input (const char *prompt_arg, const char *annotation_suffix) rl.reset (gdb_readline_no_editing (prompt)); } - cmd = handle_line_of_input (&cmd_line_buffer, rl.get (), + cmd = handle_line_of_input (cmd_line_buffer, rl.get (), 0, annotation_suffix); if (cmd == (char *) EOF) { |