Bugs in recent snapshots on Solaris

Bugs in recent snapshots on Solaris

[Manfred Hollstein]

Didn't anybody else see this? When make check is running the shared
tests in the ld directory, it'll never stop. Instead I'll get a CPU
hungry process tmpdir/shp running and running and ... I have no idea
what it's doing, size won't change (increase or decrease), but it
really hogs my CPU (top reports right now 0.0% idle!).

This is with yesterday's snapshot (gas-980312), but I've observed it
with 0303 as well. I haven't investigated, when this problem has been
introduced, but at least 980219 is working without problems.

Interestingly, this only happens on sparc-sun-solaris2.5.1, not on
sparc-sun-sunos4.1.4.

Thanks for you time.

manfred

Re: Bugs in recent snapshots on Solaris

[Manfred Hollstein]

On Fri, 13 March 1998, 09:46:33, manfred@s-direktnet.de wrote:

 > Didn't anybody else see this? When make check is running the shared
 > tests in the ld directory, it'll never stop. Instead I'll get a CPU
 > hungry process tmpdir/shp running and running and ... I have no idea
 > what it's doing, size won't change (increase or decrease), but it
 > really hogs my CPU (top reports right now 0.0% idle!).
 > 
 > This is with yesterday's snapshot (gas-980312), but I've observed it
 > with 0303 as well. I haven't investigated, when this problem has been
 > introduced, but at least 980219 is working without problems.
 > 
 > Interestingly, this only happens on sparc-sun-solaris2.5.1, not on
 > sparc-sun-sunos4.1.4.

In the meantime I tracked this down to this change from 980224:

Sun Feb 22 20:39:00 1998  Richard Henderson  <rth@cygnus.com>

	* elf32-sparc.c (elf32_sparc_adjust_dynamic_symbol): Don't create
 	a plt entry when we can determine that we authoritatively define
 	the symbol.

