Add new tor_assert_nonfatal*() macros.

Unlike tor_assert(), these macros don't abort the process.  They're
good for checking conditions we want to warn about, but which don't
warrant a full crash.

This commit also changes the default implementation for
tor_fragile_assert() to tor_assert_nonfatal_unreached_once().

Closes ticket 18613.

diff --git a/changes/assert_nonfatal b/changes/assert_nonfatal
new file mode 100644 (file)
index 0000000..0cbee44
--- /dev/null
+++ b/changes/assert_nonfatal
@@ -0,0 +1,5 @@
+  o Minor features (safety, debugging):
+
+    * Add a set of macros to check nonfatal assertions, for internal
+      use. Migrating more of our checks to these should help us avoid
+      needless crash bugs. Closes ticket 18613.

diff --git a/src/common/util_bug.c b/src/common/util_bug.c
index 139d139e8c6d39ad4ed91fc807b6e57d7f8c170d..606c66516343beccec6968660e9a87e1f935c893 100644 (file)
--- a/src/common/util_bug.c
+++ b/src/common/util_bug.c
@@ -26,3 +26,27 @@ tor_assertion_failed_(const char *fname, unsigned int line,
   log_backtrace(LOG_ERR, LD_BUG, buf);
 }
 
+
+void
+tor_bug_occurred_(const char *fname, unsigned int line,
+                  const char *func, const char *expr,
+                  int once)
+{
+  char buf[256];
+  const char *once_str = once ?
+    " (Future instances of this warning will be silenced.)": "";
+  if (! expr) {
+    log_warn(LD_BUG, "%s:%u: %s: This line should not have been reached.%s",
+             fname, line, func, once_str);
+    tor_snprintf(buf, sizeof(buf),
+                 "Line unexpectedly reached at %s at %s:%u",
+                 func, fname, line);
+  } else {
+    log_warn(LD_BUG, "%s:%u: %s: Non-fatal assertion %s failed.%s",
+             fname, line, func, expr, once_str);
+    tor_snprintf(buf, sizeof(buf),
+                 "Non-fatal assertion %s failed in %s at %s:%u",
+                 expr, func, fname, line);
+  }
+  log_backtrace(LOG_WARN, LD_BUG, buf);
+}

diff --git a/src/common/util_bug.h b/src/common/util_bug.h
index 5414c53f4ba911d50be6109da842d23952dcff7e..ce54266b205591094b7ff75b717764d86fb5dd41 100644 (file)
--- a/src/common/util_bug.h
+++ b/src/common/util_bug.h
@@ -46,13 +46,58 @@
   } STMT_END
 #endif
 
