gdbserver: fix memory leak when handling qsupported packet

When building gdbserver with AddressSanitizer, I get this annoying
little leak when gdbserver exits:

==307817==ERROR: LeakSanitizer: detected memory leaks

    Direct leak of 14 byte(s) in 1 object(s) allocated from:
        #0 0x7f7fd4256459 in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:145
        #1 0x563bef981b80 in xmalloc /home/simark/src/binutils-gdb/gdbserver/../gdb/alloc.c:60
        #2 0x563befb53301 in xstrdup /home/simark/src/binutils-gdb/libiberty/xstrdup.c:34
        #3 0x563bef9d742b in handle_query /home/simark/src/binutils-gdb/gdbserver/server.cc:2286
        #4 0x563bef9ed0b7 in process_serial_event /home/simark/src/binutils-gdb/gdbserver/server.cc:4061
        #5 0x563bef9f1d9e in handle_serial_event(int, void*) /home/simark/src/binutils-gdb/gdbserver/server.cc:4402
        #6 0x563befb0ec65 in handle_file_event /home/simark/src/binutils-gdb/gdbsupport/event-loop.cc:548
        #7 0x563befb0f49f in gdb_wait_for_event /home/simark/src/binutils-gdb/gdbsupport/event-loop.cc:673
        #8 0x563befb0d4a1 in gdb_do_one_event() /home/simark/src/binutils-gdb/gdbsupport/event-loop.cc:215
        #9 0x563bef9e721a in start_event_loop /home/simark/src/binutils-gdb/gdbserver/server.cc:3484
        #10 0x563bef9eb90a in captured_main /home/simark/src/binutils-gdb/gdbserver/server.cc:3875
        #11 0x563bef9ec2c7 in main /home/simark/src/binutils-gdb/gdbserver/server.cc:3961
        #12 0x7f7fd3330001 in __libc_start_main (/usr/lib/libc.so.6+0x27001)

    SUMMARY: AddressSanitizer: 14 byte(s) leaked in 1 allocation(s).

This is due to the handling of unknown qsupported features in
handle_query.  The `qsupported` vector is built, containing all the
feature names received from GDB.  As we iterate on them, when we
encounter unknown ones, we move them at the beginning of the vector, in
preparation of passing this vector of unknown features down to the
target (which may know about them).

When moving these unknown features to other slots in the vector, we
overwrite other pointers without freeing them, which therefore leak.

An easy fix would be to add a `free` when doing the move.  However, I
think this is a good opportunity to sprinkle a bit of automatic memory
management in this code.

So, use a vector of std::string which owns all the entries.  And use a
separate vector (that doesn't own the entries) for the unknown ones,
which is then passed to target_process_qsupported.

Given that the `c_str` method of std::string returns a `const char *`,
it follows that process_stratum_target::process_qsupported must accept a
`const char **` instead of a `char **`.  And while at it, change the
pointer + size paramters to use an array_view instead.

gdbserver/ChangeLog:

	* server.cc (handle_query): Use std::vector of
	std::string for `qsupported` vector.  Use separate
	vector for unknowns.
	* target.h (class process_stratum_target) <process_qsupported>:
	Change parameters to array_view of const char *.
	(target_process_qsupported): Remove `count` parameter.
	* target.cc (process_stratum_target::process_qsupported): Change
	parameters to array_view of const char *.
	* linux-x86-low.cc (class x86_target) <process_qsupported>:
	Likewise.

Change-Id: I97f133825faa6d7abbf83a58504eb0ba77462812

1 files changed, 6 insertions, 4 deletions

diff --git a/gdbserver/target.h b/gdbserver/target.h
index 701c8ef..13f069f 100644
--- a/gdbserver/target.h
+++ b/gdbserver/target.h
@@ -27,6 +27,7 @@
 #include "target/wait.h"
 #include "target/waitstatus.h"
 #include "mem-break.h"
+#include "gdbsupport/array-view.h"
 #include "gdbsupport/btrace-common.h"
 #include <vector>
 
@@ -315,8 +316,9 @@ public:
 			    unsigned char *myaddr, unsigned int len);
 
   /* Target specific qSupported support.  FEATURES is an array of
-     features with COUNT elements.  */
-  virtual void process_qsupported (char **features, int count);
+     features unsupported by the core of GDBserver.  */
+  virtual void process_qsupported
+    (gdb::array_view<const char * const> features);
 
   /* Return true if the target supports tracepoints, false otherwise.  */
   virtual bool supports_tracepoints ();
@@ -547,8 +549,8 @@ int kill_inferior (process_info *proc);
 #define target_async(enable) \
   the_target->async (enable)
 
-#define target_process_qsupported(features, count)	\
-  the_target->process_qsupported (features, count)
+#define target_process_qsupported(features) \
+  the_target->process_qsupported (features)
 
 #define target_supports_catch_syscall()              	\
   the_target->supports_catch_syscall ()