Re: the setrlimit changes in glibc 2.1.3

Re: the setrlimit changes in glibc 2.1.3

[Cristian Gafton]

On Wed, 12 Jan 2000, Geoff Keating wrote:

> So how did the shared lib get a reference to an unversioned setrlimit?
> Was it linked with glibc 2.0?

In glibc 2.1.2 the setrlimit was not versioned. For unversioned references
the linker assumes GLIBC_2.0. Which in this case is not correct.

> I suspect it was not linked with any libc at all, which would be the
> problem.

It was linked against libc (glibc 2.1.2). The test case is attached.

Install glibc 2.1.2 (before the setrlimit changes), do a "make libfoo.so",
then upgrade to the current glibc and in the same directory do a "make
test". The link will fail.

Cristian
--
----------------------------------------------------------------------
Cristian Gafton     --     gafton@redhat.com      --     Red Hat, Inc.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  "How could this be a problem in a country where we have Intel and 
   Microsoft?"  --Al Gore on Y2K

Re: the setrlimit changes in glibc 2.1.3

[H . J . Lu]

On Wed, Jan 12, 2000 at 06:41:30PM -0500, Cristian Gafton wrote:
> 
> Install glibc 2.1.2 (before the setrlimit changes), do a "make libfoo.so",
> then upgrade to the current glibc and in the same directory do a "make
> test". The link will fail.
> 

Thanks. It is what I need. I will try to fix the linker.


H.J.

Re: the setrlimit changes in glibc 2.1.3

[Thorsten Kukuk]

On Wed, Jan 12, H . J . Lu wrote:

> On Wed, Jan 12, 2000 at 06:41:30PM -0500, Cristian Gafton wrote:
> > 
> > Install glibc 2.1.2 (before the setrlimit changes), do a "make libfoo.so",
> > then upgrade to the current glibc and in the same directory do a "make
> > test". The link will fail.
> > 
> 
> Thanks. It is what I need. I will try to fix the linker.

Just curious: I have reported this problem when we have introduced 
symbol versioning, and everybody here explained me this is the expected
behaviour because library and main program should never call 2
different versions of the same function. Now it is a bug ?

  Thorsten

-- 
Thorsten Kukuk       http://www.suse.de/~kukuk/       kukuk@suse.de
SuSE GmbH            Schanzaeckerstr. 10            90443 Nuernberg
Linux is like a Vorlon.  It is incredibly powerful, gives terse,
cryptic answers and has a lot of things going on in the background.

Re: the setrlimit changes in glibc 2.1.3

[Mark Kettenis]

   Date: Thu, 13 Jan 2000 07:21:06 +0100
   From: Thorsten Kukuk <kukuk@suse.de>

   Just curious: I have reported this problem when we have introduced 
   symbol versioning, and everybody here explained me this is the expected
   behaviour because library and main program should never call 2
   different versions of the same function. Now it is a bug ?

No, it's not a bug.  It was designed this way.  As soon as we
introduce a new version for a symbol, the old one will become hidden
and the (static) linker will refuse to use it.

Mark

Re: the setrlimit changes in glibc 2.1.3

[H . J . Lu]

On Thu, Jan 13, 2000 at 07:21:06AM +0100, Thorsten Kukuk wrote:
> On Wed, Jan 12, H . J . Lu wrote:
> 
> > On Wed, Jan 12, 2000 at 06:41:30PM -0500, Cristian Gafton wrote:
> > > 
> > > Install glibc 2.1.2 (before the setrlimit changes), do a "make libfoo.so",
> > > then upgrade to the current glibc and in the same directory do a "make
> > > test". The link will fail.
> > > 
> > 
> > Thanks. It is what I need. I will try to fix the linker.
> 
> Just curious: I have reported this problem when we have introduced 
> symbol versioning, and everybody here explained me this is the expected
> behaviour because library and main program should never call 2
> different versions of the same function. Now it is a bug ?

I was on vacation for a few weeks. I just came back. Besides, a
testcase makes a big difference.



H.J.

A patch for binutils (Re: the setrlimit changes in glibc 2.1.3)

