[PATCH] Include alloca.h for bfd

[PATCH] Include alloca.h for bfd

[Thiemo Seufer]

Hi All,

this checks for inclusion of alloca.h, which is required on IRIX6.5
to compile e.g. linker.c.


Thiemo


2001-08-21  Thiemo Seufer <seufer@csv.ica.uni-stuttgart.de>

	/bfd/ChangeLog
	* sysdep.h (HAVE_ALLOCA): Check for inclusion of alloca.h, needed
	on irix6.


diff -BurpNX /bigdisk/src/binutils-exclude src-orig/bfd/sysdep.h src/bfd/sysdep.h
--- src-orig/bfd/sysdep.h	Wed Mar 14 17:02:30 2001
+++ src/bfd/sysdep.h	Mon Aug 20 23:31:03 2001
@@ -26,6 +26,18 @@ Foundation, Inc., 59 Temple Place - Suit
 
 #include "config.h"
 
+#ifdef HAVE_ALLOCA
+#ifdef HAVE_ALLOCA_H
+#include <alloca.h>
+#else
+#ifdef NEED_DECLARATION_ALLOCA
+extern PTR alloca ();
+#endif
+#endif
+#else
+#error "alloca() is required but not available."
+#endif
+
 #ifdef HAVE_STDDEF_H
 #include <stddef.h>
 #endif

Re: [PATCH] Include alloca.h for bfd

[Ian Lance Taylor]

Thiemo Seufer <ica2_ts@csv.ica.uni-stuttgart.de> writes:

> this checks for inclusion of alloca.h, which is required on IRIX6.5
> to compile e.g. linker.c.

This patch is incorrect.  Any declaration of alloca or inclusions of
alloca.h must be done as in gas/as.h.

In the past I did not permit alloca to be used in BFD, because on
systems which do not have alloca the version in libiberty would call
xmalloc, which might cause the program to crash when it ran out of
memory.  I believe that BFD should never crash because it runs out of
memory; instead, it should return the appropriate error to the caller.
(That's why I wrote the objalloc interface when obstacks were changed
to no longer support a failure to allocate memory.)  However, I do not
know what the current thinking is on alloca in BFD.

Ian

Re: [PATCH] Include alloca.h for bfd

[Thiemo Seufer]

Ian Lance Taylor wrote:
> Thiemo Seufer <ica2_ts@csv.ica.uni-stuttgart.de> writes:
> 
> > this checks for inclusion of alloca.h, which is required on IRIX6.5
> > to compile e.g. linker.c.
> 
> This patch is incorrect.  Any declaration of alloca or inclusions of
> alloca.h must be done as in gas/as.h.

Uh, that's a really ugly piece of code.

> In the past I did not permit alloca to be used in BFD, because on
> systems which do not have alloca the version in libiberty would call
> xmalloc, which might cause the program to crash when it ran out of
> memory.  I believe that BFD should never crash because it runs out of
> memory; instead, it should return the appropriate error to the caller.
> (That's why I wrote the objalloc interface when obstacks were changed
> to no longer support a failure to allocate memory.)  However, I do not
> know what the current thinking is on alloca in BFD.

Well, alloca() is called in BFD:
bfd/elfxx-ia64.c:             char *once_name = alloca (len2 + strlen (sname) - len + 1);
bfd/elfxx-ia64.c:      addr_name = alloca (len);
bfd/som.c:  unsigned char *tmp_space = alloca (tmp_space_size);
bfd/som.c:            tmp_space = alloca (tmp_space_size);
bfd/som.c:  unsigned char *tmp_space = alloca (tmp_space_size);
bfd/som.c:                tmp_space = alloca (tmp_space_size);
bfd/som.c:            tmp_space = alloca (tmp_space_size);
bfd/elf64-hppa.c:             new_name = alloca (strlen (h->root.root.string) + 2);
bfd/elf64-hppa.c:         new_name = alloca (strlen (h->root.root.string) + 2);
bfd/vms-hdr.c:      fname = (char *) alloca (strlen (fptr) + 1);
bfd/linker.c:         char *buf = alloca (strlen (h->root.string) + 10);

The last one causes the build with native tools on IRIX6.5 to fail.
This patch lets it compile.


Thiemo


2001-08-21  Thiemo Seufer <seufer@csv.ica.uni-stuttgart.de>

	/bfd/ChangeLog
	* sysdep.h (HAVE_ALLOCA): Check for inclusion of alloca.h, needed
	on irix6.


