[RFAv2] Fix leak in linespec parser.

[RFAv2] Fix leak in linespec parser.

[Philippe Waroquiers]

Valgrind reports the below leak.
linespec_parser_new allocates a std::vector<symtab *> at line 2756,
and stores the pointer to this vector in PARSER_RESULT (parser)->file_symtabs.
At 3 different places in linespec.c, another std::vector is assigned to
a linespec->file_symtabs, without first deleting the current value.

The leak is fixed by delete-ing linespec->file_symtabs before
assigning a new value to it, at 3 different places.
At one of these places, declare a symtab_vector_up r, so as
to do the delete of the previous value once a new value has
been properly built.

Tested on debian/amd64, + a bunch of tests re-run under valgrind
(including the test that throws an error).

Ok to push ?

gdb/ChangeLog
2018-11-25  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* linespec.c (create_sals_line_offset): Fix leak by deleting
	previous value before assigning new value.
	(convert_explicit_location_to_linespec): First build the
	symtab_vector_up r.  Then likewise.
	(parse_linespec): Likewise.

==798== VALGRIND_GDB_ERROR_BEGIN
==798== 32 (24 direct, 8 indirect) bytes in 1 blocks are definitely lost in loss record 447 of 3,143
==798==    at 0x4C2C48C: operator new(unsigned long) (vg_replace_malloc.c:334)
==798==    by 0x51D401: linespec_parser_new(ls_parser*, int, language_defn const*, program_space*, symtab*, int, linespec_result*) (linespec.c:2756)
==798==    by 0x524BF7: decode_line_full(event_location const*, int, program_space*, symtab*, int, linespec_result*, char const*, char const*) (linespec.c:3271)
==798==    by 0x3E8893: parse_breakpoint_sals(event_location const*, linespec_result*) (breakpoint.c:9067)
==798==    by 0x3E4E7F: create_breakpoint(gdbarch*, event_location const*, char const*, int, char const*, int, int, bptype, int, auto_boolean, breakpoint_ops const*, int, int, int, unsigned int) (breakpoint.c:9248)
==798==    by 0x3E55F5: break_command_1(char const*, int, int) (breakpoint.c:9434)
==798==    by 0x40BA68: cmd_func(cmd_list_element*, char const*, int) (cli-decode.c:1888)
==798==    by 0x665300: execute_command(char const*, int) (top.c:630)
...
---
 gdb/linespec.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 00f59f9c28..567211f1de 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2117,8 +2117,9 @@ create_sals_line_offset (struct linespec_state *self,
       set_default_source_symtab_and_line ();
       initialize_defaults (&self->default_symtab, &self->default_line);
       fullname = symtab_to_fullname (self->default_symtab);
-      symtab_vector_up r =
-	collect_symtabs_from_filename (fullname, self->search_pspace);
+      symtab_vector_up r
+	= collect_symtabs_from_filename (fullname, self->search_pspace);
+      delete ls->file_symtabs;
       ls->file_symtabs = r.release ();
       use_default = 1;
     }
