[Patch]: Fix crash in gprof (SunOS 5.8 only)

[Patch]: Fix crash in gprof (SunOS 5.8 only)

[Tristan Gingold]

Hi,

bsearch is slightly bugged in Solaris 8, as it calls the comparator function even if the number of entries is 0.
There are two instances of such bsearch in gprof/corefile.c.  Guarding them is very simple, and done in this patch.

Ok for trunk ?

Simply tested on an example as there is no gprof test suite.

PS: should we drop tahoe ?
PPS: should we drop '-c' ?  It is supported by very few architectures (i386, alpha, tax, sparc, tahoe and mips) and looks broken at least on i386.

Tristan.

gprof/
2012-02-23  Tristan Gingold  <gingold@adacore.com>

	* corefile.c (core_create_function_syms): Do not call bsearch if
	symbol_map_count is 0.

diff --git a/gprof/corefile.c b/gprof/corefile.c
index e25d19b..9f93cee 100644
--- a/gprof/corefile.c
+++ b/gprof/corefile.c
@@ -582,7 +582,7 @@ core_create_function_syms (void)
   bfd_vma max_vma = 0;
   int cxxclass;
   long i;
-  struct function_map * found;
+  struct function_map * found = NULL;
   int core_has_func_syms = 0;
 
   switch (core_bfd->xvec->flavour)
@@ -609,10 +609,14 @@ core_create_function_syms (void)
       /* Don't create a symtab entry for a function that has
 	 a mapping to a file, unless it's the first function
 	 in the file.  */
-      found = (struct function_map *) bsearch (core_syms[i]->name, symbol_map,
-                                               symbol_map_count,
-                                               sizeof (struct function_map),
-                                               search_mapped_symbol);
+      if (symbol_map_count != 0)
+	{
+	  /* Note: some systems (SunOS 5.8) crash if bsearch base argument
+	     is NULL.  */
+	  found = (struct function_map *) bsearch
+	    (core_syms[i]->name, symbol_map, symbol_map_count,
+	     sizeof (struct function_map), search_mapped_symbol);
+	}
       if (found == NULL || found->is_first)
 	++symtab.len;
     }
@@ -643,9 +647,14 @@ core_create_function_syms (void)
 	  continue;
 	}
 
-      found = (struct function_map *) bsearch (core_syms[i]->name, symbol_map,
-                                               symbol_map_count,
-		       sizeof (struct function_map), search_mapped_symbol);
+      if (symbol_map_count != 0)
+	{
+	  /* Note: some systems (SunOS 5.8) crash if bsearch base argument
+	     is NULL.  */
+	  found = (struct function_map *) bsearch
+	    (core_syms[i]->name, symbol_map, symbol_map_count,
+	     sizeof (struct function_map), search_mapped_symbol);
+	}
       if (found && ! found->is_first)
 	continue;
 

Re: [Patch]: Fix crash in gprof (SunOS 5.8 only)

[nick clifton]

Hi Tristan,

> Ok for trunk ?

Approved - please apply.


> PS: should we drop tahoe ?

Not really apropos the email's topic, but I think that we could 
reasonably add tahoe to the list of obsolete targets and see if anyone 
complains.


> PPS: should we drop '-c' ?  It is supported by very few architectures (i386, alpha, tax, sparc, tahoe and mips) and looks broken at least on i386.

The -c option to gprof, I assume you mean ?  Hmm, not sure. Is it the 
case that the heuristic(s) involved just need updating, or is the 
concept itself flawed ?

Cheers
   Nick

Re: [Patch]: Fix crash in gprof (SunOS 5.8 only)

[Tristan Gingold]

On Feb 23, 2012, at 5:56 PM, nick clifton wrote:

> Hi Tristan,
> 
>> Ok for trunk ?
> 
> Approved - please apply.

Thanks, committed.

>> PS: should we drop tahoe ?
> 
> Not really apropos the email's topic, but I think that we could reasonably add tahoe to the list of obsolete targets and see if anyone complains.

Isn't it overly cautious ?  Tahoe was already removed from gas in 2005 by Alan:

2005-08-11  Alan Modra  <amodra@bigpond.net.au>
[…]
	* Makefile.am: Remove a29k, h8500, m88k, tahoe, tic80, w65,
	bout and hp300 support.

And I am not sure that BFD really supports it.

Anyway, do we already have a list of obsolete targets ?

>> PPS: should we drop '-c' ?  It is supported by very few architectures (i386, alpha, tax, sparc, tahoe and mips) and looks broken at least on i386.
> 
> The -c option to gprof, I assume you mean ?  Hmm, not sure. Is it the case that the heuristic(s) involved just need updating, or is the concept itself flawed ?

I have to dig more deeper.  At least the current implementation is somewhat flawed…

Tristan.