public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* Re: PATCH for uninitialized junk in .dynsym
  1999-07-01  0:00     ` Richard Henderson
  1999-07-01  0:00       ` mark
@ 1999-07-01  0:00       ` Ian Lance Taylor
  1 sibling, 0 replies; 17+ messages in thread
From: Ian Lance Taylor @ 1999-07-01  0:00 UTC (permalink / raw)
  To: rth; +Cc: mark, binutils

   Date: Sun, 13 Jun 1999 17:37:25 -0700
   From: Richard Henderson <rth@cygnus.com>

   On Sun, Jun 13, 1999 at 08:29:45PM -0400, Ian Lance Taylor wrote:
   >    > +  /* Remove the section from the output list.  */
   >    > +  for (spp = &abfd->sections;
   >    > +       *spp != section->output_section;
   >    > +       spp = &(*spp)->next)
   >    > +    ;
   >    > +  *spp = section->output_section->next;
   >    > +  --abfd->section_count;
   > 
   > ... In this case, there is no input section to pass it.

   Certainly there is: `section'.  That's where `section->output_section'
   comes from.

Whoops, you're right, I'm badly confused.

Yes, this code should be changed to use
_bfd_strip_section_from_output.

Ian

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: PATCH for uninitialized junk in .dynsym
  1999-07-01  0:00           ` mark
@ 1999-07-01  0:00             ` Franz Sirl
  1999-07-01  0:00               ` Ian Lance Taylor
  1999-07-01  0:00               ` mark
  0 siblings, 2 replies; 17+ messages in thread
From: Franz Sirl @ 1999-07-01  0:00 UTC (permalink / raw)
  To: mark; +Cc: ian, binutils

Am Die, 15 Jun 1999 schrieb mark@codesourcery.com:
>>>>>> "Franz" == Franz Sirl <Franz.Sirl-kernel@lauterbach.com> writes:
>
>    Franz> After the 2nd some dynindx I checked are zero.
>
>    Franz> Am I on the right track here?
>
>It's because of this:
>
>      for (c = 0, s = output_bfd->sections; s != NULL; s = s->next)
>	{
>	  if ((s->flags & SEC_LINKER_CREATED) != 0
>	      || (s->flags & SEC_ALLOC) == 0)
>	    {
>	      elf_section_data (s)->dynindx = -1;
>	      continue;
>	    }
>
>	  /* These symbols will have no names, so we don't need to
>	     fiddle with dynstr_index.  */
>
>	  elf_section_data (s)->dynindx = c + 1;
>
>	  c++;
>	}
>
>For some reason the elf32-ppc back-end is giving some sections dynindx
>-1.  It should use zero instead; that's the convention for "no dynamic
>symbol table index" in elf_section_data, so far as I can tell.  If you
>make that -1 into a zero, my guess is your problems will be over.

That didn't help :-(. And I don't think this is correct either. dynindx is
checked against -1 all over the place. Maybe this comment I found clarifies it
a bit:

              /* h->dynindx may be -1 if this symbol was marked to
                 become local.  */
 
And AFA I could see dynindx entries containing eg. 2 were zeroed too,
decremented by one on every call.

Franz.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: PATCH for uninitialized junk in .dynsym
  1999-07-01  0:00 ` Richard Henderson
@ 1999-07-01  0:00   ` Ian Lance Taylor
  1999-07-01  0:00     ` Richard Henderson
  0 siblings, 1 reply; 17+ messages in thread
From: Ian Lance Taylor @ 1999-07-01  0:00 UTC (permalink / raw)
  To: rth; +Cc: mark, binutils

   Date: Sun, 13 Jun 1999 17:16:34 -0700
   From: Richard Henderson <rth@cygnus.com>

   On Sun, Jun 13, 1999 at 01:51:07PM -0700, mark@codesourcery.com wrote:
   > +  /* Remove the section from the output list.  */
   > +  for (spp = &abfd->sections;
   > +       *spp != section->output_section;
   > +       spp = &(*spp)->next)
   > +    ;
   > +  *spp = section->output_section->next;
   > +  --abfd->section_count;

   Please use _bfd_strip_section_from_output instead.  I know it does
   slightly more than required for these specific examples, but...

I thought of that, but I don't think it works.  I believe that
_bfd_strip_section_from_output takes an input section as the
parameter.  In this case, there is no input section to pass it.

