diff options
author | Pedro Alves <palves@redhat.com> | 2019-01-10 17:52:39 +0000 |
---|---|---|
committer | Pedro Alves <palves@redhat.com> | 2019-01-10 18:04:02 +0000 |
commit | 67aa1f3c2881e607081d9e1b57be3e7544c2c45c (patch) | |
tree | 445e67a51e5704a6560ba2665df76a37d6607705 /gdb/breakpoint.c | |
parent | 2f667667e24357ff54701f3e046820cf08d649cf (diff) | |
download | fsf-binutils-gdb-67aa1f3c2881e607081d9e1b57be3e7544c2c45c.zip fsf-binutils-gdb-67aa1f3c2881e607081d9e1b57be3e7544c2c45c.tar.gz fsf-binutils-gdb-67aa1f3c2881e607081d9e1b57be3e7544c2c45c.tar.bz2 |
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.
Diffstat (limited to 'gdb/breakpoint.c')
-rw-r--r-- | gdb/breakpoint.c | 6 |
1 files changed, 3 insertions, 3 deletions
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 48a1c64..2ab8a76 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -14686,7 +14686,7 @@ read_uploaded_action (void) if (next_cmd < this_utp->cmd_strings.size ()) { - rslt = this_utp->cmd_strings[next_cmd]; + rslt = this_utp->cmd_strings[next_cmd].get (); next_cmd++; } @@ -14707,7 +14707,7 @@ create_tracepoint_from_upload (struct uploaded_tp *utp) struct tracepoint *tp; if (utp->at_string) - addr_str = utp->at_string; + addr_str = utp->at_string.get (); else { /* In the absence of a source location, fall back to raw @@ -14731,7 +14731,7 @@ create_tracepoint_from_upload (struct uploaded_tp *utp) current_language); if (!create_breakpoint (get_current_arch (), location.get (), - utp->cond_string, -1, addr_str, + utp->cond_string.get (), -1, addr_str, 0 /* parse cond/thread */, 0 /* tempflag */, utp->type /* type_wanted */, |