Exclude internal frames from backtrace

This changeset suppresses backtrace locations like
`<internal:array>:211` as much as possible.

Before the patch:
```
$ ruby -e '[1].fetch_values(42)'
<internal:array>:211:in 'Array#fetch': index 42 outside of array bounds: -1...1 (IndexError)
        from <internal:array>:211:in 'block in Array#fetch_values'
        from <internal:array>:211:in 'Array#map!'
        from <internal:array>:211:in 'Array#fetch_values'
        from -e:1:in '<main>'
```

After the patch:
```
$ ./miniruby -e '[1].fetch_values(42)'
-e:1:in 'Array#fetch_values': index 42 outside of array bounds: -1...1 (IndexError)
        from -e:1:in '<main>'
```

Specifically:

* The special backtrace handling of BUILTIN_ATTR_C_TRACE is now always
  applied to frames with `<internal:...>`.
* When multiple consecutive internal frames appear, all but the bottom
  (caller-side) frame are removed.

[Misc #20968]

Author: mame

spec/ruby/core/kernel/caller_locations_spec.rb

@@ -83,7 +83,7 @@
       end
     end
 
-    ruby_version_is "3.4" do
+    ruby_version_is "3.4"..."3.5" do
       it "includes core library methods defined in Ruby" do
         file, line = Kernel.instance_method(:tap).source_location
         file.should.start_with?('<internal:')
@@ -94,5 +94,17 @@
         loc.path.should.start_with? "<internal:"
       end
     end
+
+    ruby_version_is "3.5" do
+      it "does not include core library methods defined in Ruby" do
+        file, line = Kernel.instance_method(:tap).source_location
+        file.should.start_with?('<internal:')
+
+        loc = nil
+        tap { loc = caller_locations(1, 1)[0] }
+        loc.label.should == "Kernel#tap"
+        loc.path.should == __FILE__
+      end
+    end
   end
 end

spec/ruby/core/kernel/caller_spec.rb

@@ -84,13 +84,26 @@
   end
 
   guard -> { Kernel.instance_method(:tap).source_location } do
-    it "includes core library methods defined in Ruby" do
-      file, line = Kernel.instance_method(:tap).source_location
-      file.should.start_with?('<internal:')
+    ruby_version_is ""..."3.5" do
+      it "includes core library methods defined in Ruby" do
+        file, line = Kernel.instance_method(:tap).source_location
+        file.should.start_with?('<internal:')
+
+        loc = nil
+        tap { loc = caller(1, 1)[0] }
+        loc.should =~ /\A<internal:.*in [`'](?:Kernel#)?tap'\z/
+      end
+    end
+
+    ruby_version_is "3.5" do
+      it "includes core library methods defined in Ruby" do
+        file, line = Kernel.instance_method(:tap).source_location
+        file.should.start_with?('<internal:')
 
-      loc = nil
-      tap { loc = caller(1, 1)[0] }
-      loc.should =~ /\A<internal:.*in [`'](?:Kernel#)?tap'\z/
+        loc = nil
+        tap { loc = caller(1, 1)[0] }
+        loc.should =~ /\A#{ __FILE__ }:.*in [`'](?:Kernel#)?tap'\z/
+      end
     end
   end
 end

vm_backtrace.c

@@ -262,6 +262,15 @@ calculate_iseq_label(VALUE owner, const rb_iseq_t *iseq)
     }
 }
 
+static bool
+is_internal_location(const rb_iseq_t *iseq)
+{
+    static const char prefix[] = "<internal:";
+    const size_t prefix_len = sizeof(prefix) - 1;
+    VALUE file = rb_iseq_path(iseq);
+    return strncmp(prefix, RSTRING_PTR(file), prefix_len) == 0;
+}
+
 // Return true if a given location is a C method or supposed to behave like one.
 static inline bool
 location_cfunc_p(rb_backtrace_location_t *loc)
@@ -272,7 +281,7 @@ location_cfunc_p(rb_backtrace_location_t *loc)
       case VM_METHOD_TYPE_CFUNC:
         return true;
       case VM_METHOD_TYPE_ISEQ:
-        return rb_iseq_attr_p(loc->cme->def->body.iseq.iseqptr, BUILTIN_ATTR_C_TRACE);
+        return is_internal_location(loc->cme->def->body.iseq.iseqptr);
       default:
         return false;
     }
@@ -604,15 +613,6 @@ backtrace_size(const rb_execution_context_t *ec)
     return start_cfp - last_cfp + 1;
 }
 
-static bool
-is_internal_location(const rb_control_frame_t *cfp)
-{
-    static const char prefix[] = "<internal:";
-    const size_t prefix_len = sizeof(prefix) - 1;
-    VALUE file = rb_iseq_path(cfp->iseq);
-    return strncmp(prefix, RSTRING_PTR(file), prefix_len) == 0;
-}
-
 static bool
 is_rescue_or_ensure_frame(const rb_control_frame_t *cfp)
 {
@@ -691,16 +691,26 @@ rb_ec_partial_backtrace_object(const rb_execution_context_t *ec, long start_fram
                 if (start_frame > 0) {
                     start_frame--;
                 }
-                else if (!(skip_internal && is_internal_location(cfp))) {
+                else {
+                    bool internal = is_internal_location(cfp->iseq);
+                    if (skip_internal && internal) continue;
                     if (!skip_next_frame) {
                         const rb_iseq_t *iseq = cfp->iseq;
                         const VALUE *pc = cfp->pc;
+                        if (internal && backpatch_counter > 0) {
+                            // To keep only one internal frame, discard the previous backpatch frames
+                            bt->backtrace_size -= backpatch_counter;
+                            backpatch_counter = 0;
+                        }
                         loc = &bt->backtrace[bt->backtrace_size++];
                         RB_OBJ_WRITE(btobj, &loc->cme, rb_vm_frame_method_entry(cfp));
-                        // Ruby methods with `Primitive.attr! :c_trace` should behave like C methods
-                        if (rb_iseq_attr_p(cfp->iseq, BUILTIN_ATTR_C_TRACE)) {
-                            loc->iseq = NULL;
-                            loc->pc = NULL;
+                        // internal frames (`<internal:...>`) should behave like C methods
+                        if (internal) {
+                            // Typically, these iseq and pc are not needed because they will be backpatched later.
+                            // But when the call stack starts with an internal frame (i.e., prelude.rb),
+                            // they will be used to show the `<internal:...>` location.
+                            RB_OBJ_WRITE(btobj, &loc->iseq, iseq);
+                            loc->pc = pc;
                             backpatch_counter++;
                         }
                         else {
@@ -736,7 +746,7 @@ rb_ec_partial_backtrace_object(const rb_execution_context_t *ec, long start_fram
     // is the one of the caller Ruby frame, so if the last entry is a C frame we find the caller Ruby frame here.
     if (backpatch_counter > 0) {
         for (; cfp != end_cfp; cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp)) {
-            if (cfp->iseq && cfp->pc && !(skip_internal && is_internal_location(cfp))) {
+            if (cfp->iseq && cfp->pc && !(skip_internal && is_internal_location(cfp->iseq))) {
                 VM_ASSERT(!skip_next_frame); // ISEQ_TYPE_RESCUE/ISEQ_TYPE_ENSURE should have a caller Ruby ISEQ, not a cfunc
                 bt_backpatch_loc(backpatch_counter, loc, cfp->iseq, cfp->pc);
                 RB_OBJ_WRITTEN(btobj, Qundef, cfp->iseq);