Ian

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: PATCH for uninitialized junk in .dynsym
  1999-07-01  0:00         ` Franz Sirl
@ 1999-07-01  0:00           ` mark
  1999-07-01  0:00             ` Franz Sirl
  0 siblings, 1 reply; 17+ messages in thread
From: mark @ 1999-07-01  0:00 UTC (permalink / raw)
  To: Franz.Sirl-kernel; +Cc: ian, binutils

>>>>> "Franz" == Franz Sirl <Franz.Sirl-kernel@lauterbach.com> writes:

    Franz> After the 2nd some dynindx I checked are zero.

    Franz> Am I on the right track here?

It's because of this:

      for (c = 0, s = output_bfd->sections; s != NULL; s = s->next)
	{
	  if ((s->flags & SEC_LINKER_CREATED) != 0
	      || (s->flags & SEC_ALLOC) == 0)
	    {
	      elf_section_data (s)->dynindx = -1;
	      continue;
	    }

	  /* These symbols will have no names, so we don't need to
	     fiddle with dynstr_index.  */

	  elf_section_data (s)->dynindx = c + 1;

	  c++;
	}

For some reason the elf32-ppc back-end is giving some sections dynindx
-1.  It should use zero instead; that's the convention for "no dynamic
symbol table index" in elf_section_data, so far as I can tell.  If you
make that -1 into a zero, my guess is your problems will be over.

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: PATCH for uninitialized junk in .dynsym
  1999-07-01  0:00               ` Ian Lance Taylor
@ 1999-07-01  0:00                 ` Ian Lance Taylor
  0 siblings, 0 replies; 17+ messages in thread
From: Ian Lance Taylor @ 1999-07-01  0:00 UTC (permalink / raw)
  To: ian; +Cc: Franz.Sirl-kernel, mark, binutils

   Date: 15 Jun 1999 18:06:26 -0400
   From: Ian Lance Taylor <ian@zembu.com>

   However, in general, I agree.  I think -1, not 0, should be used as
   the value to indicate that the section has no associated dynamic
   symbol.

Well, maybe.  I see that the field is initialized to 0 when the tdata
is created, so maybe 0 really is a better choice.  Most things I see
checking it check for a value > 0, and that is probably a good idea.

Of course, any choice ought to be commented in elf-bfd.h.

This has never mattered before, because I believe that before Mark's
change the section dynindx field was always used exclusively within a
single target.  It had no required global meaning.

Ian

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: PATCH for uninitialized junk in .dynsym
  1999-07-01  0:00             ` Franz Sirl
  1999-07-01  0:00               ` Ian Lance Taylor
@ 1999-07-01  0:00               ` mark
  1999-07-01  0:00                 ` Franz Sirl
  1 sibling, 1 reply; 17+ messages in thread
From: mark @ 1999-07-01  0:00 UTC (permalink / raw)
  To: Franz.Sirl-kernel; +Cc: ian, binutils

>>>>> "Franz" == Franz Sirl <Franz.Sirl-kernel@lauterbach.com> writes:

    Franz> That didn't help :-(. And I don't think this is correct
    Franz> either. dynindx is checked against -1 all over the
    Franz> place.

That's the for hash-table entries, though, not for sections.  I
believe sections have their dynindices initialized to zero.

    Franz> And AFA I could see dynindx entries containing eg. 2 were
    Franz> zeroed too, decremented by one on every call.

The idea is that the PPC back-end is assigning dynindices to most
sections, in ouput order.  So:
 
          Section Index     Dynamic Index
  section 1                 -1
  section 2                 1
  section 3                 -1
  section 4                 2
  section 5                 3
  ...

Now, if we remove sections with section index 2 and 4, say, section 5,
which used to have dynamic index 3, should now have dynamic index 1.
But, the -1s will make it have dynamic index -1!  

Still, if you say that didn't fix it, I'm not sure what *else* is
causing problems.  But, I am sure the -1 there will conflict with the
assumptions in the code I checked in; either that code is wrong, or
the ppc back-end is, now.  There are indeed other places in the PPC
back-end where -1 is used for sections; these shold be changed to.
Here's one:

      for (s = output_bfd->sections; s != NULL; s = s->next)
	{
	  int indx, dindx;

	  sym.st_value = s->vma;

	  indx = elf_section_data (s)->this_idx;
	  dindx = elf_section_data (s)->dynindx;
	  if (dindx != -1)
	    {
	      BFD_ASSERT(indx > 0);
	      BFD_ASSERT(dindx > 0);

	      if (dindx > maxdindx)
		maxdindx = dindx;

	      sym.st_shndx = indx;

	      bfd_elf32_swap_symbol_out (output_bfd, &sym,
					 (PTR) (((Elf32_External_Sym *)
						 sdynsym->contents)
						+ dindx));
	    }

Make that -1 a zero.  Note that this code implies that zero is not a
valid value; it uses `BFD_ASSERT (dindx > 0)'.

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: PATCH for uninitialized junk in .dynsym
  1999-07-01  0:00 PATCH for uninitialized junk in .dynsym mark
