[PATCH] ldconfig: Test the behaviour of _dl_cache_libcmp (Bug, 26101)

[Carlos O'Donell <carlos@redhat.com>]

We move _dl_cache_libcmp to compile the implementation of the
library name sorting routine into a distinct *.os file during
the PIE build.  Then we add a new PIE test tst-_dl_cache_libcmp
which links directly to the compiled implementation and provides
unit testing for the implementation.  We test the cases of the
numeric values sorting before non-numeric, we test a-zA-z and
their sorting, and then we test UTF-8 name sorting is sorted by
byte-value as expected.  We also add some tests for the unexpected
problems users have had over the years e.g. using *.bak files and
expecting ldconfig to ignore them (which it does not).

This is half of the fix for bug 26101, the other half is updating
the manual to describe how ldconfig behaves in plain language.
---
 elf/Makefile               |   7 +-
 elf/dl-cache-libcmp.c      |  55 +++++++++++++
 elf/dl-cache.c             |  40 ----------
 elf/ldconfig.c             |   3 +-
 elf/tst-_dl_cache_libcmp.c | 158 +++++++++++++++++++++++++++++++++++++
 5 files changed, 221 insertions(+), 42 deletions(-)
 create mode 100644 elf/dl-cache-libcmp.c
 create mode 100644 elf/tst-_dl_cache_libcmp.c

diff --git a/elf/Makefile b/elf/Makefile
index 6fe1df90bb..d152fe5317 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -25,7 +25,7 @@ headers		= elf.h bits/elfclass.h link.h bits/link.h
 routines	= $(all-dl-routines) dl-support dl-iteratephdr \
 		  dl-addr dl-addr-obj enbl-secure dl-profstub \
 		  dl-origin dl-libc dl-sym dl-sysdep dl-error \
-		  dl-reloc-static-pie libc_early_init
+		  dl-reloc-static-pie libc_early_init dl-cache-libcmp \
 
 # The core dynamic linking functions are in libc for the static and
 # profiled libraries.
@@ -443,6 +443,11 @@ tests-internal += tst-_dl_addr_inside_object
 tests-pie += tst-_dl_addr_inside_object
 $(objpfx)tst-_dl_addr_inside_object: $(objpfx)dl-addr-obj.os
 CFLAGS-tst-_dl_addr_inside_object.c += $(PIE-ccflag)
+
+tests-internal += tst-_dl_cache_libcmp
+tests-pie += tst-_dl_cache_libcmp
+$(objpfx)tst-_dl_cache_libcmp: $(objpfx)dl-cache-libcmp.os
+CFLAGS-tst-_dl_cache_libmp.c += $(PIE-ccflag)
 endif
 
 # We can only test static libcrypt use if libcrypt has been built,
diff --git a/elf/dl-cache-libcmp.c b/elf/dl-cache-libcmp.c
new file mode 100644
index 0000000000..bf49b5c61e
--- /dev/null
+++ b/elf/dl-cache-libcmp.c
@@ -0,0 +1,55 @@
+/* Implementation of _dl_cache_libcmp.
+   Copyright (C) 1996-2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+int
+_dl_cache_libcmp (const char *p1, const char *p2)
+{
+  while (*p1 != '\0')
+    {
+      if (*p1 >= '0' && *p1 <= '9')
+        {
+          if (*p2 >= '0' && *p2 <= '9')
+            {
+	      /* Must compare this numerically.  */
+	      int val1;
+	      int val2;
+
+	      val1 = *p1++ - '0';
+	      val2 = *p2++ - '0';
+	      while (*p1 >= '0' && *p1 <= '9')
+	        val1 = val1 * 10 + *p1++ - '0';
+	      while (*p2 >= '0' && *p2 <= '9')
+	        val2 = val2 * 10 + *p2++ - '0';
+	      if (val1 != val2)
+		return val1 - val2;
+	    }
+	  else
+            return 1;
+        }
+      else if (*p2 >= '0' && *p2 <= '9')
+        return -1;
+      else if (*p1 != *p2)
+        return *p1 - *p2;
+      else
+	{
+	  ++p1;
+	  ++p2;
+	}
+    }
+  return *p1 - *p2;
+}
diff --git a/elf/dl-cache.c b/elf/dl-cache.c
index 3eedd9afcf..b106144f1b 100644
--- a/elf/dl-cache.c
+++ b/elf/dl-cache.c
@@ -132,46 +132,6 @@ do									      \
   }									      \
 while (0)
 
