Fix tracepoint.c:parse_tracepoint_definition leak (and one more)

Coverity points out that gdb/tracepoint.c:parse_tracepoint_definition
can leak 'cond' in this line:

      cond = (char *) xmalloc (2 * xlen + 1);

That can leak because we're in a loop and 'cond' may have already been
xmalloc'ed into in a previous iteration.  That won't normally happen,
because we don't expect to see a tracepoint definition with multiple
conditions listed, but, it doesn't hurt to be pedantically correct,
in case some stub manages to send something odd back to GDB.

At first I thought I'd just replace the xmalloc call with:

      cond = (char *) xrealloc (cond, 2 * xlen + 1);

and be done with it.  However, my pedantic self realizes that
warning() can throw as well (due to pagination + Ctrl-C), so I fixed
it using gdb::unique_xmalloc_ptr instead.

While doing this, I noticed that these vectors in struct uploaded_tp:

  std::vector<char *> actions;
  std::vector<char *> step_actions;

hold heap-allocated strings, but nothing is freeing the strings,
AFAICS.

So I ended up switching all the heap-allocated strings in uploaded_tp
to unique pointers.  This patch is the result of that.

I also wrote an alternative, but similar patch that uses std::string
throughout instead of gdb::unique_xmalloc_ptr, but in the end reverted
it because the code didn't look that much better, and I kind of
dislike replacing pointers with fat std::string's (3 or 4 times the
size of a pointer) in structures.

gdb/ChangeLog:
2019-01-10  Pedro Alves  <palves@redhat.com>

	* breakpoint.c (read_uploaded_action)
	(create_tracepoint_from_upload): Adjust to use
	gdb::unique_xmalloc_ptr.
	* ctf.c (ctf_write_uploaded_tp):
	(SET_ARRAY_FIELD): Use emplace_back.
	(SET_STRING_FIELD): Adjust to use gdb::unique_xmalloc_ptr.
	* tracefile-tfile.c (tfile_write_uploaded_tp):
	* tracepoint.c (parse_tracepoint_definition): Adjust to use
	gdb::unique_xmalloc_ptr.
	* tracepoint.h (struct uploaded_tp) <cond, actions, step_actions,
	at_string, cond_string, cmd_strings>: Replace char pointers
	with gdb::unique_xmalloc_ptr.
This commit is contained in:
Pedro Alves 2019-01-10 17:52:39 +00:00
parent 2f667667e2
commit 67aa1f3c28
6 changed files with 65 additions and 43 deletions

View file

@ -178,21 +178,21 @@ struct uploaded_tp
int orig_size = 0;
/* String that is the encoded form of the tracepoint's condition. */
char *cond = nullptr;
gdb::unique_xmalloc_ptr<char[]> cond;
/* Vectors of strings that are the encoded forms of a tracepoint's
actions. */
std::vector<char *> actions;
std::vector<char *> step_actions;
std::vector<gdb::unique_xmalloc_ptr<char[]>> actions;
std::vector<gdb::unique_xmalloc_ptr<char[]>> step_actions;
/* The original string defining the location of the tracepoint. */
char *at_string = nullptr;
gdb::unique_xmalloc_ptr<char[]> at_string;
/* The original string defining the tracepoint's condition. */
char *cond_string = nullptr;
gdb::unique_xmalloc_ptr<char[]> cond_string;
/* List of original strings defining the tracepoint's actions. */
std::vector<char *> cmd_strings;
std::vector<gdb::unique_xmalloc_ptr<char[]>> cmd_strings;
/* The tracepoint's current hit count. */
int hit_count = 0;