[patch] Fix using_directive memory leak pr 11236

[patch] Fix using_directive memory leak pr 11236

[Sami Wagiaalla]

[-- Attachment #1: Type: text/plain, Size: 7943 bytes --]

.. and other cleanups.

     2010-03-03  Sami Wagiaalla  <swagiaal@redhat.com>

	PR C++/11236:
     	* cp-namespace.c (cp_add_using): Deleted.
     	(cp_add_using_directive): Use obstack allocations.
     	Merged the function cp_add_using into this one.
     	Added 'struct obstack *' argument.
     	(cp_scan_for_anonymous_namespaces): Updated.
     	* cp-support.h: Updated.
     	* dwarf2read.c (read_import_statement): Updated.
     	(read_namespace): Updated.
     	(new_symbol): Updated.
     	* stabsread.c (define_symbol): Updated.

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 5e894d4..173924b 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -76,7 +76,8 @@ static void maintenance_cplus_namespace (char *args, 
int from_tty);
  #define ANONYMOUS_NAMESPACE_LEN 21

  void
-cp_scan_for_anonymous_namespaces (const struct symbol *symbol)
+cp_scan_for_anonymous_namespaces (const struct symbol *symbol,
+                                  struct obstack *obstack)
  {
    if (SYMBOL_DEMANGLED_NAME (symbol) != NULL)
      {
@@ -117,7 +118,7 @@ cp_scan_for_anonymous_namespaces (const struct 
symbol *symbol)
  		 anonymous namespace.  So add symbols in it to the
  		 namespace given by the previous component if there is
  		 one, or to the global namespace if there isn't.  */
-	      cp_add_using_directive (dest, src, NULL);
+	      cp_add_using_directive (dest, src, NULL, obstack);
  	    }
  	  /* The "+ 2" is for the "::".  */
  	  previous_component = next_component + 2;
@@ -128,11 +129,19 @@ cp_scan_for_anonymous_namespaces (const struct 
symbol *symbol)
      }
  }

-/* Add a using directive to using_list. If the using directive in question
-   has already been added, don't add it twice.  */
+
+/* Add a using directive to using_list.  If the using directive in question
+   has already been added, don't add it twice.
+   Create a new struct using direct which imports the namespace SRC 
into the
+   scope DEST.  ALIAS is the name of the imported namespace in the current
+   scope.  If ALIAS is NULL then the namespace is known by its original 
name.
+   Set its next member in the linked list to NEXT; allocate all memory
+   using xmalloc.  It copies the strings, so NAME can be a temporary
+   string.  */

  void
-cp_add_using_directive (const char *dest, const char *src, const char 
*alias)
+cp_add_using_directive (const char *dest, const char *src, const char 
*alias,
+                        struct obstack *obstack)
  {
    struct using_direct *current;
    struct using_direct *new;
@@ -142,12 +151,22 @@ cp_add_using_directive (const char *dest, const 
char *src, const char *alias)
    for (current = using_directives; current != NULL; current = 
current->next)
      {
        if (strcmp (current->import_src, src) == 0
-          && strcmp (current->import_dest, dest) == 0)
+          && strcmp (current->import_dest, dest) == 0
+          && alias && strcmp (current->alias, alias) == 0)
  	return;
      }

-  using_directives = cp_add_using (dest, src, alias, using_directives);
+  new = obstack_alloc (obstack, sizeof (struct using_direct));
+  memset (new, 0, sizeof (struct using_direct));

+  new->import_src = obsavestring (src, strlen (src), obstack);
+  new->import_dest = obsavestring (dest, strlen (dest), obstack);
+
+  if (alias != NULL)
+    new->alias = obsavestring (alias, strlen (alias), obstack);
+
+  new->next = using_directives;
+  using_directives = new;
  }

  /* Record the namespace that the function defined by SYMBOL was
@@ -198,36 +217,6 @@ cp_is_anonymous (const char *namespace)
  	  != NULL);
  }

-/* Create a new struct using direct which imports the namespace SRC 
into the
-   scope DEST.  ALIAS is the name of the imported namespace in the current
-   scope.  If ALIAS is NULL then the namespace is known by its original 
name.
-   Set its next member in the linked list to NEXT; allocate all memory
-   using xmalloc.  It copies the strings, so NAME can be a temporary
-   string.  */
-
-struct using_direct *
-cp_add_using (const char *dest,
-              const char *src,
-              const char *alias,
-	      struct using_direct *next)
-{
-  struct using_direct *retval;
-
-  retval = xmalloc (sizeof (struct using_direct));
-  retval->import_src = savestring (src, strlen(src));
-  retval->import_dest = savestring (dest, strlen(dest));
-
-  if (alias != NULL)
-    retval->alias = savestring (alias, strlen (alias));
-  else
-    retval->alias = NULL;
-
-  retval->next = next;
-  retval->searched = 0;
-
-  return retval;
-}
-
  /* The C++-specific version of name lookup for static and global
     names.  This makes sure that names get looked for in all namespaces
     that are in scope.  NAME is the natural name of the symbol that
diff --git a/gdb/cp-support.h b/gdb/cp-support.h
index a6a9af1..23ed686 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -90,12 +90,8 @@ extern int cp_is_anonymous (const char *namespace);

  extern void cp_add_using_directive (const char *dest,
                                      const char *src,
-                                    const char *alias);
-
-extern struct using_direct *cp_add_using (const char *dest,
-                                          const char *src,
-                                          const char *alias,
-					  struct using_direct *next);
+                                    const char *alias,
+                                    struct obstack *obstack);

  extern void cp_initialize_namespace (void);

@@ -108,7 +104,8 @@ extern void cp_set_block_scope (const struct symbol 
*symbol,
  				const char *processing_current_prefix,
  				int processing_has_namespace_info);

-extern void cp_scan_for_anonymous_namespaces (const struct symbol *symbol);
+extern void cp_scan_for_anonymous_namespaces (const struct symbol *symbol,
+                                              struct obstack *obstack);

  extern struct symbol *cp_lookup_symbol_nonlocal (const char *name,
  						 const char *linkage_name,
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index a05c946..c7c96c9 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -3462,10 +3462,10 @@ read_import_statement (struct die_info *die, 
struct dwarf2_cu *cu)
        strcpy (canonical_name, imported_name);
      }

-  using_directives = cp_add_using (import_prefix,
-                                   canonical_name,
-                                   import_alias,
-                                   using_directives);
+  cp_add_using_directive (import_prefix,
+                          canonical_name,
+                          import_alias,
+                          &cu->objfile->objfile_obstack);
  }

  static void
@@ -5624,7 +5624,8 @@ read_namespace (struct die_info *die, struct 
dwarf2_cu *cu)
        if (is_anonymous)
  	{
  	  const char *previous_prefix = determine_prefix (die, cu);
-	  cp_add_using_directive (previous_prefix, TYPE_NAME (type), NULL);
+	  cp_add_using_directive (previous_prefix, TYPE_NAME (type), NULL,
+	                          &objfile->objfile_obstack);
  	}
      }

@@ -8627,7 +8628,7 @@ new_symbol (struct die_info *die, struct type 
*type, struct dwarf2_cu *cu)
        if (!processing_has_namespace_info
  	  && cu->language == language_cplus
  	  && dwarf2_attr (die, DW_AT_MIPS_linkage_name, cu) != NULL)
-	cp_scan_for_anonymous_namespaces (sym);
+	cp_scan_for_anonymous_namespaces (sym, &objfile->objfile_obstack);
      }
    return (sym);
  }
diff --git a/gdb/stabsread.c b/gdb/stabsread.c
index 71c168c..bad0727 100644
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -709,7 +709,7 @@ define_symbol (CORE_ADDR valu, char *string, int 
desc, int type,
  	  memcpy (name, string, p - string);
  	  name[p - string] = '\0';
  	  new_name = cp_canonicalize_string (name);
-	  cp_scan_for_anonymous_namespaces (sym);
+	  cp_scan_for_anonymous_namespaces (sym, &objfile->objfile_obstack);
  	}
        if (new_name != NULL)
  	{

[-- Attachment #2: memoryleak.patch --]
[-- Type: text/plain, Size: 7840 bytes --]


    2010-03-03  Sami Wagiaalla  <swagiaal@redhat.com>
    
    	* cp-namespace.c (cp_add_using): Deleted.
    	(cp_add_using_directive): Use obstack allocations.
    	Merged the function cp_add_using into this one.
    	Added 'struct obstack *' argument.
    	(cp_scan_for_anonymous_namespaces): Updated.
    	* cp-support.h: Updated.
    	* dwarf2read.c (read_import_statement): Updated.
    	(read_namespace): Updated.
    	(new_symbol): Updated.
    	* stabsread.c (define_symbol): Updated.

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 5e894d4..173924b 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -76,7 +76,8 @@ static void maintenance_cplus_namespace (char *args, int from_tty);
 #define ANONYMOUS_NAMESPACE_LEN 21
 
 void
-cp_scan_for_anonymous_namespaces (const struct symbol *symbol)
+cp_scan_for_anonymous_namespaces (const struct symbol *symbol,
+                                  struct obstack *obstack)
 {
   if (SYMBOL_DEMANGLED_NAME (symbol) != NULL)
     {
@@ -117,7 +118,7 @@ cp_scan_for_anonymous_namespaces (const struct symbol *symbol)
 		 anonymous namespace.  So add symbols in it to the
 		 namespace given by the previous component if there is
 		 one, or to the global namespace if there isn't.  */
-	      cp_add_using_directive (dest, src, NULL);
+	      cp_add_using_directive (dest, src, NULL, obstack);
 	    }
 	  /* The "+ 2" is for the "::".  */
 	  previous_component = next_component + 2;
@@ -128,11 +129,19 @@ cp_scan_for_anonymous_namespaces (const struct symbol *symbol)
     }
 }
 
-/* Add a using directive to using_list. If the using directive in question
-   has already been added, don't add it twice.  */
+
+/* Add a using directive to using_list.  If the using directive in question
+   has already been added, don't add it twice.
+   Create a new struct using direct which imports the namespace SRC into the
+   scope DEST.  ALIAS is the name of the imported namespace in the current
+   scope.  If ALIAS is NULL then the namespace is known by its original name.
+   Set its next member in the linked list to NEXT; allocate all memory
+   using xmalloc.  It copies the strings, so NAME can be a temporary
+   string.  */
 
 void
-cp_add_using_directive (const char *dest, const char *src, const char *alias)
+cp_add_using_directive (const char *dest, const char *src, const char *alias,
+                        struct obstack *obstack)
 {
   struct using_direct *current;
   struct using_direct *new;
@@ -142,12 +151,22 @@ cp_add_using_directive (const char *dest, const char *src, const char *alias)
   for (current = using_directives; current != NULL; current = current->next)
     {
       if (strcmp (current->import_src, src) == 0
-          && strcmp (current->import_dest, dest) == 0)
+          && strcmp (current->import_dest, dest) == 0
+          && alias && strcmp (current->alias, alias) == 0)
 	return;
     }
 
-  using_directives = cp_add_using (dest, src, alias, using_directives);
+  new = obstack_alloc (obstack, sizeof (struct using_direct));
+  memset (new, 0, sizeof (struct using_direct));
 
+  new->import_src = obsavestring (src, strlen (src), obstack);
+  new->import_dest = obsavestring (dest, strlen (dest), obstack);
+
+  if (alias != NULL)
+    new->alias = obsavestring (alias, strlen (alias), obstack);
+
+  new->next = using_directives;
+  using_directives = new;
 }
 
 /* Record the namespace that the function defined by SYMBOL was
@@ -198,36 +217,6 @@ cp_is_anonymous (const char *namespace)
 	  != NULL);
 }
 
-/* Create a new struct using direct which imports the namespace SRC into the
-   scope DEST.  ALIAS is the name of the imported namespace in the current
-   scope.  If ALIAS is NULL then the namespace is known by its original name.
-   Set its next member in the linked list to NEXT; allocate all memory
-   using xmalloc.  It copies the strings, so NAME can be a temporary
-   string.  */
-
-struct using_direct *
-cp_add_using (const char *dest,
-              const char *src,
-              const char *alias,
-	      struct using_direct *next)
-{
-  struct using_direct *retval;
-
-  retval = xmalloc (sizeof (struct using_direct));
-  retval->import_src = savestring (src, strlen(src));
-  retval->import_dest = savestring (dest, strlen(dest));
-
-  if (alias != NULL)
-    retval->alias = savestring (alias, strlen (alias));
-  else
-    retval->alias = NULL;
-
-  retval->next = next;
-  retval->searched = 0;
-
-  return retval;
-}
-
 /* The C++-specific version of name lookup for static and global
    names.  This makes sure that names get looked for in all namespaces
    that are in scope.  NAME is the natural name of the symbol that
diff --git a/gdb/cp-support.h b/gdb/cp-support.h
index a6a9af1..23ed686 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -90,12 +90,8 @@ extern int cp_is_anonymous (const char *namespace);
 
 extern void cp_add_using_directive (const char *dest,
                                     const char *src,
-                                    const char *alias);
-
-extern struct using_direct *cp_add_using (const char *dest,
-                                          const char *src,
-                                          const char *alias,
-					  struct using_direct *next);
+                                    const char *alias,
+                                    struct obstack *obstack);
 
 extern void cp_initialize_namespace (void);
 
@@ -108,7 +104,8 @@ extern void cp_set_block_scope (const struct symbol *symbol,
 				const char *processing_current_prefix,
 				int processing_has_namespace_info);
 
-extern void cp_scan_for_anonymous_namespaces (const struct symbol *symbol);
+extern void cp_scan_for_anonymous_namespaces (const struct symbol *symbol,
+                                              struct obstack *obstack);
 
 extern struct symbol *cp_lookup_symbol_nonlocal (const char *name,
 						 const char *linkage_name,
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index a05c946..c7c96c9 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -3462,10 +3462,10 @@ read_import_statement (struct die_info *die, struct dwarf2_cu *cu)
       strcpy (canonical_name, imported_name);
     }
 
-  using_directives = cp_add_using (import_prefix,
-                                   canonical_name,
-                                   import_alias,
-                                   using_directives);
+  cp_add_using_directive (import_prefix,
+                          canonical_name,
+                          import_alias,
+                          &cu->objfile->objfile_obstack);
 }
 
 static void
@@ -5624,7 +5624,8 @@ read_namespace (struct die_info *die, struct dwarf2_cu *cu)
       if (is_anonymous)
 	{
 	  const char *previous_prefix = determine_prefix (die, cu);
-	  cp_add_using_directive (previous_prefix, TYPE_NAME (type), NULL);
+	  cp_add_using_directive (previous_prefix, TYPE_NAME (type), NULL,
+	                          &objfile->objfile_obstack);
 	}
     }
 
@@ -8627,7 +8628,7 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
       if (!processing_has_namespace_info
 	  && cu->language == language_cplus
 	  && dwarf2_attr (die, DW_AT_MIPS_linkage_name, cu) != NULL)
-	cp_scan_for_anonymous_namespaces (sym);
+	cp_scan_for_anonymous_namespaces (sym, &objfile->objfile_obstack);
     }
   return (sym);
 }
diff --git a/gdb/stabsread.c b/gdb/stabsread.c
index 71c168c..bad0727 100644
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -709,7 +709,7 @@ define_symbol (CORE_ADDR valu, char *string, int desc, int type,
 	  memcpy (name, string, p - string);
 	  name[p - string] = '\0';
 	  new_name = cp_canonicalize_string (name);
-	  cp_scan_for_anonymous_namespaces (sym);
+	  cp_scan_for_anonymous_namespaces (sym, &objfile->objfile_obstack);
 	}
       if (new_name != NULL)
 	{

Re: [patch] Fix using_directive memory leak pr 11236

[Tom Tromey]

>>>>> "Sami" == Sami Wagiaalla <swagiaal@redhat.com> writes:

Sami> 	PR C++/11236:
Sami>     	* cp-namespace.c (cp_add_using): Deleted.
Sami>     	(cp_add_using_directive): Use obstack allocations.
Sami>     	Merged the function cp_add_using into this one.
Sami>     	Added 'struct obstack *' argument.
Sami>     	(cp_scan_for_anonymous_namespaces): Updated.
Sami>     	* cp-support.h: Updated.
Sami>     	* dwarf2read.c (read_import_statement): Updated.
Sami>     	(read_namespace): Updated.
Sami>     	(new_symbol): Updated.
Sami>     	* stabsread.c (define_symbol): Updated.

Thanks for doing this.
This general idea is fine, but I have a few comments.

Sami> -cp_scan_for_anonymous_namespaces (const struct symbol *symbol)
Sami> +cp_scan_for_anonymous_namespaces (const struct symbol *symbol,
Sami> +                                  struct obstack *obstack)

I don't think you need a new argument here.
Instead, I think you can use SYMBOL_SYMTAB (symbol)->obstack.

Sami>        if (strcmp (current->import_src, src) == 0
Sami> -          && strcmp (current->import_dest, dest) == 0)
Sami> +          && strcmp (current->import_dest, dest) == 0
Sami> +          && alias && strcmp (current->alias, alias) == 0)

This logic is incorrect if current->alias == NULL.

Sami> +  new = obstack_alloc (obstack, sizeof (struct using_direct));
Sami> +  memset (new, 0, sizeof (struct using_direct));

Use OBSTACK_ZALLOC here.

thanks,
Tom

Re: [patch] Fix using_directive memory leak pr 11236

[Sami Wagiaalla]

[-- Attachment #1: Type: text/plain, Size: 6085 bytes --]

Thanks for the review. Here are the fixes:


    2010-03-08  Sami Wagiaalla  <swagiaal@redhat.com>
    
    	* cp-namespace.c (cp_add_using): Deleted.
    	(cp_add_using_directive): Use obstack allocations.
    	Merged the function cp_add_using into this one.
    	Added 'struct obstack *' argument.
    	(cp_scan_for_anonymous_namespaces): Updated.
    	* cp-support.h: Updated.
    	* dwarf2read.c (read_import_statement): Updated.
    	(read_namespace): Updated.

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 5e894d4..3880141 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -117,7 +117,8 @@ cp_scan_for_anonymous_namespaces (const struct symbol *symbol)
 		 anonymous namespace.  So add symbols in it to the
 		 namespace given by the previous component if there is
 		 one, or to the global namespace if there isn't.  */
-	      cp_add_using_directive (dest, src, NULL);
+	      cp_add_using_directive (dest, src, NULL,
+	                              &SYMBOL_SYMTAB (symbol)->objfile->objfile_obstack);
 	    }
 	  /* The "+ 2" is for the "::".  */
 	  previous_component = next_component + 2;
@@ -128,11 +129,18 @@ cp_scan_for_anonymous_namespaces (const struct symbol *symbol)
     }
 }
 
-/* Add a using directive to using_list. If the using directive in question
-   has already been added, don't add it twice.  */
+
+/* Add a using directive to using_directives.  If the using directive in
+   question has already been added, don't add it twice.
+   Create a new struct using_direct which imports the namespace SRC into the
+   scope DEST.  ALIAS is the name of the imported namespace in the current
+   scope.  If ALIAS is NULL then the namespace is known by its original name.
+   The arguments are copied into newly allocated memory so they can be 
+   temporaries.  */
 
 void
-cp_add_using_directive (const char *dest, const char *src, const char *alias)
+cp_add_using_directive (const char *dest, const char *src, const char *alias,
+                        struct obstack *obstack)
 {
   struct using_direct *current;
   struct using_direct *new;
@@ -142,12 +150,22 @@ cp_add_using_directive (const char *dest, const char *src, const char *alias)
   for (current = using_directives; current != NULL; current = current->next)
     {
       if (strcmp (current->import_src, src) == 0
-          && strcmp (current->import_dest, dest) == 0)
+          && strcmp (current->import_dest, dest) == 0
+          && ((!alias && !current->alias)
+              || strcmp (current->alias, alias) == 0))
 	return;
     }
 
-  using_directives = cp_add_using (dest, src, alias, using_directives);
+  new = OBSTACK_ZALLOC (obstack, struct using_direct);
 
