Re: A patch for default version and archive

[Ulrich Drepper <drepper at redhat dot com>]

"H . J . Lu" <hjl@valinux.com> writes:

> 1. My patch doesn't introduce any new matches. Both references to
> symbol@ver1 and symbol will match the definition of symbol@@ver1
> today.
> 2. My patch only changes how the linker deals with archives. Before
> it only checks "symbol" for "symbol@@ver1", with my patch, it will
> also check "symbol@ver1".

This all is under the premise that ld is handling versioned symbols
and that this is good.  It's not and ld should not do what you assume
as the before-behavior in the first place since this, as I've layed
out, is creating big problems.

If despite that ld matches versions in archive the above does
introduce new matches (otherwise the patch wouldn't be necessary).
And these matches can be unwanted.  The problem is the same: you don't
know in which object the version is supposed to be so that only in
that object the versioned symbol is searched.  Instead, you will
search in any archive for a symbol named 'symbol' and 'symbol@@ver'
and now also 'symbol@ver1'.  This is not how ld.so works (and ld when
it's doing it's job).  If a lookup is intercepted in another DSO
version name must match exactly *and* the DSO has determined which
versions of the other DSOs it needs.  Now to your case.  I'm just
playing along to show that you make the already bad situation worse.
The example is carefully constructed, yes, but it does not mean there
are not more obvious examples.  This all is a consequence of
implementing something without thinking about the consequences.

If you reference symbol@@ver1 this means you expect ver1 to be the
version everybody is using and not some compatibility version where
you know that the interface is different from the current version.
Otherwise you would have made a reference to symbol@var1 in the first
place.  I hope you can agree with that.

Now, this means that the wherever the data structures of this
interface are used you use the old version.  Also in using other
libraries or for own code.  Now assume one of the things you want to
do is write a callback for a function which is versioned.  The library
is defining a new and an old version of the function.  The code is
split in three pieces and I haven't tried compiling it:

1. part: the old code of the library:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/* foo.h */
struct thedata
{
  char i;
};
extern int (*fp) (struct thedata);
/* note that to actually get a reference to foo@@ver1 we have to use
   versioning here as well; could be there is a very old version; it
   doesn't matter I don't know exactly what you syntax is, you'll
   certainly figure it out */
extern int curfoo (struct thedata *);
extern int foo (struct thedata *);
asm (".symver curfoo, foo@@ver1");
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/* foo.c */
int (*fp) (struct thedata *);

int
curfoo (struct thedata *s)
{
  return fp (s);
}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

2. part: the new code of the library:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/* foo.h */
struct thedata
{
  int i;
};
struct theolddata
{
  char i;
};
extern int (*fp) (struct thedata);
/* I'm not even sure about the syntax of the following two.  In any
   case do we have a reference to foo@var1 */
extern int newfoo (struct thedata *);
extern int foo (struct thedata *);
asm (".symver newfoo, foo@@ver2");
extern int oldfoo (struct theolddata *);
asm (".symver oldfoo, foo@ver1");
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/* foo.c */
int (*fp) (struct thedata *);

int
newfoo (int (*fp) (struct thedata *), struct thedata *s)
{
  return fp (s);
}
asm (".symver newfoo, foo@@ver2");
int
oldfoo (struct theolddata *s)
{
  struct thedata d;
  d.i = s->i;
  return fp (&d);
}
asm (".symver oldfoo, foo@ver1");
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This is a logical implementation of this you'll hopefully agree.  You
cannot simply introduce a second callback function which takes a
different parameter.  This would change the interface of the library
which you avoid by using versioning in the first place.

Part 3: the program

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/* main.c */
#include "foo.h"

int
callback (struct thedata *s)
{
  return s->i;  
}

int
main(void)
{
  struct thedata d = { 42 };
  fp = callback;
  return foo (&d) != 42;
}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Now look at which foo.h you use in case you get a reference to @@ver1.
It is the first one with the definition of 'thedata' which features a
'char'.  But you are linking against the second version of the library
function which expects the callback to take a parameter of the new
'thedata' type.  The consequence is that you read uninitialized
memory.  If @ver1 would not match you at least would get notified that
there is no matching definition which is true.  If you would include
the second foo.h in main.c file you would know that the version of the
old function is @ver1 (and you would not be able to call it which is
correct since you don't have the data structure definition) and the
reference to foo() would be foo@@ver2.


This is just one example how things can go wrong.  It's all a
consequence of not having thought through the whole concept of
handling versioned undefined references and versioned symbols in
archives.  This in just one additional problem you create.  It's much
easier to come up with examples that versioned symbols must not be
handled for statically linked binaries in the first place.  I haven't
followed the development that closely in the last weeks so I don't
know whether this support is already in or not.  If it is, I request to
remove it.  It is dangerous and misleading.

When we developed the versioning handling Eric Youngdale, myself, and
to some extend Ian discussed all these things over an extended period
of time.  We knew how the result would look like and that we stretched
the original model already to a point where it is not 100% guaranteed
that you catch all errors.  You are again far too fast with your
implementation since the consequences are not clear.  Poor Nick cannot
do much but seeing that nothing breaks and I had no time to review all
this (leaving alone Eric and Ian as it seems).

I don't know who will make the decision, somebody will have to.  It is
no problem to reject all this since there are much better solutions
for the original issue than this hack and the bad consequences are not
necessary to live with (and we already have enough problems).

-- 
---------------.                          ,-.   1325 Chesapeake Terrace
Ulrich Drepper  \    ,-------------------'   \  Sunnyvale, CA 94089 USA
Red Hat          `--' drepper at redhat.com   `------------------------