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)

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)