Skip to content

mruby-regexp: relocate the lookaround offsets with the code they name #7213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
matz merged 1 commit into from
Aug 17, 2026
+72 −25
Merged

mruby-regexp: relocate the lookaround offsets with the code they name #7213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 46 additions & 25 deletions mrbgems/mruby-regexp/src/re_compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,33 @@ patch(re_compiler *c, uint32_t pos, uint16_t offset)
c->code[pos].offset = offset;
}

/* True for the opcodes whose `offset` field holds an absolute code index, so
that relocating code has to carry it along. The jumps are the obvious ones;
the four lookarounds hold the end of their sub-pattern there
(re_internal.h), which is a code index just as much as a jump target and is
just as wrong when the code it names moves. Every relocator asks this one
question, because the two that used to carry their own list disagreed with
each other and with the opcode set: both relocated the jumps alone, and a
lookaround inside a quantified, repeated or alternated group kept an offset
pointing at whatever the shift left in its place.
RE_LB_WIDTH is not here: it carries a character count in `a`, and RE_SAVE
and RE_BACKREF put a slot number and a case-fold flag in `offset` rather
than an index. */
static mrb_bool
op_holds_code_index(uint8_t op)
{
switch (op) {
case RE_JMP: case RE_SPLIT: case RE_SPLITNG:
case RE_LOOKAHEAD: case RE_NEG_LOOKAHEAD:
case RE_LOOKBEHIND: case RE_NEG_LOOKBEHIND:
return TRUE;
default:
return FALSE;
}
}

/* Insert an instruction at position `pos` by shifting code.
Adjusts jump targets so they still point at the same instructions. */
Adjusts code indices so they still point at the same instructions. */
static void
insert_inst(re_compiler *c, uint32_t pos, uint8_t op, uint8_t a, uint16_t offset)
{
Expand All @@ -118,23 +143,22 @@ insert_inst(re_compiler *c, uint32_t pos, uint8_t op, uint8_t a, uint16_t offset
c->code[pos].a = a;
c->code[pos].offset = offset;

/* Fix jump targets across the insertion. A target past `pos` shifts down by
one. A target equal to `pos` is ambiguous:
/* Fix code indices across the insertion. An index past `pos` shifts down by
one. An index equal to `pos` is ambiguous:
- code that moved (i > pos) is a backward jump -- e.g. the SPLIT that
loops `\d+` back to its class -- and meant the instruction now at
pos+1, so it must follow.
- code before the insertion (i < pos) is a forward "skip to here"
reference that should stay on the newly inserted instruction. */
reference that should stay on the newly inserted instruction. A
lookaround is always that second case, since the end of its sub-pattern
lies ahead of it: leaving the end at `pos` puts the inserted
instruction after the sub-pattern rather than inside it, which is what
the quantifier wrapping the whole group wants. */
for (uint32_t i = 0; i < c->code_len; i++) {
if (i == pos) continue;
switch (c->code[i].op) {
case RE_JMP: case RE_SPLIT: case RE_SPLITNG:
if (c->code[i].offset > pos || (c->code[i].offset == pos && i > pos)) {
c->code[i].offset++;
}
break;
default:
break;
if (op_holds_code_index(c->code[i].op) &&
(c->code[i].offset > pos || (c->code[i].offset == pos && i > pos))) {
c->code[i].offset++;
}
}
}
Expand Down Expand Up @@ -1526,26 +1550,23 @@ compile_atom(re_compiler *c)
}

/* Append a copy of the atom bytecode in [start, start+size) at the current
position. Internal jump/split targets are relocated to the copy, so a
repeated group like (a{2,3}){2} keeps each iteration self-contained instead
of jumping back into the first copy (which corrupted its captures). Capture
slots (RE_SAVE) are shared across copies on purpose: a repeated group keeps
only its last iteration, like CRuby. */
position. Internal code indices are relocated to the copy, so a repeated
group like (a{2,3}){2} keeps each iteration self-contained instead of
jumping back into the first copy (which corrupted its captures). A
lookaround's sub-pattern end is one of those indices: left pointing into
the original, the copy runs its assertion against the first iteration's
code and ends the outer match early. Capture slots (RE_SAVE) are shared
across copies on purpose: a repeated group keeps only its last iteration,
like CRuby. */
static void
emit_atom_copy(re_compiler *c, uint32_t start, uint32_t size)
{
int32_t delta = (int32_t)c->code_len - (int32_t)start;
uint32_t atom_end = start + size;
for (uint32_t j = 0; j < size; j++) {
re_inst in = c->code[start + j];
switch (in.op) {
case RE_JMP: case RE_SPLIT: case RE_SPLITNG:
if (in.offset >= start && in.offset <= atom_end) {
in.offset = (uint16_t)((int32_t)in.offset + delta);
}
break;
default:
break;
if (op_holds_code_index(in.op) && in.offset >= start && in.offset <= atom_end) {
in.offset = (uint16_t)((int32_t)in.offset + delta);
}
emit(c, in.op, in.a, in.offset);
}
Expand Down
26 changes: 26 additions & 0 deletions mrbgems/mruby-regexp/test/regexp_syntax.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -937,6 +937,32 @@
assert_equal [""], /a?b?c?d?/.match("").to_a
end

assert("Regexp - a relocated lookaround keeps the end of its sub-pattern") do
# A lookaround holds the end of its sub-pattern as an absolute code index,
# so every relocation has to carry it the way it carries a jump target.
# Neither relocator did: the stale index landed on the sub-pattern's own
# RE_MATCH, which ends the outer match early, so the answers below flipped
# in both directions and the MatchData of an apparent success held nil.
# Three shapes reach a relocator, one each.

# insert_inst, via the SPLIT a quantifier puts in front of the group
assert_nil /(?:(?=a)b)*x/.match("a")
assert_equal "x", /(?:(?!b)b)*x/.match("ax")[0]

# emit_atom_copy, via the copies {n,m} makes of the group
assert_equal "aa", /(?:(?=a)a){2}/.match("aa")[0]
assert_nil /(?:(?=a)a){2}/.match("ab")

# insert_inst again, via the SPLIT compile_alt puts in front of branch 0
# once every branch is compiled
md = /(?=a)a|z/.match("ax")
assert_equal 0, md.begin(0)
assert_equal "a", md[0]

# the same group without a relocation, which always answered correctly
assert_equal "ab", /(?:(?=a)ab)+/.match("ab")[0]
end

assert("Regexp - empty-matchable patterns find earliest match position") do
# When a regex can match zero characters via epsilon transitions, the
# first-byte skip-ahead optimization is unsafe: skipping past bytes
Expand Down
Loading