Bug #6066

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

Added by Eric Hodel about 3 years ago. Updated almost 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 almost 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 almost 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 about 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 about 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 about 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 about 3 years ago

  • Status changed from Open to Closed

r34784 uses __builtin_unreachable().

#5 Updated by Nobuyoshi Nakada about 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 about 3 years ago

Thank you, I will post an updated patch using UNREACHABLE

#7 Updated by Eric Hodel about 3 years ago

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

#8 Updated by Eric Hodel about 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 almost 3 years ago

  • Assignee set to Nobuyoshi Nakada

#10 Updated by Shyouhei Urabe almost 3 years ago

  • Status changed from Open to Assigned

#11 Updated by Eric Hodel almost 3 years ago

Nobu, may I commit this?

#12 Updated by Nobuyoshi Nakada almost 3 years ago

  • Assignee changed from Nobuyoshi Nakada to Eric Hodel

Feel free.

#13 Updated by Eric Hodel almost 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 almost 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 almost 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 almost 3 years ago

  • % Done changed from 100 to 50

#17 Updated by Nobuyoshi Nakada almost 3 years ago

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

Also available in: Atom PDF