Bug #6066

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

Added by Eric Hodel about 2 years ago. Updated about 2 years ago.

[ruby-core:42815]
Status:Closed
Priority:Normal
Assignee:Eric Hodel
Category:misc
Target version:2.0.0
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 about 2 years ago

  • encoding.c (rbenccodepoint_len): Use UNREACHABLE to avoid "control reaches end of non-void function" warnings. [ruby-trunk - Bug #6066]
  • re.c (nametobackref_number): ditto.
  • object.c (rb_Float): ditto.
  • io.c (io_readpartial): ditto.
  • io.c (ioreadnonblock): ditto.
  • pack.c (rbuvto_utf8): ditto.
  • proc.c (rbmethodentry_arity): ditto.
  • vmmethod.c (rbf_notimplement): ditto.
  • struct.c (rbstructaset_id): ditto.
  • class.c (rbscanargs): ditto.
  • process.c (rlimitresourcetype): ditto.
  • process.c (rlimitresourcevalue): ditto.
  • process.c (puidswitch): ditto.
  • process.c (pgidswitch): ditto.
  • ext/digest/digest.c (rbdigestinstance_update): ditto.
  • ext/digest/digest.c (rbdigestinstance_finish): ditto.
  • ext/digest/digest.c (rbdigestinstance_reset): ditto.
  • ext/digest/digest.c (rbdigestinstanceblocklength): ditto.
  • ext/bigdecimal/bigdecimal.c (BigDecimalCmp): ditto.
  • ext/dl/handle.c (rbdlhandleclose): ditto.
  • ext/tk/tcltklib.c (pendingexceptioncheck0): ditto.
  • ext/tk/tcltklib.c (pendingexceptioncheck1): ditto.
  • ext/tk/tcltklib.c (ipcanceleval_core): ditto.
  • ext/tk/tcltklib.c (libgetreltype_name): ditto.
  • ext/tk/tcltklib.c (createdummyencodingfortk_core): ditto.
  • ext/tk/tkutil/tkutil.c (tkhashkv): ditto.
  • ext/openssl/osslssl.c (osslsslsessionreused): ditto.
  • ext/openssl/osslpkeyec.c (ossleckeydsaverify_asn1): ditto.
  • ext/openssl/osslpkeyec.c (osslecpointisat_infinit): ditto.
  • ext/openssl/osslpkeyec.c (osslecpointison_curve): ditto.
  • ext/fiddle/conversions.c (generictovalue): ditto.
  • ext/socket/raddrinfo.c (rsockiosocket_addrinfo): ditto.
  • ext/socket/socket.c (socksgetnameinfo): ditto.
  • ext/ripper/eventids2.c (ripper_token2eventid): ditto.
  • cont.c (return_fiber): ditto.
  • dmydln.c (dln_load): ditto.
  • vminsnhelper.c (vmsearchnormalsuperclass): ditto.
  • bignum.c (big_fdiv): ditto.
  • marshal.c (r_symlink): ditto.
  • marshal.c (r_symbol): ditto.

History

#1 Updated by Eric Hodel about 2 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 2 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 2 years ago

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

clang supports _builtinunreachable():

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

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

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

#4 Updated by Yui NARUSE about 2 years ago

  • Status changed from Open to Closed

r34784 uses _builtinunreachable().

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

Thank you, I will post an updated patch using UNREACHABLE

#7 Updated by Eric Hodel about 2 years ago

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

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

  • Assignee set to Nobuyoshi Nakada

#10 Updated by Shyouhei Urabe about 2 years ago

  • Status changed from Open to Assigned

#11 Updated by Eric Hodel about 2 years ago

Nobu, may I commit this?

#12 Updated by Nobuyoshi Nakada about 2 years ago

  • Assignee changed from Nobuyoshi Nakada to Eric Hodel

Feel free.

#13 Updated by Eric Hodel about 2 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 (rbenccodepoint_len): Use UNREACHABLE to avoid "control reaches end of non-void function" warnings. [ruby-trunk - Bug #6066]
  • re.c (nametobackref_number): ditto.
  • object.c (rb_Float): ditto.
  • io.c (io_readpartial): ditto.
  • io.c (ioreadnonblock): ditto.
  • pack.c (rbuvto_utf8): ditto.
  • proc.c (rbmethodentry_arity): ditto.
  • vmmethod.c (rbf_notimplement): ditto.
  • struct.c (rbstructaset_id): ditto.
  • class.c (rbscanargs): ditto.
  • process.c (rlimitresourcetype): ditto.
  • process.c (rlimitresourcevalue): ditto.
  • process.c (puidswitch): ditto.
  • process.c (pgidswitch): ditto.
  • ext/digest/digest.c (rbdigestinstance_update): ditto.
  • ext/digest/digest.c (rbdigestinstance_finish): ditto.
  • ext/digest/digest.c (rbdigestinstance_reset): ditto.
  • ext/digest/digest.c (rbdigestinstanceblocklength): ditto.
  • ext/bigdecimal/bigdecimal.c (BigDecimalCmp): ditto.
  • ext/dl/handle.c (rbdlhandleclose): ditto.
  • ext/tk/tcltklib.c (pendingexceptioncheck0): ditto.
  • ext/tk/tcltklib.c (pendingexceptioncheck1): ditto.
  • ext/tk/tcltklib.c (ipcanceleval_core): ditto.
  • ext/tk/tcltklib.c (libgetreltype_name): ditto.
  • ext/tk/tcltklib.c (createdummyencodingfortk_core): ditto.
  • ext/tk/tkutil/tkutil.c (tkhashkv): ditto.
  • ext/openssl/osslssl.c (osslsslsessionreused): ditto.
  • ext/openssl/osslpkeyec.c (ossleckeydsaverify_asn1): ditto.
  • ext/openssl/osslpkeyec.c (osslecpointisat_infinit): ditto.
  • ext/openssl/osslpkeyec.c (osslecpointison_curve): ditto.
  • ext/fiddle/conversions.c (generictovalue): ditto.
  • ext/socket/raddrinfo.c (rsockiosocket_addrinfo): ditto.
  • ext/socket/socket.c (socksgetnameinfo): ditto.
  • ext/ripper/eventids2.c (ripper_token2eventid): ditto.
  • cont.c (return_fiber): ditto.
  • dmydln.c (dln_load): ditto.
  • vminsnhelper.c (vmsearchnormalsuperclass): ditto.
  • bignum.c (big_fdiv): ditto.
  • marshal.c (r_symlink): ditto.
  • marshal.c (r_symbol): ditto.

#14 Updated by Eric Hodel about 2 years ago

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

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

  • % Done changed from 100 to 50

#17 Updated by Nobuyoshi Nakada about 2 years ago

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

Also available in: Atom PDF