Re: bug in gdb/target.c:target_signal_to_name

Re: bug in gdb/target.c:target_signal_to_name

[Andrew Cagney]

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

> When remote target returns some invalid signal, gdb vill crash with
> segfault. The problem seems to be in function target_signal_to_name,
> which doesn't check, if signal is in bounds and returns invalid name.
> 
> This version will (at least) not segfault:
> 
> /* Return the name for a signal.  */
> char *
> target_signal_to_name (sig)
>      enum target_signal sig;
> {
>   if (sig == TARGET_SIGNAL_UNKNOWN)
>     /* I think the code which prints this will always print it along with
>        the string, so no need to be verbose.  */
>     return "?";
>   if ((sig >= TARGET_SIGNAL_FIRST) && (sig <= TARGET_SIGNAL_LAST))
>     return signals[sig].name;
>   else
>     return signals[TARGET_SIGNAL_UNKNOWN].name;
> }


Thanks.  I've committed the attached.

	Andrew




[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 1091 bytes --]

2002-01-13  Andrew Cagney  <ac131313@redhat.com>

	From Petr Ledvina <ledvinap@kae.zcu.cz>:
	* signals.c (target_signal_to_name): Verify that SIG is within the
	bounds of the signals array.

Index: signals.c
===================================================================
RCS file: /cvs/src/src/gdb/signals.c,v
retrieving revision 1.1
diff -p -r1.1 signals.c
*** signals.c	2001/07/19 18:09:11	1.1
--- signals.c	2002/01/13 21:04:30
*************** target_signal_to_name (enum target_signa
*** 214,220 ****
      /* I think the code which prints this will always print it along with
         the string, so no need to be verbose.  */
      return "?";
!   return signals[sig].name;
  }
  
  /* Given a name, return its signal.  */
--- 214,223 ----
      /* I think the code which prints this will always print it along with
         the string, so no need to be verbose.  */
      return "?";
!   else if ((sig >= TARGET_SIGNAL_FIRST) && (sig <= TARGET_SIGNAL_LAST))
!     return signals[sig].name;
!   else
!     return signals[sig].name;
  }
  
  /* Given a name, return its signal.  */

Re: bug in gdb/target.c:target_signal_to_name

[Daniel Jacobowitz]

On Sun, Jan 13, 2002 at 04:10:05PM -0500, Andrew Cagney wrote:
> --- 214,223 ----
>       /* I think the code which prints this will always print it along with
>          the string, so no need to be verbose.  */
>       return "?";
> !   else if ((sig >= TARGET_SIGNAL_FIRST) && (sig <= TARGET_SIGNAL_LAST))
> !     return signals[sig].name;
> !   else
> !     return signals[sig].name;
>   }
>   
>   /* Given a name, return its signal.  */

That's probably not what you meant to commit, since both cases are the
same.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: bug in gdb/target.c:target_signal_to_name

[Andrew Cagney]

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

> On Sun, Jan 13, 2002 at 04:10:05PM -0500, Andrew Cagney wrote:
> 
>> --- 214,223 ----
>> /* I think the code which prints this will always print it along with
>> the string, so no need to be verbose.  */
>> return "?";
>> !   else if ((sig >= TARGET_SIGNAL_FIRST) && (sig <= TARGET_SIGNAL_LAST))
>> !     return signals[sig].name;
>> !   else
>> !     return signals[sig].name;
>> }
>> > /* Given a name, return its signal.  */
> 
> 
> That's probably not what you meant to commit, since both cases are the
> same.


Er, no. Lets try the attached.  Turns out that 
signals[TARGET_SIGNAL_UNKNOWN].name is NULL.

Andrew



[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 1352 bytes --]

2002-01-13  Andrew Cagney  <ac131313@redhat.com>

	* signals.c (target_signal_to_name): Rewrite.  Only use
	signals[].name when in bounds and non-NULL.
	
Index: signals.c
===================================================================
RCS file: /cvs/src/src/gdb/signals.c,v
retrieving revision 1.2
diff -p -r1.2 signals.c
*** signals.c	2002/01/13 21:11:38	1.2
--- signals.c	2002/01/13 21:51:26
*************** target_signal_to_string (enum target_sig
*** 210,223 ****
  char *
  target_signal_to_name (enum target_signal sig)
  {
!   if (sig == TARGET_SIGNAL_UNKNOWN)
!     /* I think the code which prints this will always print it along with
!        the string, so no need to be verbose.  */
!     return "?";
!   else if ((sig >= TARGET_SIGNAL_FIRST) && (sig <= TARGET_SIGNAL_LAST))
      return signals[sig].name;
    else
!     return signals[sig].name;
  }
  
  /* Given a name, return its signal.  */
--- 210,222 ----
  char *
  target_signal_to_name (enum target_signal sig)
  {
!   if ((sig >= TARGET_SIGNAL_FIRST) && (sig <= TARGET_SIGNAL_LAST)
!       && signals[sig].name != NULL)
      return signals[sig].name;
    else
!     /* I think the code which prints this will always print it along
!        with the string, so no need to be verbose (very old comment).  */
!     return "?";
  }
  
  /* Given a name, return its signal.  */