diff -BurpNX /bigdisk/src/binutils-exclude src-orig/bfd/sysdep.h src/bfd/sysdep.h
--- src-orig/bfd/sysdep.h	Tue Aug 21 19:13:51 2001
+++ src/bfd/sysdep.h	Tue Aug 21 22:54:13 2001
@@ -26,6 +26,35 @@ Foundation, Inc., 59 Temple Place - Suit
 
 #include "config.h"
 
+/* This is the code recommended in the autoconf documentation, almost
+   verbatim.  If it doesn't work for you, let me know, and notify
+   djm@gnu.ai.mit.edu as well.  */
+/* Added void* version for STDC case.  This is to be compatible with
+   the declaration in bison.simple, used for m68k operand parsing.
+   --KR 1995.08.08 */
+/* Force void* decl for hpux.  This is what Bison uses.  --KR 1995.08.16 */
+
+#ifndef __GNUC__
+# if HAVE_ALLOCA_H
+#  include <alloca.h>
+# else
+#  ifdef _AIX
+/* Indented so that pre-ansi C compilers will ignore it, rather than
+   choke on it.  Some versions of AIX require this to be the first
+   thing in the file.  */
+ #pragma alloca
+#  else
+#   ifndef alloca /* predefined by HP cc +Olibcalls */
+#    if !defined (__STDC__) && !defined (__hpux)
+extern char *alloca ();
+#    else
+extern void *alloca ();
+#    endif /* __STDC__, __hpux */
+#   endif /* alloca */
+#  endif /* _AIX */
+# endif /* HAVE_ALLOCA_H */
+#endif /* __GNUC__ */
+
 #ifdef HAVE_STDDEF_H
 #include <stddef.h>
 #endif

Re: [PATCH] Include alloca.h for bfd

[Ian Lance Taylor]

Thiemo Seufer <ica2_ts@csv.ica.uni-stuttgart.de> writes:

> Ian Lance Taylor wrote:
> > Thiemo Seufer <ica2_ts@csv.ica.uni-stuttgart.de> writes:
> > 
> > > this checks for inclusion of alloca.h, which is required on IRIX6.5
> > > to compile e.g. linker.c.
> > 
> > This patch is incorrect.  Any declaration of alloca or inclusions of
> > alloca.h must be done as in gas/as.h.
> 
> Uh, that's a really ugly piece of code.

It's a really ugly problem.

Your suggested patch would have broken the build on a few systems
which have alloca.h when using gcc to compile.

> > In the past I did not permit alloca to be used in BFD, because on
> > systems which do not have alloca the version in libiberty would call
> > xmalloc, which might cause the program to crash when it ran out of
> > memory.  I believe that BFD should never crash because it runs out of
> > memory; instead, it should return the appropriate error to the caller.
> > (That's why I wrote the objalloc interface when obstacks were changed
> > to no longer support a failure to allocate memory.)  However, I do not
> > know what the current thinking is on alloca in BFD.
> 
> Well, alloca() is called in BFD:

Yes, I know.  Those were all added since I was the chief maintainer.

Ian

Re: [PATCH] Include alloca.h for bfd

[Nick Clifton]

Hi Thiemo, Hi Ian,

> In the past I did not permit alloca to be used in BFD, because on
> systems which do not have alloca the version in libiberty would call
> xmalloc, which might cause the program to crash when it ran out of
> memory.  I believe that BFD should never crash because it runs out of
> memory; instead, it should return the appropriate error to the caller.
> (That's why I wrote the objalloc interface when obstacks were changed
> to no longer support a failure to allocate memory.)  However, I do not
> know what the current thinking is on alloca in BFD.

I agree with your policy.  I was unaware of the problems involved with
using alloca, which is why I did not enforce the rule as you used to
do.  Now that I am aware of it I will try to enforce it in the future
and also remove the current uses of alloca.

Thiemo - rather than including alloca.h how about changing the code in
linker.c to use objalloc_alloc() instead ?

Cheers
        Nick

Re: [PATCH] Include alloca.h for bfd

[Thiemo Seufer]

Nick Clifton wrote:
> Hi Thiemo, Hi Ian,
> 
> > In the past I did not permit alloca to be used in BFD, because on
> > systems which do not have alloca the version in libiberty would call
> > xmalloc, which might cause the program to crash when it ran out of
> > memory.  I believe that BFD should never crash because it runs out of
> > memory; instead, it should return the appropriate error to the caller.
> > (That's why I wrote the objalloc interface when obstacks were changed
> > to no longer support a failure to allocate memory.)  However, I do not
> > know what the current thinking is on alloca in BFD.
> 
> I agree with your policy.  I was unaware of the problems involved with
> using alloca, which is why I did not enforce the rule as you used to
> do.  Now that I am aware of it I will try to enforce it in the future
> and also remove the current uses of alloca.
> 
> Thiemo - rather than including alloca.h how about changing the code in
> linker.c to use objalloc_alloc() instead ?