[H . J . Lu]

> > > 
> > > Install glibc 2.1.2 (before the setrlimit changes), do a "make libfoo.so",
> > > then upgrade to the current glibc and in the same directory do a "make
> > > test". The link will fail.
> > > 
> > 
> > Thanks. It is what I need. I will try to fix the linker.
> 
> I happen to have this testcase handy.  It may be easier to deal with
> (it could actually be turned into an expect test for the linker).
> 
> Just do 'make'.  It should build properly, but it doesn't.
> 

Thanks. Here is a patch for binutils 2.9.5.0.22. Ian, what do you
think?


H.J.
----
Thu Jan 13 11:03:29 2000  H.J. Lu  <hjl@gnu.org>

	* elflink.h (elf_link_add_object_symbols): Bound the external
	references to the default version to the hidden version.

Index: elflink.h
===================================================================
RCS file: /work/cvs/gnu/binutils/bfd/elflink.h,v
retrieving revision 1.14
diff -u -p -r1.14 elflink.h
--- elflink.h	1999/11/01 22:47:51	1.14
+++ elflink.h	2000/01/13 19:18:31
@@ -1463,7 +1463,12 @@ elf_link_add_object_symbols (abfd, info)
              version, we create an indirect symbol from the default
              name to the fully decorated name.  This will cause
              external references which do not specify a version to be
-             bound to this version of the symbol.  */
+             bound to this version of the symbol.
+	     
+	     If this symbol is hidden, we create an indirect symbol
+	     from the default version the hidden one. This will cause
+	     external references to the default version to be bound to
+	     this symbol. */
 	  if (definition)
 	    {
 	      char *p;
@@ -1738,6 +1743,117 @@ elf_link_add_object_symbols (abfd, info)
 				       & ELF_LINK_HASH_REF_REGULAR) != 0)
 				    dynsym = true;
 				}
