[PATCH] linker.c, check for null return from bfd_hash_allocate

[PATCH] linker.c, check for null return from bfd_hash_allocate

[msnyder]

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

Most callers of bfd_hash_allocate check for null returns.


[-- Attachment #2: linker.txt --]
[-- Type: text/plain, Size: 1154 bytes --]

2007-07-25  Michael Snyder  <msnyder@access-company.com>

	* linker.c (bfd_section_already_linked_table_insert): Check for
	NULL return from bfd_hash_allocate.
	(already_linked_newfunc): Ditto.

Index: linker.c
===================================================================
RCS file: /cvs/src/src/bfd/linker.c,v
retrieving revision 1.60
diff -p -r1.60 linker.c
*** linker.c	24 Jul 2007 23:38:13 -0000	1.60
--- linker.c	26 Jul 2007 01:55:13 -0000
*************** bfd_section_already_linked_table_insert
*** 2933,2938 ****
--- 2933,2940 ----
    /* Allocate the memory from the same obstack as the hash table is
       kept in.  */
    l = bfd_hash_allocate (&_bfd_section_already_linked_table, sizeof *l);
+   if (l == NULL)
+     return;		/* Should we abort?  */
    l->sec = sec;
    l->next = already_linked_list->entry;
    already_linked_list->entry = l;
*************** already_linked_newfunc (struct bfd_hash_
*** 2946,2951 ****
--- 2948,2956 ----
    struct bfd_section_already_linked_hash_entry *ret =
      bfd_hash_allocate (table, sizeof *ret);
  
+   if (ret == NULL)
+     return ret;
+ 
    ret->entry = NULL;
  
    return &ret->root;

Re: [PATCH] linker.c, check for null return from bfd_hash_allocate

[Alan Modra]

On Wed, Jul 25, 2007 at 06:59:54PM -0700, msnyder@sonic.net wrote:
> *** linker.c	24 Jul 2007 23:38:13 -0000	1.60
> --- linker.c	26 Jul 2007 01:55:13 -0000
> *************** bfd_section_already_linked_table_insert
> *** 2933,2938 ****
> --- 2933,2940 ----
>     /* Allocate the memory from the same obstack as the hash table is
>        kept in.  */
>     l = bfd_hash_allocate (&_bfd_section_already_linked_table, sizeof *l);
> +   if (l == NULL)
> +     return;		/* Should we abort?  */

The proper fix is to change bfd_section_already_linked_table_insert to
return a pass/fail status.  Check the status in
_bfd_elf_section_already_linked, _bfd_generic_section_already_linked,
and on failure call info->einfo with a fatal error message.

>     l->sec = sec;
>     l->next = already_linked_list->entry;
>     already_linked_list->entry = l;
> *************** already_linked_newfunc (struct bfd_hash_
> *** 2946,2951 ****
> --- 2948,2956 ----
>     struct bfd_section_already_linked_hash_entry *ret =
>       bfd_hash_allocate (table, sizeof *ret);
>   
> +   if (ret == NULL)
> +     return ret;
> + 
>     ret->entry = NULL;
>   
>     return &ret->root;

This part is OK.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] linker.c, check for null return from bfd_hash_allocate

[msnyder]

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


> The proper fix is to change bfd_section_already_linked_table_insert to
> return a pass/fail status.  Check the status in
> _bfd_elf_section_already_linked, _bfd_generic_section_already_linked,
> and on failure call info->einfo with a fatal error message.

OK.  How about the attached?

> This part is OK.

Committed, thanks.

[-- Attachment #2: fatal.txt --]
[-- Type: text/plain, Size: 6032 bytes --]

2007-07-26  Michael Snyder  <msnyder@svkmacdonelllnx>

	* linker.c (bfd_section_already_linked_table_insert): Change 
	return type from void to boolean.  Return FALSE on failure.
	(_bfd_generic_section_already_linked): Test return value of
	bfd_section_already_linked_table_insert, call fatal on error.

	* elflink.c (_bfd_elf_section_already_linked): Test return value
	of bfd_section_already_linked_table_insert, call fatal on error.

	* libbfd-in.h (bfd_section_already_linked_table_insert): Update
	return type to bfd_boolean.

	* libbfd.h: Regenerate.

Index: linker.c
===================================================================
RCS file: /cvs/src/src/bfd/linker.c,v
retrieving revision 1.62
diff -p -r1.62 linker.c
*** linker.c	26 Jul 2007 21:58:44 -0000	1.62
--- linker.c	26 Jul 2007 22:15:58 -0000
*************** bfd_section_already_linked_table_lookup 
*** 2923,2929 ****
  			   TRUE, FALSE));
  }
  