@ 1999-07-01  0:00 ` Richard Henderson
  1999-07-01  0:00   ` Ian Lance Taylor
  1999-07-01  0:00 ` Franz Sirl
  1 sibling, 1 reply; 17+ messages in thread
From: Richard Henderson @ 1999-07-01  0:00 UTC (permalink / raw)
  To: mark; +Cc: binutils

On Sun, Jun 13, 1999 at 01:51:07PM -0700, mark@codesourcery.com wrote:
> +  /* Remove the section from the output list.  */
> +  for (spp = &abfd->sections;
> +       *spp != section->output_section;
> +       spp = &(*spp)->next)
> +    ;
> +  *spp = section->output_section->next;
> +  --abfd->section_count;

Please use _bfd_strip_section_from_output instead.  I know it does
slightly more than required for these specific examples, but...

Otherwise it looks fine.


r~

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: PATCH for uninitialized junk in .dynsym
  1999-07-01  0:00       ` Ian Lance Taylor
@ 1999-07-01  0:00         ` Franz Sirl
  1999-07-01  0:00           ` mark
  0 siblings, 1 reply; 17+ messages in thread
From: Franz Sirl @ 1999-07-01  0:00 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: mark, binutils

Am Die, 15 Jun 1999 schrieb Ian Lance Taylor:
>The dynindx field should be being set for all the sections in
>    ppc_elf_size_dynamic_sections
>Is that happening?  If it is happening, where is the dynindx field
>being cleared?


Hmm, in elflink.h elf_link_remove_section_and_adjust_dynindices() is called 2
times,once around line 2651:

      verdefs = asvinfo.verdefs;

      if (verdefs == NULL)
        elf_link_remove_section_and_adjust_dynindices (output_bfd,
                                                       info,
                                                       s);
      else


then again around line 2846:

        if (elf_tdata (output_bfd)->verref == NULL)
          elf_link_remove_section_and_adjust_dynindices (output_bfd,
                                                         info,
                                                         s);
        else


After the 2nd some dynindx I checked are zero.

Am I on the right track here?

Franz.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: PATCH for uninitialized junk in .dynsym
  1999-07-01  0:00   ` mark
@ 1999-07-01  0:00     ` Franz Sirl
  1999-07-01  0:00       ` Ian Lance Taylor
  0 siblings, 1 reply; 17+ messages in thread
From: Franz Sirl @ 1999-07-01  0:00 UTC (permalink / raw)
  To: mark; +Cc: binutils

Am Die, 15 Jun 1999 schrieb mark@codesourcery.com:
>I don't really know how to debug the ppc-linux problem you're seeing
>since I don't have a ppc-linux system.  I suppose I could build a
>cross-compiler and a cross-linker.  
>
>But, I bet you can track it down.  Perhaps the PPC back-end is looking
>at the dynindx for one the gnu.version functions that has been
>stripped? 

Hmm, how can I see in gdb on which symbol it is working? It asserts in the
following code ppc_elf_finish_dynamic_sections() while checking dindx, which
always seems to be 0 then and it seems to happen in all kind of sections.

      for (s = output_bfd->sections; s != NULL; s = s->next)
        {
          int indx, dindx;

          sym.st_value = s->vma;

          indx = elf_section_data (s)->this_idx;
          dindx = elf_section_data (s)->dynindx;
          if (dindx != -1)
            {
              BFD_ASSERT(indx > 0);
              BFD_ASSERT(dindx > 0);

              if (dindx > maxdindx)
                maxdindx = dindx;

              sym.st_shndx = indx;

              bfd_elf32_swap_symbol_out (output_bfd, &sym,
                                         (PTR) (((Elf32_External_Sym *)
                                                 sdynsym->contents)
                                                + dindx));
            }
        }


The other hunk of code is in ppc_elf_relocate_section(), checking for indx,
which is always 0. Here it seems fo assert only in .rodata sections.

                        {
                          asection *osec;

                          osec = sec->output_section;
                          indx = elf_section_data (osec)->dynindx;
                          BFD_ASSERT(indx > 0);
#ifdef DEBUG
                          if (indx <= 0)
                            {
                              printf("indx=%d section=%s flags=%08x name=%s\n",
                                     indx, osec->name, osec->flags,
                                     h->root.root.string);
                            }
#endif
                        }


I'm willing to debug it, but I'm quite lost in this code. I can also offer you
an account on this machine if you want.

Franz.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: PATCH for uninitialized junk in .dynsym
  1999-07-01  0:00     ` Richard Henderson
@ 1999-07-01  0:00       ` mark
  1999-07-01  0:00       ` Ian Lance Taylor
  1 sibling, 0 replies; 17+ messages in thread
