commit 4f8086fb20e93c477f033f58da17aa31b9c29fd6 Author: Nick Mathewson nickm@torproject.org Date: Mon May 30 11:12:58 2016 -0400
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. --- configure.ac | 1 + src/common/address.c | 3 ++- src/common/address.h | 16 +++++++++++++--- src/ext/ht.h | 2 ++ src/or/channeltls.c | 1 + src/or/connection.c | 2 +- src/or/geoip.c | 8 ++++---- src/test/test_bt_cl.c | 6 ++++++ 8 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/configure.ac b/configure.ac index ec56e02..4878d2b 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 a6e0f3f..759b20a 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 3de67e1..3f0bb52 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 28d1fe4..1b6cbe6 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 2128b09..293f010 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 e70b897..86ed2fb 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 b563db0..24ec9b7 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 2f5e50f..ec03ced 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)