JRuby: test_scan_integer_base_16 failed only with ubuntu-latest + jruby-head #125
Comments
|
Probably a bug in the Java strscan logic. |
|
FWIW if you ever see a Java exception come out of executing normal Ruby stuff in JRuby, it's a bug. Even if the caller of scan_base16_integer was doing something weird, they shouldn't be able to produce a broken ByteList that would do this. |
|
I could not reproduce the crash locally... perhaps whatever test is hitting this has input that varies from run to run? I did the following attempt to re-port the C version of the code, and it works, but that doesn't say much when it doesn't crash for me in the first place: diff --git a/ext/jruby/org/jruby/ext/strscan/RubyStringScanner.java b/ext/jruby/org/jruby/ext/strscan/RubyStringScanner.java
index 7e7c492..23364d3 100644
--- a/ext/jruby/org/jruby/ext/strscan/RubyStringScanner.java
+++ b/ext/jruby/org/jruby/ext/strscan/RubyStringScanner.java
@@ -608,40 +608,44 @@ public class RubyStringScanner extends RubyObject {
throw runtime.newEncodingCompatibilityError("ASCII incompatible encoding: " + str.getEncoding());
}
-
ByteList bytes = str.getByteList();
int curr = this.curr;
- int bite = bytes.get(curr);
- if (bite == '-' || bite == '+') {
- curr++;
- bite = bytes.get(curr);
+ int remaining_len = bytes.realSize() - curr;
+
+ if (remaining_len <= 0) {
+ return context.nil;
}
- if (bite == '0' && bytes.get(curr + 1) == 'x') {
- curr += 2;
- bite = bytes.get(curr);
+ int len = 0;
+
+ if (bytes.get(len) == '-' || bytes.get(len) == '+') {
+ len++;
}
- if (!((bite >= '0' && bite <= '9') || (bite >= 'a' && bite <= 'f') || (bite >= 'A' && bite <= 'F'))) {
- return context.nil;
+ if ((remaining_len >= (len + 2)) && bytes.get(len) == '0' && bytes.get(len + 1) == 'x') {
+ len += 2;
}
- while ((bite >= '0' && bite <= '9') || (bite >= 'a' && bite <= 'f') || (bite >= 'A' && bite <= 'F')) {
- curr++;
- if (curr >= bytes.getRealSize()) {
- break;
- }
- bite = bytes.get(curr);
+ if (len >= remaining_len || !isHexChar(bytes.get(len))) {
+ return context.nil;
}
- int length = curr - this.curr;
- prev = this.curr;
- this.curr = curr;
setMatched();
adjustRegisters();
+ prev = curr;
+
+ while (len < remaining_len && isHexChar(bytes.get(len))) {
+ len++;
+ }
+
+ this.curr = curr + len;
+
+ return ConvertBytes.byteListToInum(runtime, bytes, 0, len, 16, true);
+ }
- return ConvertBytes.byteListToInum(runtime, bytes, prev, curr, 16, true);
+ private static boolean isHexChar(int c) {
+ return Character.isDigit(c) || ('a' <= c && c <= 'f') || ('A' <= c && c <= 'F');
} |
|
@headius could you submit this as a PR? |
|
As for the failure, I honestly have no idea what could have happened, but my Java knowledge is very limited, so if there's indeed a difference with the C implementation, we should start by making them converge and see if the problem happens again. |
|
I will push this as a PR along with some code to better log a backtrace if we see this happen again. As it is the Java trace only has JRuby interpreter frames in it, so I can't see what line in the test caused it to fail. |
This may help prevent ArrayIndexOutOfBounds randomly seen in CI. See ruby#125
This is to align the base10 code with the freshly-ported base16 code. See ruby#125
https://github.com/ruby/strscan/actions/runs/12288841240/job/34293324916#step:11:10
The text was updated successfully, but these errors were encountered: