[commit] dwarf2read.c (typename_concat): avoid segv

[commit] dwarf2read.c (typename_concat): avoid segv

[Doug Evans]

Hi. I checked in the following as obvious.
Running a large app with many shared libs under gdb -r caused gdb to segv.

2008-10-24  Doug Evans  <dje@google.com>

	* dwarf2read.c (typename_concat): Don't segv if prefix or suffix
	is NULL.  Simplify obs == NULL case.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.289
diff -u -p -r1.289 dwarf2read.c
--- dwarf2read.c	10 Oct 2008 16:15:42 -0000	1.289
+++ dwarf2read.c	24 Oct 2008 18:12:01 -0000
@@ -8081,19 +8081,17 @@ typename_concat (struct obstack *obs, co
   else
     sep = "::";
 
+  if (prefix == NULL)
+    prefix = "";
+  if (suffix == NULL)
+    suffix = "";
+
   if (obs == NULL)
     {
       char *retval = xmalloc (strlen (prefix) + MAX_SEP_LEN + strlen (suffix) + 1);
-      retval[0] = '\0';
-      
-      if (prefix)
-	{
-	  strcpy (retval, prefix);
-	  strcat (retval, sep);
-	}
-      if (suffix)
-	strcat (retval, suffix);
-      
+      strcpy (retval, prefix);
+      strcat (retval, sep);
+      strcat (retval, suffix);
       return retval;
     }
   else

Re: [commit] dwarf2read.c (typename_concat): avoid segv

[Daniel Jacobowitz]

On Fri, Oct 24, 2008 at 11:26:14AM -0700, Doug Evans wrote:
> Hi. I checked in the following as obvious.
> Running a large app with many shared libs under gdb -r caused gdb to segv.

Shouldn't this be in the affected caller?  I thought we wouldn't reach
here if prefix or suffix are NULL.

>    if (obs == NULL)
>      {
>        char *retval = xmalloc (strlen (prefix) + MAX_SEP_LEN + strlen (suffix) + 1);
> -      retval[0] = '\0';
> -      
> -      if (prefix)
> -	{
> -	  strcpy (retval, prefix);
> -	  strcat (retval, sep);
> -	}
> -      if (suffix)
> -	strcat (retval, suffix);
> -      
> +      strcpy (retval, prefix);
> +      strcat (retval, sep);
> +      strcat (retval, suffix);
>        return retval;
>      }
>    else

If NULL values are valid, this change is definitely not correct.
You've changed a NULL prefix and "foo" suffix from "foo" to "::foo".

-- 
Daniel Jacobowitz
CodeSourcery

Re: [commit] dwarf2read.c (typename_concat): avoid segv

[Daniel Jacobowitz]

On Fri, Oct 24, 2008 at 02:43:01PM -0400, Daniel Jacobowitz wrote:
> If NULL values are valid, this change is definitely not correct.
> You've changed a NULL prefix and "foo" suffix from "foo" to "::foo".

"Definitely" is too strong... I see that we differed with and without
an obstack :-(

-- 
Daniel Jacobowitz
CodeSourcery

Re: [commit] dwarf2read.c (typename_concat): avoid segv

[Doug Evans]

On Fri, Oct 24, 2008 at 11:47 AM, Daniel Jacobowitz <drow@false.org> wrote:
> On Fri, Oct 24, 2008 at 02:43:01PM -0400, Daniel Jacobowitz wrote:
>> If NULL values are valid, this change is definitely not correct.
>> You've changed a NULL prefix and "foo" suffix from "foo" to "::foo".
>
> "Definitely" is too strong... I see that we differed with and without
> an obstack :-(

Heh.  After I sent the message I knew I should have added a followup
pointing out that the segv is in the obs != NULL case.  Blech.

typename_concat has this:

/* Return a newly-allocated string formed by concatenating PREFIX and
   SUFFIX with appropriate separator.  If PREFIX or SUFFIX is NULL or
empty, then
   simply copy the SUFFIX or PREFIX, respectively.  If OBS is
non-null,
   perform an obconcat, otherwise allocate storage for the result.
The CU argument
   is used to determine the language and hence, the appropriate separator.  */

#define MAX_SEP_LEN 2  /* sizeof ("::")  */

static char *
typename_concat (struct obstack *obs, const char *prefix, const char *suffix,
                 struct dwarf2_cu *cu)
{
  char *sep;

  if (suffix == NULL || suffix[0] == '\0' || prefix == NULL ||
prefix[0] == '\0')
    sep = "";
  else if (cu->language == language_java)
    sep = ".";
  else
    sep = "::";


Given that, I think my patch is correct (unless one wants to require
callers to not pass NULL any longer).  Am I missing something?

Re: [commit] dwarf2read.c (typename_concat): avoid segv

[Daniel Jacobowitz]

On Fri, Oct 24, 2008 at 11:56:19AM -0700, Doug Evans wrote:
> Heh.  After I sent the message I knew I should have added a followup
> pointing out that the segv is in the obs != NULL case.  Blech.

Bah.  I suffered from a serious shortage of context.  I even LOOKED at
the context!

It looks fine now.

-- 
Daniel Jacobowitz
CodeSourcery