[PATCH] null pointer guard, target-descriptions.c

[PATCH] null pointer guard, target-descriptions.c

[msnyder]

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

Possible null pointer ref, flagged by Coverity


[-- Attachment #2: tdesc --]
[-- Type: application/octet-stream, Size: 3207 bytes --]

2007-06-28  Michael Snyder  <msnyder@access-company.com>

	* target-descriptions.c (tdesc-_named_type): Guard against null
	type-id argument which may be passed by tdesc_create_reg (Coverity).

Index: target-descriptions.c
===================================================================
RCS file: /cvs/src/src/gdb/target-descriptions.c,v
retrieving revision 1.9
diff -p -r1.9 target-descriptions.c
*** target-descriptions.c	13 Jun 2007 18:26:59 -0000	1.9
--- target-descriptions.c	28 Jun 2007 20:57:07 -0000
*************** tdesc_named_type (const struct tdesc_fea
*** 350,396 ****
    int ix;
    struct type *gdb_type;
  
!   /* First try target-defined types.  */
!   for (ix = 0; VEC_iterate (type_p, feature->types, ix, gdb_type); ix++)
!     if (strcmp (TYPE_NAME (gdb_type), id) == 0)
!       return gdb_type;
! 
!   /* Next try some predefined types.  Note that none of these types
!      depend on the current architecture; some of the builtin_type_foo
!      variables are swapped based on the architecture.  */
!   if (strcmp (id, "int8") == 0)
!     return builtin_type_int8;
  
!   if (strcmp (id, "int16") == 0)
!     return builtin_type_int16;
  
!   if (strcmp (id, "int32") == 0)
!     return builtin_type_int32;
  
!   if (strcmp (id, "int64") == 0)
!     return builtin_type_int64;
  
!   if (strcmp (id, "uint8") == 0)
!     return builtin_type_uint8;
  
!   if (strcmp (id, "uint16") == 0)
!     return builtin_type_uint16;
  
!   if (strcmp (id, "uint32") == 0)
!     return builtin_type_uint32;
  
!   if (strcmp (id, "uint64") == 0)
!     return builtin_type_uint64;
  
!   if (strcmp (id, "ieee_single") == 0)
!     return builtin_type_ieee_single;
  
!   if (strcmp (id, "ieee_double") == 0)
!     return builtin_type_ieee_double;
  
!   if (strcmp (id, "arm_fpa_ext") == 0)
!     return builtin_type_arm_ext;
  
    return NULL;
  }
  \f
--- 350,399 ----
    int ix;
    struct type *gdb_type;
  
!   if (id)
!     {
!       /* First try target-defined types.  */
!       for (ix = 0; VEC_iterate (type_p, feature->types, ix, gdb_type); ix++)
! 	if (strcmp (TYPE_NAME (gdb_type), id) == 0)
! 	  return gdb_type;
  
!       /* Next try some predefined types.  Note that none of these
! 	 types depend on the current architecture; some of the
! 	 builtin_type_foo variables are swapped based on the
! 	 architecture.  */
!       if (strcmp (id, "int8") == 0)
! 	return builtin_type_int8;
  
!       if (strcmp (id, "int16") == 0)
! 	return builtin_type_int16;
  
!       if (strcmp (id, "int32") == 0)
! 	return builtin_type_int32;
  
!       if (strcmp (id, "int64") == 0)
! 	return builtin_type_int64;
  
!       if (strcmp (id, "uint8") == 0)
! 	return builtin_type_uint8;
  
!       if (strcmp (id, "uint16") == 0)
! 	return builtin_type_uint16;
  
!       if (strcmp (id, "uint32") == 0)
! 	return builtin_type_uint32;
  
!       if (strcmp (id, "uint64") == 0)
! 	return builtin_type_uint64;
  
!       if (strcmp (id, "ieee_single") == 0)
! 	return builtin_type_ieee_single;
  
!       if (strcmp (id, "ieee_double") == 0)
! 	return builtin_type_ieee_double;
  
+       if (strcmp (id, "arm_fpa_ext") == 0)
+ 	return builtin_type_arm_ext;
+     }
    return NULL;
  }
  \f

Re: [PATCH] null pointer guard, target-descriptions.c

[Daniel Jacobowitz]

On Thu, Jun 28, 2007 at 02:01:03PM -0700, Michael Snyder wrote:
> Possible null pointer ref, flagged by Coverity
> 

> 2007-06-28  Michael Snyder  <msnyder@access-company.com>
> 
> 	* target-descriptions.c (tdesc-_named_type): Guard against null
> 	type-id argument which may be passed by tdesc_create_reg (Coverity).

Whoops, yeah - this one's my fault.  There's another place in the same
file which will crash if reg->type == NULL.  How about we never set it
to NULL instead?  Like below.

-- 
Daniel Jacobowitz
CodeSourcery

2007-07-01  Daniel Jacobowitz  <dan@codesourcery.com>

	* target-descriptions.c (tdesc_create_reg): Do not set reg->type
	to NULL.

Index: target-descriptions.c
===================================================================
RCS file: /cvs/src/src/gdb/target-descriptions.c,v
retrieving revision 1.9
diff -u -p -r1.9 target-descriptions.c
--- target-descriptions.c	13 Jun 2007 18:26:59 -0000	1.9
+++ target-descriptions.c	1 Jul 2007 15:59:10 -0000
@@ -792,7 +792,7 @@ tdesc_create_reg (struct tdesc_feature *
   reg->save_restore = save_restore;
   reg->group = group ? xstrdup (group) : NULL;
   reg->bitsize = bitsize;
-  reg->type = type ? xstrdup (type) : NULL;
+  reg->type = type ? xstrdup (type) : xstrdup ("<unknown>");
 
   /* If the register's type is target-defined, look it up now.  We may not
      have easy access to the containing feature when we want it later.  */

Re: [PATCH] null pointer guard, target-descriptions.c

[msnyder]

> On Thu, Jun 28, 2007 at 02:01:03PM -0700, Michael Snyder wrote:
>> Possible null pointer ref, flagged by Coverity
>>
>
>> 2007-06-28  Michael Snyder  <msnyder@access-company.com>
>>
>> 	* target-descriptions.c (tdesc-_named_type): Guard against null
>> 	type-id argument which may be passed by tdesc_create_reg (Coverity).
>
> Whoops, yeah - this one's my fault.  There's another place in the same
> file which will crash if reg->type == NULL.  How about we never set it
> to NULL instead?  Like below.

That certainly solves the issue for now -- since it's only called
from one place -- but if we just went ahead and checked it for null,
we wouldn't have to worry about it coming up again.

But it's your code, so your choice...  if you prefer your patch,
consider this approval.  ;-)

Re: [PATCH] null pointer guard, target-descriptions.c

[Daniel Jacobowitz]

On Mon, Jul 02, 2007 at 05:40:12PM -0700, Michael Snyder wrote:
> But it's your code, so your choice...  if you prefer your patch,
> consider this approval.  ;-)

Thanks, I committed it.

-- 
Daniel Jacobowitz
CodeSourcery