`Exception#set_backtrace` accept arrays of `Backtrace::Location`

[Feature #13557]

Setting the backtrace with an array of strings is lossy. The resulting
exception will return nil on `#backtrace_locations`.

By accepting an array of `Backtrace::Location` instance, we can rebuild
a `Backtrace` instance and have a fully functioning Exception.

Co-Authored-By: Étienne Barrié <etienne.barrie@gmail.com>

6 files changed, 104 insertions, 11 deletions

diff --git a/error.c b/error.c
index 9c5b70d1d6..7eda2ed538 100644
--- a/error.c
+++ b/error.c
@@ -1839,7 +1839,7 @@ static VALUE
 rb_check_backtrace(VALUE bt)
 {
     long i;
-    static const char err[] = "backtrace must be Array of String";
+    static const char err[] = "backtrace must be an Array of String or an Array of Thread::Backtrace::Location";
 
     if (!NIL_P(bt)) {
         if (RB_TYPE_P(bt, T_STRING)) return rb_ary_new3(1, bt);
@@ -1870,7 +1870,15 @@ rb_check_backtrace(VALUE bt)
 static VALUE
 exc_set_backtrace(VALUE exc, VALUE bt)
 {
-    return rb_ivar_set(exc, id_bt, rb_check_backtrace(bt));
+    VALUE btobj = rb_location_ary_to_backtrace(bt);
+    if (RTEST(btobj)) {
+        rb_ivar_set(exc, id_bt, btobj);
+        rb_ivar_set(exc, id_bt_locations, btobj);
+        return bt;
+    }
+    else {
+        return rb_ivar_set(exc, id_bt, rb_check_backtrace(bt));
+    }
 }
 
 VALUE
diff --git a/internal/vm.h b/internal/vm.h
index 1adce8a390..74635e6ad8 100644
--- a/internal/vm.h
+++ b/internal/vm.h
@@ -114,6 +114,7 @@ void rb_backtrace_print_as_bugreport(FILE*);
 int rb_backtrace_p(VALUE obj);
 VALUE rb_backtrace_to_str_ary(VALUE obj);
 VALUE rb_backtrace_to_location_ary(VALUE obj);
+VALUE rb_location_ary_to_backtrace(VALUE ary);
 void rb_backtrace_each(VALUE (*iter)(VALUE recv, VALUE str), VALUE output);
 int rb_frame_info_p(VALUE obj);
 int rb_get_node_id_from_frame_info(VALUE obj);
diff --git a/spec/ruby/core/exception/set_backtrace_spec.rb b/spec/ruby/core/exception/set_backtrace_spec.rb
index ba2e1bf7aa..12c1da919c 100644
--- a/spec/ruby/core/exception/set_backtrace_spec.rb
+++ b/spec/ruby/core/exception/set_backtrace_spec.rb
@@ -11,9 +11,37 @@ describe "Exception#set_backtrace" do
   it "allows the user to set the backtrace from a rescued exception" do
     bt  = ExceptionSpecs::Backtrace.backtrace
     err = RuntimeError.new
+    err.backtrace.should == nil
+    err.backtrace_locations.should == nil
 
     err.set_backtrace bt
+
     err.backtrace.should == bt
+    err.backtrace_locations.should == nil
+  end
+
+  ruby_version_is "3.4" do
+    it "allows the user to set backtrace locations from a rescued exception" do
+      bt_locations = ExceptionSpecs::Backtrace.backtrace_locations
+      err = RuntimeError.new
+      err.backtrace.should == nil
+      err.backtrace_locations.should == nil
+
+      err.set_backtrace bt_locations
+
+      err.backtrace_locations.size.should == bt_locations.size
+      err.backtrace_locations.each_with_index do |loc, index|
+        other_loc = bt_locations[index]
+
+        loc.path.should == other_loc.path
+        loc.label.should == other_loc.label
+        loc.base_label.should == other_loc.base_label
+        loc.lineno.should == other_loc.lineno
+        loc.absolute_path.should == other_loc.absolute_path
+        loc.to_s.should == other_loc.to_s
+      end
+      err.backtrace.size.should == err.backtrace_locations.size
+    end
   end
 
   it "accepts an empty Array" do
diff --git a/spec/ruby/shared/kernel/raise.rb b/spec/ruby/shared/kernel/raise.rb
index 82fb0333c8..4b60951f24 100644
--- a/spec/ruby/shared/kernel/raise.rb
+++ b/spec/ruby/shared/kernel/raise.rb
@@ -146,4 +146,12 @@ describe :kernel_raise, shared: true do
       @object.raise(ArgumentError, "message", caller)
     end.should raise_error(ArgumentError, "message")
   end
+
+  ruby_version_is "3.4" do
+    it "allows Exception, message, and backtrace_locations parameters" do
+      -> do
+        @object.raise(ArgumentError, "message", caller_locations)
+      end.should raise_error(ArgumentError, "message")
+    end
+  end
 end
diff --git a/test/ruby/test_exception.rb b/test/ruby/test_exception.rb
index 09a6541e90..aa94968699 100644
--- a/test/ruby/test_exception.rb
+++ b/test/ruby/test_exception.rb
@@ -416,7 +416,7 @@ class TestException < Test::Unit::TestCase
 
     assert_in_out_err([], "$@ = 1", [], /\$! not set \(ArgumentError\)$/)
 
-    assert_in_out_err([], <<-INPUT, [], /backtrace must be Array of String \(TypeError\)$/)
+    assert_in_out_err([], <<-INPUT, [], /backtrace must be an Array of String or an Array of Thread::Backtrace::Location \(TypeError\)$/)
       begin
         raise
       rescue
@@ -508,6 +508,16 @@ end.join
 
     assert_raise(TypeError) { e.set_backtrace(1) }
     assert_raise(TypeError) { e.set_backtrace([1]) }
+
+    error = assert_raise(TypeError) do
+      e.set_backtrace(caller_locations(1, 1) + ["foo"])
+    end
+    assert_include error.message, "backtrace must be an Array of String or an Array of Thread::Backtrace::Location"
+
+    error = assert_raise(TypeError) do
+      e.set_backtrace(["foo"] + caller_locations(1, 1))
+    end
+    assert_include error.message, "backtrace must be an Array of String or an Array of Thread::Backtrace::Location"
   end
 
   def test_exit_success_p
@@ -1421,11 +1431,7 @@ $stderr = $stdout; raise "\x82\xa0"') do |outs, errs, status|
   end
 
   def test_marshal_circular_cause
-    begin
-      raise RuntimeError, "err", [], cause: Exception.new
-    rescue => e
-    end
-    dump = Marshal.dump(e).sub(/o:\x0EException\x08;.0;.0;.0/, "@\x05")
+    dump = "\x04\bo:\x11RuntimeError\b:\tmesgI\"\berr\x06:\x06ET:\abt[\x00:\ncause@\x05"
     assert_raise_with_message(ArgumentError, /circular cause/, ->{dump.inspect}) do
       e = Marshal.load(dump)
       assert_same(e, e.cause)
diff --git a/vm_backtrace.c b/vm_backtrace.c
index e36347b406..9b256fa4e8 100644
--- a/vm_backtrace.c
+++ b/vm_backtrace.c
@@ -531,6 +531,16 @@ backtrace_alloc(VALUE klass)
     return obj;
 }
 
+static VALUE
+backtrace_alloc_capa(long num_frames, rb_backtrace_t **backtrace)
+{
+    size_t memsize = offsetof(rb_backtrace_t, backtrace) + num_frames * sizeof(rb_backtrace_location_t);
+    VALUE btobj = rb_data_typed_object_zalloc(rb_cBacktrace, memsize, &backtrace_data_type);
+    TypedData_Get_Struct(btobj, rb_backtrace_t, &backtrace_data_type, *backtrace);
+    return btobj;
+}
+
+
 static long
 backtrace_size(const rb_execution_context_t *ec)
 {
@@ -617,9 +627,7 @@ rb_ec_partial_backtrace_object(const rb_execution_context_t *ec, long start_fram
         }
     }
 
-    size_t memsize = offsetof(rb_backtrace_t, backtrace) + num_frames * sizeof(rb_backtrace_location_t);
-    btobj = rb_data_typed_object_zalloc(rb_cBacktrace, memsize, &backtrace_data_type);
-    TypedData_Get_Struct(btobj, rb_backtrace_t, &backtrace_data_type, bt);
+    btobj = backtrace_alloc_capa(num_frames, &bt);
 
     bt->backtrace_size = 0;
     if (num_frames == 0) {
@@ -783,6 +791,40 @@ rb_backtrace_to_location_ary(VALUE self)
     return bt->locary;
 }
 
+VALUE
+rb_location_ary_to_backtrace(VALUE ary)
+{
+    if (!RB_TYPE_P(ary, T_ARRAY) || !rb_frame_info_p(RARRAY_AREF(ary, 0))) {
+        return Qfalse;
+    }
+
+    rb_backtrace_t *new_backtrace;
+    long num_frames = RARRAY_LEN(ary);
+    VALUE btobj = backtrace_alloc_capa(num_frames, &new_backtrace);
+
+    for (long index = 0; index < RARRAY_LEN(ary); index++) {
+        VALUE locobj = RARRAY_AREF(ary, index);
+
+        if (!rb_frame_info_p(locobj)) {
+            return Qfalse;
+        }
+
+        struct valued_frame_info *src_vloc;
+        TypedData_Get_Struct(locobj, struct valued_frame_info, &location_data_type, src_vloc);
+
+        rb_backtrace_location_t *dst_location = &new_backtrace->backtrace[index];
+        RB_OBJ_WRITE(btobj, &dst_location->cme, src_vloc->loc->cme);
+        RB_OBJ_WRITE(btobj, &dst_location->iseq, src_vloc->loc->iseq);
+        dst_location->pc = src_vloc->loc->pc;
+
+        new_backtrace->backtrace_size++;
+
+        RB_GC_GUARD(locobj);
+    }
+
+    return btobj;
+}
+
 static VALUE
 backtrace_dump_data(VALUE self)
 {