It's a simple string, so I used bfd_malloc.


Thiemo


2001-08-23  Thiemo Seufer <seufer@csv.ica.uni-stuttgart.de>

	/bfd/ChangeLog
	* linker.c (_bfd_generic_link_add_archive_symbols): Replace alloca()
	by bfd_malloc().


diff -BurpNX /bigdisk/src/binutils-exclude src-orig/bfd/linker.c src/bfd/linker.c
--- src-orig/bfd/linker.c	Sat Aug 18 21:47:22 2001
+++ src/bfd/linker.c	Thu Aug 23 17:14:45 2001
@@ -1007,9 +1007,10 @@ _bfd_generic_link_add_archive_symbols (a
 	     let's look for its import thunk */
 	  if (info->pei386_auto_import)
 	    {
-	      char *buf = alloca (strlen (h->root.string) + 10);
+	      char *buf = (char *) bfd_malloc (strlen (h->root.string) + 10);
 	      sprintf (buf, "__imp_%s", h->root.string);
 	      arh = archive_hash_lookup (&arsym_hash, buf, false, false);
+	      free(buf);
 	    }
 	  if (arh == (struct archive_hash_entry *) NULL)
 	    {

Re: [PATCH] Include alloca.h for bfd

[Jakub Jelinek]

On Thu, Aug 23, 2001 at 05:47:16PM +0200, Thiemo Seufer wrote:
> 2001-08-23  Thiemo Seufer <seufer@csv.ica.uni-stuttgart.de>
> 
> 	/bfd/ChangeLog
> 	* linker.c (_bfd_generic_link_add_archive_symbols): Replace alloca()
> 	by bfd_malloc().
> 
> 
> diff -BurpNX /bigdisk/src/binutils-exclude src-orig/bfd/linker.c src/bfd/linker.c
> --- src-orig/bfd/linker.c	Sat Aug 18 21:47:22 2001
> +++ src/bfd/linker.c	Thu Aug 23 17:14:45 2001
> @@ -1007,9 +1007,10 @@ _bfd_generic_link_add_archive_symbols (a
>  	     let's look for its import thunk */
>  	  if (info->pei386_auto_import)
>  	    {
> -	      char *buf = alloca (strlen (h->root.string) + 10);
> +	      char *buf = (char *) bfd_malloc (strlen (h->root.string) + 10);

You need to check return value - bfd_malloc can return NULL (but will set
bfd error).

	Jakub

Re: [PATCH] Include alloca.h for bfd

[Thiemo Seufer]

Jakub Jelinek wrote:
[snip]
> > -	      char *buf = alloca (strlen (h->root.string) + 10);
> > +	      char *buf = (char *) bfd_malloc (strlen (h->root.string) + 10);
> 
> You need to check return value - bfd_malloc can return NULL (but will set
> bfd error).

Argh. I _knew_ I've forgotten something.


2001-08-23  Thiemo Seufer <seufer@csv.ica.uni-stuttgart.de>

	/bfd/ChangeLog
	* linker.c (_bfd_generic_link_add_archive_symbols): Replace alloca()
	by bfd_malloc().


diff -BurpNX /bigdisk/src/binutils-exclude src-orig/bfd/linker.c src/bfd/linker.c
--- src-orig/bfd/linker.c	Sat Aug 18 21:47:22 2001
+++ src/bfd/linker.c	Thu Aug 23 18:09:21 2001
@@ -1007,9 +1007,13 @@ _bfd_generic_link_add_archive_symbols (a
 	     let's look for its import thunk */
 	  if (info->pei386_auto_import)
 	    {
-	      char *buf = alloca (strlen (h->root.string) + 10);
+	      char *buf = (char *) bfd_malloc (strlen (h->root.string) + 10);
+	      if (buf == NULL)
+		return false;
+
 	      sprintf (buf, "__imp_%s", h->root.string);
 	      arh = archive_hash_lookup (&arsym_hash, buf, false, false);
+	      free(buf);
 	    }
 	  if (arh == (struct archive_hash_entry *) NULL)
 	    {

Re: [PATCH] Include alloca.h for bfd

[Andrew Cagney]

> 
> Argh. I _knew_ I've forgotten something.
> 
> 
> 2001-08-23  Thiemo Seufer <seufer@csv.ica.uni-stuttgart.de>
> 
> /bfd/ChangeLog
> 	* linker.c (_bfd_generic_link_add_archive_symbols): Replace alloca()
> 	by bfd_malloc().
> 
> 
> diff -BurpNX /bigdisk/src/binutils-exclude src-orig/bfd/linker.c src/bfd/linker.c
> --- src-orig/bfd/linker.c	Sat Aug 18 21:47:22 2001
> +++ src/bfd/linker.c	Thu Aug 23 18:09:21 2001
> @@ -1007,9 +1007,13 @@ _bfd_generic_link_add_archive_symbols (a
>  	     let's look for its import thunk */
>  	  if (info->pei386_auto_import)
>  	    {
> -	      char *buf = alloca (strlen (h->root.string) + 10);
> +	      char *buf = (char *) bfd_malloc (strlen (h->root.string) + 10);
> +	      if (buf == NULL)
> +		return false;
> +
>  	      sprintf (buf, "__imp_%s", h->root.string);

Hmm, shame you can't use asprintf().  It does everything you want except 
use bfd_malloc().

	Andrew

Re: [PATCH] Include alloca.h for bfd

[Ian Lance Taylor]

Andrew Cagney <ac131313@cygnus.com> writes:

> >
> > Argh. I _knew_ I've forgotten something.
> > 2001-08-23  Thiemo Seufer <seufer@csv.ica.uni-stuttgart.de>
> > /bfd/ChangeLog
> > 	* linker.c (_bfd_generic_link_add_archive_symbols): Replace alloca()
> > 	by bfd_malloc().
> > diff -BurpNX /bigdisk/src/binutils-exclude src-orig/bfd/linker.c
> > src/bfd/linker.c
> > --- src-orig/bfd/linker.c	Sat Aug 18 21:47:22 2001
> > +++ src/bfd/linker.c	Thu Aug 23 18:09:21 2001
> > @@ -1007,9 +1007,13 @@ _bfd_generic_link_add_archive_symbols (a
> >  	     let's look for its import thunk */
> >  	  if (info->pei386_auto_import)
> >  	    {
> > -	      char *buf = alloca (strlen (h->root.string) + 10);
> > +	      char *buf = (char *) bfd_malloc (strlen (h->root.string) + 10);
> > +	      if (buf == NULL)
> > +		return false;
> > +
> >  	      sprintf (buf, "__imp_%s", h->root.string);
> 
> Hmm, shame you can't use asprintf().  It does everything you want
> except use bfd_malloc().

Now that you mention it, we could use asprintf there.  There is an
implementation in libiberty.  If it can't allocate memory, it sets the
buffer to NULL.

Doesn't matter much, though.

Ian

Re: [PATCH] Include alloca.h for bfd

[Andrew Cagney]

> Hmm, shame you can't use asprintf().  It does everything you want
>> except use bfd_malloc().
> 
> 
> Now that you mention it, we could use asprintf there.  There is an
> implementation in libiberty.  If it can't allocate memory, it sets the
> buffer to NULL.
> 
> Doesn't matter much, though.

Yes.  Its the bit where bfd_malloc() sets the error message that is missing.

	Andrew

Re: [PATCH] Include alloca.h for bfd

[Jakub Jelinek]

On Thu, Aug 23, 2001 at 02:42:47PM -0400, Andrew Cagney wrote:
> > Hmm, shame you can't use asprintf().  It does everything you want
> >> except use bfd_malloc().
> > 
> > 
> > Now that you mention it, we could use asprintf there.  There is an
> > implementation in libiberty.  If it can't allocate memory, it sets the
> > buffer to NULL.
> > 
> > Doesn't matter much, though.
> 
> Yes.  Its the bit where bfd_malloc() sets the error message that is missing.

Guess
	      size_t len = strlen (h->root.string);
	      char *buf = (char *) bfd_malloc (len + sizeof ("__imp_"));
	      memcpy (buf, "__imp_", sizeof ("__imp_") - 1);
	      memcpy (buf + sizeof ("__imp_") - 1, h->root.string, len + 1);
              arh = archive_hash_lookup (&arsym_hash, buf, false, false);
	      free(buf);

will be faster anyway, so there is no need to look for asprintf for concat
of 2 strings together (well, the above would be even nicer with mempcpy).

	Jakub

Re: [PATCH] Include alloca.h for bfd

[Nick Clifton]

Hi Thiemo,

> Argh. I _knew_ I've forgotten something.

:-)

 
> 2001-08-23  Thiemo Seufer <seufer@csv.ica.uni-stuttgart.de>
> 
> 	/bfd/ChangeLog
> 	* linker.c (_bfd_generic_link_add_archive_symbols): Replace alloca()
> 	by bfd_malloc().

Approved.

Cheers
        Nick