@@ -2401,9 +2402,11 @@ convert_explicit_location_to_linespec (struct linespec_state *self,
     {
       TRY
 	{
-	  result->file_symtabs
+	  symtab_vector_up r
 	    = symtabs_from_filename (source_filename,
-				     self->search_pspace).release ();
+				     self->search_pspace);
+	  delete result->file_symtabs;
+	  result->file_symtabs = r.release ();
 	}
       CATCH (except, RETURN_MASK_ERROR)
 	{
@@ -2630,6 +2633,7 @@ parse_linespec (linespec_parser *parser, const char *arg,
 	  symtab_vector_up r
 	    = symtabs_from_filename (user_filename.get (),
 				     PARSER_STATE (parser)->search_pspace);
+	  delete PARSER_RESULT (parser)->file_symtabs;
 	  PARSER_RESULT (parser)->file_symtabs = r.release ();
 	}
       CATCH (ex, RETURN_MASK_ERROR)
-- 
2.19.1

Re: [RFAv2] Fix leak in linespec parser.

[Simon Marchi]

On 2018-11-25 6:04 p.m., Philippe Waroquiers wrote:
> Valgrind reports the below leak.
> linespec_parser_new allocates a std::vector<symtab *> at line 2756,
> and stores the pointer to this vector in PARSER_RESULT (parser)->file_symtabs.
> At 3 different places in linespec.c, another std::vector is assigned to
> a linespec->file_symtabs, without first deleting the current value.
> 
> The leak is fixed by delete-ing linespec->file_symtabs before
> assigning a new value to it, at 3 different places.
> At one of these places, declare a symtab_vector_up r, so as
> to do the delete of the previous value once a new value has
> been properly built.
> 
> Tested on debian/amd64, + a bunch of tests re-run under valgrind
> (including the test that throws an error).
> 
> Ok to push ?
> 
> gdb/ChangeLog
> 2018-11-25  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* linespec.c (create_sals_line_offset): Fix leak by deleting
> 	previous value before assigning new value.
> 	(convert_explicit_location_to_linespec): First build the
> 	symtab_vector_up r.  Then likewise.
> 	(parse_linespec): Likewise.
> 
> ==798== VALGRIND_GDB_ERROR_BEGIN
> ==798== 32 (24 direct, 8 indirect) bytes in 1 blocks are definitely lost in loss record 447 of 3,143
> ==798==    at 0x4C2C48C: operator new(unsigned long) (vg_replace_malloc.c:334)
> ==798==    by 0x51D401: linespec_parser_new(ls_parser*, int, language_defn const*, program_space*, symtab*, int, linespec_result*) (linespec.c:2756)
> ==798==    by 0x524BF7: decode_line_full(event_location const*, int, program_space*, symtab*, int, linespec_result*, char const*, char const*) (linespec.c:3271)
> ==798==    by 0x3E8893: parse_breakpoint_sals(event_location const*, linespec_result*) (breakpoint.c:9067)
> ==798==    by 0x3E4E7F: create_breakpoint(gdbarch*, event_location const*, char const*, int, char const*, int, int, bptype, int, auto_boolean, breakpoint_ops const*, int, int, int, unsigned int) (breakpoint.c:9248)
> ==798==    by 0x3E55F5: break_command_1(char const*, int, int) (breakpoint.c:9434)
> ==798==    by 0x40BA68: cmd_func(cmd_list_element*, char const*, int) (cli-decode.c:1888)
> ==798==    by 0x665300: execute_command(char const*, int) (top.c:630)

Hi Philippe,

This looks good, but I think we could simplify that a bit by returning
std::vector objects directly, not dealing with new/delete.  We would move
returned data in the existing vector.  Would the patch below work?  I think
everything is set up correctly move-semantic-wise so that there would be no
copy of vector data
.

From f135accbec31059b4bff017a1d42361dae25708b Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Sun, 25 Nov 2018 20:40:35 -0500
Subject: [PATCH] Fix leak in linespec parser.

---
 gdb/linespec.c | 38 +++++++++++++++-----------------------
 1 file changed, 15 insertions(+), 23 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 00f59f9c2866..e534cf2e81e4 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -77,10 +77,6 @@ enum class linespec_complete_what
   KEYWORD,
 };

-/* Typedef for unique_ptrs of vectors of symtabs.  */
-
-typedef std::unique_ptr<std::vector<symtab *>> symtab_vector_up;
-
 /* An address entry is used to ensure that any given location is only
    added to the result a single time.  It holds an address and the
    program space from which the address came.  */
@@ -357,7 +353,7 @@ static std::vector<symtab_and_line> decode_objc (struct linespec_state *self,
 						 linespec_p ls,
 						 const char *arg);

-static symtab_vector_up symtabs_from_filename
+static std::vector<symtab *> symtabs_from_filename
   (const char *, struct program_space *pspace);

 static std::vector<block_symbol> *find_label_symbols
@@ -389,7 +385,7 @@ static void add_all_symbol_names_from_pspace
     (struct collect_info *info, struct program_space *pspace,
      const std::vector<const char *> &names, enum search_domain search_domain);

-static symtab_vector_up
+static std::vector<symtab *>
   collect_symtabs_from_filename (const char *file,
 				 struct program_space *pspace);

@@ -2117,9 +2113,8 @@ create_sals_line_offset (struct linespec_state *self,
       set_default_source_symtab_and_line ();
       initialize_defaults (&self->default_symtab, &self->default_line);
       fullname = symtab_to_fullname (self->default_symtab);
-      symtab_vector_up r =
-	collect_symtabs_from_filename (fullname, self->search_pspace);
-      ls->file_symtabs = r.release ();
+      *ls->file_symtabs
+	= collect_symtabs_from_filename (fullname, self->search_pspace);
       use_default = 1;
     }

@@ -2401,9 +2396,8 @@ convert_explicit_location_to_linespec (struct linespec_state *self,
     {
       TRY
 	{
-	  result->file_symtabs
-	    = symtabs_from_filename (source_filename,
-				     self->search_pspace).release ();
+	  *result->file_symtabs
+	    = symtabs_from_filename (source_filename, self->search_pspace);
 	}
       CATCH (except, RETURN_MASK_ERROR)
 	{
@@ -2627,10 +2621,9 @@ parse_linespec (linespec_parser *parser, const char *arg,
       /* Check if the input is a filename.  */
       TRY
 	{
-	  symtab_vector_up r
+	  *PARSER_RESULT (parser)->file_symtabs
 	    = symtabs_from_filename (user_filename.get (),
 				     PARSER_STATE (parser)->search_pspace);
-	  PARSER_RESULT (parser)->file_symtabs = r.release ();
 	}
       CATCH (ex, RETURN_MASK_ERROR)
 	{
@@ -3790,7 +3783,6 @@ class symtab_collector
 {
 public:
   symtab_collector ()
-    : m_symtabs (new std::vector<symtab *> ())
   {
     m_symtab_table = htab_create (1, htab_hash_pointer, htab_eq_pointer,
 				  NULL);
@@ -3805,15 +3797,15 @@ public:
   /* Callable as a symbol_found_callback_ftype callback.  */
   bool operator () (symtab *sym);

-  /* Releases ownership of the collected symtabs and returns them.  */
-  symtab_vector_up release_symtabs ()
+  /* Return an rvalue reference to the collected symtabs.  */
+  std::vector<symtab *> &&release_symtabs ()
   {
     return std::move (m_symtabs);
   }

 private:
   /* The result vector of symtabs.  */
-  symtab_vector_up m_symtabs;
+  std::vector<symtab *> m_symtabs;

   /* This is used to ensure the symtabs are unique.  */
   htab_t m_symtab_table;
@@ -3828,7 +3820,7 @@ symtab_collector::operator () (struct symtab *symtab)
   if (!*slot)
     {
       *slot = symtab;
-      m_symtabs->push_back (symtab);
+      m_symtabs.push_back (symtab);
     }

   return false;
@@ -3840,7 +3832,7 @@ symtab_collector::operator () (struct symtab *symtab)
    SEARCH_PSPACE is not NULL, the search is restricted to just that
    program space.  */

-static symtab_vector_up
+static std::vector<symtab *>
 collect_symtabs_from_filename (const char *file,
 			       struct program_space *search_pspace)
 {
@@ -3872,14 +3864,14 @@ collect_symtabs_from_filename (const char *file,
 /* Return all the symtabs associated to the FILENAME.  If SEARCH_PSPACE is
    not NULL, the search is restricted to just that program space.  */

-static symtab_vector_up
+static std::vector<symtab *>
 symtabs_from_filename (const char *filename,
 		       struct program_space *search_pspace)
 {
-  symtab_vector_up result
+  std::vector<symtab *> result
     = collect_symtabs_from_filename (filename, search_pspace);

-  if (result->empty ())
+  if (result.empty ())
     {
       if (!have_full_symbols () && !have_partial_symbols ())
 	throw_error (NOT_FOUND_ERROR,
-- 
2.19.2

Re: [RFAv2] Fix leak in linespec parser.

[Philippe Waroquiers]

On Sun, 2018-11-25 at 20:42 -0500, Simon Marchi wrote:
> Hi Philippe,
> 
> This looks good, but I think we could simplify that a bit by returning
> std::vector objects directly, not dealing with new/delete.  We would move
> returned data in the existing vector.  Would the patch below work?  I think
> everything is set up correctly move-semantic-wise so that there would be no
> copy of vector data.

Hello Simon,

I tested the patch, which effectively solves the leak.
For what concerns the move-semantic-wise: I cannot really comment on that,
as my c++ knowledge is very limited (only working with c++ on GDB at home,
never really learned it).
But valgrind does not show any increase in total nr of blocks allocated:
We see in the below that the unpatched GDB has 7 blocks leaked, and does
the same total nr of block allocations than the patched GDB, so that looks all ok.

The patched GDB allocates in total 2206 bytes more, probably irrelevant
difference caused by environment/file name differences/hash table differing/...

So, FWIW, the patch looks good to me ...

Thanks

Philippe


Unpatched GDB:
==23215== xtree memory report: /bd/home/philippe/gdb/git/build_binutils-gdb/xtmemory.kcg.23215
==23215== HEAP SUMMARY:
==23215==     in use at exit: 35,368,211 bytes in 5,199 blocks
==23215==   total heap usage: 1,187,659 allocs, 1,182,460 frees, 168,153,794 bytes allocated
==23215== 
==23215== LEAK SUMMARY:
==23215==    definitely lost: 144 bytes in 6 blocks
==23215==    indirectly lost: 8 bytes in 1 blocks
==23215==      possibly lost: 61,952 bytes in 144 blocks
==23215==    still reachable: 35,306,011 bytes in 5,043 blocks
==23215==         suppressed: 96 bytes in 5 blocks

Patched GDB:
==23211== xtree memory report: /bd/home/philippe/gdb/git/build_smallthing/xtmemory.kcg.23211
==23211== HEAP SUMMARY:
==23211==     in use at exit: 35,368,053 bytes in 5,193 blocks
==23211==   total heap usage: 1,187,659 allocs, 1,182,466 frees, 168,156,000 bytes allocated
==23211== 
==23211== LEAK SUMMARY:
==23211==    definitely lost: 0 bytes in 0 blocks
==23211==    indirectly lost: 0 bytes in 0 blocks
==23211==      possibly lost: 61,952 bytes in 144 blocks
==23211==    still reachable: 35,306,005 bytes in 5,044 blocks
==23211==         suppressed: 96 bytes in 5 blocks

Re: [RFAv2] Fix leak in linespec parser.

[Simon Marchi]

On 2018-11-27 5:52 p.m., Philippe Waroquiers wrote:
> On Sun, 2018-11-25 at 20:42 -0500, Simon Marchi wrote:
>> Hi Philippe,
>>
>> This looks good, but I think we could simplify that a bit by returning
>> std::vector objects directly, not dealing with new/delete.  We would move
>> returned data in the existing vector.  Would the patch below work?  I think
>> everything is set up correctly move-semantic-wise so that there would be no
>> copy of vector data.
> 
> Hello Simon,
> 
> I tested the patch, which effectively solves the leak.
> For what concerns the move-semantic-wise: I cannot really comment on that,
> as my c++ knowledge is very limited (only working with c++ on GDB at home,
> never really learned it).
> But valgrind does not show any increase in total nr of blocks allocated:
> We see in the below that the unpatched GDB has 7 blocks leaked, and does
> the same total nr of block allocations than the patched GDB, so that looks all ok.
> 
> The patched GDB allocates in total 2206 bytes more, probably irrelevant
> difference caused by environment/file name differences/hash table differing/...
> 
> So, FWIW, the patch looks good to me ...

Ok thanks, this is what I pushed:

From 4717cec4fe4cb3a086fb13161603112e8ded787e Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Fri, 30 Nov 2018 16:49:35 -0500
Subject: [PATCH] Fix leak in linespec parser

Valgrind reports this leak:

  ==798== VALGRIND_GDB_ERROR_BEGIN
  ==798== 32 (24 direct, 8 indirect) bytes in 1 blocks are definitely lost in loss record 447 of 3,143
  ==798==    at 0x4C2C48C: operator new(unsigned long) (vg_replace_malloc.c:334)
  ==798==    by 0x51D401: linespec_parser_new(ls_parser*, int, language_defn const*, program_space*, symtab*, int, linespec_result*) (linespec.c:2756)
  ==798==    by 0x524BF7: decode_line_full(event_location const*, int, program_space*, symtab*, int, linespec_result*, char const*, char const*) (linespec.c:3271)
  ==798==    by 0x3E8893: parse_breakpoint_sals(event_location const*, linespec_result*) (breakpoint.c:9067)
  ==798==    by 0x3E4E7F: create_breakpoint(gdbarch*, event_location const*, char const*, int, char const*, int, int, bptype, int, auto_boolean, breakpoint_ops const*, int, int, int, unsigned int) (breakpoint.c:9248)
  ==798==    by 0x3E55F5: break_command_1(char const*, int, int) (breakpoint.c:9434)
  ==798==    by 0x40BA68: cmd_func(cmd_list_element*, char const*, int) (cli-decode.c:1888)
  ==798==    by 0x665300: execute_command(char const*, int) (top.c:630)
  ...

linespec_parser_new allocates a std::vector<symtab *> at line 2756, and stores
the pointer to this vector in PARSER_RESULT (parser)->file_symtabs.  At 3
different places in linespec.c, another std::vector is assigned to a
linespec->file_symtabs, without first deleting the current value.

The leak is fixed by assigning the vector itself instead of the pointer.
Everything should be moved, so there is no significant data copy
involved.

Tested on debian/amd64, + a bunch of tests re-run under valgrind
(including the test that throws an error).

gdb/ChangeLog:

	* linespec.c (symtab_vector_up): Remove.
	(symtabs_from_filename): Change return type to std::vector.
	(collect_symtabs_from_filename): Likewise.
	(create_sals_line_offset): Assign return value of
	collect_symtabs_from_filename to *ls->file_symtabs.
	(convert_explicit_location_to_linespec): Remove call to release.
	(parse_linespec): Likewise.
	(symtab_collector) <symtab_collector>: Remove initialization of
	m_symtabs.
	<release_symtabs>: Change return type to std::vector<symtab *>.
	<operator ()>: Adjust.
---
 gdb/ChangeLog  | 15 +++++++++++++++
 gdb/linespec.c | 38 +++++++++++++++-----------------------
 2 files changed, 30 insertions(+), 23 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 348eb65ec71..778eebc1b10 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,18 @@
+2018-11-30  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
+            Simon Marchi  <simon.marchi@ericsson.com>
+
+	* linespec.c (symtab_vector_up): Remove.
+	(symtabs_from_filename): Change return type to std::vector.
+	(collect_symtabs_from_filename): Likewise.
+	(create_sals_line_offset): Assign return value of
+	collect_symtabs_from_filename to *ls->file_symtabs.
+	(convert_explicit_location_to_linespec): Remove call to release.
+	(parse_linespec): Likewise.
+	(symtab_collector) <symtab_collector>: Remove initialization of
+	m_symtabs.
+	<release_symtabs>: Change return type to std::vector<symtab *>.
+	<operator ()>: Adjust.
+
 2018-11-30  John Baldwin  <jhb@FreeBSD.org>

 	* fbsd-nat.c [__FreeBSD_version >= 700009] (USE_SIGINFO): Macro
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 00f59f9c286..e534cf2e81e 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -77,10 +77,6 @@ enum class linespec_complete_what
   KEYWORD,
 };

-/* Typedef for unique_ptrs of vectors of symtabs.  */
-
-typedef std::unique_ptr<std::vector<symtab *>> symtab_vector_up;
-
 /* An address entry is used to ensure that any given location is only
    added to the result a single time.  It holds an address and the
    program space from which the address came.  */
@@ -357,7 +353,7 @@ static std::vector<symtab_and_line> decode_objc (struct linespec_state *self,
 						 linespec_p ls,
 						 const char *arg);

-static symtab_vector_up symtabs_from_filename
+static std::vector<symtab *> symtabs_from_filename
   (const char *, struct program_space *pspace);

 static std::vector<block_symbol> *find_label_symbols
@@ -389,7 +385,7 @@ static void add_all_symbol_names_from_pspace
     (struct collect_info *info, struct program_space *pspace,
      const std::vector<const char *> &names, enum search_domain search_domain);

-static symtab_vector_up
+static std::vector<symtab *>
   collect_symtabs_from_filename (const char *file,
 				 struct program_space *pspace);

@@ -2117,9 +2113,8 @@ create_sals_line_offset (struct linespec_state *self,
       set_default_source_symtab_and_line ();
       initialize_defaults (&self->default_symtab, &self->default_line);
       fullname = symtab_to_fullname (self->default_symtab);
-      symtab_vector_up r =
-	collect_symtabs_from_filename (fullname, self->search_pspace);
-      ls->file_symtabs = r.release ();
+      *ls->file_symtabs
+	= collect_symtabs_from_filename (fullname, self->search_pspace);
       use_default = 1;
     }

@@ -2401,9 +2396,8 @@ convert_explicit_location_to_linespec (struct linespec_state *self,
     {
       TRY
 	{
-	  result->file_symtabs
-	    = symtabs_from_filename (source_filename,
-				     self->search_pspace).release ();
+	  *result->file_symtabs
+	    = symtabs_from_filename (source_filename, self->search_pspace);
 	}
       CATCH (except, RETURN_MASK_ERROR)
 	{
@@ -2627,10 +2621,9 @@ parse_linespec (linespec_parser *parser, const char *arg,
       /* Check if the input is a filename.  */
       TRY
 	{
-	  symtab_vector_up r
+	  *PARSER_RESULT (parser)->file_symtabs
 	    = symtabs_from_filename (user_filename.get (),
 				     PARSER_STATE (parser)->search_pspace);
-	  PARSER_RESULT (parser)->file_symtabs = r.release ();
 	}
       CATCH (ex, RETURN_MASK_ERROR)
 	{
@@ -3790,7 +3783,6 @@ class symtab_collector
 {
 public:
   symtab_collector ()
-    : m_symtabs (new std::vector<symtab *> ())
   {
     m_symtab_table = htab_create (1, htab_hash_pointer, htab_eq_pointer,
 				  NULL);
@@ -3805,15 +3797,15 @@ public:
   /* Callable as a symbol_found_callback_ftype callback.  */
   bool operator () (symtab *sym);

-  /* Releases ownership of the collected symtabs and returns them.  */
-  symtab_vector_up release_symtabs ()
+  /* Return an rvalue reference to the collected symtabs.  */
+  std::vector<symtab *> &&release_symtabs ()
   {
     return std::move (m_symtabs);
   }

 private:
   /* The result vector of symtabs.  */
-  symtab_vector_up m_symtabs;
+  std::vector<symtab *> m_symtabs;

   /* This is used to ensure the symtabs are unique.  */
   htab_t m_symtab_table;
@@ -3828,7 +3820,7 @@ symtab_collector::operator () (struct symtab *symtab)
   if (!*slot)
     {
       *slot = symtab;
-      m_symtabs->push_back (symtab);
+      m_symtabs.push_back (symtab);
     }

   return false;
@@ -3840,7 +3832,7 @@ symtab_collector::operator () (struct symtab *symtab)
    SEARCH_PSPACE is not NULL, the search is restricted to just that
    program space.  */

-static symtab_vector_up
+static std::vector<symtab *>
 collect_symtabs_from_filename (const char *file,
 			       struct program_space *search_pspace)
 {
@@ -3872,14 +3864,14 @@ collect_symtabs_from_filename (const char *file,
 /* Return all the symtabs associated to the FILENAME.  If SEARCH_PSPACE is
    not NULL, the search is restricted to just that program space.  */

-static symtab_vector_up
+static std::vector<symtab *>
 symtabs_from_filename (const char *filename,
 		       struct program_space *search_pspace)
 {
-  symtab_vector_up result
+  std::vector<symtab *> result
     = collect_symtabs_from_filename (filename, search_pspace);

-  if (result->empty ())
+  if (result.empty ())
     {
       if (!have_full_symbols () && !have_partial_symbols ())
 	throw_error (NOT_FOUND_ERROR,
-- 
2.19.2