Remove statistics tracking from LHASH

Fixes #17928. Supercedes #17931.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/17935)

diff --git a/CHANGES.md b/CHANGES.md
index a343db2d50b52ccd3267d89d2f4241b769fcc050..d82d857782bd352ba2abf80b757b4874eb6cf4ed 100644 (file)
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -114,6 +114,15 @@ breaking changes, and mappings for the large list of deprecated functions.
 
 [Migration guide]: https://github.com/openssl/openssl/tree/master/doc/man7/migration_guide.pod
 
+### Changes between 3.0.2 and 3.0.3
+
+ * The functions `OPENSSL_LH_stats` and `OPENSSL_LH_stats_bio` now only report
+   the `num_items`, `num_nodes` and `num_alloc_nodes` statistics. All other
+   statistics are no longer supported. For compatibility, these statistics are
+   still listed in the output but are now always reported as zero.
+
+   *Hugo Landau*
+
 ### Changes between 3.0.1 and 3.0.2 [15 mar 2022]
 
  * Fixed a bug in the BN_mod_sqrt() function that can cause it to loop forever

diff --git a/crypto/lhash/lh_stats.c b/crypto/lhash/lh_stats.c
index 0d4bc72608d8ce8770efe1f997d3723f13b5bd9f..8a49bfab347e104d71720a91cfc8bd3024ffe200 100644 (file)
--- a/crypto/lhash/lh_stats.c
+++ b/crypto/lhash/lh_stats.c
@@ -61,37 +61,22 @@ void OPENSSL_LH_node_usage_stats(const OPENSSL_LHASH *lh, FILE *fp)
 
 void OPENSSL_LH_stats_bio(const OPENSSL_LHASH *lh, BIO *out)
 {
-    int omit_tsan = 0;
-
-#ifdef TSAN_REQUIRES_LOCKING
-    if (!CRYPTO_THREAD_read_lock(lh->tsan_lock)) {
-        BIO_printf(out, "unable to lock table, omitting TSAN counters\n");
-        omit_tsan = 1;
-    }
-#endif
     BIO_printf(out, "num_items             = %lu\n", lh->num_items);
     BIO_printf(out, "num_nodes             = %u\n",  lh->num_nodes);
     BIO_printf(out, "num_alloc_nodes       = %u\n",  lh->num_alloc_nodes);
-    BIO_printf(out, "num_expands           = %lu\n", lh->num_expands);
-    BIO_printf(out, "num_expand_reallocs   = %lu\n", lh->num_expand_reallocs);
-    BIO_printf(out, "num_contracts         = %lu\n", lh->num_contracts);
-    BIO_printf(out, "num_contract_reallocs = %lu\n", lh->num_contract_reallocs);
-    if (!omit_tsan) {
-        BIO_printf(out, "num_hash_calls        = %lu\n", lh->num_hash_calls);
-        BIO_printf(out, "num_comp_calls        = %lu\n", lh->num_comp_calls);
-    }
-    BIO_printf(out, "num_insert            = %lu\n", lh->num_insert);
-    BIO_printf(out, "num_replace           = %lu\n", lh->num_replace);
-    BIO_printf(out, "num_delete            = %lu\n", lh->num_delete);
-    BIO_printf(out, "num_no_delete         = %lu\n", lh->num_no_delete);
-    if (!omit_tsan) {
-        BIO_printf(out, "num_retrieve          = %lu\n", lh->num_retrieve);
-        BIO_printf(out, "num_retrieve_miss     = %lu\n", lh->num_retrieve_miss);
-        BIO_printf(out, "num_hash_comps        = %lu\n", lh->num_hash_comps);
-#ifdef TSAN_REQUIRES_LOCKING
-        CRYPTO_THREAD_unlock(lh->tsan_lock);
-#endif
-    }
+    BIO_printf(out, "num_expands           = 0\n");
+    BIO_printf(out, "num_expand_reallocs   = 0\n");
+    BIO_printf(out, "num_contracts         = 0\n");
+    BIO_printf(out, "num_contract_reallocs = 0\n");
+    BIO_printf(out, "num_hash_calls        = 0\n");
+    BIO_printf(out, "num_comp_calls        = 0\n");
+    BIO_printf(out, "num_insert            = 0\n");
+    BIO_printf(out, "num_replace           = 0\n");
+    BIO_printf(out, "num_delete            = 0\n");
+    BIO_printf(out, "num_no_delete         = 0\n");
+    BIO_printf(out, "num_retrieve          = 0\n");
+    BIO_printf(out, "num_retrieve_miss     = 0\n");
+    BIO_printf(out, "num_hash_comps        = 0\n");
 }
 
 void OPENSSL_LH_node_stats_bio(const OPENSSL_LHASH *lh, BIO *out)

