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 a343db2..d82d857 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -114,6 +114,15 @@
[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 0d4bc72..8a49bfa 100644
--- a/crypto/lhash/lh_stats.c
+++ b/crypto/lhash/lh_stats.c
@@ -61,37 +61,22 @@
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 b99b63f..2a574fb 100644
--- a/crypto/lhash/lhash.c
+++ b/crypto/lhash/lhash.c
@@ -44,22 +44,6 @@
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 @@
}
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
lh->error++;
return;
}
- lh->num_contract_reallocs++;
lh->num_alloc_nodes /= 2;
lh->pmax /= 2;
lh->p = lh->pmax - 1;
@@ -311,7 +274,6 @@
lh->p--;
lh->num_nodes--;
- lh->num_contracts++;
n1 = lh->b[(int)lh->p];
if (n1 == NULL)
@@ -329,14 +291,8 @@
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 @@
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 35c0c5b..5edd8e6 100644
--- a/crypto/lhash/lhash_local.h
+++ b/crypto/lhash/lhash_local.h
@@ -27,21 +27,5 @@
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 a3f8570..5b25599 100644
--- a/doc/man3/OPENSSL_LH_stats.pod
+++ b/doc/man3/OPENSSL_LH_stats.pod
@@ -23,9 +23,10 @@
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.
