[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>
if (prompt == NULL)
prompt = "> ";
- args = command_line_input (prompt, annotation_suffix);
+ std::string buffer;
+ args = command_line_input (buffer, prompt, annotation_suffix);
if (args == NULL)
error_no_arg (_("one or more choice numbers"));
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;
static enum command_control_type
recurse_read_control_structure
- (gdb::function_view<const char * ()> read_next_line_func,
+ (read_next_line_ftype read_next_line_func,
struct command_line *current_cmd,
gdb::function_view<void (const char *)> validator);
static void do_document_command (const char *comname, int from_tty,
const counted_command_line *commands);
-static const char *read_next_line ();
+static const char *read_next_line (std::string &buffer);
/* Level of control structure when reading. */
static int control_level;
from stdin. */
static const char *
-read_next_line ()
+read_next_line (std::string &buffer)
{
struct ui *ui = current_ui;
char *prompt_ptr, control_prompt[256];
else
prompt_ptr = NULL;
- return command_line_input (prompt_ptr, "commands");
+ return command_line_input (buffer, prompt_ptr, "commands");
}
/* Given an input line P, skip the command and return a pointer to the
obtain lines of the command. */
static enum command_control_type
-recurse_read_control_structure (gdb::function_view<const char * ()> read_next_line_func,
+recurse_read_control_structure (read_next_line_ftype read_next_line_func,
struct command_line *current_cmd,
gdb::function_view<void (const char *)> validator)
{
{
dont_repeat ();
+ std::string buffer;
next = nullptr;
- val = process_next_line (read_next_line_func (), &next,
+ val = process_next_line (read_next_line_func (buffer), &next,
current_cmd->control_type != python_control
&& current_cmd->control_type != guile_control
&& current_cmd->control_type != compile_control,
obtained using READ_NEXT_LINE_FUNC. */
counted_command_line
-read_command_lines_1 (gdb::function_view<const char * ()> read_next_line_func,
+read_command_lines_1 (read_next_line_ftype read_next_line_func,
int parse_commands,
gdb::function_view<void (const char *)> validator)
{
while (1)
{
dont_repeat ();
- val = process_next_line (read_next_line_func (), &next, parse_commands,
+
+ std::string buffer;
+ val = process_next_line (read_next_line_func (buffer), &next, parse_commands,
validator);
/* Ignore blank lines or comments. */
}
};
+/* Prototype for a function to call to get one more input line.
+
+ If the function needs to return a dynamically allocated string, it can place
+ in the passed-in buffer, and return a pointer to it. Otherwise, it can
+ simply ignore it. */
+
+using read_next_line_ftype = gdb::function_view<const char * (std::string &)>;
+
extern counted_command_line read_command_lines
(const char *, int, int, gdb::function_view<void (const char *)>);
extern counted_command_line read_command_lines_1
- (gdb::function_view<const char * ()>, int,
- gdb::function_view<void (const char *)>);
+ (read_next_line_ftype, int, gdb::function_view<void (const char *)>);
/* Exported to cli/cli-cmds.c */
extern char *gdb_readline_wrapper (const char *);
-extern const char *command_line_input (const char *, const char *);
+extern const char *command_line_input (std::string &cmd_line_buffer,
+ const char *, const char *);
extern void print_prompt (void);
struct ui *current_ui;
struct ui *ui_list;
-/* Get a pointer to the current UI's line buffer. This is used to
+/* Get a reference to the current UI's line buffer. This is used to
construct a whole line of input from partial input. */
-static struct buffer *
+static std::string &
get_command_line_buffer (void)
{
- return ¤t_ui->line_buffer;
+ return current_ui->line_buffer;
}
/* When there is an event ready on the stdin file descriptor, instead
}
}
-/* Append RL, an input line returned by readline or one of its
- emulations, to CMD_LINE_BUFFER. Returns the command line if we
- have a whole command line ready to be processed by the command
- interpreter or NULL if the command line isn't complete yet (input
- line ends in a backslash). */
+/* Append RL, an input line returned by readline or one of its emulations, to
+ CMD_LINE_BUFFER. Return true if we have a whole command line ready to be
+ processed by the command interpreter or false if the command line isn't
+ complete yet (input line ends in a backslash). */
-static char *
-command_line_append_input_line (struct buffer *cmd_line_buffer, const char *rl)
+static bool
+command_line_append_input_line (std::string &cmd_line_buffer, const char *rl)
{
- char *cmd;
- size_t len;
-
- len = strlen (rl);
+ size_t len = strlen (rl);
if (len > 0 && rl[len - 1] == '\\')
{
/* Don't copy the backslash and wait for more. */
- buffer_grow (cmd_line_buffer, rl, len - 1);
- cmd = NULL;
+ cmd_line_buffer.append (rl, len - 1);
+ return false;
}
else
{
/* Copy whole line including terminating null, and we're
done. */
- buffer_grow (cmd_line_buffer, rl, len + 1);
- cmd = cmd_line_buffer->buffer;
+ cmd_line_buffer.append (rl, len + 1);
+ return true;
}
-
- return cmd;
}
/* Handle a line of input coming from readline.
- If the read line ends with a continuation character (backslash),
- save the partial input in CMD_LINE_BUFFER (except the backslash),
- and return NULL. Otherwise, save the partial input and return a
- pointer to CMD_LINE_BUFFER's buffer (null terminated), indicating a
- whole command line is ready to be executed.
+ If the read line ends with a continuation character (backslash), return
+ nullptr. Otherwise, return a pointer to the command line, indicating a whole
+ command line is ready to be executed.
- Returns EOF on end of file.
+ The returned pointer may or may not point to CMD_LINE_BUFFER's internal
+ buffer.
+
+ Return EOF on end of file.
If REPEAT, handle command repetitions:
command instead of the empty input line.
*/
-char *
-handle_line_of_input (struct buffer *cmd_line_buffer,
+const char *
+handle_line_of_input (std::string &cmd_line_buffer,
const char *rl, int repeat,
const char *annotation_suffix)
{
struct ui *ui = current_ui;
int from_tty = ui->instream == ui->stdin_stream;
- char *p1;
- char *cmd;
if (rl == NULL)
return (char *) EOF;
- cmd = command_line_append_input_line (cmd_line_buffer, rl);
- if (cmd == NULL)
+ bool complete = command_line_append_input_line (cmd_line_buffer, rl);
+ if (!complete)
return NULL;
- /* 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;
-
if (from_tty && annotation_level > 1)
printf_unfiltered (("\n\032\032post-%s\n"), annotation_suffix);
#define SERVER_COMMAND_PREFIX "server "
- server_command = startswith (cmd, SERVER_COMMAND_PREFIX);
+ server_command = startswith (cmd_line_buffer.c_str (), SERVER_COMMAND_PREFIX);
if (server_command)
{
/* Note that we don't call `save_command_line'. Between this
and the check in dont_repeat, this insures that repeating
will still do the right thing. */
- return cmd + strlen (SERVER_COMMAND_PREFIX);
+ return cmd_line_buffer.c_str () + strlen (SERVER_COMMAND_PREFIX);
}
/* Do history expansion if that is wished. */
char *cmd_expansion;
int expanded;
- expanded = history_expand (cmd, &cmd_expansion);
+ /* Note: here, we pass a pointer to the std::string's internal buffer as
+ a `char *`. At the time of writing, readline's history_expand does
+ not modify the passed-in string. Ideally, readline should be modified
+ to make that parameter `const char *`. */
+ expanded = history_expand (&cmd_line_buffer[0], &cmd_expansion);
gdb::unique_xmalloc_ptr<char> history_value (cmd_expansion);
if (expanded)
{
- size_t len;
-
/* Print the changes. */
printf_unfiltered ("%s\n", history_value.get ());
/* If there was an error, call this function again. */
if (expanded < 0)
- return cmd;
-
- /* history_expand returns an allocated string. Just replace
- our buffer with it. */
- len = strlen (history_value.get ());
- xfree (buffer_finish (cmd_line_buffer));
- cmd_line_buffer->buffer = history_value.get ();
- cmd_line_buffer->buffer_size = len + 1;
- cmd = history_value.release ();
+ return cmd_line_buffer.c_str ();
+
+ cmd_line_buffer = history_value.get ();
}
}
/* If we just got an empty line, and that is supposed to repeat the
previous command, return the previously saved command. */
- for (p1 = cmd; *p1 == ' ' || *p1 == '\t'; p1++)
+ const char *p1;
+ for (p1 = cmd_line_buffer.c_str (); *p1 == ' ' || *p1 == '\t'; p1++)
;
if (repeat && *p1 == '\0')
return get_saved_command_line ();
and then later fetch it from the value history and remove the
'#'. The kill ring is probably better, but some people are in
the habit of commenting things out. */
- if (*cmd != '\0' && from_tty && current_ui->input_interactive_p ())
- gdb_add_history (cmd);
+ if (cmd_line_buffer[0] != '\0' && from_tty && current_ui->input_interactive_p ())
+ gdb_add_history (cmd_line_buffer.c_str ());
/* Save into global buffer if appropriate. */
if (repeat)
{
- save_command_line (cmd);
+ save_command_line (cmd_line_buffer.c_str ());
+
+ /* It is important that we return a pointer to the saved command line
+ here, for the `cmd_start == saved_command_line` check in
+ execute_command to work. */
return get_saved_command_line ();
}
- else
- return cmd;
+
+ return cmd_line_buffer.c_str ();
}
/* See event-top.h. */
void
command_line_handler (gdb::unique_xmalloc_ptr<char> &&rl)
{
- struct buffer *line_buffer = get_command_line_buffer ();
+ std::string &line_buffer = get_command_line_buffer ();
struct ui *ui = current_ui;
- char *cmd;
- cmd = handle_line_of_input (line_buffer, rl.get (), 1, "prompt");
+ const char *cmd = handle_line_of_input (line_buffer, rl.get (), 1, "prompt");
if (cmd == (char *) EOF)
{
/* stdin closed. The connection with the terminal is gone.
{
ui->prompt_state = PROMPT_NEEDED;
+ /* Ensure the UI's line buffer is empty for the next command. */
+ SCOPE_EXIT { line_buffer.clear (); };
+
command_handler (cmd);
if (ui->prompt_state != PROMPTED)
{
prompt = "> ";
}
- args = command_line_input (prompt, "overload-choice");
+
+ std::string buffer;
+ args = command_line_input (buffer, prompt, "overload-choice");
if (args == 0 || *args == 0)
error_no_arg (_("one or more choice numbers"));
int count = 1;
auto reader
- = [&] ()
+ = [&] (std::string &buffer)
{
const char *result = nullptr;
if (count < argc)
bool first = true;
char *save_ptr = nullptr;
auto reader
- = [&] ()
+ = [&] (std::string &buffer)
{
const char *result = strtok_r (first ? commands.get () : nullptr,
"\n", &save_ptr);
{
int n;
const char *p = NULL;
+ std::string buffer;
char *q;
try
{
- p = command_line_input (prompt, "python");
+ p = command_line_input (buffer, prompt, "python");
}
/* Handle errors by raising Python exceptions. */
catch (const gdb_exception &except)
bool first = true;
char *save_ptr = nullptr;
auto reader
- = [&] ()
+ = [&] (std::string &buffer)
{
const char *result = strtok_r (first ? &arg_copy[0] : nullptr,
"\n", &save_ptr);
m_gdb_stderr (new stderr_file (errstream)),
m_gdb_stdlog (m_gdb_stderr)
{
- buffer_init (&line_buffer);
-
unbuffer_stream (instream_);
if (ui_list == NULL)
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
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);
}
}
}
-/* 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. */
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);
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)
{
/* The UI's command line buffer. This is to used to accumulate
input until we have a whole command line. */
- struct buffer line_buffer;
+ std::string line_buffer;
/* The callback used by the event loop whenever an event is detected
on the UI's input file descriptor. This function incrementally
extern void set_verbose (const char *, int, struct cmd_list_element *);
-extern char *handle_line_of_input (struct buffer *cmd_line_buffer,
- const char *rl, int repeat,
- const char *annotation_suffix);
+extern const char *handle_line_of_input (std::string &cmd_line_buffer,
+ const char *rl, int repeat,
+ const char *annotation_suffix);
/* Call at startup to see if the user has requested that gdb start up
quietly. */
projects / binutils-gdb.git / commitdiff