summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYusuke Endoh <mame@ruby-lang.org>2020-09-05 21:18:45 +0900
committerYusuke Endoh <mame@ruby-lang.org>2020-09-06 13:57:41 +0900
commit369cfabd5936ccb522f8e95e0f9cc65b59ea4039 (patch)
tree81eea199cc4f0fa663d709056eebe1f4f1a139b5
parentc12b2703bc05f8c7eaaace49253f63a5e0f28273 (diff)
Make it possible to dump and load an exception object
A backtrace object in an exception had never supported marshalling correctly: `Marshal.load(Marshal.dump(exc)).backtrace_locations` dumped core. An Exception object has two hidden instance varibles for backtrace data: one is "bt", which has an Array of Strings, and the other is "bt_locations", which has an Array of Thread::Backtrace::Locations. However, Exception's dump outputs data so that the two variables are the same Array of Strings. Thus, "bt_locations" had a wrong-type object. For the compatibility, it is difficult to change the dump format. This changeset fixes the issue by ignoring data for "bt_locations" at the loading phase if "bt_locations" refers to the same object as "bt". Future work: Exception's dump should output "bt_locations" appropriately. https://bugs.ruby-lang.org/issues/17150
Notes
Notes: Merged: https://github.com/ruby/ruby/pull/3521
-rw-r--r--error.c46
-rw-r--r--test/ruby/test_marshal.rb31
2 files changed, 77 insertions, 0 deletions
diff --git a/error.c b/error.c
index 70f24a00f0..61fa86b2d2 100644
--- a/error.c
+++ b/error.c
@@ -2599,10 +2599,56 @@ syserr_eqq(VALUE self, VALUE exc)
* * fatal
*/
+static VALUE
+exception_alloc(VALUE klass)
+{
+ return rb_class_allocate_instance(klass);
+}
+
+static VALUE
+exception_dumper(VALUE exc)
+{
+ // TODO: Currently, the instance variables "bt" and "bt_locations"
+ // refers to the same object (Array of String). But "bt_locations"
+ // should have an Array of Thread::Backtrace::Locations.
+
+ return exc;
+}
+
+static int
+ivar_copy_i(st_data_t key, st_data_t val, st_data_t exc)
+{
+ rb_ivar_set((VALUE) exc, (ID) key, (VALUE) val);
+ return ST_CONTINUE;
+}
+
+static VALUE
+exception_loader(VALUE exc, VALUE obj)
+{
+ // The loader function of rb_marshal_define_compat seems to be called for two events:
+ // one is for fixup (r_fixup_compat), the other is for TYPE_USERDEF.
+ // In the former case, the first argument is an instance of Exception (because
+ // we pass rb_eException to rb_marshal_define_compat). In the latter case, the first
+ // argument is a class object (see TYPE_USERDEF case in r_object0).
+ // We want to copy all instance variables (but "bt_locations) from obj to exc.
+ // But we do not want to do so in the second case, so the following branch is for that.
+ if (RB_TYPE_P(exc, T_CLASS)) return obj; // maybe called from Marshal's TYPE_USERDEF
+
+ rb_ivar_foreach(obj, ivar_copy_i, exc);
+
+ if (rb_ivar_get(exc, id_bt) == rb_ivar_get(exc, id_bt_locations)) {
+ rb_ivar_set(exc, id_bt_locations, Qnil);
+ }
+
+ return exc;
+}
+
void
Init_Exception(void)
{
rb_eException = rb_define_class("Exception", rb_cObject);
+ rb_define_alloc_func(rb_eException, exception_alloc);
+ rb_marshal_define_compat(rb_eException, rb_eException, exception_dumper, exception_loader);
rb_define_singleton_method(rb_eException, "exception", rb_class_new_instance, -1);
rb_define_singleton_method(rb_eException, "to_tty?", exc_s_to_tty_p, 0);
rb_define_method(rb_eException, "exception", exc_exception, -1);
diff --git a/test/ruby/test_marshal.rb b/test/ruby/test_marshal.rb
index 1d096adb18..fd62ea774b 100644
--- a/test/ruby/test_marshal.rb
+++ b/test/ruby/test_marshal.rb
@@ -780,4 +780,35 @@ class TestMarshal < Test::Unit::TestCase
hash = Marshal.load(Marshal.dump(flagged_hash))
assert_equal(42, ruby2_keywords_test(*[hash]))
end
+
+ def exception_test
+ raise
+ end
+
+ def test_marshal_exception
+ begin
+ exception_test
+ rescue => e
+ e2 = Marshal.load(Marshal.dump(e))
+ assert_equal(e.message, e2.message)
+ assert_equal(e.backtrace, e2.backtrace)
+ assert_nil(e2.backtrace_locations) # temporal
+ end
+ end
+
+ def nameerror_test
+ unknown_method
+ end
+
+ def test_marshal_nameerror
+ begin
+ nameerror_test
+ rescue NameError => e
+ e2 = Marshal.load(Marshal.dump(e))
+ assert_equal(e.message, e2.message)
+ assert_equal(e.name, e2.name)
+ assert_equal(e.backtrace, e2.backtrace)
+ assert_nil(e2.backtrace_locations) # temporal
+ end
+ end
end