Enable -Wnull-dereference (GCC >=6.1), and fix the easy cases

This warning, IIUC, means that the compiler doesn't like it when it
sees a NULL check _after_ we've already dereferenced the
variable. In such cases, it considers itself free to eliminate the
NULL check.

There are a couple of tricky cases:

One was the case related to the fact that tor_addr_to_in6() can
return NULL if it gets a non-AF_INET6 address.  The fix was to
create a variant which asserts on the address type, and never
returns NULL.

8 files changed, 30 insertions, 9 deletions

diff --git a/configure.ac b/configure.ac
index ec56e022cd..4878d2ba7e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1776,6 +1776,7 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([], [
 
  if test "x$have_gcc6" = "xyes"; then
      CFLAGS="$CFLAGS -Wignored-attributes -Wshift-negative-value -Wshift-overflow=2"
+     CFLAGS="$CFLAGS -Wnull-dereference"
   fi
 
   if test "x$have_shorten64_flag" = "xyes"; then
diff --git a/src/common/address.c b/src/common/address.c
index a6e0f3f491..759b20a094 100644
--- a/src/common/address.c
+++ b/src/common/address.c
@@ -131,7 +131,8 @@ tor_addr_to_sockaddr(const tor_addr_t *a,
 #endif
     sin6->sin6_family = AF_INET6;
     sin6->sin6_port = htons(port);
-    memcpy(&sin6->sin6_addr, tor_addr_to_in6(a), sizeof(struct in6_addr));
+    memcpy(&sin6->sin6_addr, tor_addr_to_in6_assert(a),
+           sizeof(struct in6_addr));
     return sizeof(struct sockaddr_in6);
   } else {
     return 0;
diff --git a/src/common/address.h b/src/common/address.h
index 3de67e1c74..3f0bb521cd 100644
--- a/src/common/address.h
+++ b/src/common/address.h
@@ -74,6 +74,7 @@ typedef struct tor_addr_port_t
 #define TOR_ADDR_NULL {AF_UNSPEC, {0}}
 
 static inline const struct in6_addr *tor_addr_to_in6(const tor_addr_t *a);
+static inline const struct in6_addr *tor_addr_to_in6_assert(const tor_addr_t *a);
 static inline uint32_t tor_addr_to_ipv4n(const tor_addr_t *a);
 static inline uint32_t tor_addr_to_ipv4h(const tor_addr_t *a);
 static inline uint32_t tor_addr_to_mapped_ipv4h(const tor_addr_t *a);
@@ -97,21 +98,30 @@ tor_addr_to_in6(const tor_addr_t *a)
   return a->family == AF_INET6 ? &a->addr.in6_addr : NULL;
 }
 
+/** As tor_addr_to_in6, but assert that the address truly is an IPv6 address. */
+static inline const struct in6_addr *
+tor_addr_to_in6_assert(const tor_addr_t *a)
+{
+  tor_assert(a->family == AF_INET6);
+  return &a->addr.in6_addr;
+}
+
 /** Given an IPv6 address <b>x</b>, yield it as an array of uint8_t.
  *
  * Requires that <b>x</b> is actually an IPv6 address.
  */
-#define tor_addr_to_in6_addr8(x) tor_addr_to_in6(x)->s6_addr
+#define tor_addr_to_in6_addr8(x) tor_addr_to_in6_assert(x)->s6_addr
+
 /** Given an IPv6 address <b>x</b>, yield it as an array of uint16_t.
  *
  * Requires that <b>x</b> is actually an IPv6 address.
  */
-#define tor_addr_to_in6_addr16(x) S6_ADDR16(*tor_addr_to_in6(x))
+#define tor_addr_to_in6_addr16(x) S6_ADDR16(*tor_addr_to_in6_assert(x))
 /** Given an IPv6 address <b>x</b>, yield it as an array of uint32_t.
  *
  * Requires that <b>x</b> is actually an IPv6 address.
  */
-#define tor_addr_to_in6_addr32(x) S6_ADDR32(*tor_addr_to_in6(x))
+#define tor_addr_to_in6_addr32(x) S6_ADDR32(*tor_addr_to_in6_assert(x))
 
 /** Return an IPv4 address in network order for <b>a</b>, or 0 if
  * <b>a</b> is not an IPv4 address. */
diff --git a/src/ext/ht.h b/src/ext/ht.h
index 28d1fe49d5..1b6cbe6632 100644
--- a/src/ext/ht.h
+++ b/src/ext/ht.h
@@ -203,6 +203,7 @@ ht_string_hash(const char *s)
       name##_HT_GROW(head, head->hth_n_entries+1);                      \
     HT_SET_HASH_(elm, field, hashfn);                                   \
     p = name##_HT_FIND_P_(head, elm);                                   \
+    HT_ASSERT_(p != NULL); /* this holds because we called HT_GROW */   \
     r = *p;                                                             \
     *p = elm;                                                           \
     if (r && (r!=elm)) {                                                \
@@ -470,6 +471,7 @@ ht_string_hash(const char *s)
       name##_HT_GROW(var##_head_, var##_head_->hth_n_entries+1);        \
     HT_SET_HASH_((elm), field, hashfn);                                 \
     var = name##_HT_FIND_P_(var##_head_, (elm));                        \
+    HT_ASSERT_(var); /* Holds because we called HT_GROW */              \
     if (*var) {                                                         \
       y;                                                                \
     } else {                                                            \
diff --git a/src/or/channeltls.c b/src/or/channeltls.c
index 2128b0924d..293f01070b 100644
--- a/src/or/channeltls.c
+++ b/src/or/channeltls.c
@@ -797,6 +797,7 @@ static int
 channel_tls_write_packed_cell_method(channel_t *chan,
                                      packed_cell_t *packed_cell)
 {
+  tor_assert(chan);
   channel_tls_t *tlschan = BASE_CHAN_TO_TLS(chan);
   size_t cell_network_size = get_cell_network_size(chan->wide_circ_ids);
   int written = 0;
diff --git a/src/or/connection.c b/src/or/connection.c
index e70b89767e..86ed2fbccf 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -2240,7 +2240,7 @@ connection_send_socks5_connect(connection_t *conn)
   } else { /* AF_INET6 */
     buf[3] = 4;
     reqsize += 16;
-    memcpy(buf + 4, tor_addr_to_in6(&conn->addr), 16);
+    memcpy(buf + 4, tor_addr_to_in6_addr8(&conn->addr), 16);
     memcpy(buf + 20, &port, 2);
   }
 
diff --git a/src/or/geoip.c b/src/or/geoip.c
index b563db0418..24ec9b7b15 100644
--- a/src/or/geoip.c
+++ b/src/or/geoip.c
@@ -80,9 +80,9 @@ geoip_add_entry(const tor_addr_t *low, const tor_addr_t *high,
   intptr_t idx;
   void *idxplus1_;
 
-  if (tor_addr_family(low) != tor_addr_family(high))
+  IF_BUG_ONCE(tor_addr_family(low) != tor_addr_family(high))
     return;
-  if (tor_addr_compare(high, low, CMP_EXACT) < 0)
+  IF_BUG_ONCE(tor_addr_compare(high, low, CMP_EXACT) < 0)
     return;
 
   idxplus1_ = strmap_get_lc(country_idxplus1_by_lc_code, country);
@@ -110,8 +110,8 @@ geoip_add_entry(const tor_addr_t *low, const tor_addr_t *high,
     smartlist_add(geoip_ipv4_entries, ent);
   } else if (tor_addr_family(low) == AF_INET6) {
     geoip_ipv6_entry_t *ent = tor_malloc_zero(sizeof(geoip_ipv6_entry_t));
-    ent->ip_low = *tor_addr_to_in6(low);
-    ent->ip_high = *tor_addr_to_in6(high);
+    ent->ip_low = *tor_addr_to_in6_assert(low);
+    ent->ip_high = *tor_addr_to_in6_assert(high);
     ent->country = idx;
     smartlist_add(geoip_ipv6_entries, ent);
   }
diff --git a/src/test/test_bt_cl.c b/src/test/test_bt_cl.c
index 2f5e50fbf5..ec03cedd0a 100644
--- a/src/test/test_bt_cl.c
+++ b/src/test/test_bt_cl.c
@@ -28,6 +28,9 @@ int a_tangled_web(int x) NOINLINE;
 int we_weave(int x) NOINLINE;
 static void abort_handler(int s) NORETURN;
 
+#if GCC_VERSION >= 601
+DISABLE_GCC_WARNING(null-dereference)
+#endif
 int
 crash(int x)
 {
@@ -47,6 +50,9 @@ crash(int x)
   crashtype *= x;
   return crashtype;
 }
+#if GCC_VERSION >= 601
+ENABLE_GCC_WARNING(null-dereference)
+#endif
 
 int
 oh_what(int x)