Re: [PATCH] dlsym: Make RTLD_NEXT prefer default version definition [#BZ #14932]

[Fangrui Song <maskray@google.com>]

On 2022-05-27, Florian Weimer wrote:
>I've been looking at this for a while.
>
>We currently have this code in elf/dl-lookup.c:
>
>      /* No specific version is selected.  There are two ways we
>	 can got here:
>
>	 - a binary which does not include versioning information
>	 is loaded
>
>	 - dlsym() instead of dlvsym() is used to get a symbol which
>	 might exist in more than one form
>
>	 If the library does not provide symbol version information
>	 there is no problem at all: we simply use the symbol if it
>	 is defined.
>
>	 These two lookups need to be handled differently if the
>	 library defines versions.  In the case of the old
>	 unversioned application the oldest (default) version
>	 should be used.  In case of a dlsym() call the latest and
>	 public interface should be returned.  */
>      if (verstab != NULL)
>	{
>	  if ((verstab[symidx] & 0x7fff)
>	      >= ((flags & DL_LOOKUP_RETURN_NEWEST) ? 2 : 3))
>	    {
>	      /* Don't accept hidden symbols.  */
>	      if ((verstab[symidx] & 0x8000) == 0
>		  && (*num_versions)++ == 0)
>		/* No version so far.  */
>		*versioned_sym = sym;
>
>	      return NULL;
>	    }
>	}
>
>The numbers 2 and 3 look suspicious.  The condition involving
>num_versions ensures that we only store the first matching symbol in
>*versioned_sym.  But the skipped version indices are not special.
>Indices are not specific to individual symbols, so it is no clear that
>index 2 is special and contains the oldest possible version for that
>particular symbol, and the next index will have the latest version.
>
>Aligning dlvsym with dlsym still makes sense, so I suggest to proceed
>with this patch.  But I think there are more bugs in this area.
>
>> diff --git a/elf/nextmod3.c b/elf/nextmod3.c
>> new file mode 100644
>> index 0000000000..96608a65c0
>> --- /dev/null
>> +++ b/elf/nextmod3.c
>> @@ -0,0 +1,19 @@
>> +int
>> +foo_v1 (int a)
>> +{
>> +  return 1;
>> +}
>> +asm (".symver foo_v1, foo@v1");
>> +
>> +int
>> +foo_v2 (int a)
>> +{
>> +  return 2;
>> +}
>> +asm (".symver foo_v2, foo@v2");
>> +
>> +int
>> +foo (int a)
>> +{
>> +  return 3;
>> +}
>
>Please set foo@@v3 explicitly.

Ack. Having asm (".symver foo, foo@@@v3") is clearer, though it is
redundant since the version script specifies foo in the v3 version node.

>> diff --git a/elf/nextmod3.map b/elf/nextmod3.map
>> new file mode 100644
>> index 0000000000..0a8e4e4ee3
>> --- /dev/null
>> +++ b/elf/nextmod3.map
>> @@ -0,0 +1,3 @@
>> +v1 { };
>> +v2 { };
>> +v3 { foo; };
>
>These versions are not ordered.  Maybe use this instead?
>
>v1 { };
>v2 { } v1;
>v3 { foo; } v2;

The dependencies are a no-op in lld but tracks version definition
dependencies in GNU ld and gold (see readelf -V output: `Parent 1`).

The vd_cnt member is ignored in glibc and FreeBSD rtld, so omitting it
should be fine. That said, I'll add it...