+  new->import_src = obsavestring (src, strlen (src), obstack);
+  new->import_dest = obsavestring (dest, strlen (dest), obstack);
+
+  if (alias != NULL)
+    new->alias = obsavestring (alias, strlen (alias), obstack);
+
+  new->next = using_directives;
+  using_directives = new;
 }
 
 /* Record the namespace that the function defined by SYMBOL was
@@ -198,36 +216,6 @@ cp_is_anonymous (const char *namespace)
 	  != NULL);
 }
 
-/* Create a new struct using direct which imports the namespace SRC into the
-   scope DEST.  ALIAS is the name of the imported namespace in the current
-   scope.  If ALIAS is NULL then the namespace is known by its original name.
-   Set its next member in the linked list to NEXT; allocate all memory
-   using xmalloc.  It copies the strings, so NAME can be a temporary
-   string.  */
-
-struct using_direct *
-cp_add_using (const char *dest,
-              const char *src,
-              const char *alias,
-	      struct using_direct *next)
-{
-  struct using_direct *retval;
-
-  retval = xmalloc (sizeof (struct using_direct));
-  retval->import_src = savestring (src, strlen(src));
-  retval->import_dest = savestring (dest, strlen(dest));
-
-  if (alias != NULL)
-    retval->alias = savestring (alias, strlen (alias));
-  else
-    retval->alias = NULL;
-
-  retval->next = next;
-  retval->searched = 0;
-
-  return retval;
-}
-
 /* The C++-specific version of name lookup for static and global
    names.  This makes sure that names get looked for in all namespaces
    that are in scope.  NAME is the natural name of the symbol that
diff --git a/gdb/cp-support.h b/gdb/cp-support.h
index a6a9af1..3921a5f 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -90,12 +90,8 @@ extern int cp_is_anonymous (const char *namespace);
 
 extern void cp_add_using_directive (const char *dest,
                                     const char *src,
-                                    const char *alias);
-
-extern struct using_direct *cp_add_using (const char *dest,
-                                          const char *src,
-                                          const char *alias,
-					  struct using_direct *next);
+                                    const char *alias,
+                                    struct obstack *obstack);
 
 extern void cp_initialize_namespace (void);
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index a05c946..447424e 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -3462,10 +3462,10 @@ read_import_statement (struct die_info *die, struct dwarf2_cu *cu)
       strcpy (canonical_name, imported_name);
     }
 
-  using_directives = cp_add_using (import_prefix,
-                                   canonical_name,
-                                   import_alias,
-                                   using_directives);
+  cp_add_using_directive (import_prefix,
+                          canonical_name,
+                          import_alias,
+                          &cu->objfile->objfile_obstack);
 }
 
 static void
@@ -5624,7 +5624,8 @@ read_namespace (struct die_info *die, struct dwarf2_cu *cu)
       if (is_anonymous)
 	{
 	  const char *previous_prefix = determine_prefix (die, cu);
-	  cp_add_using_directive (previous_prefix, TYPE_NAME (type), NULL);
+	  cp_add_using_directive (previous_prefix, TYPE_NAME (type), NULL,
+	                          &objfile->objfile_obstack);
 	}
     }
 

[-- Attachment #2: memoryleak.patch --]
[-- Type: text/plain, Size: 6041 bytes --]


    2010-03-08  Sami Wagiaalla  <swagiaal@redhat.com>
    
    	* cp-namespace.c (cp_add_using): Deleted.
    	(cp_add_using_directive): Use obstack allocations.
    	Merged the function cp_add_using into this one.
    	Added 'struct obstack *' argument.
    	(cp_scan_for_anonymous_namespaces): Updated.
    	* cp-support.h: Updated.
    	* dwarf2read.c (read_import_statement): Updated.
    	(read_namespace): Updated.

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 5e894d4..3880141 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -117,7 +117,8 @@ cp_scan_for_anonymous_namespaces (const struct symbol *symbol)
 		 anonymous namespace.  So add symbols in it to the
 		 namespace given by the previous component if there is
 		 one, or to the global namespace if there isn't.  */
-	      cp_add_using_directive (dest, src, NULL);
+	      cp_add_using_directive (dest, src, NULL,
+	                              &SYMBOL_SYMTAB (symbol)->objfile->objfile_obstack);
 	    }
 	  /* The "+ 2" is for the "::".  */
 	  previous_component = next_component + 2;
@@ -128,11 +129,18 @@ cp_scan_for_anonymous_namespaces (const struct symbol *symbol)
     }
 }
 
-/* Add a using directive to using_list. If the using directive in question
-   has already been added, don't add it twice.  */
+
+/* Add a using directive to using_directives.  If the using directive in
+   question has already been added, don't add it twice.
+   Create a new struct using_direct which imports the namespace SRC into the
+   scope DEST.  ALIAS is the name of the imported namespace in the current
+   scope.  If ALIAS is NULL then the namespace is known by its original name.
+   The arguments are copied into newly allocated memory so they can be 
+   temporaries.  */
 
 void
-cp_add_using_directive (const char *dest, const char *src, const char *alias)
+cp_add_using_directive (const char *dest, const char *src, const char *alias,
+                        struct obstack *obstack)
 {
   struct using_direct *current;
   struct using_direct *new;
@@ -142,12 +150,22 @@ cp_add_using_directive (const char *dest, const char *src, const char *alias)
   for (current = using_directives; current != NULL; current = current->next)
     {
       if (strcmp (current->import_src, src) == 0
-          && strcmp (current->import_dest, dest) == 0)
+          && strcmp (current->import_dest, dest) == 0
+          && ((!alias && !current->alias)
+              || strcmp (current->alias, alias) == 0))
 	return;
     }
 
-  using_directives = cp_add_using (dest, src, alias, using_directives);
+  new = OBSTACK_ZALLOC (obstack, struct using_direct);
 
+  new->import_src = obsavestring (src, strlen (src), obstack);
+  new->import_dest = obsavestring (dest, strlen (dest), obstack);
+
+  if (alias != NULL)
+    new->alias = obsavestring (alias, strlen (alias), obstack);
+
+  new->next = using_directives;
+  using_directives = new;
 }
 
 /* Record the namespace that the function defined by SYMBOL was
@@ -198,36 +216,6 @@ cp_is_anonymous (const char *namespace)
 	  != NULL);
 }
 
-/* Create a new struct using direct which imports the namespace SRC into the
-   scope DEST.  ALIAS is the name of the imported namespace in the current
-   scope.  If ALIAS is NULL then the namespace is known by its original name.
-   Set its next member in the linked list to NEXT; allocate all memory
-   using xmalloc.  It copies the strings, so NAME can be a temporary
-   string.  */
-
-struct using_direct *
-cp_add_using (const char *dest,
-              const char *src,
-              const char *alias,
-	      struct using_direct *next)
-{
-  struct using_direct *retval;
-
-  retval = xmalloc (sizeof (struct using_direct));
-  retval->import_src = savestring (src, strlen(src));
-  retval->import_dest = savestring (dest, strlen(dest));
-
-  if (alias != NULL)
-    retval->alias = savestring (alias, strlen (alias));
-  else
-    retval->alias = NULL;
-
-  retval->next = next;
-  retval->searched = 0;
-
-  return retval;
-}
-
 /* The C++-specific version of name lookup for static and global
    names.  This makes sure that names get looked for in all namespaces
    that are in scope.  NAME is the natural name of the symbol that
diff --git a/gdb/cp-support.h b/gdb/cp-support.h
index a6a9af1..3921a5f 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -90,12 +90,8 @@ extern int cp_is_anonymous (const char *namespace);
 
 extern void cp_add_using_directive (const char *dest,
                                     const char *src,
-                                    const char *alias);
-
-extern struct using_direct *cp_add_using (const char *dest,
-                                          const char *src,
-                                          const char *alias,
-					  struct using_direct *next);
+                                    const char *alias,
+                                    struct obstack *obstack);
 
 extern void cp_initialize_namespace (void);
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index a05c946..447424e 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -3462,10 +3462,10 @@ read_import_statement (struct die_info *die, struct dwarf2_cu *cu)
       strcpy (canonical_name, imported_name);
     }
 
-  using_directives = cp_add_using (import_prefix,
-                                   canonical_name,
-                                   import_alias,
-                                   using_directives);
+  cp_add_using_directive (import_prefix,
+                          canonical_name,
+                          import_alias,
+                          &cu->objfile->objfile_obstack);
 }
 
 static void
@@ -5624,7 +5624,8 @@ read_namespace (struct die_info *die, struct dwarf2_cu *cu)
       if (is_anonymous)
 	{
 	  const char *previous_prefix = determine_prefix (die, cu);
-	  cp_add_using_directive (previous_prefix, TYPE_NAME (type), NULL);
+	  cp_add_using_directive (previous_prefix, TYPE_NAME (type), NULL,
+	                          &objfile->objfile_obstack);
 	}
     }
 

Re: [patch] Fix using_directive memory leak pr 11236

[Tom Tromey]

>>>>> "Sami" == Sami Wagiaalla <swagiaal@redhat.com> writes:

Sami>     2010-03-08  Sami Wagiaalla  <swagiaal@redhat.com>
Sami>     	* cp-namespace.c (cp_add_using): Deleted.

Mention the PR in the ChangeLog entry.

Sami>        if (strcmp (current->import_src, src) == 0
Sami> -          && strcmp (current->import_dest, dest) == 0)
Sami> +          && strcmp (current->import_dest, dest) == 0
Sami> +          && ((!alias && !current->alias)
Sami> +              || strcmp (current->alias, alias) == 0))

I think this test is still wrong.

First, I think this will crash if alias==NULL but current->alias!=NULL,
or vice versa, because the code can still reach the strcmp.

Second, it doesn't really test the right thing.  I think you want
something like:

  && ((alias == NULL && current->alias == NULL)
      || (alias != NULL && current->alias != NULL
          && strcmp (alias, current->alias) == 0))

Tom

Re: [patch] Fix using_directive memory leak pr 11236

[Sami Wagiaalla]

[-- Attachment #1: Type: text/plain, Size: 6507 bytes --]


> 
> Sami>         if (strcmp (current->import_src, src) == 0
> Sami>  -&&  strcmp (current->import_dest, dest) == 0)
> Sami>  +&&  strcmp (current->import_dest, dest) == 0
> Sami>  +&&  ((!alias&&  !current->alias)
> Sami>  +              || strcmp (current->alias, alias) == 0))
> 
> I think this test is still wrong.
> 

Totally missed that :) Here is a corrected patch:


    2010-03-09  Sami Wagiaalla  <swagiaal@redhat.com>
    
    	PR C++/11236:
    	* cp-namespace.c (cp_add_using): Deleted.
    	(cp_add_using_directive): Use obstack allocations.
    	Merged the function cp_add_using into this one.
    	Added 'struct obstack *' argument.
    	(cp_scan_for_anonymous_namespaces): Updated.
    	* cp-support.h: Updated.
    	* dwarf2read.c (read_import_statement): Updated.
    	(read_namespace): Updated.

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 5e894d4..7593475 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -117,7 +117,8 @@ cp_scan_for_anonymous_namespaces (const struct symbol *symbol)
 		 anonymous namespace.  So add symbols in it to the
 		 namespace given by the previous component if there is
 		 one, or to the global namespace if there isn't.  */
-	      cp_add_using_directive (dest, src, NULL);
+	      cp_add_using_directive (dest, src, NULL,
+	                              &SYMBOL_SYMTAB (symbol)->objfile->objfile_obstack);
 	    }
 	  /* The "+ 2" is for the "::".  */
 	  previous_component = next_component + 2;
@@ -128,11 +129,18 @@ cp_scan_for_anonymous_namespaces (const struct symbol *symbol)
     }
 }
 
-/* Add a using directive to using_list. If the using directive in question
-   has already been added, don't add it twice.  */
+
+/* Add a using directive to using_directives.  If the using directive in
+   question has already been added, don't add it twice.
+   Create a new struct using_direct which imports the namespace SRC into the
+   scope DEST.  ALIAS is the name of the imported namespace in the current
+   scope.  If ALIAS is NULL then the namespace is known by its original name.
+   The arguments are copied into newly allocated memory so they can be 
+   temporaries.  */
 
 void
