[GR-19220] Fix bounds checking issues with MatchData#[Integer, Integer] (#2638)

PullRequest: truffleruby/3278

Author: bjfish

CHANGELOG.md

@@ -6,6 +6,8 @@ New features:
 Bug fixes:
 
 * Fix `rb_id2name` to ensure the native string will have the same lifetime as the id (#2630, @aardvark179).
+* Fix `MatchData#[]` exception when passing a length argument larger than the number of match values (#2636, @nirvdrum).
+* Fix `MatchData#[]` exception when supplying a large negative index along with a length argument (@nirvdrum).
 
 Compatibility:
 
@@ -82,7 +84,7 @@ Performance:
 * Reduce memory footprint by tracking `VALUE`s created during C extension init separately (@aardvark179).
 * Rewrote `ArrayEachIteratorNode` to optimize performance for a constant-sized array and reduce specializations to 1 general case (#2587, @MattAlp)
 * Reduce conversion of `VALUE`s to native handle during common operations in C extensions (@aardvark179).
-* Improved performance of regex boolean matches (e.g., `Regexp#match?`) by avoiding match data allocation in TRegex (#2558, @nirvdrum).
+* Improved performance of regex boolean matches (e.g., `Regexp#match?`) by avoiding match data allocation in TRegex (#2588, @nirvdrum).
 * Remove overhead when getting using `RDATA_PTR` (@aardvark179).
 * Additional copy operations have been reduced when performing IO (#2536, @aardvark179).
 

spec/ruby/core/matchdata/element_reference_spec.rb

@@ -16,6 +16,12 @@
   it "supports accessors [start, length]" do
     /(.)(.)(\d+)(\d)/.match("THX1138.")[1, 2].should == %w|H X|
     /(.)(.)(\d+)(\d)/.match("THX1138.")[-3, 2].should == %w|X 113|
+
+    # negative index is larger than the number of match values
+    /(.)(.)(\d+)(\d)/.match("THX1138.")[-30, 2].should == nil
+
+    # length argument larger than number of match values is capped to match value length
+    /(.)(.)(\d+)(\d)/.match("THX1138.")[3, 10].should == %w|113 8|
   end
 
   it "supports ranges [start..end]" do

src/main/java/org/truffleruby/core/regexp/MatchDataNodes.java

@@ -285,14 +285,27 @@ protected Object getIndex(RubyMatchData matchData, int index, NotProvided length
         }
 
         @Specialization
-        protected RubyArray getIndex(RubyMatchData matchData, int index, int length,
-                @Cached ConditionProfile normalizedIndexProfile) {
-            // TODO BJF 15-May-2015 Need to handle negative indexes and lengths and out of bounds
+        protected Object getIndex(RubyMatchData matchData, int index, int length,
+                @Cached ConditionProfile normalizedIndexProfile,
+                @Cached ConditionProfile indexOutOfBoundsProfile,
+                @Cached ConditionProfile tooLargeTotalProfile) {
             final Object[] values = getValuesNode.execute(matchData);
+
             if (normalizedIndexProfile.profile(index < 0)) {
                 index += values.length;
+
+                if (indexOutOfBoundsProfile.profile(index < 0)) {
+                    return nil;
+                }
+            }
+
+            int endIndex = index + length;
+            if (tooLargeTotalProfile.profile(endIndex > values.length)) {
+                endIndex = values.length;
             }
-            final Object[] store = Arrays.copyOfRange(values, index, index + length);
+
+            final Object[] store = Arrays.copyOfRange(values, index, endIndex);
+
             return createArray(store);
         }