[v9,2/6] elf: Add tests for the dl hash funcs (_dl_new_hash and _dl_elf_hash)

If we want to further optimize the functions tests are needed.
---
 elf/Makefile      |   1 +
 elf/tst-dl-hash.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 148 insertions(+)
 create mode 100644 elf/tst-dl-hash.c

On 17/05/2022 02:00, Noah Goldstein via Libc-alpha wrote:
> If we want to further optimize the functions tests are needed.
> ---
>   elf/Makefile      |   1 +
>   elf/tst-dl-hash.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 148 insertions(+)
>   create mode 100644 elf/tst-dl-hash.c
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index fc9860edee..0e72f913a0 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -309,6 +309,7 @@ tests := \
>     tst-array4 \
>     tst-array5 \
>     tst-auxv \
> +  tst-dl-hash \
>     tst-leaks1 \
>     tst-stringtable \
>     tst-tls9 \
> diff --git a/elf/tst-dl-hash.c b/elf/tst-dl-hash.c
> new file mode 100644
> index 0000000000..e806a274ca
> --- /dev/null
> +++ b/elf/tst-dl-hash.c
> @@ -0,0 +1,147 @@
> +/* Copyright (C) 2022 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/>.  */
> +/* Simple implementation of ELF ABI hash function. */

The one line description is typically the first line at the top, just 
before the copyright notice.  And perhaps you want to call it "Test ELF 
ABI hash functions" or something similar :)

> +
> +#include <dl-hash.h>
> +#include <dl-new-hash.h>
> +#include <support/support.h>
> +#include <support/check.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdlib.h>
> +
> +typedef unsigned int (*hash_f) (const char *);
> +
> +static unsigned int
> +simple_dl_new_hash (const char *s)
> +{
> +  uint32_t h = 5381;
> +  for (unsigned char c = *s; c != '\0'; c = *++s)
> +    h = h * 33 + c;
> +  return h;
> +}

Maybe just `#define dl_new_hash simple_dl_new_hash` and include 
elf/dl-new-hash.h here?  And then don't get rid of elf/dl-new-hash.h in 
6/6, let it remain the reference implementation to test against. 
Perhaps also add a comment in that file stating that it is a reference 
implementation to test against and that sysdeps has the actual 
implementation that gets used, depending on the target.

> +
> +static unsigned int
> +simple_dl_elf_hash (const char *name_arg)
> +{
> +  unsigned long int hash = 0;
> +  for (unsigned char c = *name_arg; c != '\0'; c = *(++name_arg))
> +    {
> +      unsigned long int hi;
> +      hash = (hash << 4) + c;
> +      hi = hash & 0xf0000000;
> +      hash ^= hi >> 24;
> +      hash &= 0x0fffffff;
> +    }
> +  return hash;
> +}

Likewise, add elf/dl-hash.h with this reference implementation.