-cp_add_using_directive (const char *dest, const char *src, const char *alias)
+cp_add_using_directive (const char *dest, const char *src, const char *alias,
+                        struct obstack *obstack)
 {
   struct using_direct *current;
   struct using_direct *new;
@@ -142,12 +150,23 @@ cp_add_using_directive (const char *dest, const char *src, const char *alias)
   for (current = using_directives; current != NULL; current = current->next)
     {
       if (strcmp (current->import_src, src) == 0
-          && strcmp (current->import_dest, dest) == 0)
+          && strcmp (current->import_dest, dest) == 0
+          && ((alias == NULL && current->alias == NULL)
+              || (alias != NULL && current->alias != NULL
+        	  && strcmp (alias, current->alias) == 0)))
 	return;
     }
 
-  using_directives = cp_add_using (dest, src, alias, using_directives);
+  new = OBSTACK_ZALLOC (obstack, struct using_direct);
 
+  new->import_src = obsavestring (src, strlen (src), obstack);
+  new->import_dest = obsavestring (dest, strlen (dest), obstack);
+
+  if (alias != NULL)
+    new->alias = obsavestring (alias, strlen (alias), obstack);
+
+  new->next = using_directives;
+  using_directives = new;
 }
 
 /* Record the namespace that the function defined by SYMBOL was
@@ -198,36 +217,6 @@ cp_is_anonymous (const char *namespace)
 	  != NULL);
 }
 
-/* Create a new struct using direct which imports the namespace SRC into the
-   scope DEST.  ALIAS is the name of the imported namespace in the current
-   scope.  If ALIAS is NULL then the namespace is known by its original name.
-   Set its next member in the linked list to NEXT; allocate all memory
-   using xmalloc.  It copies the strings, so NAME can be a temporary
-   string.  */
-
-struct using_direct *
-cp_add_using (const char *dest,
-              const char *src,
-              const char *alias,
-	      struct using_direct *next)
-{
-  struct using_direct *retval;
-
-  retval = xmalloc (sizeof (struct using_direct));
-  retval->import_src = savestring (src, strlen(src));
-  retval->import_dest = savestring (dest, strlen(dest));
-
-  if (alias != NULL)
-    retval->alias = savestring (alias, strlen (alias));
-  else
-    retval->alias = NULL;
-
-  retval->next = next;
-  retval->searched = 0;
-
-  return retval;
-}
-
 /* The C++-specific version of name lookup for static and global
    names.  This makes sure that names get looked for in all namespaces
    that are in scope.  NAME is the natural name of the symbol that
diff --git a/gdb/cp-support.h b/gdb/cp-support.h
index a6a9af1..3921a5f 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -90,12 +90,8 @@ extern int cp_is_anonymous (const char *namespace);
 
 extern void cp_add_using_directive (const char *dest,
                                     const char *src,
-                                    const char *alias);
-
-extern struct using_direct *cp_add_using (const char *dest,
-                                          const char *src,
-                                          const char *alias,
-					  struct using_direct *next);
+                                    const char *alias,
+                                    struct obstack *obstack);
 
 extern void cp_initialize_namespace (void);
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index a05c946..447424e 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -3462,10 +3462,10 @@ read_import_statement (struct die_info *die, struct dwarf2_cu *cu)
       strcpy (canonical_name, imported_name);
     }
 
-  using_directives = cp_add_using (import_prefix,
-                                   canonical_name,
-                                   import_alias,
-                                   using_directives);
+  cp_add_using_directive (import_prefix,
+                          canonical_name,
+                          import_alias,
+                          &cu->objfile->objfile_obstack);
 }
 
 static void
@@ -5624,7 +5624,8 @@ read_namespace (struct die_info *die, struct dwarf2_cu *cu)
       if (is_anonymous)
 	{
 	  const char *previous_prefix = determine_prefix (die, cu);
-	  cp_add_using_directive (previous_prefix, TYPE_NAME (type), NULL);
+	  cp_add_using_directive (previous_prefix, TYPE_NAME (type), NULL,
+	                          &objfile->objfile_obstack);
 	}
     }
 

[-- Attachment #2: memoryleak.patch --]
[-- Type: text/plain, Size: 6131 bytes --]


    2010-03-09  Sami Wagiaalla  <swagiaal@redhat.com>
    
    	PR C++/11236:
    	* cp-namespace.c (cp_add_using): Deleted.
    	(cp_add_using_directive): Use obstack allocations.
    	Merged the function cp_add_using into this one.
    	Added 'struct obstack *' argument.
    	(cp_scan_for_anonymous_namespaces): Updated.
    	* cp-support.h: Updated.
    	* dwarf2read.c (read_import_statement): Updated.
    	(read_namespace): Updated.

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 5e894d4..7593475 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -117,7 +117,8 @@ cp_scan_for_anonymous_namespaces (const struct symbol *symbol)
 		 anonymous namespace.  So add symbols in it to the
 		 namespace given by the previous component if there is
 		 one, or to the global namespace if there isn't.  */
-	      cp_add_using_directive (dest, src, NULL);
+	      cp_add_using_directive (dest, src, NULL,
+	                              &SYMBOL_SYMTAB (symbol)->objfile->objfile_obstack);
 	    }
 	  /* The "+ 2" is for the "::".  */
 	  previous_component = next_component + 2;
