Bug #6066

Fix "control may reach end of non-void function" warnings for clang

Added by Eric Hodel over 3 years ago. Updated over 3 years ago.

[ruby-core:42815]
Status:Closed
Priority:Normal
Assignee:Eric Hodel
ruby -v:ruby 2.0.0dev (2012-02-23 trunk 34755) [x86_64-darwin11.3.0] Backport:

Description

=begin
clang 3.1 is more picky about "control may reach end of non-void function"

The attached patches contain add (({return Qnil; /* not reached */})) or the equivalent where the warnings occurred.
=end

fiddle.return.patch Magnifier - ext/fiddle patch (404 Bytes) Eric Hodel, 02/23/2012 09:39 AM

ripper.return.patch Magnifier - ext/ripper patch (391 Bytes) Eric Hodel, 02/23/2012 09:39 AM

socket.return.patch Magnifier - ext/socket patch (774 Bytes) Eric Hodel, 02/23/2012 09:39 AM

tk.return.patch Magnifier - ext/tk patch (1.46 KB) Eric Hodel, 02/23/2012 09:39 AM

return.combined.patch Magnifier - Combined patch for core and ext/ (10.4 KB) Eric Hodel, 02/23/2012 09:39 AM

core.return.patch Magnifier - Patch for C files in the project root (5.75 KB) Eric Hodel, 02/23/2012 09:39 AM

bigdecimal.return.patch Magnifier - ext/bigdecimal patch (409 Bytes) Eric Hodel, 02/23/2012 09:39 AM

digest.return.patch Magnifier - ext/digest patch (904 Bytes) Eric Hodel, 02/23/2012 09:39 AM

dl.return.patch Magnifier - ext/dl patch (354 Bytes) Eric Hodel, 02/23/2012 09:39 AM

return.UNREACHABLE.patch Magnifier - Updated combined patch using UNREACHABLE (10.9 KB) Eric Hodel, 02/26/2012 01:39 PM

return.not_reached.convert.patch Magnifier - update /* not reached */ to UNREACHABLE (10.7 KB) Eric Hodel, 02/26/2012 02:08 PM

Associated revisions

