diff options
author | Rich Salz <rsalz@akamai.com> | 2021-05-19 11:09:49 -0400 |
---|---|---|
committer | Richard Levitte <levitte@openssl.org> | 2021-05-28 10:04:31 +0200 |
commit | a935791d54078f43209ffbc1886ac5e68772ce34 (patch) | |
tree | 3f130fe3e12b6671584ff421984542579d5268a8 /crypto | |
parent | 6bf3692d311ad15d3667e7015bbe1a8f849f3c7b (diff) |
Rework and make DEBUG macros consistent.
Remove unused -DCONF_DEBUG and -DBN_CTX_DEBUG.
Rename REF_PRINT to REF_DEBUG for consistency, and add a new
tracing category and use it for printing reference counts.
Rename -DDEBUG_UNUSED to -DUNUSED_RESULT_DEBUG
Fix BN_DEBUG_RAND so it compiles and, when set, force DEBUG_RAND to
be set also.
Rename engine_debug_ref to be ENGINE_REF_PRINT also for consistency.
Fixes #15357
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15353)
Diffstat (limited to 'crypto')
-rw-r--r-- | crypto/asn1/tasn_utl.c | 4 | ||||
-rw-r--r-- | crypto/bio/bio_local.h | 2 | ||||
-rw-r--r-- | crypto/bn/bn_exp.c | 4 | ||||
-rw-r--r-- | crypto/bn/bn_gf2m.c | 4 | ||||
-rw-r--r-- | crypto/bn/bn_local.h | 25 | ||||
-rw-r--r-- | crypto/ec/ecp_nistp224.c | 2 | ||||
-rw-r--r-- | crypto/ec/ecp_nistp256.c | 2 | ||||
-rw-r--r-- | crypto/ec/ecp_nistp521.c | 2 | ||||
-rw-r--r-- | crypto/engine/eng_init.c | 6 | ||||
-rw-r--r-- | crypto/engine/eng_lib.c | 4 | ||||
-rw-r--r-- | crypto/engine/eng_list.c | 12 | ||||
-rw-r--r-- | crypto/engine/eng_local.h | 12 | ||||
-rw-r--r-- | crypto/engine/tb_asnmth.c | 2 | ||||
-rw-r--r-- | crypto/trace.c | 2 |
14 files changed, 44 insertions, 39 deletions
diff --git a/crypto/asn1/tasn_utl.c b/crypto/asn1/tasn_utl.c index 28a4b23aa4..e5f25d88df 100644 --- a/crypto/asn1/tasn_utl.c +++ b/crypto/asn1/tasn_utl.c @@ -97,9 +97,7 @@ int ossl_asn1_do_lock(ASN1_VALUE **pval, int op, const ASN1_ITEM *it) case -1: if (!CRYPTO_DOWN_REF(lck, &ret, *lock)) return -1; /* failed */ -#ifdef REF_PRINT - fprintf(stderr, "%p:%4d:%s\n", (void*)it, ret, it->sname); -#endif + REF_PRINT_EX(it->sname, ret, (void *)it); REF_ASSERT_ISNT(ret < 0); if (ret == 0) { CRYPTO_THREAD_lock_free(*lock); diff --git a/crypto/bio/bio_local.h b/crypto/bio/bio_local.h index 134ad748a1..749e8f810c 100644 --- a/crypto/bio/bio_local.h +++ b/crypto/bio/bio_local.h @@ -9,7 +9,6 @@ #include "e_os.h" #include "internal/sockets.h" -#include "internal/refcount.h" /* BEGIN BIO_ADDRINFO/BIO_ADDR stuff. */ @@ -88,6 +87,7 @@ union bio_addr_st { #include "internal/cryptlib.h" #include "internal/bio.h" +#include "internal/refcount.h" typedef struct bio_f_buffer_ctx_struct { /*- diff --git a/crypto/bn/bn_exp.c b/crypto/bn/bn_exp.c index 12dd6d554c..2ee85aa0cf 100644 --- a/crypto/bn/bn_exp.c +++ b/crypto/bn/bn_exp.c @@ -819,7 +819,7 @@ int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p, /* * BN_to_montgomery can contaminate words above .top [in - * BN_DEBUG[_DEBUG] build]... + * BN_DEBUG build... */ for (i = am.top; i < top; i++) am.d[i] = 0; @@ -924,7 +924,7 @@ int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p, /* * BN_to_montgomery can contaminate words above .top [in - * BN_DEBUG[_DEBUG] build]... + * BN_DEBUG build... */ for (i = am.top; i < top; i++) am.d[i] = 0; diff --git a/crypto/bn/bn_gf2m.c b/crypto/bn/bn_gf2m.c index c9524eaece..82aad3f599 100644 --- a/crypto/bn/bn_gf2m.c +++ b/crypto/bn/bn_gf2m.c @@ -715,8 +715,8 @@ static int BN_GF2m_mod_inv_vartime(BIGNUM *r, const BIGNUM *a, ret = 1; err: -# ifdef BN_DEBUG /* BN_CTX_end would complain about the - * expanded form */ +# ifdef BN_DEBUG + /* BN_CTX_end would complain about the expanded form */ bn_correct_top(c); bn_correct_top(u); bn_correct_top(v); diff --git a/crypto/bn/bn_local.h b/crypto/bn/bn_local.h index d9e9977291..3c8534e1f7 100644 --- a/crypto/bn/bn_local.h +++ b/crypto/bn/bn_local.h @@ -28,14 +28,19 @@ /* * These preprocessor symbols control various aspects of the bignum headers * and library code. They're not defined by any "normal" configuration, as - * they are intended for development and testing purposes. NB: defining all - * three can be useful for debugging application code as well as openssl + * they are intended for development and testing purposes. NB: defining + * them can be useful for debugging application code as well as openssl * itself. BN_DEBUG - turn on various debugging alterations to the bignum - * code BN_DEBUG_RAND - uses random poisoning of unused words to trip up - * mismanagement of bignum internals. You must also define BN_DEBUG. + * code BN_RAND_DEBUG - uses random poisoning of unused words to trip up + * mismanagement of bignum internals. Enable BN_RAND_DEBUG is known to + * break some of the OpenSSL tests. */ -/* #define BN_DEBUG */ -/* #define BN_DEBUG_RAND */ +# if defined(BN_RAND_DEBUG) && !defined(BN_DEBUG) +# define BN_DEBUG +# endif +# if defined(BN_RAND_DEBUG) +# include <openssl/rand.h> +# endif # ifndef OPENSSL_SMALL_FOOTPRINT # define BN_MUL_COMBA @@ -127,7 +132,7 @@ * bn_check_top() is as before. * - if BN_DEBUG *is* defined; * - bn_check_top() tries to pollute unused words even if the bignum 'top' is - * consistent. (ed: only if BN_DEBUG_RAND is defined) + * consistent. (ed: only if BN_RAND_DEBUG is defined) * - bn_fix_top() maps to bn_check_top() rather than "fixing" anything. * The idea is to have debug builds flag up inconsistent bignums when they * occur. If that occurs in a bn_fix_top(), we examine the code in question; if @@ -153,7 +158,7 @@ * all operations manipulating the bit in question in non-BN_DEBUG build. */ # define BN_FLG_FIXED_TOP 0x10000 -# ifdef BN_DEBUG_RAND +# ifdef BN_RAND_DEBUG # define bn_pollute(a) \ do { \ const BIGNUM *_bnum1 = (a); \ @@ -164,7 +169,7 @@ * wouldn't be constructed with top!=dmax. */ \ BN_ULONG *_not_const; \ memcpy(&_not_const, &_bnum1->d, sizeof(_not_const)); \ - RAND_bytes(&_tmp_char, 1); /* Debug only - safe to ignore error return */\ + (void)RAND_bytes(&_tmp_char, 1); /* Debug only - safe to ignore error return */\ memset(_not_const + _bnum1->top, _tmp_char, \ sizeof(*_not_const) * (_bnum1->dmax - _bnum1->top)); \ } \ @@ -451,7 +456,7 @@ unsigned __int64 _umul128(unsigned __int64 a, unsigned __int64 b, # endif /* cpu */ # endif /* OPENSSL_NO_ASM */ -# ifdef BN_DEBUG_RAND +# ifdef BN_RAND_DEBUG # define bn_clear_top2max(a) \ { \ int ind = (a)->dmax - (a)->top; \ diff --git a/crypto/ec/ecp_nistp224.c b/crypto/ec/ecp_nistp224.c index 47f33825ad..5ab0dd7bef 100644 --- a/crypto/ec/ecp_nistp224.c +++ b/crypto/ec/ecp_nistp224.c @@ -1270,7 +1270,7 @@ void EC_nistp224_pre_comp_free(NISTP224_PRE_COMP *p) return; CRYPTO_DOWN_REF(&p->references, &i, p->lock); - REF_PRINT_COUNT("EC_nistp224", x); + REF_PRINT_COUNT("EC_nistp224", p); if (i > 0) return; REF_ASSERT_ISNT(i < 0); diff --git a/crypto/ec/ecp_nistp256.c b/crypto/ec/ecp_nistp256.c index 67d2dce9b7..4a55f925c4 100644 --- a/crypto/ec/ecp_nistp256.c +++ b/crypto/ec/ecp_nistp256.c @@ -1881,7 +1881,7 @@ void EC_nistp256_pre_comp_free(NISTP256_PRE_COMP *pre) return; CRYPTO_DOWN_REF(&pre->references, &i, pre->lock); - REF_PRINT_COUNT("EC_nistp256", x); + REF_PRINT_COUNT("EC_nistp256", pre); if (i > 0) return; REF_ASSERT_ISNT(i < 0); diff --git a/crypto/ec/ecp_nistp521.c b/crypto/ec/ecp_nistp521.c index 694031b45d..c449b93a0a 100644 --- a/crypto/ec/ecp_nistp521.c +++ b/crypto/ec/ecp_nistp521.c @@ -1723,7 +1723,7 @@ void EC_nistp521_pre_comp_free(NISTP521_PRE_COMP *p) return; CRYPTO_DOWN_REF(&p->references, &i, p->lock); - REF_PRINT_COUNT("EC_nistp521", x); + REF_PRINT_COUNT("EC_nistp521", p); if (i > 0) return; REF_ASSERT_ISNT(i < 0); diff --git a/crypto/engine/eng_init.c b/crypto/engine/eng_init.c index 20147a5263..c204eb1899 100644 --- a/crypto/engine/eng_init.c +++ b/crypto/engine/eng_init.c @@ -34,8 +34,8 @@ int engine_unlocked_init(ENGINE *e) */ e->struct_ref++; e->funct_ref++; - engine_ref_debug(e, 0, 1); - engine_ref_debug(e, 1, 1); + ENGINE_REF_PRINT(e, 0, 1); + ENGINE_REF_PRINT(e, 1, 1); } return to_return; } @@ -57,7 +57,7 @@ int engine_unlocked_finish(ENGINE *e, int unlock_for_handlers) * to 0 without either calling finish(). */ e->funct_ref--; - engine_ref_debug(e, 1, -1); + ENGINE_REF_PRINT(e, 1, -1); if ((e->funct_ref == 0) && e->finish) { if (unlock_for_handlers) CRYPTO_THREAD_unlock(global_engine_lock); diff --git a/crypto/engine/eng_lib.c b/crypto/engine/eng_lib.c index a022fc1d54..44e997e77b 100644 --- a/crypto/engine/eng_lib.c +++ b/crypto/engine/eng_lib.c @@ -34,7 +34,7 @@ ENGINE *ENGINE_new(void) return NULL; } ret->struct_ref = 1; - engine_ref_debug(ret, 0, 1); + ENGINE_REF_PRINT(ret, 0, 1); if (!CRYPTO_new_ex_data(CRYPTO_EX_INDEX_ENGINE, ret, &ret->ex_data)) { OPENSSL_free(ret); return NULL; @@ -77,7 +77,7 @@ int engine_free_util(ENGINE *e, int not_locked) CRYPTO_DOWN_REF(&e->struct_ref, &i, global_engine_lock); else i = --e->struct_ref; - engine_ref_debug(e, 0, -1); + ENGINE_REF_PRINT(e, 0, -1); if (i > 0) return 1; REF_ASSERT_ISNT(i < 0); diff --git a/crypto/engine/eng_list.c b/crypto/engine/eng_list.c index 7ea1ef675a..fec0ef7129 100644 --- a/crypto/engine/eng_list.c +++ b/crypto/engine/eng_list.c @@ -91,7 +91,7 @@ static int engine_list_add(ENGINE *e) * Having the engine in the list assumes a structural reference. */ e->struct_ref++; - engine_ref_debug(e, 0, 1); + ENGINE_REF_PRINT(e, 0, 1); /* However it came to be, e is the last item in the list. */ engine_list_tail = e; e->next = NULL; @@ -143,7 +143,7 @@ ENGINE *ENGINE_get_first(void) ret = engine_list_head; if (ret) { ret->struct_ref++; - engine_ref_debug(ret, 0, 1); + ENGINE_REF_PRINT(ret, 0, 1); } CRYPTO_THREAD_unlock(global_engine_lock); return ret; @@ -163,7 +163,7 @@ ENGINE *ENGINE_get_last(void) ret = engine_list_tail; if (ret) { ret->struct_ref++; - engine_ref_debug(ret, 0, 1); + ENGINE_REF_PRINT(ret, 0, 1); } CRYPTO_THREAD_unlock(global_engine_lock); return ret; @@ -183,7 +183,7 @@ ENGINE *ENGINE_get_next(ENGINE *e) if (ret) { /* Return a valid structural reference to the next ENGINE */ ret->struct_ref++; - engine_ref_debug(ret, 0, 1); + ENGINE_REF_PRINT(ret, 0, 1); } CRYPTO_THREAD_unlock(global_engine_lock); /* Release the structural reference to the previous ENGINE */ @@ -204,7 +204,7 @@ ENGINE *ENGINE_get_prev(ENGINE *e) if (ret) { /* Return a valid structural reference to the next ENGINE */ ret->struct_ref++; - engine_ref_debug(ret, 0, 1); + ENGINE_REF_PRINT(ret, 0, 1); } CRYPTO_THREAD_unlock(global_engine_lock); /* Release the structural reference to the previous ENGINE */ @@ -316,7 +316,7 @@ ENGINE *ENGINE_by_id(const char *id) } } else { iterator->struct_ref++; - engine_ref_debug(iterator, 0, 1); + ENGINE_REF_PRINT(iterator, 0, 1); } } CRYPTO_THREAD_unlock(global_engine_lock); diff --git a/crypto/engine/eng_local.h b/crypto/engine/eng_local.h index ef96a5769f..9f5840e837 100644 --- a/crypto/engine/eng_local.h +++ b/crypto/engine/eng_local.h @@ -20,14 +20,14 @@ extern CRYPTO_RWLOCK *global_engine_lock; /* - * This prints the engine's pointer address (truncated to unsigned int), - * "struct" or "funct" to indicate the reference type, the before and after - * reference count, and the file:line-number pair. The "engine_ref_debug" - * statements must come *after* the change. + * This prints the engine's pointer address, "struct" or "funct" to + * indicate the reference type, the before and after reference count, and + * the file:line-number pair. The "ENGINE_REF_PRINT" statements must come + * *after* the change. */ -# define engine_ref_debug(e, isfunct, diff) \ +# define ENGINE_REF_PRINT(e, isfunct, diff) \ OSSL_TRACE6(ENGINE_REF_COUNT, \ - "engine: %p %s from %d to %d (%s:%d)\n", \ + "engine: %p %s from %d to %d (%s:%d)\n", \ (void *)(e), (isfunct ? "funct" : "struct"), \ ((isfunct) \ ? ((e)->funct_ref - (diff)) \ diff --git a/crypto/engine/tb_asnmth.c b/crypto/engine/tb_asnmth.c index fffd4888f9..e3a5c82e99 100644 --- a/crypto/engine/tb_asnmth.c +++ b/crypto/engine/tb_asnmth.c @@ -206,7 +206,7 @@ const EVP_PKEY_ASN1_METHOD *ENGINE_pkey_asn1_find_str(ENGINE **pe, /* If found obtain a structural reference to engine */ if (fstr.e) { fstr.e->struct_ref++; - engine_ref_debug(fstr.e, 0, 1); + ENGINE_REF_PRINT(fstr.e, 0, 1); } *pe = fstr.e; CRYPTO_THREAD_unlock(global_engine_lock); diff --git a/crypto/trace.c b/crypto/trace.c index 0c3a90b002..f012b617ab 100644 --- a/crypto/trace.c +++ b/crypto/trace.c @@ -16,6 +16,7 @@ #include <openssl/trace.h> #include "internal/bio.h" #include "internal/nelem.h" +#include "internal/refcount.h" #include "crypto/cryptlib.h" #include "e_os.h" /* strcasecmp for Windows */ @@ -138,6 +139,7 @@ static const struct trace_category_st trace_categories[] = { TRACE_CATEGORY_(STORE), TRACE_CATEGORY_(DECODER), TRACE_CATEGORY_(ENCODER), + TRACE_CATEGORY_(REF_COUNT) }; const char *OSSL_trace_get_category_name(int num) |