+			    }
+			}
+		    }
+		}
+	      else if (p != NULL)
+		{
+		  char *defaultversion;
+		  struct elf_link_hash_entry *hi;
+		  boolean override;
+
+		  defaultversion = bfd_hash_allocate (&info->hash->table,
+						 strlen (name) + 2);
+		  if (defaultversion == NULL)
+		    goto error_return;
+		  strncpy (defaultversion, name, p - name + 1);
+		  defaultversion[p - name + 1] = ELF_VER_CHR;
+		  strcpy (&defaultversion[p - name + 2], p + 1);
+
+		  /* We are going to create a new symbol.  Merge it
+                     with any existing symbol with this name.  For the
+                     purposes of the merge, act as though we were
+                     defining the symbol we just defined, although we
+                     actually going to define an indirect symbol.  */
+		  type_change_ok = false;
+		  size_change_ok = false;
+		  if (! elf_merge_symbol (abfd, info, defaultversion, &sym, &sec,
+					  &value, &hi, &override,
+					  &type_change_ok, &size_change_ok))
+		    goto error_return;
+
+		  if (override
+		      || ! (_bfd_generic_link_add_one_symbol
+			    (info, abfd, defaultversion, BSF_INDIRECT,
+			     bfd_ind_section_ptr, (bfd_vma) 0, name, false,
+			     collect, (struct bfd_link_hash_entry **) &hi)))
+		    goto error_return;
+
+		  /* If there is a duplicate definition somewhere,
+		     then HI may not point to an indirect symbol.  We
+		     will have reported an error to the user in that
+		     case.  */
+
+		  if (hi->root.type == bfd_link_hash_indirect)
+		    {
+		      struct elf_link_hash_entry *ht;
+
+		      /* If the symbol became indirect, then we assume
+			 that we have not seen a definition before.  */
+		      BFD_ASSERT ((hi->elf_link_hash_flags
+				   & (ELF_LINK_HASH_DEF_DYNAMIC
+				      | ELF_LINK_HASH_DEF_REGULAR))
+				  == 0);
+
+		      ht = (struct elf_link_hash_entry *) hi->root.u.i.link;
+
+		      /* Copy down any references that we may have
+			 already seen to the symbol which just became
+			 indirect.  */
+		      ht->elf_link_hash_flags |=
+			(hi->elf_link_hash_flags
+			 & (ELF_LINK_HASH_REF_DYNAMIC
+			    | ELF_LINK_HASH_REF_REGULAR
+			    | ELF_LINK_HASH_REF_REGULAR_NONWEAK
+			    | ELF_LINK_NON_GOT_REF));
+
+		      /* Copy over the global and procedure linkage table
+			 offset entries.  These may have been already set
+			 up by a check_relocs routine.  */
+		      if (ht->got.offset == (bfd_vma) -1)
+			{
+			  ht->got.offset = hi->got.offset;
+			  hi->got.offset = (bfd_vma) -1;
+			}
+		      BFD_ASSERT (hi->got.offset == (bfd_vma) -1);
+
+		      if (ht->plt.offset == (bfd_vma) -1)
+			{
+			  ht->plt.offset = hi->plt.offset;
+			  hi->plt.offset = (bfd_vma) -1;
+			}
+		      BFD_ASSERT (hi->plt.offset == (bfd_vma) -1);
+
+		      if (ht->dynindx == -1)
+			{
+			  ht->dynindx = hi->dynindx;
+			  ht->dynstr_index = hi->dynstr_index;
+			  hi->dynindx = -1;
+			  hi->dynstr_index = 0;
+			}
+		      BFD_ASSERT (hi->dynindx == -1);
+
+		      /* FIXME: There may be other information to copy
+			 over for particular targets.  */
+
+		      /* See if the new flags lead us to realize that
+			 the symbol must be dynamic.  */
+		      if (! dynsym)
+			{
+			  if (! dynamic)
+			    {
+			      if (info->shared
+				  || ((hi->elf_link_hash_flags
+				       & ELF_LINK_HASH_REF_DYNAMIC)
+				      != 0))
+				dynsym = true;
+			    }
+			  else
+			    {
+			      if ((hi->elf_link_hash_flags
+				   & ELF_LINK_HASH_REF_REGULAR) != 0)
+				dynsym = true;
 			    }
 			}
 		    }

Re: A patch for binutils (Re: the setrlimit changes in glibc 2.1.3)

[Ian Lance Taylor]

   Date: Thu, 13 Jan 2000 11:23:31 -0800
   From: "H . J . Lu" <hjl@lucon.org>

   Thanks. Here is a patch for binutils 2.9.5.0.22. Ian, what do you
   think?

   Thu Jan 13 11:03:29 2000  H.J. Lu  <hjl@gnu.org>

	   * elflink.h (elf_link_add_object_symbols): Bound the external
	   references to the default version to the hidden version.

There is something odd about this case.  libtest.so has an external
reference to symbol foo in version FOO_1.0.  Why are we looking up
foo@@FOO_1.0?  Why aren't we looking up foo@FOO_1.0?  If I understand
your patch, the latter is defined while the former is not.  That also
matches my intuition.

We use the double @ to indicate the default version of a symbol.  But
that is only applicable to a definition.  It does not make sense to
refer to the default version of a symbol and to simultaneously specify
a specific version.  When we do use the double @, we also set up a
reference from the unqualified symbol name, and from the symbol name
with a single @.

So I suspect that your patch, which sets up an indirection from
foo@@FOO_1.0 to foo@FOO_1.0, is fixing the wrong thing.  We should be
asking why there would ever be a reference to foo@@FOO_1.0.

Perhaps the fix is as simple as changing the code to not add a second
ELF_VER_CHR if sym.st_shndx is SHN_UNDEF.

Ian

Re: A patch for binutils (Re: the setrlimit changes in glibc 2.1.3)

[H . J . Lu]

On Thu, Jan 13, 2000 at 03:21:29PM -0500, Ian Lance Taylor wrote:
>    Date: Thu, 13 Jan 2000 11:23:31 -0800
>    From: "H . J . Lu" <hjl@lucon.org>
> 
>    Thanks. Here is a patch for binutils 2.9.5.0.22. Ian, what do you
>    think?
> 
>    Thu Jan 13 11:03:29 2000  H.J. Lu  <hjl@gnu.org>
> 
> 	   * elflink.h (elf_link_add_object_symbols): Bound the external
> 	   references to the default version to the hidden version.
> 
> There is something odd about this case.  libtest.so has an external
> reference to symbol foo in version FOO_1.0.  Why are we looking up
> foo@@FOO_1.0?  Why aren't we looking up foo@FOO_1.0?  If I understand

It is because libtest.so is linked against the older libfoo.so,
libfoo.so.1.0, where the FOO_1.0 of foo is the default. You got the
reference to foo@@FOO_1.0. The new libfoo.so, libfoo.so.1.1, has
the FOO_1.1 of foo as default and foo@FOO_1.0 is hidden.

> your patch, the latter is defined while the former is not.  That also
> matches my intuition.
> 
> We use the double @ to indicate the default version of a symbol.  But
> that is only applicable to a definition.  It does not make sense to
> refer to the default version of a symbol and to simultaneously specify
> a specific version.  When we do use the double @, we also set up a
> reference from the unqualified symbol name, and from the symbol name
> with a single @.

Please remember a specific version may change from default to hidden.
However, foo@FOO_1.0 will always be 100 binary compatible with
foo@@FOO_1.0.

> 
> So I suspect that your patch, which sets up an indirection from
> foo@@FOO_1.0 to foo@FOO_1.0, is fixing the wrong thing.  We should be
> asking why there would ever be a reference to foo@@FOO_1.0.
> 
> Perhaps the fix is as simple as changing the code to not add a second
> ELF_VER_CHR if sym.st_shndx is SHN_UNDEF.
> 

You are right. Here is the new patch. Please ignore my previous one.

Thanks.

H.J.
--
Index: elflink.h
===================================================================
RCS file: /work/cvs/gnu/binutils/bfd/elflink.h,v
retrieving revision 1.14
diff -u -p -r1.14 elflink.h
--- elflink.h	1999/11/01 22:47:51	1.14
+++ elflink.h	2000/01/13 20:45:07
@@ -1293,7 +1442,13 @@ elf_link_add_object_symbols (abfd, info)
 		  strcpy (newname, name);
 		  p = newname + namelen;
 		  *p++ = ELF_VER_CHR;
-		  if ((iver.vs_vers & VERSYM_HIDDEN) == 0)
+		  /* For the undefined versioned symbol, we always
+		     reference the hidden version. The default version
+		     will resolve the reference to the hidden one. It
+		     solves the problem where a version changes from
+		     default to higgen. */
+		  if ((iver.vs_vers & VERSYM_HIDDEN) == 0
+		      && sym.st_shndx != SHN_UNDEF)
 		    *p++ = ELF_VER_CHR;
 		  strcpy (p, verstr);
 

Re: A patch for binutils (Re: the setrlimit changes in glibc 2.1.3)

[H . J . Lu]

On Thu, Jan 13, 2000 at 12:46:58PM -0800, H . J . Lu wrote:
> > Perhaps the fix is as simple as changing the code to not add a second
> > ELF_VER_CHR if sym.st_shndx is SHN_UNDEF.
> > 
> 
> You are right. Here is the new patch. Please ignore my previous one.
> 

Ooops. There is a typo in comment. Here is the fixed one.


H.J.
Index: elflink.h
===================================================================
RCS file: /work/cvs/gnu/binutils/bfd/elflink.h,v
retrieving revision 1.14
retrieving revision 1.18
diff -u -p -r1.14 -r1.18
--- elflink.h	1999/11/01 22:47:51	1.14
+++ elflink.h	2000/01/13 20:52:34	1.18
@@ -1293,7 +1442,13 @@ elf_link_add_object_symbols (abfd, info)
 		  strcpy (newname, name);
 		  p = newname + namelen;
 		  *p++ = ELF_VER_CHR;
-		  if ((iver.vs_vers & VERSYM_HIDDEN) == 0)
+		  /* For the undefined versioned symbol, we always
+		     reference the hidden version. The default version
+		     will resolve the reference to the hidden one. It
+		     solves the problem where a version changes from
+		     default to hidden. */
+		  if ((iver.vs_vers & VERSYM_HIDDEN) == 0
+		      && sym.st_shndx != SHN_UNDEF)
 		    *p++ = ELF_VER_CHR;
 		  strcpy (p, verstr);
 

Re: A patch for binutils (Re: the setrlimit changes in glibc 2.1.3)

[Ian Lance Taylor]

   Date: Thu, 13 Jan 2000 12:46:58 -0800
   From: "H . J . Lu" <hjl@lucon.org>

   On Thu, Jan 13, 2000 at 03:21:29PM -0500, Ian Lance Taylor wrote:
   >    Date: Thu, 13 Jan 2000 11:23:31 -0800
   >    From: "H . J . Lu" <hjl@lucon.org>
   > 
   >    Thanks. Here is a patch for binutils 2.9.5.0.22. Ian, what do you
   >    think?
   > 
   >    Thu Jan 13 11:03:29 2000  H.J. Lu  <hjl@gnu.org>
   > 
   > 	   * elflink.h (elf_link_add_object_symbols): Bound the external
   > 	   references to the default version to the hidden version.
   > 
   > There is something odd about this case.  libtest.so has an external
   > reference to symbol foo in version FOO_1.0.  Why are we looking up
   > foo@@FOO_1.0?  Why aren't we looking up foo@FOO_1.0?  If I understand

   It is because libtest.so is linked against the older libfoo.so,
   libfoo.so.1.0, where the FOO_1.0 of foo is the default. You got the
   reference to foo@@FOO_1.0. The new libfoo.so, libfoo.so.1.1, has
   the FOO_1.1 of foo as default and foo@FOO_1.0 is hidden.

I think you probably understood my point.  Just in case, though, I'll
expand further.

The @ characters do not appear in the dynamic symbol table.  All that
is in the file is the symbol name, the associated version name, and a
flag indicating whether this is a hidden version or not.  The linker
introduces the @ characters as, essentially, a trick to combine the
version name into the symbol name for the linker hash table.

The linker uses a double @ for the default version.  Now that I look
at the code, I'm not sure why it does that (I didn't write this code).
I mean, I see why the linker needs to know whether the symbol is the
default version, but I don't see why it needs to be recorded in the
symbol name itself.  That seems like it's trying to overload the
symbol name unnecessarily.