> +static int
> +do_fill_test (size_t len, int fill, const char *name, hash_f testf,
> +	      hash_f expecf)
> +{
> +  uint32_t expec, res;
> +  char buf[len + 1];
> +  memset (buf, fill, len);
> +  buf[len] = '\0';
> +
> +  expec = expecf (buf);
> +  res = testf (buf);
> +  if (expec != res)
> +    {
> +      FAIL_EXIT1 ("FAIL: fill(%d) %s(%zu), %x != %x\n", fill, name, len, expec,
> +		  res);
> +    }
> +
> +  return 0;
> +}
> +
> +static int
> +do_fill_tests (size_t len, int fill)
> +{
> +  if (do_fill_test (len, fill, "dl_new_hash", &_dl_new_hash,
> +		    &simple_dl_new_hash))
> +    {

Redundant paranthesis.

> +      return 1;
> +    }
> +  return do_fill_test (len, fill, "dl_elf_hash", &_dl_elf_hash,
> +		       &simple_dl_elf_hash);
> +}
> +
> +static int
> +do_rand_test (size_t len, const char *name, hash_f testf, hash_f expecf)
> +{
> +  uint32_t expec, res;
> +  size_t i;
> +  char buf[len + 1];
> +  char v;
> +  for (i = 0; i < len; ++i)
> +    {
> +      v = random ();
> +      if (v == 0)
> +	{

Likewise.

> +	  v = 1;
> +	}
> +      buf[i] = v;
> +    }
> +  buf[len] = '\0';
> +
> +  expec = expecf (buf);
> +  res = testf (buf);
> +  if (expec != res)
> +    {
> +      printf ("FAIL: random %s(%zu), %x != %x\n", name, len, expec, res);
> +      return 1;
> +    }
> +
> +  return 0;
> +}
> +
> +static int
> +do_rand_tests (size_t len)
> +{
> +  if (do_rand_test (len, "dl_new_hash", &_dl_new_hash, &simple_dl_new_hash))
> +    {

Likewise.

> +      return 1;
> +    }
> +  return do_rand_test (len, "dl_elf_hash", &_dl_elf_hash, &simple_dl_elf_hash);
> +}
> +
> +static int
> +do_test (void)
> +{
> +  size_t i, j;
> +  for (i = 0; i < 100; ++i)
> +    {
> +      for (j = 0; j < 8192; ++j)
> +	{
> +	  if (do_rand_tests (i))
> +	    {

Likewise.

> +	      return 1;
> +	    }
> +
> +	  if (do_fill_tests (i, -1) || do_fill_tests (i, 1)
> +	      || do_fill_tests (i, 0x80) || do_fill_tests (i, 0x88))
> +	    {

Likewise.

> +	      return 1;
> +	    }
> +	}
> +    }
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>

On Mon, May 16, 2022 at 11:19 PM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>
> On 17/05/2022 02:00, Noah Goldstein via Libc-alpha wrote:
> > If we want to further optimize the functions tests are needed.
> > ---
> >   elf/Makefile      |   1 +
> >   elf/tst-dl-hash.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 148 insertions(+)
> >   create mode 100644 elf/tst-dl-hash.c
> >
> > diff --git a/elf/Makefile b/elf/Makefile
> > index fc9860edee..0e72f913a0 100644
> > --- a/elf/Makefile
> > +++ b/elf/Makefile
> > @@ -309,6 +309,7 @@ tests := \
> >     tst-array4 \
> >     tst-array5 \
> >     tst-auxv \
> > +  tst-dl-hash \
> >     tst-leaks1 \
> >     tst-stringtable \
> >     tst-tls9 \
> > diff --git a/elf/tst-dl-hash.c b/elf/tst-dl-hash.c
> > new file mode 100644
> > index 0000000000..e806a274ca
> > --- /dev/null
> > +++ b/elf/tst-dl-hash.c
> > @@ -0,0 +1,147 @@
> > +/* Copyright (C) 2022 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/>.  */
> > +/* Simple implementation of ELF ABI hash function. */
>
> The one line description is typically the first line at the top, just
> before the copyright notice.  And perhaps you want to call it "Test ELF
> ABI hash functions" or something similar :)

Fixed in V10.
>
> > +
> > +#include <dl-hash.h>
> > +#include <dl-new-hash.h>
> > +#include <support/support.h>
> > +#include <support/check.h>
> > +#include <stdio.h>
> > +#include <string.h>
> > +#include <stdlib.h>
> > +
> > +typedef unsigned int (*hash_f) (const char *);
> > +
> > +static unsigned int
> > +simple_dl_new_hash (const char *s)
> > +{
> > +  uint32_t h = 5381;
> > +  for (unsigned char c = *s; c != '\0'; c = *++s)
> > +    h = h * 33 + c;
> > +  return h;
> > +}
>
> Maybe just `#define dl_new_hash simple_dl_new_hash` and include
> elf/dl-new-hash.h here?  And then don't get rid of elf/dl-new-hash.h in
> 6/6, let it remain the reference implementation to test against.
> Perhaps also add a comment in that file stating that it is a reference
> implementation to test against and that sysdeps has the actual
> implementation that gets used, depending on the target.

Done in V10.
>
> > +
> > +static unsigned int
> > +simple_dl_elf_hash (const char *name_arg)
> > +{
> > +  unsigned long int hash = 0;
> > +  for (unsigned char c = *name_arg; c != '\0'; c = *(++name_arg))
> > +    {
> > +      unsigned long int hi;
> > +      hash = (hash << 4) + c;
> > +      hi = hash & 0xf0000000;
> > +      hash ^= hi >> 24;
> > +      hash &= 0x0fffffff;
> > +    }
> > +  return hash;
> > +}
>
> Likewise, add elf/dl-hash.h with this reference implementation.

Done in V10.
>
> > +static int
> > +do_fill_test (size_t len, int fill, const char *name, hash_f testf,
> > +           hash_f expecf)
> > +{
> > +  uint32_t expec, res;
> > +  char buf[len + 1];
> > +  memset (buf, fill, len);
> > +  buf[len] = '\0';
> > +
> > +  expec = expecf (buf);
> > +  res = testf (buf);
> > +  if (expec != res)
> > +    {
> > +      FAIL_EXIT1 ("FAIL: fill(%d) %s(%zu), %x != %x\n", fill, name, len, expec,
> > +               res);
> > +    }
> > +
> > +  return 0;
> > +}
> > +
> > +static int
> > +do_fill_tests (size_t len, int fill)
> > +{
> > +  if (do_fill_test (len, fill, "dl_new_hash", &_dl_new_hash,
> > +                 &simple_dl_new_hash))
> > +    {
>
> Redundant paranthesis.
Fixed in V10.
>
> > +      return 1;
> > +    }
> > +  return do_fill_test (len, fill, "dl_elf_hash", &_dl_elf_hash,
> > +                    &simple_dl_elf_hash);
> > +}
> > +
> > +static int
> > +do_rand_test (size_t len, const char *name, hash_f testf, hash_f expecf)
> > +{
> > +  uint32_t expec, res;
> > +  size_t i;
> > +  char buf[len + 1];
> > +  char v;
> > +  for (i = 0; i < len; ++i)
> > +    {
> > +      v = random ();
> > +      if (v == 0)
> > +     {
>
> Likewise.
Fixed in V10.
>
> > +       v = 1;
> > +     }
> > +      buf[i] = v;
> > +    }
> > +  buf[len] = '\0';
> > +
> > +  expec = expecf (buf);
> > +  res = testf (buf);
> > +  if (expec != res)
> > +    {
> > +      printf ("FAIL: random %s(%zu), %x != %x\n", name, len, expec, res);
> > +      return 1;
> > +    }
> > +
> > +  return 0;
> > +}
> > +
> > +static int
> > +do_rand_tests (size_t len)
> > +{
> > +  if (do_rand_test (len, "dl_new_hash", &_dl_new_hash, &simple_dl_new_hash))
> > +    {
>
> Likewise.
Fixed in V10.
>
> > +      return 1;
> > +    }
> > +  return do_rand_test (len, "dl_elf_hash", &_dl_elf_hash, &simple_dl_elf_hash);
> > +}
> > +
> > +static int
> > +do_test (void)
> > +{
> > +  size_t i, j;
> > +  for (i = 0; i < 100; ++i)
> > +    {
> > +      for (j = 0; j < 8192; ++j)
> > +     {
> > +       if (do_rand_tests (i))
> > +         {
>
> Likewise.
Fixed in V10.
>
> > +           return 1;
> > +         }
> > +
> > +       if (do_fill_tests (i, -1) || do_fill_tests (i, 1)
> > +           || do_fill_tests (i, 0x80) || do_fill_tests (i, 0x88))
> > +         {
>
> Likewise.
Fixed in V10.
>
> > +           return 1;
> > +         }
> > +     }
> > +    }
> > +  return 0;
> > +}
> > +
> > +#include <support/test-driver.c>
>