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

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

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
  • Status changed from Closed to Assigned
  • Target version set to 2.0.0

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