From 44cb87e1b7f7a57ae03cab308a5ca9b4cf23fda3 Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Tue, 3 Dec 2024 15:33:05 +0200 Subject: [PATCH 1/4] Add rb_syserr_fail_str C function --- CHANGELOG.md | 1 + .../include/truffleruby/truffleruby-abi-version.h | 2 +- spec/ruby/optional/capi/ext/kernel_spec.c | 8 ++++++++ spec/ruby/optional/capi/kernel_spec.rb | 14 ++++++++++++++ src/main/c/cext/exception.c | 9 +++++++++ tool/generate-cext-trampoline.rb | 1 + 6 files changed, 34 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37e8e83be97d..7e18aee581ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ Compatibility: * Support serializing of `Data` instances into Marshal format (#3726, @andrykonchin). * `Array#pack` now raises `ArgumentError` for unknown directives (#3681, @Th3-M4jor). * `String#unpack` now raises `ArgumentError` for unknown directives (#3681, @Th3-M4jor). +* Add `rb_syserr_fail_str()` (#3732, @andrykonchin). Performance: diff --git a/lib/cext/include/truffleruby/truffleruby-abi-version.h b/lib/cext/include/truffleruby/truffleruby-abi-version.h index 9a19f5e409de..e60bddf0a25a 100644 --- a/lib/cext/include/truffleruby/truffleruby-abi-version.h +++ b/lib/cext/include/truffleruby/truffleruby-abi-version.h @@ -20,6 +20,6 @@ // $RUBY_VERSION must be the same as TruffleRuby.LANGUAGE_VERSION. // $ABI_NUMBER starts at 1 and is incremented for every ABI-incompatible change. -#define TRUFFLERUBY_ABI_VERSION "3.3.5.6" +#define TRUFFLERUBY_ABI_VERSION "3.3.5.7" #endif diff --git a/spec/ruby/optional/capi/ext/kernel_spec.c b/spec/ruby/optional/capi/ext/kernel_spec.c index f2ce94bd8914..abd8d20ff4f2 100644 --- a/spec/ruby/optional/capi/ext/kernel_spec.c +++ b/spec/ruby/optional/capi/ext/kernel_spec.c @@ -251,6 +251,13 @@ VALUE kernel_spec_rb_syserr_fail(VALUE self, VALUE err, VALUE msg) { return Qnil; } +VALUE kernel_spec_rb_syserr_fail_str(VALUE self, VALUE err, VALUE msg) { + if (self != Qundef) { + rb_syserr_fail_str(NUM2INT(err), msg); + } + return Qnil; +} + VALUE kernel_spec_rb_warn(VALUE self, VALUE msg) { rb_warn("%s", StringValuePtr(msg)); return Qnil; @@ -415,6 +422,7 @@ void Init_kernel_spec(void) { rb_define_method(cls, "rb_catch_obj", kernel_spec_rb_catch_obj, 2); rb_define_method(cls, "rb_sys_fail", kernel_spec_rb_sys_fail, 1); rb_define_method(cls, "rb_syserr_fail", kernel_spec_rb_syserr_fail, 2); + rb_define_method(cls, "rb_syserr_fail_str", kernel_spec_rb_syserr_fail_str, 2); rb_define_method(cls, "rb_warn", kernel_spec_rb_warn, 1); rb_define_method(cls, "rb_yield", kernel_spec_rb_yield, 1); rb_define_method(cls, "rb_yield_indirected", kernel_spec_rb_yield_indirected, 1); diff --git a/spec/ruby/optional/capi/kernel_spec.rb b/spec/ruby/optional/capi/kernel_spec.rb index 43eea07182c9..5c0db9eded24 100644 --- a/spec/ruby/optional/capi/kernel_spec.rb +++ b/spec/ruby/optional/capi/kernel_spec.rb @@ -197,6 +197,20 @@ class CApiKernelSpecs::Exc < StandardError end end + describe "rb_syserr_fail_str" do + it "raises an exception from the given error" do + -> do + @s.rb_syserr_fail_str(Errno::EINVAL::Errno, "additional info") + end.should raise_error(Errno::EINVAL, /additional info/) + end + + it "can take nil as a message" do + -> do + @s.rb_syserr_fail_str(Errno::EINVAL::Errno, nil) + end.should raise_error(Errno::EINVAL) + end + end + describe "rb_yield" do it "yields passed argument" do ret = nil diff --git a/src/main/c/cext/exception.c b/src/main/c/cext/exception.c index fb240a32d81f..00db66949047 100644 --- a/src/main/c/cext/exception.c +++ b/src/main/c/cext/exception.c @@ -66,6 +66,15 @@ void rb_syserr_fail(int eno, const char *message) { UNREACHABLE; } +void rb_syserr_fail_str(int eno, VALUE message) { + if (message == Qnil) { + message = rb_str_new_cstr(""); + } + + polyglot_invoke(RUBY_CEXT, "rb_syserr_fail", eno, rb_tr_unwrap(message)); + UNREACHABLE; +} + #undef rb_sys_fail void rb_sys_fail(const char *message) { int n = errno; diff --git a/tool/generate-cext-trampoline.rb b/tool/generate-cext-trampoline.rb index 56a535330970..86b99ea892b7 100755 --- a/tool/generate-cext-trampoline.rb +++ b/tool/generate-cext-trampoline.rb @@ -19,6 +19,7 @@ rb_exc_raise rb_jump_tag rb_syserr_fail + rb_syserr_fail_str rb_sys_fail rb_sys_fail_str rb_throw From 431177dec04f199f546fdfcfe74d628468796cfc Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Tue, 3 Dec 2024 16:20:53 +0200 Subject: [PATCH 2/4] Fix SystemCallError.new called with nil message value --- .../core/exception/system_call_error_spec.rb | 16 ++++++++++++++++ src/main/ruby/truffleruby/core/exception.rb | 3 ++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/spec/ruby/core/exception/system_call_error_spec.rb b/spec/ruby/core/exception/system_call_error_spec.rb index 73167bc28892..517b67d8a4fe 100644 --- a/spec/ruby/core/exception/system_call_error_spec.rb +++ b/spec/ruby/core/exception/system_call_error_spec.rb @@ -53,6 +53,11 @@ def initialize e.should be_an_instance_of(@example_errno_class) end + it "sets an error message corresponding to an appropriate Errno class" do + e = SystemCallError.new(@example_errno) + e.message.should == 'Invalid argument' + end + it "accepts an optional custom message preceding the errno" do exc = SystemCallError.new("custom message", @example_errno) exc.should be_an_instance_of(@example_errno_class) @@ -81,6 +86,17 @@ def initialize SystemCallError.new('foo', 2.9).should == SystemCallError.new('foo', 2) end + it "treats nil errno as unknown error value" do + SystemCallError.new(nil).should be_an_instance_of(SystemCallError) + end + + it "treats nil custom message as if it is not passed at all" do + exc = SystemCallError.new(nil, @example_errno) + exc.should be_an_instance_of(@example_errno_class) + exc.errno.should == @example_errno + exc.message.should == 'Invalid argument' + end + it "converts to Integer if errno is a Complex convertible to Integer" do SystemCallError.new('foo', Complex(2.9, 0)).should == SystemCallError.new('foo', 2) end diff --git a/src/main/ruby/truffleruby/core/exception.rb b/src/main/ruby/truffleruby/core/exception.rb index 3bae02ec9b43..5e445a2b03ac 100644 --- a/src/main/ruby/truffleruby/core/exception.rb +++ b/src/main/ruby/truffleruby/core/exception.rb @@ -298,7 +298,8 @@ def self.new(*args) message = nil else errno = nil - message = StringValue(args.first) + message = args.first + message = StringValue(message) unless Primitive.nil?(message) end location = nil when 2 From 7d8b1617f987fb628f9eb15e569dd3bae67de522 Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Tue, 3 Dec 2024 16:58:14 +0200 Subject: [PATCH 3/4] Fix SystemCallError message when errno is unknown --- .../core/exception/system_call_error_spec.rb | 10 +++++++-- spec/ruby/optional/capi/kernel_spec.rb | 20 +++++++++++++---- .../java/org/truffleruby/cext/CExtNodes.java | 2 +- .../core/exception/ErrnoErrorNode.java | 22 ++++++++++++------- .../core/exception/ExceptionNodes.java | 4 ++-- .../core/time/RubyDateFormatter.java | 4 ++-- src/main/ruby/truffleruby/core/exception.rb | 10 ++------- .../core/truffle/exception_operations.rb | 11 +++++----- 8 files changed, 50 insertions(+), 33 deletions(-) diff --git a/spec/ruby/core/exception/system_call_error_spec.rb b/spec/ruby/core/exception/system_call_error_spec.rb index 517b67d8a4fe..7235aa9c268c 100644 --- a/spec/ruby/core/exception/system_call_error_spec.rb +++ b/spec/ruby/core/exception/system_call_error_spec.rb @@ -92,11 +92,17 @@ def initialize it "treats nil custom message as if it is not passed at all" do exc = SystemCallError.new(nil, @example_errno) - exc.should be_an_instance_of(@example_errno_class) - exc.errno.should == @example_errno exc.message.should == 'Invalid argument' end + it "sets an 'unknown error' message when an unknown error number" do + SystemCallError.new(-1).message.should =~ /Unknown error(:)? -1/ + end + + it "adds a custom error message to an 'unknown error' message when an unknown error number and a custom message specified" do + SystemCallError.new("custom message", -1).message.should =~ /Unknown error(:)? -1 - custom message/ + end + it "converts to Integer if errno is a Complex convertible to Integer" do SystemCallError.new('foo', Complex(2.9, 0)).should == SystemCallError.new('foo', 2) end diff --git a/spec/ruby/optional/capi/kernel_spec.rb b/spec/ruby/optional/capi/kernel_spec.rb index 5c0db9eded24..96d61ee003c4 100644 --- a/spec/ruby/optional/capi/kernel_spec.rb +++ b/spec/ruby/optional/capi/kernel_spec.rb @@ -187,13 +187,19 @@ class CApiKernelSpecs::Exc < StandardError it "raises an exception from the given error" do -> do @s.rb_syserr_fail(Errno::EINVAL::Errno, "additional info") - end.should raise_error(Errno::EINVAL, /additional info/) + end.should raise_error(Errno::EINVAL, "Invalid argument - additional info") end it "can take a NULL message" do -> do @s.rb_syserr_fail(Errno::EINVAL::Errno, nil) - end.should raise_error(Errno::EINVAL) + end.should raise_error(Errno::EINVAL, "Invalid argument") + end + + it "uses an 'unknown error' message when errno is unknown" do + -> do + @s.rb_syserr_fail(-10, nil) + end.should raise_error(SystemCallError, /Unknown error(:)? -1/) # a new class Errno::E-01 is generated on the fly end end @@ -201,13 +207,19 @@ class CApiKernelSpecs::Exc < StandardError it "raises an exception from the given error" do -> do @s.rb_syserr_fail_str(Errno::EINVAL::Errno, "additional info") - end.should raise_error(Errno::EINVAL, /additional info/) + end.should raise_error(Errno::EINVAL, "Invalid argument - additional info") end it "can take nil as a message" do -> do @s.rb_syserr_fail_str(Errno::EINVAL::Errno, nil) - end.should raise_error(Errno::EINVAL) + end.should raise_error(Errno::EINVAL, "Invalid argument") + end + + it "uses an 'unknown error' message when errno is unknown" do + -> do + @s.rb_syserr_fail_str(-1, nil) + end.should raise_error(SystemCallError, /Unknown error(:)? -1/) # a new class Errno::E-01 is generated on the fly end end diff --git a/src/main/java/org/truffleruby/cext/CExtNodes.java b/src/main/java/org/truffleruby/cext/CExtNodes.java index 9098ba6d957e..08863a10281e 100644 --- a/src/main/java/org/truffleruby/cext/CExtNodes.java +++ b/src/main/java/org/truffleruby/cext/CExtNodes.java @@ -1313,7 +1313,7 @@ public abstract static class RbSysErrFail extends CoreMethodArrayArgumentsNode { Object rbSysErrFail(int errno, Object string, @Cached ErrnoErrorNode errnoErrorNode) { final Backtrace backtrace = getContext().getCallStack().getBacktrace(this); - throw new RaiseException(getContext(), errnoErrorNode.execute(null, errno, string, backtrace)); + throw new RaiseException(getContext(), errnoErrorNode.execute(null, errno, string, null, backtrace)); } } diff --git a/src/main/java/org/truffleruby/core/exception/ErrnoErrorNode.java b/src/main/java/org/truffleruby/core/exception/ErrnoErrorNode.java index 01921f8965f1..603c9705b7bd 100644 --- a/src/main/java/org/truffleruby/core/exception/ErrnoErrorNode.java +++ b/src/main/java/org/truffleruby/core/exception/ErrnoErrorNode.java @@ -18,6 +18,7 @@ import org.truffleruby.core.klass.RubyClass; import org.truffleruby.core.string.RubyString; import org.truffleruby.core.string.ImmutableRubyString; +import org.truffleruby.language.Nil; import org.truffleruby.language.RubyBaseNode; import org.truffleruby.language.backtrace.Backtrace; import org.truffleruby.language.dispatch.DispatchNode; @@ -33,10 +34,11 @@ public static ErrnoErrorNode create() { @Child private DispatchNode formatMessageNode; public abstract RubySystemCallError execute(RubyClass rubyClass, int errno, Object extraMessage, - Backtrace backtrace); + Object location, Backtrace backtrace); @Specialization - RubySystemCallError errnoError(RubyClass rubyClass, int errno, Object extraMessage, Backtrace backtrace, + RubySystemCallError errnoError( + RubyClass rubyClass, int errno, Object extraMessage, Object location, Backtrace backtrace, @Cached TruffleString.FromJavaStringNode fromJavaStringNode) { final String errnoName = getContext().getCoreLibrary().getErrnoName(errno); @@ -57,26 +59,30 @@ RubySystemCallError errnoError(RubyClass rubyClass, int errno, Object extraMessa Encodings.UTF_8); } - final RubyString errorMessage = formatMessage(errnoDescription, errno, extraMessage); + final RubyString errorMessage = formatMessage(errnoDescription, errno, extraMessage, location); return ExceptionOperations .createSystemCallError(getContext(), errnoClass, errorMessage, errno, backtrace); } - private RubyString formatMessage(Object errnoDescription, int errno, Object extraMessage) { - assert extraMessage instanceof RubyString || extraMessage instanceof ImmutableRubyString; + private RubyString formatMessage(Object errnoDescription, int errno, Object extraMessage, Object location) { + assert extraMessage instanceof RubyString || extraMessage instanceof ImmutableRubyString || + extraMessage instanceof Nil; if (formatMessageNode == null) { CompilerDirectives.transferToInterpreterAndInvalidate(); formatMessageNode = insert(DispatchNode.create()); } + if (location == null) { + location = nil; + } + + var arguments = new Object[]{ errnoDescription, errno, extraMessage, location }; return (RubyString) formatMessageNode.call( getContext().getCoreLibrary().truffleExceptionOperationsModule, "format_errno_error_message", - errnoDescription, - errno, - extraMessage); + arguments); } } diff --git a/src/main/java/org/truffleruby/core/exception/ExceptionNodes.java b/src/main/java/org/truffleruby/core/exception/ExceptionNodes.java index 60ff586cd094..db40d5ef92af 100644 --- a/src/main/java/org/truffleruby/core/exception/ExceptionNodes.java +++ b/src/main/java/org/truffleruby/core/exception/ExceptionNodes.java @@ -321,8 +321,8 @@ public abstract static class ExceptionErrnoErrorPrimitiveNode extends PrimitiveA @Child ErrnoErrorNode errnoErrorNode = ErrnoErrorNode.create(); @Specialization - RubySystemCallError exceptionErrnoError(RubyClass errorClass, Object message, int errno) { - return errnoErrorNode.execute(errorClass, errno, message, null); + RubySystemCallError exceptionErrnoError(RubyClass errorClass, Object extraMessage, int errno, Object location) { + return errnoErrorNode.execute(errorClass, errno, extraMessage, location, null); } } diff --git a/src/main/java/org/truffleruby/core/time/RubyDateFormatter.java b/src/main/java/org/truffleruby/core/time/RubyDateFormatter.java index 7321b5c29d78..d476935e02b8 100644 --- a/src/main/java/org/truffleruby/core/time/RubyDateFormatter.java +++ b/src/main/java/org/truffleruby/core/time/RubyDateFormatter.java @@ -546,11 +546,11 @@ public static TStringBuilder formatToTStringBuilder(Token[] compiledPattern, Zon } catch (IndexOutOfBoundsException ioobe) { final Backtrace backtrace = context.getCallStack().getBacktrace(currentNode); final RubyString message = StringOperations.createUTF8String(context, language, "strftime"); + final int errno = context.getCoreLibrary().getErrnoValue("ERANGE"); throw new RaiseException( context, - errnoErrorNode.execute(null, context.getCoreLibrary().getErrnoValue("ERANGE"), message, - backtrace)); + errnoErrorNode.execute(null, errno, message, null, backtrace)); } // reset formatter diff --git a/src/main/ruby/truffleruby/core/exception.rb b/src/main/ruby/truffleruby/core/exception.rb index 5e445a2b03ac..5a4215c1ee5e 100644 --- a/src/main/ruby/truffleruby/core/exception.rb +++ b/src/main/ruby/truffleruby/core/exception.rb @@ -274,12 +274,6 @@ def success? class SystemCallError < StandardError - def self.errno_error(klass, message, errno, location) - message = message ? " - #{message}" : +'' - message = " @ #{location}#{message}" if location - Primitive.exception_errno_error klass, message, errno - end - # We use .new here because when errno is set, we attempt to # lookup and return a subclass of SystemCallError, specifically, # one of the Errno subclasses. @@ -315,7 +309,7 @@ def self.new(*args) # If it corresponds to a known Errno class, create and return it now if errno errno = Primitive.rb_num2long(errno) - error = SystemCallError.errno_error(self, message, errno, location) + error = Primitive.exception_errno_error(self, message, errno, location) return error unless Primitive.nil? error end super(message, errno, location) @@ -334,7 +328,7 @@ def self.new(*args) end if defined?(self::Errno) && Primitive.is_a?(self::Errno, Integer) - error = SystemCallError.errno_error(self, message, self::Errno, location) + error = Primitive.exception_errno_error(self, message, self::Errno, location) if error && Primitive.equal?(Primitive.class(error), self) return error end diff --git a/src/main/ruby/truffleruby/core/truffle/exception_operations.rb b/src/main/ruby/truffleruby/core/truffle/exception_operations.rb index 243eb4db4f14..4c5b9303db2f 100644 --- a/src/main/ruby/truffleruby/core/truffle/exception_operations.rb +++ b/src/main/ruby/truffleruby/core/truffle/exception_operations.rb @@ -352,12 +352,11 @@ def self.comparison_error_message(x, y) format("super: no superclass method `%s'", exception.name) end - def self.format_errno_error_message(errno_description, errno, extra_message) - if Primitive.nil? errno_description - "unknown error: #{errno} - #{extra_message}" - else - "#{errno_description}#{extra_message}" - end + def self.format_errno_error_message(errno_description, errno, extra_message, location) + message = errno_description || "Unknown error: #{errno}" + message += " @ #{location}" if location + message += " - #{extra_message}" if extra_message + +message end end end From a19a172eb9946b4b38b243f06a011e6ffc3656a5 Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Tue, 3 Dec 2024 17:07:27 +0200 Subject: [PATCH 4/4] Refactor rb_syserr_fail and rb_syserr_fail_str to call rb_syserr_fail Ruby method with nil instead of "" when custom message is NULL or Qnil --- lib/cext/ABI_check.txt | 2 +- src/main/c/cext/exception.c | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/cext/ABI_check.txt b/lib/cext/ABI_check.txt index b8626c4cff28..7ed6ff82de6b 100644 --- a/lib/cext/ABI_check.txt +++ b/lib/cext/ABI_check.txt @@ -1 +1 @@ -4 +5 diff --git a/src/main/c/cext/exception.c b/src/main/c/cext/exception.c index 00db66949047..8c204cd82a66 100644 --- a/src/main/c/cext/exception.c +++ b/src/main/c/cext/exception.c @@ -62,15 +62,12 @@ VALUE rb_errinfo(void) { } void rb_syserr_fail(int eno, const char *message) { - polyglot_invoke(RUBY_CEXT, "rb_syserr_fail", eno, rb_tr_unwrap(rb_str_new_cstr(message == NULL ? "" : message))); + VALUE messageValue = (message == NULL) ? Qnil : rb_str_new_cstr(message); + polyglot_invoke(RUBY_CEXT, "rb_syserr_fail", eno, rb_tr_unwrap(messageValue)); UNREACHABLE; } void rb_syserr_fail_str(int eno, VALUE message) { - if (message == Qnil) { - message = rb_str_new_cstr(""); - } - polyglot_invoke(RUBY_CEXT, "rb_syserr_fail", eno, rb_tr_unwrap(message)); UNREACHABLE; }