! void
  bfd_section_already_linked_table_insert
    (struct bfd_section_already_linked_hash_entry *already_linked_list,
     asection *sec)
--- 2923,2929 ----
  			   TRUE, FALSE));
  }
  
! bfd_boolean
  bfd_section_already_linked_table_insert
    (struct bfd_section_already_linked_hash_entry *already_linked_list,
     asection *sec)
*************** bfd_section_already_linked_table_insert
*** 2933,2941 ****
--- 2933,2944 ----
    /* Allocate the memory from the same obstack as the hash table is
       kept in.  */
    l = bfd_hash_allocate (&_bfd_section_already_linked_table, sizeof *l);
+   if (l == NULL)
+     return FALSE;
    l->sec = sec;
    l->next = already_linked_list->entry;
    already_linked_list->entry = l;
+   return TRUE;
  }
  
  static struct bfd_hash_entry *
*************** already_linked_newfunc (struct bfd_hash_
*** 2947,2953 ****
      bfd_hash_allocate (table, sizeof *ret);
  
    if (ret == NULL)
!     return ret;
  
    ret->entry = NULL;
  
--- 2950,2956 ----
      bfd_hash_allocate (table, sizeof *ret);
  
    if (ret == NULL)
!     return NULL;
  
    ret->entry = NULL;
  
*************** bfd_section_already_linked_table_free (v
*** 2973,2979 ****
  
  void
  _bfd_generic_section_already_linked (bfd *abfd, asection *sec,
! 				     struct bfd_link_info *info ATTRIBUTE_UNUSED)
  {
    flagword flags;
    const char *name;
--- 2976,2982 ----
  
  void
  _bfd_generic_section_already_linked (bfd *abfd, asection *sec,
! 				     struct bfd_link_info *info)
  {
    flagword flags;
    const char *name;
*************** _bfd_generic_section_already_linked (bfd
*** 3074,3080 ****
      }
  
    /* This is the first section with this name.  Record it.  */
!   bfd_section_already_linked_table_insert (already_linked_list, sec);
  }
  
  /* Convert symbols in excluded output sections to use a kept section.  */
--- 3077,3084 ----
      }
  
    /* This is the first section with this name.  Record it.  */
!   if (! bfd_section_already_linked_table_insert (already_linked_list, sec))
!     info->callbacks->einfo (_("%F%P: internal error at %D"));
  }
  
  /* Convert symbols in excluded output sections to use a kept section.  */
Index: elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.273
diff -p -r1.273 elflink.c
*** elflink.c	26 Jul 2007 13:45:59 -0000	1.273
--- elflink.c	26 Jul 2007 22:15:59 -0000
*************** _bfd_elf_section_already_linked (bfd *ab
*** 12235,12241 ****
  	}
  
    /* This is the first section with this name.  Record it.  */
!   bfd_section_already_linked_table_insert (already_linked_list, sec);
  }
  
  bfd_boolean
--- 12235,12242 ----
  	}
  
    /* This is the first section with this name.  Record it.  */
!   if (! bfd_section_already_linked_table_insert (already_linked_list, sec))
!     info->callbacks->einfo (_("%F%P: internal error at %D"));
  }
  
  bfd_boolean
Index: libbfd-in.h
===================================================================
RCS file: /cvs/src/src/bfd/libbfd-in.h,v
retrieving revision 1.71
diff -p -r1.71 libbfd-in.h
*** libbfd-in.h	3 Jul 2007 14:26:42 -0000	1.71
--- libbfd-in.h	26 Jul 2007 22:15:59 -0000
*************** struct bfd_section_already_linked
*** 726,732 ****
  
  extern struct bfd_section_already_linked_hash_entry *
    bfd_section_already_linked_table_lookup (const char *);