-
-int
-_dl_cache_libcmp (const char *p1, const char *p2)
-{
-  while (*p1 != '\0')
-    {
-      if (*p1 >= '0' && *p1 <= '9')
-        {
-          if (*p2 >= '0' && *p2 <= '9')
-            {
-	      /* Must compare this numerically.  */
-	      int val1;
-	      int val2;
-
-	      val1 = *p1++ - '0';
-	      val2 = *p2++ - '0';
-	      while (*p1 >= '0' && *p1 <= '9')
-	        val1 = val1 * 10 + *p1++ - '0';
-	      while (*p2 >= '0' && *p2 <= '9')
-	        val2 = val2 * 10 + *p2++ - '0';
-	      if (val1 != val2)
-		return val1 - val2;
-	    }
-	  else
-            return 1;
-        }
-      else if (*p2 >= '0' && *p2 <= '9')
-        return -1;
-      else if (*p1 != *p2)
-        return *p1 - *p2;
-      else
-	{
-	  ++p1;
-	  ++p2;
-	}
-    }
-  return *p1 - *p2;
-}
-
-
 /* Look up NAME in ld.so.cache and return the file name stored there, or null
    if none is found.  The cache is loaded if it was not already.  If loading
    the cache previously failed there will be no more attempts to load it.
diff --git a/elf/ldconfig.c b/elf/ldconfig.c
index 0c090dca15..21072d3ea9 100644
--- a/elf/ldconfig.c
+++ b/elf/ldconfig.c
@@ -967,7 +967,8 @@ search_dir (const struct dir_entry *entry)
 	  if (strcmp (dlib_ptr->soname, soname) == 0)
 	    {
 	      /* Prefer a file to a link, otherwise check which one
-		 is newer.  */
+		 is a later version number based on the existing cache
+		 choices made by _dl_cache_libcmp.  */
 	      if ((!is_link && dlib_ptr->is_link)
 		  || (is_link == dlib_ptr->is_link
 		      && _dl_cache_libcmp (dlib_ptr->name, direntry->d_name) < 0))