diff --git a/crypto/lhash/lhash.c b/crypto/lhash/lhash.c
index b99b63fd6aabd1b97eb3e31df6b11c427258a52c..2a574fbfe6aa82fd6afe13f04eb598ee5d2d8aee 100644 (file)
--- a/crypto/lhash/lhash.c
+++ b/crypto/lhash/lhash.c
@@ -44,22 +44,6 @@ static int expand(OPENSSL_LHASH *lh);
 static void contract(OPENSSL_LHASH *lh);
 static OPENSSL_LH_NODE **getrn(OPENSSL_LHASH *lh, const void *data, unsigned long *rhash);
 
-static ossl_inline int tsan_lock(const OPENSSL_LHASH *lh)
-{
-#ifdef TSAN_REQUIRES_LOCKING
-    if (!CRYPTO_THREAD_write_lock(lh->tsan_lock))
-        return 0;
-#endif
-    return 1;
-}
-
-static ossl_inline void tsan_unlock(const OPENSSL_LHASH *lh)
-{
-#ifdef TSAN_REQUIRES_LOCKING
-    CRYPTO_THREAD_unlock(lh->tsan_lock);
-#endif
-}
-
 OPENSSL_LHASH *OPENSSL_LH_new(OPENSSL_LH_HASHFUNC h, OPENSSL_LH_COMPFUNC c)
 {
     OPENSSL_LHASH *ret;
@@ -74,10 +58,6 @@ OPENSSL_LHASH *OPENSSL_LH_new(OPENSSL_LH_HASHFUNC h, OPENSSL_LH_COMPFUNC c)
     }
     if ((ret->b = OPENSSL_zalloc(sizeof(*ret->b) * MIN_NODES)) == NULL)
         goto err;
-#ifdef TSAN_REQUIRES_LOCKING
-    if ((ret->tsan_lock = CRYPTO_THREAD_lock_new()) == NULL)
-        goto err;
-#endif
     ret->comp = ((c == NULL) ? (OPENSSL_LH_COMPFUNC)strcmp : c);
     ret->hash = ((h == NULL) ? (OPENSSL_LH_HASHFUNC)OPENSSL_LH_strhash : h);
     ret->num_nodes = MIN_NODES / 2;
@@ -99,9 +79,6 @@ void OPENSSL_LH_free(OPENSSL_LHASH *lh)
         return;
 
     OPENSSL_LH_flush(lh);
-#ifdef TSAN_REQUIRES_LOCKING
-    CRYPTO_THREAD_lock_free(lh->tsan_lock);
-#endif
     OPENSSL_free(lh->b);
     OPENSSL_free(lh);
 }
@@ -147,12 +124,10 @@ void *OPENSSL_LH_insert(OPENSSL_LHASH *lh, void *data)
         nn->hash = hash;
         *rn = nn;
         ret = NULL;
-        lh->num_insert++;
         lh->num_items++;
     } else {                    /* replace same key */
         ret = (*rn)->data;
         (*rn)->data = data;
-        lh->num_replace++;
     }
     return ret;
 }
@@ -167,14 +142,12 @@ void *OPENSSL_LH_delete(OPENSSL_LHASH *lh, const void *data)
     rn = getrn(lh, data, &hash);
 
     if (*rn == NULL) {
-        lh->num_no_delete++;
         return NULL;
     } else {
         nn = *rn;
         *rn = nn->next;
         ret = nn->data;
         OPENSSL_free(nn);
-        lh->num_delete++;
     }
 
     lh->num_items--;
@@ -190,18 +163,11 @@ void *OPENSSL_LH_retrieve(OPENSSL_LHASH *lh, const void *data)
     unsigned long hash;
     OPENSSL_LH_NODE **rn;
 
-    /*-
-     * This should be atomic without tsan.
-     * It's not clear why it was done this way and not elsewhere.
-     */
-    tsan_store((TSAN_QUALIFIER int *)&lh->error, 0);
+    if (lh->error != 0)
+        lh->error = 0;
 
     rn = getrn(lh, data, &hash);
 
-    if (tsan_lock(lh)) {
-        tsan_counter(*rn == NULL ? &lh->num_retrieve_miss : &lh->num_retrieve);
-        tsan_unlock(lh);
-    }
     return *rn == NULL ? NULL : (*rn)->data;
 }
 
@@ -262,14 +228,12 @@ static int expand(OPENSSL_LHASH *lh)
         memset(n + nni, 0, sizeof(*n) * (j - nni));
         lh->pmax = nni;
         lh->num_alloc_nodes = j;