diff -u -r --new-file --show-function-line=^[A-Za-z_] /home/raeburn/offsite/snap/old/gas-980219/bfd/elf32-sparc.c ./bfd/elf32-sparc.c
--- /home/raeburn/offsite/snap/old/gas-980219/bfd/elf32-sparc.c	Thu Feb 19 01:14:03 1998
+++ ./bfd/elf32-sparc.c	Tue Feb 24 01:14:08 1998
@@ -593,14 +593,22 @@ elf32_sparc_adjust_dynamic_symbol (info,
 	      || h->root.type == bfd_link_hash_defweak)
 	  && (h->root.u.def.section->flags & SEC_CODE) != 0))
     {
-      if (! elf_hash_table (info)->dynamic_sections_created)
+      if (! elf_hash_table (info)->dynamic_sections_created
+	  || ((!info->shared || info->symbolic || h->dynindx == -1)
+	      && (h->elf_link_hash_flags
+		  & ELF_LINK_HASH_DEF_REGULAR) != 0))
 	{
 	  /* This case can occur if we saw a WPLT30 reloc in an input
-             file, but none of the input files were dynamic objects.
-             In such a case, we don't actually need to build a
-             procedure linkage table, and we can just do a WDISP30
-             reloc instead.  */
-	  BFD_ASSERT ((h->elf_link_hash_flags & ELF_LINK_HASH_NEEDS_PLT) != 0);
+	     file, but none of the input files were dynamic objects.
+	     Or, when linking the main application or a -Bsymbolic
+	     shared library against PIC code.  Or when a global symbol
+	     has been made private, e.g. via versioning.
+
+	     In these cases we know what value the symbol will resolve
+	     to, so we don't actually need to build a procedure linkage
+	     table, and we can just do a WDISP30 reloc instead.  */
+
+	  h->elf_link_hash_flags &= ~ELF_LINK_HASH_NEEDS_PLT;
 	  return true;
 	}
 
@@ -618,17 +626,11 @@ elf32_sparc_adjust_dynamic_symbol (info,
 	  return false;
 	}
 
-      /* If this symbol is not defined in a regular file, and we are
-	 not generating a shared library, then set the symbol to this
-	 location in the .plt.  This is required to make function
-	 pointers compare as equal between the normal executable and
-	 the shared library.  */
-      if (! info->shared
-	  && (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR) == 0)
-	{
-	  h->root.u.def.section = s;
-	  h->root.u.def.value = s->_raw_size;
-	}
+      /* Set the symbol to this location in the .plt.  This is
+	 required to make function pointers compare as equal between
+	 the normal executable and the shared library.  */
+      h->root.u.def.section = s;
+      h->root.u.def.value = s->_raw_size;
 
       h->plt_offset = s->_raw_size;
 

I reverted this patch, re-built and did make check - now it runs!
Richard, can you please take a look at it?

Thanks

manfred

Re: Bugs in recent snapshots on Solaris

[Ken Raeburn]

Yes, it appears that the symbol shlib_shlibvar1, which names a
function in the text section that has its address taken at various
points, has gotten moved in the shared library to the .plt section.
(The correct code is still in the text section, just without a name.)

The code at that .plt location, at run time, becomes a very tight
loop.  In the .so file:

DYNAMIC RELOCATION RECORDS
OFFSET   TYPE              VALUE 
...
00010790 R_SPARC_JMP_SLOT  shlib_shlibvar1
...

Disassembly of section .plt:

00010754 <shlib_shlibcalled-30>:
        ...

00010784 <shlib_shlibcalled>:
   10784:       03 00 00 30     sethi  %hi(0xc000), %g1
   10788:       30 bf ff f3     b,a   10754 <_PROCEDURE_LINKAGE_TABLE_>
   1078c:       01 00 00 00     nop 

00010790 <shlib_shlibvar1>:
   10790:       03 00 00 3c     sethi  %hi(0xf000), %g1
   10794:       30 bf ff f0     b,a   10754 <_PROCEDURE_LINKAGE_TABLE_>
   10798:       01 00 00 00     nop 
   1079c:       03 00 00 48     sethi  %hi(0x12000), %g1
   107a0:       30 bf ff ed     b,a   10754 <_PROCEDURE_LINKAGE_TABLE_>
   107a4:       01 00 00 00     nop 
   107a8:       01 00 00 00     nop 

And run time:

(gdb) x/20i &shlib_shlibvar1
0xef7a0790 <shlib_shlibvar1>:   sethi  %hi(0xf000), %g1
0xef7a0794 <shlib_shlibvar1+4>: sethi  %hi(0xef7a0400), %g1
0xef7a0798 <shlib_shlibvar1+8>:
    jmp  %g1 + 0x390    ! 0xef7a0790 <shlib_shlibvar1>
0xef7a079c <shlib_shlibvar1+12>:        sethi  %hi(0x12000), %g1


I'm not familiar with the shared library linker support, so for the
moment, I'll leave the rest to Richard.

Ken

Re: Bugs in recent snapshots on Solaris

[Richard Henderson]

> -      /* If this symbol is not defined in a regular file, and we are
> -	 not generating a shared library, then set the symbol to this
> -	 location in the .plt.  This is required to make function
> -	 pointers compare as equal between the normal executable and
> -	 the shared library.  */
> -      if (! info->shared
> -	  && (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR) == 0)
> -	{
> -	  h->root.u.def.section = s;
> -	  h->root.u.def.value = s->_raw_size;
> -	}
> +      /* Set the symbol to this location in the .plt.  This is
> +	 required to make function pointers compare as equal between
> +	 the normal executable and the shared library.  */
> +      h->root.u.def.section = s;
> +      h->root.u.def.value = s->_raw_size;

This was the offending bit.  The other part is righteous.
Somewhere I got a bit overeager.

Sorry about that.


r~

Re: Bugs in recent snapshots on Solaris

[H.J. Lu]

> 
> > -      /* If this symbol is not defined in a regular file, and we are
> > -	 not generating a shared library, then set the symbol to this
> > -	 location in the .plt.  This is required to make function
> > -	 pointers compare as equal between the normal executable and
> > -	 the shared library.  */
> > -      if (! info->shared
> > -	  && (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR) == 0)
> > -	{
> > -	  h->root.u.def.section = s;
> > -	  h->root.u.def.value = s->_raw_size;
> > -	}
> > +      /* Set the symbol to this location in the .plt.  This is
> > +	 required to make function pointers compare as equal between
> > +	 the normal executable and the shared library.  */
> > +      h->root.u.def.section = s;
> > +      h->root.u.def.value = s->_raw_size;
> 
> This was the offending bit.  The other part is righteous.
> Somewhere I got a bit overeager.
> 

Is that fixed now in the snapshot? I need to make a new binutils for
Linux to get this and an x86 bug fixed.

Thanks.


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