A patch for elf.c

A patch for elf.c

[H.J. Lu]

> 
> 
> On Mon, 7 Dec 1998, H.J. Lu wrote:
> 
> > Could you please tell me on which platform you had this problem?
> 
> Alpha running Debian Linux.

Thanks for this information. BTW, have you mentioned this in your first
bug report? I think this patch fixes the bug. 

Ian, this bug happens on ELF/Alpha when you strip more than one shared
libraries at a time. It is not the first bfd_zalloc/bfd_alloc bug in
bfd. It is quite annoying.


-- 
H.J. Lu (hjl@gnu.org)
----
Tue Dec  8 07:51:09 1998  H.J. Lu  (hjl@gnu.org)

	* elf.c (copy_private_bfd_data): Use bfd_zalloc instead of
	bfd_alloc.

Index: elf.c
===================================================================
RCS file: /home/work/cvs/gnu/binutils/bfd/elf.c,v
retrieving revision 1.40
diff -u -p -r1.40 elf.c
--- elf.c	1998/12/05 03:38:53	1.40
+++ elf.c	1998/12/08 17:48:01
@@ -3317,7 +3317,7 @@ copy_private_bfd_data (ibfd, obfd)
       /* Allocate a segment map big enough to contain all of the
 	 sections we have selected.  */
       m = ((struct elf_segment_map *)
-	   bfd_alloc (obfd,
+	   bfd_zalloc (obfd,
 		      (sizeof (struct elf_segment_map)
 		       + ((size_t) csecs - 1) * sizeof (asection *))));
       if (m == NULL)

Re: A patch for elf.c

[Christopher C Chimelis]

On Tue, 8 Dec 1998, H.J. Lu wrote:

> Thanks for this information. BTW, have you mentioned this in your first
> bug report? I think this patch fixes the bug. 

Unsure, but I think I mentioned it (sorry if I didn't).  Thanks for your
attention on this, btw.

I'll test the patch tonight and get back to you tomorrow on it (my 533 MHz
Alpha is offline right now so it'll have to build on the Multia).

Thanks again...
C

Re: A patch for elf.c

[Ian Lance Taylor]

   From: hjl@lucon.org (H.J. Lu)
   Date: Tue, 8 Dec 1998 09:57:19 -0800 (PST)

   > On Mon, 7 Dec 1998, H.J. Lu wrote:
   > 
   > > Could you please tell me on which platform you had this problem?
   > 
   > Alpha running Debian Linux.

   Thanks for this information. BTW, have you mentioned this in your first
   bug report? I think this patch fixes the bug. 

   Ian, this bug happens on ELF/Alpha when you strip more than one shared
   libraries at a time. It is not the first bfd_zalloc/bfd_alloc bug in
   bfd. It is quite annoying.

This bug was reported on bug-gnu-utils.  I believe I have already
fixed this problem with the appended patch.

We've disagreed before on whether memory should be cleared when it is
allocated, and I think we will continue to disagree.  This was a bug
introduced with a change to the function a few months ago.  The fix is
to avoid introducing bugs; clearing memory is just treating the
symptom.

Ian

Index: elf.c
===================================================================
RCS file: /cvs/cvsfiles/devo/bfd/elf.c,v
retrieving revision 1.222
retrieving revision 1.223
diff -u -r1.222 -r1.223
--- elf.c	1998/11/18 04:26:38	1.222
+++ elf.c	1998/12/07 00:33:14	1.223
@@ -3444,6 +3444,8 @@
       m->includes_filehdr = (p->p_offset == 0
 			     && p->p_filesz >= iehdr->e_ehsize);
 
+      m->includes_phdrs = 0;
+
       if (! phdr_included || p->p_type != PT_LOAD)
 	{
 	  m->includes_phdrs =

Re: A patch for elf.c

[Christopher C Chimelis]

On Tue, 8 Dec 1998, Ian Lance Taylor wrote:

> This bug was reported on bug-gnu-utils.  I believe I have already
> fixed this problem with the appended patch.
> 
> We've disagreed before on whether memory should be cleared when it is
> allocated, and I think we will continue to disagree.  This was a bug
> introduced with a change to the function a few months ago.  The fix is
> to avoid introducing bugs; clearing memory is just treating the
> symptom.

I'll try both in turn and see.  If Ian's patch is effective, then I'll
stick to it.  Thanks to both of you for this :-)

C

Re: A patch for elf.c

[H.J. Lu]

> 
> We've disagreed before on whether memory should be cleared when it is
> allocated, and I think we will continue to disagree.  This was a bug

We can agree on that :-).

> introduced with a change to the function a few months ago.  The fix is
> to avoid introducing bugs; clearing memory is just treating the
> symptom.
> 

To me, clearing memory is the super set of setting one field to
zero. I am a better safe than sorry person. I have no time
to second guess if there is another field I should clear.


-- 
H.J. Lu (hjl@gnu.org)

Re: A patch for elf.c

[Roland McGrath]

> To me, clearing memory is the super set of setting one field to
> zero. I am a better safe than sorry person. I have no time
> to second guess if there is another field I should clear.

It is indeed the superset of setting one field to zero.  The question is
whether, generically speaking, setting a field to zero is the appropriate
way to initialize it.  It is not appropriate in all cases, so clearing
memory is not necessarily a superset of appropriately initializing each field.

If I may presume to explain another's position, I think Ian (like me), also
considers himself a "better safe than sorry" person.  For us, that means it
is better to be safe by individually addressing each case with a human eye
and being sure that case is handled properly, than to be sorry by presuming
that some nonspecific blanket solution like zeroing everything will be the
appropriate way to initialize each and every field.

(What I prefer to do myself is use a debugging version of malloc that fills
newly-allocated space with a distinctive nonzero bit pattern, e.g. 0xa5a5a5a5,
that makes it unlikely that the program could happen to work if a field is
left uninitialized.)

Re: A patch for elf.c

[Ian Lance Taylor]

   From: hjl@lucon.org (H.J. Lu)
   Date: Wed, 9 Dec 1998 07:42:46 -0800 (PST)

   > introduced with a change to the function a few months ago.  The fix is
   > to avoid introducing bugs; clearing memory is just treating the
   > symptom.

   To me, clearing memory is the super set of setting one field to
   zero. I am a better safe than sorry person. I have no time
   to second guess if there is another field I should clear.

I know you don't mean this, but to me ``I have no time to second
guess'' sounds like ``I have no time to figure out how the code works,
or what the right patch is.''  In a long running project like GNU,
anybody who wants to make changes has to take the time to understand
them.  We're not quickly hacking stuff together for the big demo next
Wednesday; we're writing code that should, ideally, survive for twenty
years.

I also agree with Roland's point.

Ian

Re: A patch for elf.c

[Jeffrey A Law]

  In message < 199812091735.MAA29788@subrogation.cygnus.com >you write:
  >    > introduced with a change to the function a few months ago.  The fix is
  >    > to avoid introducing bugs; clearing memory is just treating the
  >    > symptom.
  > 
  >    To me, clearing memory is the super set of setting one field to
  >    zero. I am a better safe than sorry person. I have no time
  >    to second guess if there is another field I should clear.
  > 
  > I know you don't mean this, but to me ``I have no time to second
  > guess'' sounds like ``I have no time to figure out how the code works,
  > or what the right patch is.''  In a long running project like GNU,
  > anybody who wants to make changes has to take the time to understand
  > them.  We're not quickly hacking stuff together for the big demo next
  > Wednesday; we're writing code that should, ideally, survive for twenty
  > years.
  > 
  > I also agree with Roland's point.
IMHO, clearing a block of memory to fix a problem without understanding
precisely why clearing the block fixes the problem is just papering over a
bug.

Once one understands precisely why clearing one or more memory locations is
fixing a bug, once can decide if clearing one or more fields or a whole
structure/array is the best solution.

I believe gdb operates under a similar guidelines.


jeff

Re: A patch for elf.c

[Stan Shebs]

law@hurl.cygnus.com (Jeffrey A Law) writes:

> Once one understands precisely why clearing one or more memory locations is
> fixing a bug, once can decide if clearing one or more fields or a whole
> structure/array is the best solution.
> 
> I believe gdb operates under a similar guidelines.

Yes.  There is actually a real reason to not just automatically zero
everything - in some cases, the blocks are very large, and it's known
that they will be overwritten soon, such as when BFD reads in a whole
section.  So zeroing all allocated blocks chews execution time
uselessly.

This is just a GNU standard.  In my other program (Xconq), everything
is zeroed upon allocation, and the rest of the code is designed to
exploit that.

							Stan