From: mark @ 1999-07-01  0:00 UTC (permalink / raw)
  To: rth; +Cc: ian, binutils

>>>>> "Richard" == Richard Henderson <rth@cygnus.com> writes:

    Richard> On Sun, Jun 13, 1999 at 08:29:45PM -0400, Ian Lance
    Richard> Taylor wrote:
    >> > + /* Remove the section from the output list.  */ > + for
    >> (spp = &abfd->sections; > + *spp != section->output_section; >
    >> + spp = &(*spp)->next) > + ; > + *spp =
    >> section->output_section->next; > + --abfd->section_count;
    >> 
    >> ... In this case, there is no input section to pass it.

    Richard> Certainly there is: `section'.  That's where
    Richard> `section->output_section' comes from.

I just moved the code in question from one place to another; I didn't
really investigate its properties.

I've committed the following patch.

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

1999-06-13  Mark Mitchell  <mark@codesourcery.com>

	* elflink.h (elf_link_remove_section_and_adjust_dynindices):
	Remove abfd parameter.  Use _bfd_strip_section_from_output.
	(bfd_elf_size_dynamic_sections): Adjust callers accordingly.

Index: elflink.h
===================================================================
RCS file: /cvs/binutils/binutils/bfd/elflink.h,v
retrieving revision 1.3
diff -u -p -r1.3 elflink.h
--- elflink.h	1999/06/13 14:49:48	1.3
+++ elflink.h	1999/06/14 01:33:12
@@ -55,7 +55,7 @@ static boolean elf_collect_hash_codes
 static boolean elf_link_read_relocs_from_section 
   PARAMS ((bfd *, Elf_Internal_Shdr *, PTR, Elf_Internal_Rela *));
 static void elf_link_remove_section_and_adjust_dynindices 
-  PARAMS ((bfd *, struct bfd_link_info *, asection *));
+  PARAMS ((struct bfd_link_info *, asection *));
 
 /* Given an ELF BFD, add symbols to the global hash table as
    appropriate.  */
@@ -2408,20 +2408,14 @@ compute_bucket_count (info)
    subsequent entries.  */
 
 static void