+#define tor_assert_unreached() tor_assert(0)
+
+/* Non-fatal bug assertions. The "unreached" variants mean "this line should
+ * never be reached." The "once" variants mean "Don't log a warning more than
+ * once".
+ */
+
+#ifdef ALL_BUGS_ARE_FATAL
+#define tor_assert_nonfatal_unreached() tor_assert(0)
+#define tor_assert_nonfatal(cond) tor_assert((cond))
+#define tor_assert_nonfatal_unreached_once() tor_assert(0)
+#define tor_assert_nonfatal_once(cond) tor_assert((cond))
+#elif defined(TOR_UNIT_TESTS) && defined(DISABLE_ASSERTS_IN_UNIT_TESTS)
+#define tor_assert_nonfatal_unreached() STMT_NIL
+#define tor_assert_nonfatal(cond) ((void)(cond))
+#define tor_assert_nonfatal_unreached_once() STMT_NIL
+#define tor_assert_nonfatal_once(cond) ((void)(cond))
+#else /* Normal case, !ALL_BUGS_ARE_FATAL, !DISABLE_ASSERTS_IN_UNIT_TESTS */
+#define tor_assert_nonfatal_unreached() STMT_BEGIN                      \
+  tor_bug_occurred_(SHORT_FILE__, __LINE__, __func__, NULL, 0);         \
+  STMT_END
+#define tor_assert_nonfatal(cond) STMT_BEGIN                            \
+  if (PREDICT_UNLIKELY(!(cond))) {                                      \
+    tor_bug_occurred_(SHORT_FILE__, __LINE__, __func__, #cond, 0);  \
+  }                                                                     \
+  STMT_END
+#define tor_assert_nonfatal_unreached_once() STMT_BEGIN                 \
+  static int warning_logged__ = 0;                                      \
+  if (!warning_logged__) {                                              \
+    warning_logged__ = 1;                                               \
+    tor_bug_occurred_(SHORT_FILE__, __LINE__, __func__, NULL, 1);       \
+  }                                                                     \
+  STMT_END
+#define tor_assert_nonfatal_once(cond) STMT_BEGIN                       \
+  static int warning_logged__ = 0;                                      \
+  if (!warning_logged__ && PREDICT_UNLIKELY(!(cond))) {                 \
+    warning_logged__ = 1;                                               \
+    tor_bug_occurred_(SHORT_FILE__, __LINE__, __func__, #cond, 1);      \
+  }                                                                     \
+  STMT_END
+#endif
+
 /** Define this if you want Tor to crash when any problem comes up,
  * so you can get a coredump and track things down. */
-// #define tor_fragile_assert() tor_assert(0)
-#define tor_fragile_assert()
+// #define tor_fragile_assert() tor_assert_unreached(0)
+#define tor_fragile_assert() tor_assert_nonfatal_unreached_once()
 
 void tor_assertion_failed_(const char *fname, unsigned int line,
                            const char *func, const char *expr);
+void tor_bug_occurred_(const char *fname, unsigned int line,
+                       const char *func, const char *expr,
+                       int once);
 
 #endif
 

diff --git a/src/or/main.c b/src/or/main.c
index a2cf5b110173c80f8fbe9c3144e18ba8003f33df..7c5e6855eb73df03d7e0dee5a376ae89474772d9 100644 (file)
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -1643,8 +1643,8 @@ rotate_x509_certificate_callback(time_t now, const or_options_t *options)
    * TLS context. */
   log_info(LD_GENERAL,"Rotating tls context.");
   if (router_initialize_tls_context() < 0) {
-    log_warn(LD_BUG, "Error reinitializing TLS context");
-    tor_assert(0);
+    log_err(LD_BUG, "Error reinitializing TLS context");
+    tor_assert_unreached();
   }
 
   /* We also make sure to rotate the TLS connections themselves if they've
@@ -2563,9 +2563,7 @@ run_main_loop_once(void)
         return -1;
 #endif
     } else {
-      if (ERRNO_IS_EINPROGRESS(e))
-        log_warn(LD_BUG,
-                 "libevent call returned EINPROGRESS? Please report.");
+      tor_assert_nonfatal_once(! ERRNO_IS_EINPROGRESS(e));
       log_debug(LD_NET,"libevent call interrupted.");
       /* You can't trust the results of this poll(). Go back to the
        * top of the big for loop. */

diff --git a/src/or/policies.c b/src/or/policies.c
index f9718b6a9514baeebbd694099091009ef8101558..2703d7edefd0c94ff656693ac3722e7f871996cb 100644 (file)
--- a/src/or/policies.c
+++ b/src/or/policies.c
@@ -103,7 +103,7 @@ policy_expand_private(smartlist_t **policy)
        if (tor_addr_parse_mask_ports(private_nets[i], 0,
                                &newpolicy.addr,
                                &newpolicy.maskbits, &port_min, &port_max)<0) {
-         tor_assert(0);
+         tor_assert_unreached();
        }
        smartlist_add(tmp, addr_policy_get_canonical_entry(&newpolicy));
      }

diff --git a/src/or/rephist.c b/src/or/rephist.c
index fe0ca91c25e177c5d778a1ebf512eb1e8a900f1a..b94ad2965066bea15603517d820c7dc565d97292 100644 (file)
--- a/src/or/rephist.c
+++ b/src/or/rephist.c
@@ -3214,7 +3214,7 @@ rep_hist_free_all(void)
   rep_hist_desc_stats_term();
   total_descriptor_downloads = 0;
 
-  tor_assert(rephist_total_alloc == 0);
-  tor_assert(rephist_total_num == 0);
+  tor_assert_nonfatal(rephist_total_alloc == 0);
+  tor_assert_nonfatal_once(rephist_total_num == 0);
 }