diff --git a/elf/tst-_dl_cache_libcmp.c b/elf/tst-_dl_cache_libcmp.c
new file mode 100644
index 0000000000..6e2a3fee0f
--- /dev/null
+++ b/elf/tst-_dl_cache_libcmp.c
@@ -0,0 +1,158 @@
+/* Testing of the ld.so/ldconfig library version comparison.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <string.h>
+#include <support/check.h>
+
+/* Verify the correct operation of _dl_cache_libcmp against what we
+   expect for the sorting.  */
+extern int _dl_cache_libcmp (const char *p1, const char *p2);
+
+static int
+do_test (void)
+{
+  /* The algorithm in _dl_cache_libcmp follows these rules:
+     * All input is treated as byte values.
+     * All ASCII numbers are sorted ahead of non-numbers.
+     * All ASCII numbers are sorted in numerically increasing order.
+     * All ASCII characters are sorted by their byte values.  */
+
+  /* Please note that the testing values overlap.  They are arranged
+     in this order to test corner cases of the algorithm.  We could
+     start just by testing the entire ASCII range followed by a more
+     complex conditional to exclude the numeric range, but the following
+     incremental testing steps should provide clearer diagnostics should
+     anything immediately fail.  Also testing the entire ASCII range in
+     all permutations is an 8-bit x 8-bit testing matrix and takes too
+     long for quick developer testing.  */
+
+  /* Test 1: Verify numbers sort before letters in ASCII.  */
+  char p1[2], p2[2];
+  p1[1]='\0';
+  p2[1]='\0';
+  /* First argument has non-numbers.  */
+  for (p1[0] = 'a'; p1[0] <= 'z'; p1[0]++)
+    for (p2[0] = '0'; p2[0] <= '9'; p2[0]++)
+      TEST_VERIFY (_dl_cache_libcmp (&p1[0], &p2[0]) < 0);
+  for (p1[0] = 'A'; p1[0] <= 'Z'; p1[0]++)
+    for (p2[0] = '0'; p2[0] <= '9'; p2[0]++)
+      TEST_VERIFY (_dl_cache_libcmp (&p1[0], &p2[0]) < 0);
+  /* Second argument has non-numbers.  */
+  for (p1[0] = '0'; p1[0] <= '9'; p1[0]++)
+    for (p2[0] = 'a'; p2[0] <= 'z'; p2[0]++)
+      TEST_VERIFY (_dl_cache_libcmp (&p1[0], &p2[0]) > 0);
+  for (p1[0] = '0'; p1[0] <= '9'; p1[0]++)
+    for (p2[0] = 'A'; p2[0] <= 'Z'; p2[0]++)
+      TEST_VERIFY (_dl_cache_libcmp (&p1[0], &p2[0]) > 0);
+
+  /* Test 2: Verify numbers sort in numerically increasing order.  */
+  for (p1[0] = '0'; p1[0] <= '9'; p1[0]++)
+    for (p2[0] = '0'; p2[0] <= '9'; p2[0]++)
+      if (p1[0] > p2 [0])
+        TEST_VERIFY (_dl_cache_libcmp (&p1[0], &p2[0]) > 0);
+      else if (p1[0] == p2[0])
+        TEST_VERIFY (_dl_cache_libcmp (&p1[0], &p2[0]) == 0);
+      else
+        TEST_VERIFY (_dl_cache_libcmp (&p1[0], &p2[0]) < 0);
+
+  /* Test 3: Verify non-numbers are sorted in increasing order.  */
+  for (p1[0] = 'a'; p1[0] <= 'z'; p1[0]++)
+    for (p2[0] = 'a'; p2[0] <= 'z'; p2[0]++)
+      if (p1[0] > p2 [0])
+        TEST_VERIFY (_dl_cache_libcmp (&p1[0], &p2[0]) > 0);
+      else if (p1[0] == p2[0])
+        TEST_VERIFY (_dl_cache_libcmp (&p1[0], &p2[0]) == 0);
+      else
+        TEST_VERIFY (_dl_cache_libcmp (&p1[0], &p2[0]) < 0);
+  for (p1[0] = 'A'; p1[0] <= 'Z'; p1[0]++)
+    for (p2[0] = 'A'; p2[0] <= 'Z'; p2[0]++)
+      if (p1[0] > p2 [0])
+        TEST_VERIFY (_dl_cache_libcmp (&p1[0], &p2[0]) > 0);
+      else if (p1[0] == p2[0])
+        TEST_VERIFY (_dl_cache_libcmp (&p1[0], &p2[0]) == 0);
+      else
+        TEST_VERIFY (_dl_cache_libcmp (&p1[0], &p2[0]) < 0);
+
+  /* Test 4: Verify some byte sequences sort in increasing order
+             excluding the numeric range.  Also test transitivity
+	     in each test.  */
+  /* Corner case ASCII just before 0.  */
+  p1[0] = '/';
+  p2[0] = '0';
+  TEST_VERIFY (_dl_cache_libcmp (&p1[0], &p2[0]) < 0);
+  TEST_VERIFY (_dl_cache_libcmp (&p2[0], &p1[0]) > 0);
+  /* Corner case ASCII just after 9.  */
+  p1[0] = ':';
+  p2[0] = '9';
+  TEST_VERIFY (_dl_cache_libcmp (&p1[0], &p2[0]) < 0);
+  TEST_VERIFY (_dl_cache_libcmp (&p2[0], &p1[0]) > 0);
+  /* Corner case lowest byte value.  */
+  p1[0] = 1;
+  p2[0] = 2;
+  TEST_VERIFY (_dl_cache_libcmp (&p1[0], &p2[0]) < 0);
+  TEST_VERIFY (_dl_cache_libcmp (&p2[0], &p1[0]) > 0);
+  /* Corner case highest byte value.  */
+  p1[0] = 254;
+  p2[0] = 255;
+  TEST_VERIFY (_dl_cache_libcmp (&p1[0], &p2[0]) < 0);
+  TEST_VERIFY (_dl_cache_libcmp (&p2[0], &p1[0]) > 0);
+
+  /* Test 5: Specific example names.  */
+  char name1[256];
+  char name2[256];
+  /* libc.so.6 is newer than libc.so.5.  */
+  strcpy (name1, "libc.so.6");
+  strcpy (name2, "libc.so.5");
+  TEST_VERIFY (_dl_cache_libcmp (&name1[0], &name2[0]) > 0);
+  TEST_VERIFY (_dl_cache_libcmp (&name2[0], &name1[0]) < 0);
+  /* ld-2.31.so is newer than ld-2.30.so.  */
+  strcpy (name1, "ld-2.31.so");
+  strcpy (name2, "ld-2.30.so");
+  TEST_VERIFY (_dl_cache_libcmp (&name1[0], &name2[0]) > 0);
+  TEST_VERIFY (_dl_cache_libcmp (&name2[0], &name1[0]) < 0);
+  /* Sometimes users put *.bak files into /lib64 and ldconfig still
+     picks up on these files even if they end in *.bak because such
+     files still constitute a possible implementation name.  In these
+     cases they are sorted according to their numeric values and processed
+     according. See bug 26101.  */
+  strcpy (name1, "libc-2.31.so.bak");
+  strcpy (name2, "libc-2.30.so");
+  TEST_VERIFY (_dl_cache_libcmp (&name1[0], &name2[0]) > 0);
+  TEST_VERIFY (_dl_cache_libcmp (&name2[0], &name1[0]) < 0);
+  /* Test a UTF-8 libname example with 2-byte UTF-8 values.
+     With strcoll we would expect this sorting in en_US.UTF-8:
+     U00E0 (LATIN SMALL LETTER A WITH GRAVE)
+     < U00C0 (LATIN CAPITAL LETTER A WITH GRAVE)
+     This is based on the ISO 14651 T1 common sorting.
+     We don't get that because multi-byte UTF-8 sorting sorts is this:
+     0xc3 0xa0 (LATIN SMALL LETTER A WITH GRAVE)
+     > 0xc3 0x80 (LATIN CAPITAL LETTER A WITH GRAVE)
+     We chose this specific example because it highlights the case
+     where locale-specific collation is the opposite of multi-byte
+     value sorting.  */
+  strcpy (name1, "libÀ.so");
+  strcpy (name2, "libà.so");
+  TEST_VERIFY (_dl_cache_libcmp (&name1[0], &name2[0]) < 0);
+  TEST_VERIFY (_dl_cache_libcmp (&name2[0], &name1[0]) > 0);
+
+  return 0;
+}
+
+#define TIMEOUT 120
+#include <support/test-driver.c>
-- 
2.26.2