-elf_link_remove_section_and_adjust_dynindices (abfd, info, section)
-     bfd *abfd;
+elf_link_remove_section_and_adjust_dynindices (info, section)
      struct bfd_link_info *info;
      asection *section;
 {
   asection **spp;
 
   /* Remove the section from the output list.  */
-  for (spp = &abfd->sections;
-       *spp != section->output_section;
-       spp = &(*spp)->next)
-    ;
-  *spp = section->output_section->next;
-  --abfd->section_count;
+  _bfd_strip_section_from_output (section);
 
   if (elf_section_data (section->output_section)->dynindx)
     {
@@ -2648,9 +2642,7 @@ NAME(bfd_elf,size_dynamic_sections) (out
       verdefs = asvinfo.verdefs;
 
       if (verdefs == NULL)
-	elf_link_remove_section_and_adjust_dynindices (output_bfd,
-						       info,
-						       s);
+	elf_link_remove_section_and_adjust_dynindices (info, s);
       else
 	{
 	  unsigned int cdefs;
@@ -2843,9 +2835,7 @@ NAME(bfd_elf,size_dynamic_sections) (out
 				(PTR) &sinfo);
 
 	if (elf_tdata (output_bfd)->verref == NULL)
-	  elf_link_remove_section_and_adjust_dynindices (output_bfd,
-							 info,
-							 s);
+	  elf_link_remove_section_and_adjust_dynindices (info, s);
 	else
 	  {
 	    Elf_Internal_Verneed *t;
@@ -2945,9 +2935,7 @@ NAME(bfd_elf,size_dynamic_sections) (out
       if (dynsymcount == 0
 	  || (verdefs == NULL && elf_tdata (output_bfd)->verref == NULL))
 	{
-	  elf_link_remove_section_and_adjust_dynindices (output_bfd,
-							 info,
-							 s);
+	  elf_link_remove_section_and_adjust_dynindices (info, s);
 	  /* The DYNSYMCOUNT might have changed if we were going to
 	     output a dynamic symbol table entry for S.  */
 	  dynsymcount = elf_hash_table (info)->dynsymcount;

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: PATCH for uninitialized junk in .dynsym
  1999-07-01  0:00               ` mark
@ 1999-07-01  0:00                 ` Franz Sirl
  0 siblings, 0 replies; 17+ messages in thread
From: Franz Sirl @ 1999-07-01  0:00 UTC (permalink / raw)
  To: mark; +Cc: ian, binutils, geoffk

Am Mit, 16 Jun 1999 schrieb mark@codesourcery.com:
>>>>>> "Franz" == Franz Sirl <Franz.Sirl-kernel@lauterbach.com> writes:
>
>    Franz> That didn't help :-(. And I don't think this is correct
>    Franz> either. dynindx is checked against -1 all over the
>    Franz> place.
>
>That's the for hash-table entries, though, not for sections.  I
>believe sections have their dynindices initialized to zero.
>
>    Franz> And AFA I could see dynindx entries containing eg. 2 were
>    Franz> zeroed too, decremented by one on every call.
>
>The idea is that the PPC back-end is assigning dynindices to most
>sections, in ouput order.  So:
> 
>          Section Index     Dynamic Index
>  section 1                 -1
>  section 2                 1
>  section 3                 -1
>  section 4                 2
>  section 5                 3
>  ...
>
>Now, if we remove sections with section index 2 and 4, say, section 5,
>which used to have dynamic index 3, should now have dynamic index 1.
>But, the -1s will make it have dynamic index -1!  
>
>Still, if you say that didn't fix it, I'm not sure what *else* is
>causing problems.  But, I am sure the -1 there will conflict with the
>assumptions in the code I checked in; either that code is wrong, or
>the ppc back-end is, now.  There are indeed other places in the PPC
>back-end where -1 is used for sections; these shold be changed to.
>Here's one:
>
>      for (s = output_bfd->sections; s != NULL; s = s->next)
>	{
>	  int indx, dindx;
>
>	  sym.st_value = s->vma;
>
>	  indx = elf_section_data (s)->this_idx;
>	  dindx = elf_section_data (s)->dynindx;
>	  if (dindx != -1)
>	    {
>	      BFD_ASSERT(indx > 0);
>	      BFD_ASSERT(dindx > 0);
>
>	      if (dindx > maxdindx)
>		maxdindx = dindx;
>
>	      sym.st_shndx = indx;
>
>	      bfd_elf32_swap_symbol_out (output_bfd, &sym,
>					 (PTR) (((Elf32_External_Sym *)
>						 sdynsym->contents)
>						+ dindx));
>	    }
>
>Make that -1 a zero.  Note that this code implies that zero is not a
>valid value; it uses `BFD_ASSERT (dindx > 0)'.

Ok, this one turned the testsuite results back to normal, but I'm not feeling
comfortable...

Franz.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: PATCH for uninitialized junk in .dynsym
  1999-07-01  0:00 PATCH for uninitialized junk in .dynsym mark
  1999-07-01  0:00 ` Richard Henderson
@ 1999-07-01  0:00 ` Franz Sirl
  1999-07-01  0:00   ` mark
  1 sibling, 1 reply; 17+ messages in thread
From: Franz Sirl @ 1999-07-01  0:00 UTC (permalink / raw)
  To: mark; +Cc: binutils

At 22:51 13.06.99 , mark@codesourcery.com wrote:

>The MIPS back-end uses mips_elf_size_dynamic_sections to create
>dynamic symbol table entries for every section in the output file.
>These include the .gnu.version, .gnu.version_d, and .gnu.version_r
>sections.  These sections are removed by
>bfd_elfXX_size_dynamic_sections if they are not needed, but the
>corresponding dynamic symbol table entries are not removed, resulting
>in uninitialized junk in the output file.
>
>This patch fixes the problem.  OK to check in?
>
>--
>Mark Mitchell                   mark@codesourcery.com
>CodeSourcery, LLC               http://www.codesourcery.com
>
>1999-06-13  Mark Mitchell  <mark@codesourcery.com>
>
>         * elflink.h (elf_link_adjust_dynindx): New function.
>         (elf_link_remove_section_and_adjust_dynindices): Likewise.
>         (bfd_elf_size_dynamic_sections): Use them.

Hi Mark,

this patch badly breaks powerpc-linux-gnu, the testsuite log is full of 
things like that:

gcc -g -O2  -fpic -B/home/fsirl/cvsx/binutils/obj/ld/tmpdir/gas/ 
-I/home/fsirl/cvsx/binutils/ld/testsuite/ld-shared -g -O2 -c 
/home/fsirl/cvsx/bi
nutils/ld/testsuite/ld-shared/main.c -o tmpdir/mainp.o
/home/fsirl/cvsx/binutils/obj/ld/ld-new -o tmpdir/shmpnp.so 
-shared  tmpdir/sh1np.o tmpdir/sh2np.o
lt-ld-new: bfd assertion fail ../../bfd/elf32-ppc.c:3368
lt-ld-new: bfd assertion fail ../../bfd/elf32-ppc.c:3368
lt-ld-new: bfd assertion fail ../../bfd/elf32-ppc.c:3368
lt-ld-new: bfd assertion fail ../../bfd/elf32-ppc.c:2955
lt-ld-new: bfd assertion fail ../../bfd/elf32-ppc.c:2955
lt-ld-new: bfd assertion fail ../../bfd/elf32-ppc.c:2955
lt-ld-new: bfd assertion fail ../../bfd/elf32-ppc.c:2955
lt-ld-new: bfd assertion fail ../../bfd/elf32-ppc.c:2955
lt-ld-new: bfd assertion fail ../../bfd/elf32-ppc.c:2955
lt-ld-new: bfd assertion fail ../../bfd/elf32-ppc.c:2955
lt-ld-new: bfd assertion fail ../../bfd/elf32-ppc.c:2955
lt-ld-new: bfd assertion fail ../../bfd/elf32-ppc.c:2955
lt-ld-new: bfd assertion fail ../../bfd/elf32-ppc.c:2955
lt-ld-new: bfd assertion fail ../../bfd/elf32-ppc.c:2955
FAIL: shared (PIC main, non PIC so)

I dunno if you triggered a bug on PPC or if your patch is at fault.

Franz.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* PATCH for uninitialized junk in .dynsym
@ 1999-07-01  0:00 mark
  1999-07-01  0:00 ` Richard Henderson
  1999-07-01  0:00 ` Franz Sirl
  0 siblings, 2 replies; 17+ messages in thread
From: mark @ 1999-07-01  0:00 UTC (permalink / raw)
  To: binutils

The MIPS back-end uses mips_elf_size_dynamic_sections to create
dynamic symbol table entries for every section in the output file.
These include the .gnu.version, .gnu.version_d, and .gnu.version_r
sections.  These sections are removed by
bfd_elfXX_size_dynamic_sections if they are not needed, but the
corresponding dynamic symbol table entries are not removed, resulting
in uninitialized junk in the output file.

This patch fixes the problem.  OK to check in?

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

1999-06-13  Mark Mitchell  <mark@codesourcery.com>

	* elflink.h (elf_link_adjust_dynindx): New function.
	(elf_link_remove_section_and_adjust_dynindices): Likewise.
	(bfd_elf_size_dynamic_sections): Use them.

Index: elflink.h
===================================================================
RCS file: /cvs/binutils/binutils/bfd/elflink.h,v
retrieving revision 1.2
diff -u -p -r1.2 elflink.h
--- elflink.h	1999/06/13 01:13:24	1.2
+++ elflink.h	1999/06/13 20:41:40
@@ -54,6 +54,10 @@ static boolean elf_collect_hash_codes
   PARAMS ((struct elf_link_hash_entry *, PTR));
 static boolean elf_link_read_relocs_from_section 
   PARAMS ((bfd *, Elf_Internal_Shdr *, PTR, Elf_Internal_Rela *));
+static boolean elf_link_adjust_dynindx 
+  PARAMS ((struct elf_link_hash_entry *, PTR));
+static void elf_link_remove_section_and_adjust_dynindices 
+  PARAMS ((bfd *, struct bfd_link_info *, asection *));
 
 /* Given an ELF BFD, add symbols to the global hash table as
    appropriate.  */
@@ -2401,6 +2405,65 @@ compute_bucket_count (info)
   return best_size;
 }
 
+/* Increase the index at which H will appear in the dynamic symbol
+   table by INCREMENT (which is really an `int *').  Called via
+   elf_link_hash_traverse.  */
+
+static boolean
+elf_link_adjust_dynindx (h, increment)
+     struct elf_link_hash_entry *h;
+     PTR increment;
+{
+  if (h->dynindx != -1)
+    h->dynindx += *((int *) increment);
+    
+  return true;
+}
+
+/* Remove SECTION from the BFD.  If a symbol for SECTION was going to
+   be put into the dynamic symbol table, remove it, and renumber
+   subsequent entries.  */
+
+static void
+elf_link_remove_section_and_adjust_dynindices (abfd, info, section)
+     bfd *abfd;
+     struct bfd_link_info *info;
+     asection *section;
+{
+  asection **spp;
+
+  /* Remove the section from the output list.  */
+  for (spp = &abfd->sections;
+       *spp != section->output_section;
+       spp = &(*spp)->next)
+    ;
+  *spp = section->output_section->next;
+  --abfd->section_count;
+
+  if (elf_section_data (section->output_section)->dynindx)
+    {
+      asection *s;
+      int increment = -1;
+
+      /* We were going to output an entry in the dynamic symbol table
+	 for the symbol corresponding to this section.  Now, the
+	 section is gone.  So, we must renumber the dynamic indices of
+	 all subsequent sections and all other entries in the dynamic
+	 symbol table.  */
+      elf_section_data (section->output_section)->dynindx = 0;
+      for (s = section->output_section->next; s; s = s->next)
+	if (elf_section_data (s)->dynindx)
+	  --elf_section_data (s)->dynindx;
+      
+      elf_link_hash_traverse (elf_hash_table (info),
+			      elf_link_adjust_dynindx,
+			      &increment);
+
+      /* There is one less dynamic symbol than there was before.  */
+      --elf_hash_table (info)->dynsymcount;
+    }
+}
+
 /* Set up the sizes and contents of the ELF dynamic sections.  This is
    called by the ELF linker emulation before_allocation routine.  We
    must set the sizes of the sections before the linker sets the
@@ -2602,17 +2665,9 @@ NAME(bfd_elf,size_dynamic_sections) (out
       verdefs = asvinfo.verdefs;
 
       if (verdefs == NULL)
-	{
-	  asection **spp;
-
-	  /* Don't include this section in the output file.  */
-	  for (spp = &output_bfd->sections;
-	       *spp != s->output_section;
-	       spp = &(*spp)->next)
-	    ;
-	  *spp = s->output_section->next;
-	  --output_bfd->section_count;
-	}
+	elf_link_remove_section_and_adjust_dynindices (output_bfd,
+						       info,
+						       s);
       else
 	{
 	  unsigned int cdefs;
@@ -2805,19 +2860,9 @@ NAME(bfd_elf,size_dynamic_sections) (out
 				(PTR) &sinfo);
 
 	if (elf_tdata (output_bfd)->verref == NULL)
-	  {
-	    asection **spp;
-
-	    /* We don't have any version definitions, so we can just
-               remove the section.  */
-
-	    for (spp = &output_bfd->sections;
-		 *spp != s->output_section;
-		 spp = &(*spp)->next)
-	      ;
-	    *spp = s->output_section->next;
-	    --output_bfd->section_count;
-	  }
+	  elf_link_remove_section_and_adjust_dynindices (output_bfd,
+							 info,
+							 s);
 	else
 	  {
 	    Elf_Internal_Verneed *t;
@@ -2917,16 +2962,12 @@ NAME(bfd_elf,size_dynamic_sections) (out
       if (dynsymcount == 0
 	  || (verdefs == NULL && elf_tdata (output_bfd)->verref == NULL))
 	{
-	  asection **spp;
-
-	  /* We don't need any symbol versions; just discard the
-             section.  */
-	  for (spp = &output_bfd->sections;
-	       *spp != s->output_section;
-	       spp = &(*spp)->next)
-	    ;
-	  *spp = s->output_section->next;
-	  --output_bfd->section_count;
+	  elf_link_remove_section_and_adjust_dynindices (output_bfd,
+							 info,
+							 s);
+	  /* The DYNSYMCOUNT might have changed if we were going to
+	     output a dynamic symbol table entry for S.  */
+	  dynsymcount = elf_hash_table (info)->dynsymcount;
 	}
       else
 	{

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: PATCH for uninitialized junk in .dynsym
  1999-07-01  0:00     ` Franz Sirl
@ 1999-07-01  0:00       ` Ian Lance Taylor
  1999-07-01  0:00         ` Franz Sirl
  0 siblings, 1 reply; 17+ messages in thread
From: Ian Lance Taylor @ 1999-07-01  0:00 UTC (permalink / raw)
  To: Franz.Sirl-kernel; +Cc: mark, binutils

   From: Franz Sirl <Franz.Sirl-kernel@lauterbach.com>
   Date: Tue, 15 Jun 1999 20:42:50 +0200

   Hmm, how can I see in gdb on which symbol it is working? It asserts in the
   following code ppc_elf_finish_dynamic_sections() while checking dindx, which
   always seems to be 0 then and it seems to happen in all kind of sections.

	 for (s = output_bfd->sections; s != NULL; s = s->next)
	   {
	     int indx, dindx;

	     sym.st_value = s->vma;

	     indx = elf_section_data (s)->this_idx;
	     dindx = elf_section_data (s)->dynindx;
	     if (dindx != -1)
	       {
		 BFD_ASSERT(indx > 0);
		 BFD_ASSERT(dindx > 0);

		 if (dindx > maxdindx)
		   maxdindx = dindx;

		 sym.st_shndx = indx;

		 bfd_elf32_swap_symbol_out (output_bfd, &sym,
					    (PTR) (((Elf32_External_Sym *)
						    sdynsym->contents)
						   + dindx));
	       }
	   }

The dynindx field should be being set for all the sections in
    ppc_elf_size_dynamic_sections
Is that happening?  If it is happening, where is the dynindx field
being cleared?

Ian

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: PATCH for uninitialized junk in .dynsym
  1999-07-01  0:00             ` Franz Sirl
@ 1999-07-01  0:00               ` Ian Lance Taylor
  1999-07-01  0:00                 ` Ian Lance Taylor
  1999-07-01  0:00               ` mark
  1 sibling, 1 reply; 17+ messages in thread
From: Ian Lance Taylor @ 1999-07-01  0:00 UTC (permalink / raw)
  To: Franz.Sirl-kernel; +Cc: mark, binutils

   From: Franz Sirl <Franz.Sirl-kernel@lauterbach.com>
   Date: Tue, 15 Jun 1999 23:46:24 +0200

   >It's because of this:
   >
   >      for (c = 0, s = output_bfd->sections; s != NULL; s = s->next)
   >	{
   >	  if ((s->flags & SEC_LINKER_CREATED) != 0
   >	      || (s->flags & SEC_ALLOC) == 0)
   >	    {
   >	      elf_section_data (s)->dynindx = -1;
   >	      continue;
   >	    }
   >
   >	  /* These symbols will have no names, so we don't need to
   >	     fiddle with dynstr_index.  */
   >
   >	  elf_section_data (s)->dynindx = c + 1;
   >
   >	  c++;
   >	}
   >
   >For some reason the elf32-ppc back-end is giving some sections dynindx
   >-1.  It should use zero instead; that's the convention for "no dynamic
   >symbol table index" in elf_section_data, so far as I can tell.  If you
   >make that -1 into a zero, my guess is your problems will be over.

   That didn't help :-(. And I don't think this is correct either. dynindx is
   checked against -1 all over the place. Maybe this comment I found clarifies it
   a bit:

		 /* h->dynindx may be -1 if this symbol was marked to
		    become local.  */

   And AFA I could see dynindx entries containing eg. 2 were zeroed too,
   decremented by one on every call.

Actually, that is a different field in a different struct which also
happens to be named dynindx.

However, in general, I agree.  I think -1, not 0, should be used as
the value to indicate that the section has no associated dynamic
symbol.

If there is a section dynindx field which is decremented such that it
becomes 0, I think there may be a problem somewhere.

Ian

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: PATCH for uninitialized junk in .dynsym
  1999-07-01  0:00 ` Franz Sirl
@ 1999-07-01  0:00   ` mark
  1999-07-01  0:00     ` Franz Sirl
  0 siblings, 1 reply; 17+ messages in thread
From: mark @ 1999-07-01  0:00 UTC (permalink / raw)
  To: Franz.Sirl-kernel; +Cc: binutils

I don't really know how to debug the ppc-linux problem you're seeing
since I don't have a ppc-linux system.  I suppose I could build a
cross-compiler and a cross-linker.  

But, I bet you can track it down.  Perhaps the PPC back-end is looking
at the dynindx for one the gnu.version functions that has been
stripped? 

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: PATCH for uninitialized junk in .dynsym
  1999-07-01  0:00   ` Ian Lance Taylor
@ 1999-07-01  0:00     ` Richard Henderson
  1999-07-01  0:00       ` mark
  1999-07-01  0:00       ` Ian Lance Taylor
  0 siblings, 2 replies; 17+ messages in thread
From: Richard Henderson @ 1999-07-01  0:00 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: mark, binutils

On Sun, Jun 13, 1999 at 08:29:45PM -0400, Ian Lance Taylor wrote:
>    > +  /* Remove the section from the output list.  */
>    > +  for (spp = &abfd->sections;
>    > +       *spp != section->output_section;
>    > +       spp = &(*spp)->next)
>    > +    ;
>    > +  *spp = section->output_section->next;
>    > +  --abfd->section_count;
> 
> ... In this case, there is no input section to pass it.

Certainly there is: `section'.  That's where `section->output_section'
comes from.


r~

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~1999-07-01  0:00 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
1999-07-01  0:00 PATCH for uninitialized junk in .dynsym mark
1999-07-01  0:00 ` Richard Henderson
1999-07-01  0:00   ` Ian Lance Taylor
1999-07-01  0:00     ` Richard Henderson
1999-07-01  0:00       ` mark
1999-07-01  0:00       ` Ian Lance Taylor
1999-07-01  0:00 ` Franz Sirl
1999-07-01  0:00   ` mark
1999-07-01  0:00     ` Franz Sirl
1999-07-01  0:00       ` Ian Lance Taylor
1999-07-01  0:00         ` Franz Sirl
1999-07-01  0:00           ` mark
1999-07-01  0:00             ` Franz Sirl
1999-07-01  0:00               ` Ian Lance Taylor
1999-07-01  0:00                 ` Ian Lance Taylor
1999-07-01  0:00               ` mark
1999-07-01  0:00                 ` Franz Sirl

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).