From 0112936ef7bc7bf4de39a17ce8b4cc37a9825f83 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Thu, 3 Jan 2019 08:12:19 +0000 Subject: [PATCH] Replace assert(false) with an unreachable() macro. Taking a leaf out of the LLVM code base: this macro still includes an assert(false) so that the message will show up in a typical build, but it follows it up with a call to a function explicitly marked as no- return. So this ought to do a better job of convincing compilers that once a code path hits this function it _really doesn't_ have to still faff about with making up a bogus return value or filling in a variable that 'might be used uninitialised' in the following code that won't be reached anyway. I've gone through the existing code looking for the assert(false) / assert(0) idiom and replaced all the ones I found with the new macro, which also meant I could remove a few pointless return statements and variable initialisations that I'd already had to put in to placate compiler front ends. --- cmdgen.c | 8 ++++---- import.c | 11 +++++------ misc.h | 18 ++++++++++++++++++ nocmdline.c | 2 +- portfwd.c | 2 +- ssh1connection-client.c | 6 ++---- ssh1connection-server.c | 5 ++--- ssh1login.c | 3 +-- ssh2connection-client.c | 12 +++++------- ssh2connection-server.c | 26 +++++++++++++------------- ssh2transport.c | 2 +- sshaes.c | 18 +++++++++--------- sshcommon.c | 8 ++++---- sshpubk.c | 2 +- sshsh256.c | 2 +- sshsha.c | 2 +- sshverstring.c | 9 ++++----- terminal.c | 2 +- unix/gtkwin.c | 2 +- unix/uxnet.c | 4 ++-- unix/uxpgnt.c | 4 ++-- windows/window.c | 2 +- windows/winnet.c | 2 +- windows/winpgen.c | 4 +++- windows/winpgnt.c | 4 +++- windows/winpgntc.c | 2 +- x11fwd.c | 2 +- 27 files changed, 89 insertions(+), 75 deletions(-) diff --git a/cmdgen.c b/cmdgen.c index da2f98de..bbb2cc14 100644 --- a/cmdgen.c +++ b/cmdgen.c @@ -104,7 +104,7 @@ void sk_cleanup(void) } void queue_idempotent_callback(IdempotentCallback *ic) { - assert(0); + unreachable("queue_idempotent_callback called in cmdgen"); } void showversion(void) @@ -594,7 +594,7 @@ int main(int argc, char **argv) case SSH_KEYTYPE_OPENSSH_AUTO: default: - assert(0 && "Should never see these types on an input file"); + unreachable("Should never see these types on an input file"); } } @@ -851,7 +851,7 @@ int main(int argc, char **argv) break; default: - assert(0); + unreachable("bad input key type"); } if (error) { @@ -1044,7 +1044,7 @@ int main(int argc, char **argv) real_outtype = SSH_KEYTYPE_SSHCOM; break; default: - assert(0 && "control flow goof"); + unreachable("control flow goof"); } ret = export_ssh2(outfilename, real_outtype, ssh2key, new_passphrase); if (!ret) { diff --git a/import.c b/import.c index 71caa859..e5893c0a 100644 --- a/import.c +++ b/import.c @@ -750,7 +750,7 @@ static struct ssh2_userkey *openssh_pem_read( } } else { - assert(0 && "Bad key type from load_openssh_pem_key"); + unreachable("Bad key type from load_openssh_pem_key"); errmsg = "Bad key type from load_openssh_pem_key"; goto error; } @@ -973,8 +973,7 @@ static bool openssh_pem_write( header = "-----BEGIN EC PRIVATE KEY-----\n"; footer = "-----END EC PRIVATE KEY-----\n"; } else { - assert(0); /* zoinks! */ - exit(1); /* XXX: GCC doesn't understand assert() on some systems. */ + unreachable("bad key alg in openssh_pem_write"); } /* @@ -1363,7 +1362,7 @@ static struct ssh2_userkey *openssh_new_read( keysize = 48; /* 32 byte key + 16 byte IV */ break; default: - assert(0 && "Bad cipher enumeration value"); + unreachable("Bad cipher enumeration value"); } assert(keysize <= sizeof(keybuf)); switch (key->kdf) { @@ -1378,7 +1377,7 @@ static struct ssh2_userkey *openssh_new_read( keybuf, keysize); break; default: - assert(0 && "Bad kdf enumeration value"); + unreachable("Bad kdf enumeration value"); } switch (key->cipher) { case ON_E_NONE: @@ -1408,7 +1407,7 @@ static struct ssh2_userkey *openssh_new_read( } break; default: - assert(0 && "Bad cipher enumeration value"); + unreachable("Bad cipher enumeration value"); } } diff --git a/misc.h b/misc.h index 39219d62..1b52fa51 100644 --- a/misc.h +++ b/misc.h @@ -11,8 +11,10 @@ #include /* for FILE * */ #include /* for va_list */ +#include /* for abort */ #include /* for struct tm */ #include /* for INT_MAX/MIN */ +#include /* for assert (obviously) */ unsigned long parse_blocksize(const char *bs); char ctrlparse(char *s, char **next); @@ -169,6 +171,22 @@ bool smemeq(const void *av, const void *bv, size_t len); char *buildinfo(const char *newline); +/* + * A function you can put at points in the code where execution should + * never reach in the first place. Better than assert(false), or even + * assert(false && "some explanatory message"), because some compilers + * don't interpret assert(false) as a declaration of unreachability, + * so they may still warn about pointless things like some variable + * not being initialised on the unreachable code path. + * + * I follow the assertion with a call to abort() just in case someone + * compiles with -DNDEBUG, and I wrap that abort inside my own + * function labelled NORETURN just in case some unusual kind of system + * header wasn't foresighted enough to label abort() itself that way. + */ +static inline NORETURN void unreachable_internal(void) { abort(); } +#define unreachable(msg) (assert(false && msg), unreachable_internal()) + /* * Debugging functions. * diff --git a/nocmdline.c b/nocmdline.c index 090bfba7..05a79baf 100644 --- a/nocmdline.c +++ b/nocmdline.c @@ -33,7 +33,7 @@ int cmdline_get_passwd_input(prompts_t *p) int cmdline_process_param(const char *p, char *value, int need_save, Conf *conf) { - assert(false && "cmdline_process_param should never be called"); + unreachable("cmdline_process_param should never be called"); } /* diff --git a/portfwd.c b/portfwd.c index eaba9820..e410850b 100644 --- a/portfwd.c +++ b/portfwd.c @@ -243,7 +243,7 @@ static void pfd_receive(Plug *plug, int urgent, char *data, int len) switch (pf->socks_state) { case SOCKS_INITIAL: case SOCKS_NONE: - assert(0 && "These case values cannot appear"); + unreachable("These case values cannot appear"); case SOCKS_4: /* SOCKS 4/4A connect message */ diff --git a/ssh1connection-client.c b/ssh1connection-client.c index 214d4482..8c46483d 100644 --- a/ssh1connection-client.c +++ b/ssh1connection-client.c @@ -519,12 +519,10 @@ struct ssh_rportfwd *ssh1_rportfwd_alloc( SshChannel *ssh1_serverside_x11_open( ConnectionLayer *cl, Channel *chan, const SocketPeerInfo *pi) { - assert(false && "Should never be called in the client"); - return NULL; + unreachable("Should never be called in the client"); } SshChannel *ssh1_serverside_agent_open(ConnectionLayer *cl, Channel *chan) { - assert(false && "Should never be called in the client"); - return NULL; + unreachable("Should never be called in the client"); } diff --git a/ssh1connection-server.c b/ssh1connection-server.c index 92e1ec99..eecb773a 100644 --- a/ssh1connection-server.c +++ b/ssh1connection-server.c @@ -253,7 +253,7 @@ bool ssh1_handle_direction_specific_packet( SshChannel *ssh1_session_open(ConnectionLayer *cl, Channel *chan) { - assert(false && "Should never be called in the server"); + unreachable("Should never be called in the server"); } struct ssh_rportfwd *ssh1_rportfwd_alloc( @@ -262,8 +262,7 @@ struct ssh_rportfwd *ssh1_rportfwd_alloc( int addressfamily, const char *log_description, PortFwdRecord *pfr, ssh_sharing_connstate *share_ctx) { - assert(false && "Should never be called in the server"); - return NULL; + unreachable("Should never be called in the server"); } static int ssh1sesschan_write(SshChannel *sc, bool is_stderr, diff --git a/ssh1login.c b/ssh1login.c index 0a81e1d9..b9fe760d 100644 --- a/ssh1login.c +++ b/ssh1login.c @@ -690,8 +690,7 @@ static void ssh1_login_process_queue(PacketProtocolLayer *ppl) got_passphrase = false; /* and try again */ } else { - assert(0 && "unexpected return from rsa_ssh1_loadkey()"); - got_passphrase = false; /* placate optimisers */ + unreachable("unexpected return from rsa_ssh1_loadkey()"); } } diff --git a/ssh2connection-client.c b/ssh2connection-client.c index 3853ac97..b5a07c64 100644 --- a/ssh2connection-client.c +++ b/ssh2connection-client.c @@ -304,14 +304,12 @@ SshChannel *ssh2_session_open(ConnectionLayer *cl, Channel *chan) SshChannel *ssh2_serverside_x11_open( ConnectionLayer *cl, Channel *chan, const SocketPeerInfo *pi) { - assert(false && "Should never be called in the client"); - return 0; /* placate optimiser */ + unreachable("Should never be called in the client"); } SshChannel *ssh2_serverside_agent_open(ConnectionLayer *cl, Channel *chan) { - assert(false && "Should never be called in the client"); - return 0; /* placate optimiser */ + unreachable("Should never be called in the client"); } static void ssh2_channel_response( @@ -358,19 +356,19 @@ bool ssh2channel_start_subsystem( void ssh2channel_send_exit_status(SshChannel *sc, int status) { - assert(false && "Should never be called in the client"); + unreachable("Should never be called in the client"); } void ssh2channel_send_exit_signal( SshChannel *sc, ptrlen signame, bool core_dumped, ptrlen msg) { - assert(false && "Should never be called in the client"); + unreachable("Should never be called in the client"); } void ssh2channel_send_exit_signal_numeric( SshChannel *sc, int signum, bool core_dumped, ptrlen msg) { - assert(false && "Should never be called in the client"); + unreachable("Should never be called in the client"); } void ssh2channel_request_x11_forwarding( diff --git a/ssh2connection-server.c b/ssh2connection-server.c index c68e36f7..38c37e14 100644 --- a/ssh2connection-server.c +++ b/ssh2connection-server.c @@ -142,17 +142,17 @@ struct ssh_rportfwd *ssh2_rportfwd_alloc( int addressfamily, const char *log_description, PortFwdRecord *pfr, ssh_sharing_connstate *share_ctx) { - assert(false && "Should never be called in the server"); + unreachable("Should never be called in the server"); } void ssh2_rportfwd_remove(ConnectionLayer *cl, struct ssh_rportfwd *rpf) { - assert(false && "Should never be called in the server"); + unreachable("Should never be called in the server"); } SshChannel *ssh2_session_open(ConnectionLayer *cl, Channel *chan) { - assert(false && "Should never be called in the server"); + unreachable("Should never be called in the server"); } SshChannel *ssh2_serverside_x11_open( @@ -202,19 +202,19 @@ SshChannel *ssh2_serverside_agent_open(ConnectionLayer *cl, Channel *chan) void ssh2channel_start_shell(SshChannel *sc, bool want_reply) { - assert(false && "Should never be called in the server"); + unreachable("Should never be called in the server"); } void ssh2channel_start_command( SshChannel *sc, bool want_reply, const char *command) { - assert(false && "Should never be called in the server"); + unreachable("Should never be called in the server"); } bool ssh2channel_start_subsystem( SshChannel *sc, bool want_reply, const char *subsystem) { - assert(false && "Should never be called in the server"); + unreachable("Should never be called in the server"); } void ssh2channel_send_exit_status(SshChannel *sc, int status) @@ -262,38 +262,38 @@ void ssh2channel_request_x11_forwarding( SshChannel *sc, bool want_reply, const char *authproto, const char *authdata, int screen_number, bool oneshot) { - assert(false && "Should never be called in the server"); + unreachable("Should never be called in the server"); } void ssh2channel_request_agent_forwarding(SshChannel *sc, bool want_reply) { - assert(false && "Should never be called in the server"); + unreachable("Should never be called in the server"); } void ssh2channel_request_pty( SshChannel *sc, bool want_reply, Conf *conf, int w, int h) { - assert(false && "Should never be called in the server"); + unreachable("Should never be called in the server"); } bool ssh2channel_send_env_var( SshChannel *sc, bool want_reply, const char *var, const char *value) { - assert(false && "Should never be called in the server"); + unreachable("Should never be called in the server"); } bool ssh2channel_send_serial_break(SshChannel *sc, bool want_reply, int length) { - assert(false && "Should never be called in the server"); + unreachable("Should never be called in the server"); } bool ssh2channel_send_signal( SshChannel *sc, bool want_reply, const char *signame) { - assert(false && "Should never be called in the server"); + unreachable("Should never be called in the server"); } void ssh2channel_send_terminal_size_change(SshChannel *sc, int w, int h) { - assert(false && "Should never be called in the server"); + unreachable("Should never be called in the server"); } diff --git a/ssh2transport.c b/ssh2transport.c index ab813ac2..a8bc1f3e 100644 --- a/ssh2transport.c +++ b/ssh2transport.c @@ -927,7 +927,7 @@ static bool ssh2_scan_kexinits( break; default: - assert(false && "Bad list index in scan_kexinits"); + unreachable("Bad list index in scan_kexinits"); } } diff --git a/sshaes.c b/sshaes.c index cdcff238..eb1fd1d7 100644 --- a/sshaes.c +++ b/sshaes.c @@ -860,7 +860,7 @@ static void aes_encrypt_cbc_sw(unsigned char *blk, int len, AESContext * ctx) ENCLASTROUND; break; default: - assert(0); + unreachable("bad AES round count"); } for (i = 0; i < 4; i++) PUT_32BIT_MSB_FIRST(blk + 4 * i, block[i]); @@ -905,7 +905,7 @@ static void aes_sdctr_sw(unsigned char *blk, int len, AESContext *ctx) ENCLASTROUND; break; default: - assert(0); + unreachable("bad AES round count"); } for (i = 0; i < 4; i++) { tmp = GET_32BIT_MSB_FIRST(blk + 4 * i); @@ -956,7 +956,7 @@ static void aes_decrypt_cbc_sw(unsigned char *blk, int len, AESContext * ctx) DECLASTROUND; break; default: - assert(0); + unreachable("bad AES round count"); } for (i = 0; i < 4; i++) { PUT_32BIT_MSB_FIRST(blk + 4 * i, iv[i] ^ block[i]); @@ -1505,7 +1505,7 @@ static void aes_encrypt_cbc_ni(unsigned char *blk, int len, AESContext * ctx) enc = _mm_aesenclast_si128(enc, *(++keysched)); break; default: - assert(0); + unreachable("bad AES round count"); } /* Store and go to next block */ @@ -1552,7 +1552,7 @@ static void aes_decrypt_cbc_ni(unsigned char *blk, int len, AESContext * ctx) dec = _mm_aesdeclast_si128(dec, *(++keysched)); break; default: - assert(0); + unreachable("bad AES round count"); } /* Xor data with IV */ @@ -1610,7 +1610,7 @@ static void aes_sdctr_ni(unsigned char *blk, int len, AESContext *ctx) enc = _mm_aesenclast_si128(enc, *(++keysched)); break; default: - assert(0); + unreachable("bad AES round count"); } /* Xor with block and store result */ @@ -1725,7 +1725,7 @@ static void aes_setup_ni(AESContext * ctx, AES_256_Key_Expansion (key, keysched); break; default: - assert(0); + unreachable("bad AES key length"); } /* @@ -1742,7 +1742,7 @@ static void aes_setup_ni(AESContext * ctx, aes_inv_key_14(ctx); break; default: - assert(0); + unreachable("bad AES key length"); } } @@ -1750,7 +1750,7 @@ static void aes_setup_ni(AESContext * ctx, static void aes_setup_ni(AESContext * ctx, const unsigned char *key, int keylen) { - assert(0); + unreachable("aes_setup_ni called when not compiled in"); } INLINE static bool supports_aes_ni() diff --git a/sshcommon.c b/sshcommon.c index c541e9d3..ebd10fd1 100644 --- a/sshcommon.c +++ b/sshcommon.c @@ -342,12 +342,12 @@ static bool zombiechan_want_close(Channel *chan, bool sent_eof, bool rcvd_eof) void chan_remotely_opened_confirmation(Channel *chan) { - assert(0 && "this channel type should never receive OPEN_CONFIRMATION"); + unreachable("this channel type should never receive OPEN_CONFIRMATION"); } void chan_remotely_opened_failure(Channel *chan, const char *errtext) { - assert(0 && "this channel type should never receive OPEN_FAILURE"); + unreachable("this channel type should never receive OPEN_FAILURE"); } bool chan_default_want_close( @@ -435,7 +435,7 @@ bool chan_no_change_window_size( void chan_no_request_response(Channel *chan, bool success) { - assert(0 && "this channel type should never send a want-reply request"); + unreachable("this channel type should never send a want-reply request"); } /* ---------------------------------------------------------------------- @@ -556,7 +556,7 @@ struct ssh_ttymodes get_ttymodes_from_conf(Seat *seat, Conf *conf) ival = (atoi(sval) != 0); break; default: - assert(0 && "Bad mode->type"); + unreachable("Bad mode->type"); } modes.have_mode[mode->opcode] = true; diff --git a/sshpubk.c b/sshpubk.c index 9a187b5c..d9006465 100644 --- a/sshpubk.c +++ b/sshpubk.c @@ -1478,7 +1478,7 @@ void ssh2_write_pubkey(FILE *fp, const char *comment, fprintf(fp, "%s\n", buffer); sfree(buffer); } else { - assert(0 && "Bad key type in ssh2_write_pubkey"); + unreachable("Bad key type in ssh2_write_pubkey"); } } diff --git a/sshsh256.c b/sshsh256.c index da6782f1..d8438e25 100644 --- a/sshsh256.c +++ b/sshsh256.c @@ -632,7 +632,7 @@ static void SHA256_ni(SHA256_State * s, const unsigned char *q, int len) static void SHA256_ni(SHA256_State * s, const unsigned char *q, int len) { - assert(0); + unreachable("SHA256_ni not compiled in"); } #endif /* COMPILER_SUPPORTS_AES_NI */ diff --git a/sshsha.c b/sshsha.c index 66d9aca5..b49355c5 100644 --- a/sshsha.c +++ b/sshsha.c @@ -683,7 +683,7 @@ static void sha1_ni(SHA_State * s, const unsigned char *q, int len) static void sha1_ni(SHA_State * s, const unsigned char *q, int len) { - assert(0); + unreachable("sha1_ni not compiled in"); } bool supports_sha_ni(void) diff --git a/sshverstring.c b/sshverstring.c index cf40cbaa..d1641e7c 100644 --- a/sshverstring.c +++ b/sshverstring.c @@ -407,16 +407,15 @@ void ssh_verstring_handle_input(BinaryPacketProtocol *bpp) static PktOut *ssh_verstring_new_pktout(int type) { - assert(0 && "Should never try to send packets during SSH version " - "string exchange"); - return NULL; + unreachable("Should never try to send packets during SSH version " + "string exchange"); } static void ssh_verstring_handle_output(BinaryPacketProtocol *bpp) { if (pq_peek(&bpp->out_pq)) { - assert(0 && "Should never try to send packets during SSH version " - "string exchange"); + unreachable("Should never try to send packets during SSH version " + "string exchange"); } } diff --git a/terminal.c b/terminal.c index 1d55d6be..6f4b15ae 100644 --- a/terminal.c +++ b/terminal.c @@ -6540,7 +6540,7 @@ int format_small_keypad_key(char *buf, Terminal *term, SmallKeypadKey key) case SKK_END: code = 4; break; case SKK_PGUP: code = 5; break; case SKK_PGDN: code = 6; break; - default: assert(false && "bad small keypad key enum value"); + default: unreachable("bad small keypad key enum value"); } /* Reorder edit keys to physical order */ diff --git a/unix/gtkwin.c b/unix/gtkwin.c index 9ba3f7fd..03c161da 100644 --- a/unix/gtkwin.c +++ b/unix/gtkwin.c @@ -679,7 +679,7 @@ static void update_mouseptr(GtkFrontend *inst) inst->waitcursor); break; default: - assert(0); + unreachable("Bad busy_status"); } } diff --git a/unix/uxnet.c b/unix/uxnet.c index a676656a..910d290e 100644 --- a/unix/uxnet.c +++ b/unix/uxnet.c @@ -454,7 +454,7 @@ void sk_addrcopy(SockAddr *addr, char *buf) memcpy(buf, &((struct sockaddr_in6 *)step.ai->ai_addr)->sin6_addr, sizeof(struct in6_addr)); else - assert(false); + unreachable("bad address family in sk_addrcopy"); #else struct in_addr a; @@ -723,7 +723,7 @@ static int try_connect(NetSocket *sock) break; default: - assert(0 && "unknown address family"); + unreachable("unknown address family"); exit(1); /* XXX: GCC doesn't understand assert() on some systems. */ } diff --git a/unix/uxpgnt.c b/unix/uxpgnt.c index dd6933cb..eea901b8 100644 --- a/unix/uxpgnt.c +++ b/unix/uxpgnt.c @@ -201,7 +201,7 @@ void pageant_print_env(int pid) socketname, pid); break; case SHELL_AUTO: - assert(0 && "Can't get here"); + unreachable("SHELL_AUTO should have been eliminated by now"); break; } } @@ -708,7 +708,7 @@ void run_client(void) } break; default: - assert(0 && "Invalid client action found"); + unreachable("Invalid client action found"); } } diff --git a/windows/window.c b/windows/window.c index dab900cc..0b432e88 100644 --- a/windows/window.c +++ b/windows/window.c @@ -1145,7 +1145,7 @@ static void update_mouse_pointer(void) force_visible = true; break; default: - assert(0); + unreachable("Bad busy_status"); } { HCURSOR cursor = LoadCursor(NULL, curstype); diff --git a/windows/winnet.c b/windows/winnet.c index bf91634f..590b98e9 100644 --- a/windows/winnet.c +++ b/windows/winnet.c @@ -784,7 +784,7 @@ void sk_addrcopy(SockAddr *addr, char *buf) memcpy(buf, &((struct sockaddr_in6 *)step.ai->ai_addr)->sin6_addr, sizeof(struct in6_addr)); else - assert(false); + unreachable("bad address family in sk_addrcopy"); } else #endif if (family == AF_INET) { diff --git a/windows/winpgen.c b/windows/winpgen.c index 005cd818..4c486fbc 100644 --- a/windows/winpgen.c +++ b/windows/winpgen.c @@ -61,7 +61,9 @@ void nonfatal(const char *fmt, ...) } /* Stubs needed to link against misc.c */ -void queue_idempotent_callback(IdempotentCallback *ic) { assert(0); } +void queue_idempotent_callback(IdempotentCallback *ic) { + unreachable("No callbacks in this application"); +} /* ---------------------------------------------------------------------- * Progress report code. This is really horrible :-) diff --git a/windows/winpgnt.c b/windows/winpgnt.c index 29e9cacf..c3d1984c 100644 --- a/windows/winpgnt.c +++ b/windows/winpgnt.c @@ -86,7 +86,9 @@ void modalfatalbox(const char *fmt, ...) } /* Stubs needed to link against misc.c */ -void queue_idempotent_callback(IdempotentCallback *ic) { assert(0); } +void queue_idempotent_callback(IdempotentCallback *ic) { + unreachable("No callbacks in this application"); +} static bool has_security; diff --git a/windows/winpgntc.c b/windows/winpgntc.c index 470fa847..08f5066a 100644 --- a/windows/winpgntc.c +++ b/windows/winpgntc.c @@ -27,7 +27,7 @@ bool agent_exists(void) void agent_cancel_query(agent_pending_query *q) { - assert(0 && "Windows agent queries are never asynchronous!"); + unreachable("Windows agent queries are never asynchronous!"); } agent_pending_query *agent_query( diff --git a/x11fwd.c b/x11fwd.c index faf19b16..3d0356a3 100644 --- a/x11fwd.c +++ b/x11fwd.c @@ -663,7 +663,7 @@ void x11_format_auth_for_authfile( put_uint16(bs, 6); /* indicates IPv6 */ put_stringpl_xauth(bs, make_ptrlen(ipv6buf, 16)); } else { - assert(false && "Bad address type in x11_format_auth_for_authfile"); + unreachable("Bad address type in x11_format_auth_for_authfile"); } {