Revision 35321
Added by Eric Hodel over 3 years ago

  • encoding.c (rb_enc_codepoint_len): Use UNREACHABLE to avoid "control reaches end of non-void function" warnings. [ruby-trunk - Bug #6066]
  • re.c (name_to_backref_number): ditto.
  • object.c (rb_Float): ditto.
  • io.c (io_readpartial): ditto.
  • io.c (io_read_nonblock): ditto.
  • pack.c (rb_uv_to_utf8): ditto.
  • proc.c (rb_method_entry_arity): ditto.
  • vm_method.c (rb_f_notimplement): ditto.
  • struct.c (rb_struct_aset_id): ditto.
  • class.c (rb_scan_args): ditto.
  • process.c (rlimit_resource_type): ditto.
  • process.c (rlimit_resource_value): ditto.
  • process.c (p_uid_switch): ditto.
  • process.c (p_gid_switch): ditto.
  • ext/digest/digest.c (rb_digest_instance_update): ditto.
  • ext/digest/digest.c (rb_digest_instance_finish): ditto.
  • ext/digest/digest.c (rb_digest_instance_reset): ditto.
  • ext/digest/digest.c (rb_digest_instance_block_length): ditto.
  • ext/bigdecimal/bigdecimal.c (BigDecimalCmp): ditto.
  • ext/dl/handle.c (rb_dlhandle_close): ditto.
  • ext/tk/tcltklib.c (pending_exception_check0): ditto.
  • ext/tk/tcltklib.c (pending_exception_check1): ditto.
  • ext/tk/tcltklib.c (ip_cancel_eval_core): ditto.
  • ext/tk/tcltklib.c (lib_get_reltype_name): ditto.
  • ext/tk/tcltklib.c (create_dummy_encoding_for_tk_core): ditto.
  • ext/tk/tkutil/tkutil.c (tk_hash_kv): ditto.
  • ext/openssl/ossl_ssl.c (ossl_ssl_session_reused): ditto.
  • ext/openssl/ossl_pkey_ec.c (ossl_ec_key_dsa_verify_asn1): ditto.
  • ext/openssl/ossl_pkey_ec.c (ossl_ec_point_is_at_infinit): ditto.
  • ext/openssl/ossl_pkey_ec.c (ossl_ec_point_is_on_curve): ditto.
  • ext/fiddle/conversions.c (generic_to_value): ditto.
  • ext/socket/raddrinfo.c (rsock_io_socket_addrinfo): ditto.
  • ext/socket/socket.c (sock_s_getnameinfo): ditto.
  • ext/ripper/eventids2.c (ripper_token2eventid): ditto.
  • cont.c (return_fiber): ditto.
  • dmydln.c (dln_load): ditto.
  • vm_insnhelper.c (vm_search_normal_superclass): ditto.
  • bignum.c (big_fdiv): ditto.
  • marshal.c (r_symlink): ditto.
  • marshal.c (r_symbol): ditto.

Revision 35321
Added by Eric Hodel over 3 years ago

  • encoding.c (rb_enc_codepoint_len): Use UNREACHABLE to avoid "control reaches end of non-void function" warnings. [ruby-trunk - Bug #6066]
  • re.c (name_to_backref_number): ditto.
  • object.c (rb_Float): ditto.
  • io.c (io_readpartial): ditto.
  • io.c (io_read_nonblock): ditto.
  • pack.c (rb_uv_to_utf8): ditto.
  • proc.c (rb_method_entry_arity): ditto.
  • vm_method.c (rb_f_notimplement): ditto.
  • struct.c (rb_struct_aset_id): ditto.
  • class.c (rb_scan_args): ditto.
  • process.c (rlimit_resource_type): ditto.
  • process.c (rlimit_resource_value): ditto.
  • process.c (p_uid_switch): ditto.
  • process.c (p_gid_switch): ditto.
  • ext/digest/digest.c (rb_digest_instance_update): ditto.
  • ext/digest/digest.c (rb_digest_instance_finish): ditto.
  • ext/digest/digest.c (rb_digest_instance_reset): ditto.
  • ext/digest/digest.c (rb_digest_instance_block_length): ditto.
  • ext/bigdecimal/bigdecimal.c (BigDecimalCmp): ditto.
  • ext/dl/handle.c (rb_dlhandle_close): ditto.
  • ext/tk/tcltklib.c (pending_exception_check0): ditto.
  • ext/tk/tcltklib.c (pending_exception_check1): ditto.
  • ext/tk/tcltklib.c (ip_cancel_eval_core): ditto.
  • ext/tk/tcltklib.c (lib_get_reltype_name): ditto.
  • ext/tk/tcltklib.c (create_dummy_encoding_for_tk_core): ditto.
  • ext/tk/tkutil/tkutil.c (tk_hash_kv): ditto.
  • ext/openssl/ossl_ssl.c (ossl_ssl_session_reused): ditto.
  • ext/openssl/ossl_pkey_ec.c (ossl_ec_key_dsa_verify_asn1): ditto.
  • ext/openssl/ossl_pkey_ec.c (ossl_ec_point_is_at_infinit): ditto.
  • ext/openssl/ossl_pkey_ec.c (ossl_ec_point_is_on_curve): ditto.
  • ext/fiddle/conversions.c (generic_to_value): ditto.
  • ext/socket/raddrinfo.c (rsock_io_socket_addrinfo): ditto.
  • ext/socket/socket.c (sock_s_getnameinfo): ditto.
  • ext/ripper/eventids2.c (ripper_token2eventid): ditto.
  • cont.c (return_fiber): ditto.
  • dmydln.c (dln_load): ditto.
  • vm_insnhelper.c (vm_search_normal_superclass): ditto.
  • bignum.c (big_fdiv): ditto.
  • marshal.c (r_symlink): ditto.
  • marshal.c (r_symbol): ditto.

History

#1 Updated by Eric Hodel over 3 years ago

Patches were not added in-order.

Note that return.combined.patch contains all the other patches grouped together

#2 Updated by Nobuyoshi Nakada over 3 years ago

clang 3.1 is more picky about "control may reach end of non-void function"

or doesn't deal with noreturn attribute properly?

Anyway, (ID)NULL seems odd.

#3 Updated by Eric Hodel over 3 years ago

=begin
Yes, (ID)NULL seems odd to me too.

clang supports __builtin_unreachable():

http://clang.llvm.org/docs/LanguageExtensions.html#__builtin_unreachable

It looks like replacing (({return Qnil; /* NOT REACHED */})) lines with __builtin_unreachable() may improve optimizations with clang.

Adding configure support to detect _builltin_unreachable() looks like it is beyond my knowledge based on the _builtin_setjmp() detection, can you help me?
=end

#4 Updated by Yui NARUSE over 3 years ago

  • Status changed from Open to Closed

r34784 uses __builtin_unreachable().

#5 Updated by Nobuyoshi Nakada over 3 years ago

  • Status changed from Closed to Open

No, the commit just checks the function, and defines UNREACHABLE macro.

#6 Updated by Eric Hodel over 3 years ago

Thank you, I will post an updated patch using UNREACHABLE

#7 Updated by Eric Hodel over 3 years ago

This patch updates all -Wreturn-type warning locations to use the UNREACHABLE macro.

#8 Updated by Eric Hodel over 3 years ago

=begin
This patch updates existing (({return Qnil; /* not reached */})) with (({UNREACHABLE;}))

I did not change (({trace_en()})) in variable.c as it seems to return, I think the (({/* not reached */})) should be removed, but can you check it?
=end

#9 Updated by Koichi Sasada over 3 years ago

  • Assignee set to Nobuyoshi Nakada

#10 Updated by Shyouhei Urabe over 3 years ago

  • Status changed from Open to Assigned

#11 Updated by Eric Hodel over 3 years ago

Nobu, may I commit this?

#12 Updated by Nobuyoshi Nakada over 3 years ago

  • Assignee changed from Nobuyoshi Nakada to Eric Hodel

Feel free.

#13 Updated by Eric Hodel over 3 years ago

  • % Done changed from 0 to 100
  • Status changed from Assigned to Closed

This issue was solved with changeset r35321.
Eric, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • encoding.c (rb_enc_codepoint_len): Use UNREACHABLE to avoid "control reaches end of non-void function" warnings. [ruby-trunk - Bug #6066]
  • re.c (name_to_backref_number): ditto.
  • object.c (rb_Float): ditto.
  • io.c (io_readpartial): ditto.
  • io.c (io_read_nonblock): ditto.
  • pack.c (rb_uv_to_utf8): ditto.
  • proc.c (rb_method_entry_arity): ditto.
  • vm_method.c (rb_f_notimplement): ditto.
  • struct.c (rb_struct_aset_id): ditto.
  • class.c (rb_scan_args): ditto.
  • process.c (rlimit_resource_type): ditto.
  • process.c (rlimit_resource_value): ditto.
  • process.c (p_uid_switch): ditto.
  • process.c (p_gid_switch): ditto.
  • ext/digest/digest.c (rb_digest_instance_update): ditto.
  • ext/digest/digest.c (rb_digest_instance_finish): ditto.
  • ext/digest/digest.c (rb_digest_instance_reset): ditto.
  • ext/digest/digest.c (rb_digest_instance_block_length): ditto.
  • ext/bigdecimal/bigdecimal.c (BigDecimalCmp): ditto.
  • ext/dl/handle.c (rb_dlhandle_close): ditto.
  • ext/tk/tcltklib.c (pending_exception_check0): ditto.
  • ext/tk/tcltklib.c (pending_exception_check1): ditto.
  • ext/tk/tcltklib.c (ip_cancel_eval_core): ditto.
  • ext/tk/tcltklib.c (lib_get_reltype_name): ditto.
  • ext/tk/tcltklib.c (create_dummy_encoding_for_tk_core): ditto.
  • ext/tk/tkutil/tkutil.c (tk_hash_kv): ditto.
  • ext/openssl/ossl_ssl.c (ossl_ssl_session_reused): ditto.
  • ext/openssl/ossl_pkey_ec.c (ossl_ec_key_dsa_verify_asn1): ditto.
  • ext/openssl/ossl_pkey_ec.c (ossl_ec_point_is_at_infinit): ditto.
  • ext/openssl/ossl_pkey_ec.c (ossl_ec_point_is_on_curve): ditto.
  • ext/fiddle/conversions.c (generic_to_value): ditto.
  • ext/socket/raddrinfo.c (rsock_io_socket_addrinfo): ditto.
  • ext/socket/socket.c (sock_s_getnameinfo): ditto.
  • ext/ripper/eventids2.c (ripper_token2eventid): ditto.
  • cont.c (return_fiber): ditto.
  • dmydln.c (dln_load): ditto.
  • vm_insnhelper.c (vm_search_normal_superclass): ditto.
  • bignum.c (big_fdiv): ditto.
  • marshal.c (r_symlink): ditto.
  • marshal.c (r_symbol): ditto.

#14 Updated by Eric Hodel over 3 years ago

r35322 is also part of this bug, but I forgot to mention it in my commit message.

#15 Updated by Nobuyoshi Nakada over 3 years ago

  • Category set to misc
  • Target version set to 2.0.0
  • Status changed from Closed to Assigned

Thank you for the commit, and found r35322 let older compilers and non-gcc compilers warn unreachable code.
UNREACHABLE should not replace those `return's, but be appended, I think.

#16 Updated by Nobuyoshi Nakada over 3 years ago

  • % Done changed from 100 to 50

#17 Updated by Nobuyoshi Nakada over 3 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 50 to 100

Also available in: Atom PDF