I know that we permit a source file to use a double @ to indicate the
default version, but that's just a hack to ease life for library
maintainers.  I'm not sure why it should slip into the linker hash
table.

When the linker create a symbol with a double @, it also introduces an
indirection from the symbol of the same name name but with a single @.

Anyhow, this all means that the shared library has no reference to the
literal string "foo@@FOO_1.0".  What it has is a reference to symbol
foo, version FOO_1.0, and the version is not hidden.

So I was asking why we were trying to look up foo@@FOO_1.0.  What we
should have been looking up was symbol foo, version FOO_1.0.  In the
hash table that is represented by foo@FOO_1.0 with a single @.

   > We use the double @ to indicate the default version of a symbol.  But
   > that is only applicable to a definition.  It does not make sense to
   > refer to the default version of a symbol and to simultaneously specify
   > a specific version.  When we do use the double @, we also set up a
   > reference from the unqualified symbol name, and from the symbol name
   > with a single @.

   Please remember a specific version may change from default to hidden.
   However, foo@FOO_1.0 will always be 100 binary compatible with
   foo@@FOO_1.0.

In fact, I believe the two names always name precisely the same
symbol.  It's not a case of compatibility.  foo@FOO_1.0 and
foo@@FOO_1.0 are identical.

   > So I suspect that your patch, which sets up an indirection from
   > foo@@FOO_1.0 to foo@FOO_1.0, is fixing the wrong thing.  We should be
   > asking why there would ever be a reference to foo@@FOO_1.0.
   > 
   > Perhaps the fix is as simple as changing the code to not add a second
   > ELF_VER_CHR if sym.st_shndx is SHN_UNDEF.

   You are right. Here is the new patch. Please ignore my previous one.

Thanks for the patch.

Ian