undefined (#27772)

Here are the warnings from src/app/tor itself:

  CCLD     src/app/tor
src/feature/nodelist/torcert.c: In function ‘tor_cert_sign_impl.constprop’:
src/feature/nodelist/torcert.c:55:19: warning: potential null pointer dereference [-Wnull-dereference]
   cert->exp_field = (uint32_t) CEIL_DIV(now + lifetime, 3600);
                   ^
src/feature/nodelist/torcert.c:56:23: warning: potential null pointer dereference [-Wnull-dereference]
   cert->cert_key_type = signed_key_type;
                       ^
src/feature/nodelist/torcert.c:54:19: warning: potential null pointer dereference [-Wnull-dereference]
   cert->cert_type = cert_type;
                   ^
src/lib/meminfo/meminfo.c: In function ‘tor_log_mallinfo’:
src/lib/meminfo/meminfo.c:48:8: warning: function call has aggregate value [-Waggregate-return]
   mi = mallinfo();
        ^
src/lib/crypt_ops/crypto_pwbox.c: In function ‘crypto_pwbox’:
src/trunnel/pwbox.c:161:56: warning: potential null pointer dereference [-Wnull-dereference]
                  &inp->skey_header.n_, inp->skey_header.elts_, newlen,
                                                        ^
src/core/mainloop/connection.c: In function ‘connection_free_minimal’:
src/core/mainloop/connection.c:665:12: warning: null pointer dereference [-Wnull-dereference]
       if (!CHANNEL_FINISHED(TLS_CHAN_TO_BASE(or_conn->chan))) {
            ^
src/feature/nodelist/routerparse.c: In function ‘router_parse_addr_policy.part.2’:
lto1: warning: function may return address of local variable [-Wreturn-local-addr]
src/feature/nodelist/routerparse.c:4406:17: note: declared here
   addr_policy_t newe;
                 ^
src/core/proto/proto_socks.c: In function ‘socks_request_set_socks5_error’:
src/trunnel/socks5.c:3075:16: warning: null pointer dereference [-Wnull-dereference]
   inp->version = val;
                ^
src/trunnel/socks5.c:3563:14: warning: null pointer dereference [-Wnull-dereference]
   inp->reply = val;
              ^
src/trunnel/socks5.c:3116:14: warning: null pointer dereference [-Wnull-dereference]
   inp->atype = val;
              ^
src/core/or/scheduler_kist.c: In function ‘update_socket_info_impl’:
src/core/or/scheduler_kist.c:198:5: warning: potential null pointer dereference [-Wnull-dereference]
     TO_CONN(BASE_CHAN_TO_TLS((channel_t *) ent->chan)->conn)->s;
     ^
src/core/proto/proto_socks.c: In function ‘fetch_from_buf_socks’:
src/trunnel/socks5.c:2690:16: warning: potential null pointer dereference [-Wnull-dereference]
   inp->version = val;
                ^
src/core/or/scheduler_kist.c: In function ‘channel_write_to_kernel’:
src/core/or/scheduler_kist.c:457:27: warning: potential null pointer dereference [-Wnull-dereference]
   connection_handle_write(TO_CONN(BASE_CHAN_TO_TLS(chan)->conn), 0);
                           ^
src/core/or/channelpadding.c: In function ‘channelpadding_send_enable_command’:
src/core/or/channelpadding.c:331:3: warning: potential null pointer dereference [-Wnull-dereference]
   tor_assert(BASE_CHAN_TO_TLS(chan)->conn->link_proto >=
   ^
src/core/or/channelpadding.c: In function ‘channelpadding_send_disable_command’:
src/core/or/channelpadding.c:299:3: warning: potential null pointer dereference [-Wnull-dereference]
   tor_assert(BASE_CHAN_TO_TLS(chan)->conn->link_proto >=
   ^
src/core/or/channel.c: In function ‘channel_rsa_id_group_set_badness.isra.10.part.11’:
src/core/or/channel.c:3436:54: warning: potential null pointer dereference [-Wnull-dereference]
     smartlist_add(or_conns, BASE_CHAN_TO_TLS(channel)->conn);
                                                      ^
src/lib/meminfo/meminfo.c: In function ‘signal_callback’:
src/lib/meminfo/meminfo.c:48:8: warning: function call has aggregate value [-Waggregate-return]
   mi = mallinfo();
        ^

See my branch 'bug27772' with PR at https://github.com/torproject/tor/pull/356 .

I haven't written a changes for this yet, because I'd like a reviewer's opinion on how many of these to backport how far.

There are remaining warnings: I think there is some bug in GCC's LTO where it doesn't handle "#pragma diagnostic" properly -- since these are warnings we have explicitly suppressed in the code:

src/lib/meminfo/meminfo.c: In function ‘tor_log_mallinfo’:
src/lib/meminfo/meminfo.c:48:8: warning: function call has aggregate value [-Waggregate-return]
   mi = mallinfo();
        ^
src/test/test_bt_cl.c: In function ‘crash’:
src/test/test_bt_cl.c:43:24: warning: null pointer dereference [-Wnull-dereference]
     *(volatile int *)0 = 0;
                        ^

I ported part of this to my bug25573-034-typefix branch in case we eventually merge #25573 (moved) to 0.3.4

Trac:
Status: assigned to needs_review

Trac:
Reviewer: N/A to ahf

Trac:
Priority: Medium to High

I think all of these commits can go in as they are generally cleaning up code.

I do however think that some of the comments that are added to suppress warnings from GCC will become confusing when you look at the code later on. For example: https://github.com/torproject/tor/pull/356/commits/620108ea7770608de72dcbea4ca73d6fb99c1109#diff-23701c678d2a4dce3ba19f90e4bf5b00R4440

The patches seems reasonably "small" to maybe backport all of them, or should we just backport the ones that causes compilation errors? Will any of our users who are using an older version of Tor be compiling Tor with such a new version of GCC?

Trac:
Status: needs_review to merge_ready

IMO anybody who wants LTO should be sticking with a fairly recent version. Looking over the commits again, I didn't see anything that looked like a scary bug in need of a backport... so if you didn't either, that should be fine.

I'll go over the comments to try to make them better.

okay, I've squashed, fixed the comments, merged, fixed the conflicts. TY!

Trac:
Status: merge_ready to closed
Resolution: N/A to fixed

Activity