! extern void bfd_section_already_linked_table_insert
    (struct bfd_section_already_linked_hash_entry *, asection *);
  extern void bfd_section_already_linked_table_traverse
    (bfd_boolean (*) (struct bfd_section_already_linked_hash_entry *,
--- 726,732 ----
  
  extern struct bfd_section_already_linked_hash_entry *
    bfd_section_already_linked_table_lookup (const char *);
! extern bfd_boolean bfd_section_already_linked_table_insert
    (struct bfd_section_already_linked_hash_entry *, asection *);
  extern void bfd_section_already_linked_table_traverse
    (bfd_boolean (*) (struct bfd_section_already_linked_hash_entry *,
Index: libbfd.h
===================================================================
RCS file: /cvs/src/src/bfd/libbfd.h,v
retrieving revision 1.195
diff -p -r1.195 libbfd.h
*** libbfd.h	3 Jul 2007 14:26:42 -0000	1.195
--- libbfd.h	26 Jul 2007 22:15:59 -0000
*************** struct bfd_section_already_linked
*** 731,737 ****
  
  extern struct bfd_section_already_linked_hash_entry *
    bfd_section_already_linked_table_lookup (const char *);
! extern void bfd_section_already_linked_table_insert
    (struct bfd_section_already_linked_hash_entry *, asection *);
  extern void bfd_section_already_linked_table_traverse
    (bfd_boolean (*) (struct bfd_section_already_linked_hash_entry *,
--- 731,737 ----
  
  extern struct bfd_section_already_linked_hash_entry *
    bfd_section_already_linked_table_lookup (const char *);
! extern bfd_boolean bfd_section_already_linked_table_insert
    (struct bfd_section_already_linked_hash_entry *, asection *);
  extern void bfd_section_already_linked_table_traverse
    (bfd_boolean (*) (struct bfd_section_already_linked_hash_entry *,

Re: [PATCH] linker.c, check for null return from bfd_hash_allocate

[Alan Modra]

On Thu, Jul 26, 2007 at 03:16:45PM -0700, msnyder@sonic.net wrote:
> 2007-07-26  Michael Snyder  <msnyder@svkmacdonelllnx>
> 
> 	* linker.c (bfd_section_already_linked_table_insert): Change 
> 	return type from void to boolean.  Return FALSE on failure.
> 	(_bfd_generic_section_already_linked): Test return value of
> 	bfd_section_already_linked_table_insert, call fatal on error.
> 
> 	* elflink.c (_bfd_elf_section_already_linked): Test return value
> 	of bfd_section_already_linked_table_insert, call fatal on error.
> 
> 	* libbfd-in.h (bfd_section_already_linked_table_insert): Update
> 	return type to bfd_boolean.
> 
> 	* libbfd.h: Regenerate.

This is OK,

> !   if (! bfd_section_already_linked_table_insert (already_linked_list, sec))
> !     info->callbacks->einfo (_("%F%P: internal error at %D"));

except that it isn't correct to call this an "internal error".  (It's
just out of memory.)  Use "%F%P: already_linked_table: %E" instead.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] linker.c, check for null return from bfd_hash_allocate

[msnyder]

> On Thu, Jul 26, 2007 at 03:16:45PM -0700, msnyder@sonic.net wrote:
>> 2007-07-26  Michael Snyder  <msnyder@svkmacdonelllnx>
>>
>> 	* linker.c (bfd_section_already_linked_table_insert): Change
>> 	return type from void to boolean.  Return FALSE on failure.
>> 	(_bfd_generic_section_already_linked): Test return value of
>> 	bfd_section_already_linked_table_insert, call fatal on error.
>>
>> 	* elflink.c (_bfd_elf_section_already_linked): Test return value
>> 	of bfd_section_already_linked_table_insert, call fatal on error.
>>
>> 	* libbfd-in.h (bfd_section_already_linked_table_insert): Update
>> 	return type to bfd_boolean.
>>
>> 	* libbfd.h: Regenerate.
>
> This is OK,
>
>> !   if (! bfd_section_already_linked_table_insert (already_linked_list,
>> sec))
>> !     info->callbacks->einfo (_("%F%P: internal error at %D"));
>
> except that it isn't correct to call this an "internal error".  (It's
> just out of memory.)  Use "%F%P: already_linked_table: %E" instead.

Done and committed.
Thanks.