-        lh->num_expand_reallocs++;
         lh->p = 0;
     } else {
         lh->p++;
     }
 
     lh->num_nodes++;
-    lh->num_expands++;
     n1 = &(lh->b[p]);
     n2 = &(lh->b[p + pmax]);
     *n2 = NULL;
@@ -302,7 +266,6 @@ static void contract(OPENSSL_LHASH *lh)
             lh->error++;
             return;
         }
-        lh->num_contract_reallocs++;
         lh->num_alloc_nodes /= 2;
         lh->pmax /= 2;
         lh->p = lh->pmax - 1;
@@ -311,7 +274,6 @@ static void contract(OPENSSL_LHASH *lh)
         lh->p--;
 
     lh->num_nodes--;
-    lh->num_contracts++;
 
     n1 = lh->b[(int)lh->p];
     if (n1 == NULL)
@@ -329,14 +291,8 @@ static OPENSSL_LH_NODE **getrn(OPENSSL_LHASH *lh,
     OPENSSL_LH_NODE **ret, *n1;
     unsigned long hash, nn;
     OPENSSL_LH_COMPFUNC cf;
-    int do_tsan = 1;
 
-#ifdef TSAN_REQUIRES_LOCKING
-    do_tsan = tsan_lock(lh);
-#endif
     hash = (*(lh->hash)) (data);
-    if (do_tsan)
-        tsan_counter(&lh->num_hash_calls);
     *rhash = hash;
 
     nn = hash % lh->pmax;
@@ -346,20 +302,14 @@ static OPENSSL_LH_NODE **getrn(OPENSSL_LHASH *lh,
     cf = lh->comp;
     ret = &(lh->b[(int)nn]);
     for (n1 = *ret; n1 != NULL; n1 = n1->next) {
-        if (do_tsan)
-            tsan_counter(&lh->num_hash_comps);
         if (n1->hash != hash) {
             ret = &(n1->next);
             continue;
         }
-        if (do_tsan)
-            tsan_counter(&lh->num_comp_calls);
         if (cf(n1->data, data) == 0)
             break;
         ret = &(n1->next);
     }
-    if (do_tsan)
-        tsan_unlock(lh);
     return ret;
 }
 

diff --git a/crypto/lhash/lhash_local.h b/crypto/lhash/lhash_local.h
index 35c0c5b49c919ed6a6a351e8455a39cab08c42bf..5edd8e6c6627e607b4aa418983ae915e9cd90184 100644 (file)
--- a/crypto/lhash/lhash_local.h
+++ b/crypto/lhash/lhash_local.h
@@ -27,21 +27,5 @@ struct lhash_st {
     unsigned long up_load;      /* load times 256 */
     unsigned long down_load;    /* load times 256 */
     unsigned long num_items;
-    unsigned long num_expands;
-    unsigned long num_expand_reallocs;
-    unsigned long num_contracts;
-    unsigned long num_contract_reallocs;
-    TSAN_QUALIFIER unsigned long num_hash_calls;
-    TSAN_QUALIFIER unsigned long num_comp_calls;
-    unsigned long num_insert;
-    unsigned long num_replace;
-    unsigned long num_delete;
-    unsigned long num_no_delete;
-    TSAN_QUALIFIER unsigned long num_retrieve;
-    TSAN_QUALIFIER unsigned long num_retrieve_miss;
-    TSAN_QUALIFIER unsigned long num_hash_comps;
     int error;
-#ifdef TSAN_REQUIRES_LOCKING
-    CRYPTO_RWLOCK *tsan_lock;
-#endif
 };

diff --git a/doc/man3/OPENSSL_LH_stats.pod b/doc/man3/OPENSSL_LH_stats.pod
index a3f85700b46e4ac81861953c42752ab6f6dcef6c..5b2559911ccf98a9356ae72a1de38edb577f53e3 100644 (file)
--- a/doc/man3/OPENSSL_LH_stats.pod
+++ b/doc/man3/OPENSSL_LH_stats.pod
@@ -23,9 +23,10 @@ OPENSSL_LH_node_stats_bio, OPENSSL_LH_node_usage_stats_bio - LHASH statistics
 The B<LHASH> structure records statistics about most aspects of
 accessing the hash table.
 
-OPENSSL_LH_stats() prints out statistics on the size of the hash table, how
-many entries are in it, and the number and result of calls to the
-routines in this library.
+OPENSSL_LH_stats() prints out statistics on the size of the hash table and how
+many entries are in it. For historical reasons, this function also outputs a
+number of additional statistics, but the tracking of these statistics is no
+longer supported and these statistics are always reported as zero.
 
 OPENSSL_LH_node_stats() prints the number of entries for each 'bucket' in the
 hash table.