@@ -128,11 +129,18 @@ cp_scan_for_anonymous_namespaces (const struct symbol *symbol)
     }
 }
 
-/* Add a using directive to using_list. If the using directive in question
-   has already been added, don't add it twice.  */
+
+/* Add a using directive to using_directives.  If the using directive in
+   question has already been added, don't add it twice.
+   Create a new struct using_direct which imports the namespace SRC into the
+   scope DEST.  ALIAS is the name of the imported namespace in the current
+   scope.  If ALIAS is NULL then the namespace is known by its original name.
+   The arguments are copied into newly allocated memory so they can be 
+   temporaries.  */
 
 void
-cp_add_using_directive (const char *dest, const char *src, const char *alias)
+cp_add_using_directive (const char *dest, const char *src, const char *alias,
+                        struct obstack *obstack)
 {
   struct using_direct *current;
   struct using_direct *new;
@@ -142,12 +150,23 @@ cp_add_using_directive (const char *dest, const char *src, const char *alias)
   for (current = using_directives; current != NULL; current = current->next)
     {
       if (strcmp (current->import_src, src) == 0
-          && strcmp (current->import_dest, dest) == 0)
+          && strcmp (current->import_dest, dest) == 0
+          && ((alias == NULL && current->alias == NULL)
+              || (alias != NULL && current->alias != NULL
+        	  && strcmp (alias, current->alias) == 0)))
 	return;
     }
 
-  using_directives = cp_add_using (dest, src, alias, using_directives);
+  new = OBSTACK_ZALLOC (obstack, struct using_direct);
 
+  new->import_src = obsavestring (src, strlen (src), obstack);
+  new->import_dest = obsavestring (dest, strlen (dest), obstack);
+
+  if (alias != NULL)
+    new->alias = obsavestring (alias, strlen (alias), obstack);
+
+  new->next = using_directives;
+  using_directives = new;
 }
 
 /* Record the namespace that the function defined by SYMBOL was
@@ -198,36 +217,6 @@ cp_is_anonymous (const char *namespace)
 	  != NULL);
 }
 
-/* Create a new struct using direct which imports the namespace SRC into the
-   scope DEST.  ALIAS is the name of the imported namespace in the current
-   scope.  If ALIAS is NULL then the namespace is known by its original name.
-   Set its next member in the linked list to NEXT; allocate all memory
-   using xmalloc.  It copies the strings, so NAME can be a temporary
-   string.  */
-
-struct using_direct *
-cp_add_using (const char *dest,
-              const char *src,
-              const char *alias,
-	      struct using_direct *next)
-{
-  struct using_direct *retval;
-
-  retval = xmalloc (sizeof (struct using_direct));
-  retval->import_src = savestring (src, strlen(src));
-  retval->import_dest = savestring (dest, strlen(dest));
-
-  if (alias != NULL)
-    retval->alias = savestring (alias, strlen (alias));
-  else
-    retval->alias = NULL;
-
-  retval->next = next;
-  retval->searched = 0;
-
-  return retval;
-}
-
 /* The C++-specific version of name lookup for static and global
    names.  This makes sure that names get looked for in all namespaces
    that are in scope.  NAME is the natural name of the symbol that
diff --git a/gdb/cp-support.h b/gdb/cp-support.h
index a6a9af1..3921a5f 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -90,12 +90,8 @@ extern int cp_is_anonymous (const char *namespace);
 
 extern void cp_add_using_directive (const char *dest,
                                     const char *src,
-                                    const char *alias);
-
-extern struct using_direct *cp_add_using (const char *dest,
-                                          const char *src,
-                                          const char *alias,
-					  struct using_direct *next);
+                                    const char *alias,
+                                    struct obstack *obstack);
 
 extern void cp_initialize_namespace (void);
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index a05c946..447424e 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -3462,10 +3462,10 @@ read_import_statement (struct die_info *die, struct dwarf2_cu *cu)
       strcpy (canonical_name, imported_name);
     }
 
-  using_directives = cp_add_using (import_prefix,
-                                   canonical_name,
-                                   import_alias,
-                                   using_directives);
+  cp_add_using_directive (import_prefix,
+                          canonical_name,
+                          import_alias,
+                          &cu->objfile->objfile_obstack);
 }
 
 static void
@@ -5624,7 +5624,8 @@ read_namespace (struct die_info *die, struct dwarf2_cu *cu)
       if (is_anonymous)
 	{
 	  const char *previous_prefix = determine_prefix (die, cu);
-	  cp_add_using_directive (previous_prefix, TYPE_NAME (type), NULL);
+	  cp_add_using_directive (previous_prefix, TYPE_NAME (type), NULL,
+	                          &objfile->objfile_obstack);
 	}
     }
 

Re: [patch] Fix using_directive memory leak pr 11236

[Tom Tromey]

>>>>> "Sami" == Sami Wagiaalla <swagiaal@redhat.com> writes:

Sami>    2010-03-09  Sami Wagiaalla  <swagiaal@redhat.com>
Sami>     	PR C++/11236:
Sami>    	* cp-namespace.c (cp_add_using): Deleted.
Sami>    	(cp_add_using_directive): Use obstack allocations.
Sami>    	Merged the function cp_add_using into this one.
Sami>    	Added 'struct obstack *' argument.
Sami>    	(cp_scan_for_anonymous_namespaces): Updated.
Sami>    	* cp-support.h: Updated.
Sami>    	* dwarf2read.c (read_import_statement): Updated.
Sami>    	(read_namespace): Updated.

This is ok, thanks.

Tom