Skip to content

fix(pcre): inline (?m) flag inside group activates multiline mid-pattern #56

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
jbachorik merged 4 commits into from
May 9, 2026
Merged

fix(pcre): inline (?m) flag inside group activates multiline mid-pattern #56

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cb0c3c8
fix: address review findings for inline (?m) flag implementation
jbachorik May 9, 2026
d2f1b9a
fix: use requiresMultilineStartAnchor() in OnePassBytecodeGenerator
jbachorik May 9, 2026
9114c11
fix: bail tryInlineEpsilonClosure on anchor states
jbachorik May 9, 2026
6fc9e63
fix: STRING_END (\Z) semantics in RecursiveDescent; doc fix
jbachorik May 9, 2026
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
47 changes: 30 additions & 17 deletions reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/automaton/NFA.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,36 +162,49 @@ public boolean hasStringStartAnchor() {
* requiresStartAnchor() returns true.
*/
public boolean requiresStartAnchor() {
// BFS to find states with character transitions reachable without going through anchor
Set<NFAState> reachableWithoutAnchor = new HashSet<>();
Queue<NFAState> queue = new LinkedList<>();
queue.add(startState);
reachableWithoutAnchor.add(startState);
return requiresAnchorOnAllPaths(AnchorType.START, AnchorType.STRING_START);
}

/**
* Check if a multiline start anchor is REQUIRED to match this pattern. Returns true only if ALL
* paths to character transitions go through a {@code START_MULTILINE}, {@code START}, or {@code
* STRING_START} anchor. {@code START} and {@code STRING_START} are treated as sufficient barriers
* because a pattern anchored at the absolute start of input is trivially also anchored at a line
* start, so the same find()-position optimization applies.
*
* <p>This is used to optimize find() operations - we can skip positions not following '\n' only
* when all paths require the multiline anchor.
*/
public boolean requiresMultilineStartAnchor() {
boolean hasMultilineAnchor =
states.stream().anyMatch(s -> s.anchor == AnchorType.START_MULTILINE);
if (!hasMultilineAnchor) {
return false;
}
return requiresAnchorOnAllPaths(
AnchorType.START_MULTILINE, AnchorType.START, AnchorType.STRING_START);
}
Comment thread
jbachorik marked this conversation as resolved.

private boolean requiresAnchorOnAllPaths(AnchorType... barriers) {
Set<AnchorType> barrierSet = EnumSet.copyOf(Arrays.asList(barriers));
Set<NFAState> visited = new HashSet<>();
Queue<NFAState> queue = new ArrayDeque<>();
queue.add(startState);
visited.add(startState);
while (!queue.isEmpty()) {
NFAState state = queue.poll();

// If this state has an anchor, don't follow its transitions
// (paths through this state require the anchor)
if (state.anchor == AnchorType.START || state.anchor == AnchorType.STRING_START) {
if (barrierSet.contains(state.anchor)) {
continue;
}

// If this state has character transitions, we can match without anchor
if (!state.getTransitions().isEmpty()) {
return false;
}

// Follow epsilon transitions (but not through anchor states)
for (NFAState next : state.getEpsilonTransitions()) {
if (!reachableWithoutAnchor.contains(next)) {
reachableWithoutAnchor.add(next);
if (visited.add(next)) {
queue.add(next);
}
}
}

// Couldn't find any character transitions reachable without going through an anchor
return true;
}

Expand Down
182 changes: 174 additions & 8 deletions reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/codegen/NFABytecodeGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1885,11 +1885,12 @@ private void generateOptimizedEpsilonClosureWithPrecomputation(
int posVar,
LocalVariableAllocator allocator,
EpsilonClosureSlots preAllocSlots) {
// Check if NFA has assertions - if so, use runtime closure
boolean hasAssertions = nfa.getStates().stream().anyMatch(s -> s.assertionType != null);
// Check if NFA has assertions or anchors - if so, use runtime closure
boolean hasAssertions =
nfa.getStates().stream().anyMatch(s -> s.assertionType != null || s.anchor != null);
Comment thread
jbachorik marked this conversation as resolved.

if (hasAssertions) {
// Assertions require runtime checks, use standard closure with pre-allocated slots
// Assertions/anchors require runtime checks, use standard closure with pre-allocated slots
generateEpsilonClosure(mv, statesVar, inputVar, posVar, allocator, preAllocSlots);
Comment thread
jbachorik marked this conversation as resolved.
return;
}
Expand Down Expand Up @@ -2117,6 +2118,84 @@ private void generateEpsilonClosure(
// No group tracking in this context (-1, -1)
generateAssertionCheck(
mv, state, inputVar, posVar, statesVar, worklistVar, stateIdVar, -1, -1, allocator);
} else if (state.anchor != null) {
// Inline position guard: only follow epsilon transitions if anchor passes
Label anchorPassed = new Label();
switch (state.anchor) {
case START:
case STRING_START:
// pos == 0
mv.visitVarInsn(ILOAD, posVar);
mv.visitJumpInsn(IFNE, worklistLoop);
break;
case START_MULTILINE:
// pos == 0 || input.charAt(pos-1) == '\n'
mv.visitVarInsn(ILOAD, posVar);
mv.visitJumpInsn(IFEQ, anchorPassed);
mv.visitVarInsn(ALOAD, inputVar);
mv.visitVarInsn(ILOAD, posVar);
mv.visitInsn(ICONST_1);
mv.visitInsn(ISUB);
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/String", "charAt", "(I)C", false);
pushInt(mv, '\n');
mv.visitJumpInsn(IF_ICMPNE, worklistLoop);
break;
case END:
case STRING_END_ABSOLUTE:
// pos == input.length()
mv.visitVarInsn(ILOAD, posVar);
mv.visitVarInsn(ALOAD, inputVar);
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/String", "length", "()I", false);
mv.visitJumpInsn(IF_ICMPNE, worklistLoop);
break;
case STRING_END:
// \Z: pos == length || (pos == length-1 && charAt(pos) == '\n')
mv.visitVarInsn(ILOAD, posVar);
mv.visitVarInsn(ALOAD, inputVar);
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/String", "length", "()I", false);
mv.visitJumpInsn(IF_ICMPEQ, anchorPassed);
mv.visitVarInsn(ALOAD, inputVar);
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/String", "length", "()I", false);
mv.visitInsn(ICONST_1);
mv.visitInsn(ISUB);
mv.visitVarInsn(ILOAD, posVar);
mv.visitJumpInsn(IF_ICMPNE, worklistLoop);
mv.visitVarInsn(ALOAD, inputVar);
mv.visitVarInsn(ILOAD, posVar);
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/String", "charAt", "(I)C", false);
pushInt(mv, '\n');
mv.visitJumpInsn(IF_ICMPNE, worklistLoop);
break;
case END_MULTILINE:
// pos == input.length() || input.charAt(pos) == '\n'
mv.visitVarInsn(ILOAD, posVar);
mv.visitVarInsn(ALOAD, inputVar);
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/String", "length", "()I", false);
mv.visitJumpInsn(IF_ICMPEQ, anchorPassed);
mv.visitVarInsn(ALOAD, inputVar);
mv.visitVarInsn(ILOAD, posVar);
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/String", "charAt", "(I)C", false);
pushInt(mv, '\n');
mv.visitJumpInsn(IF_ICMPNE, worklistLoop);
break;
default:
// Other anchor types (e.g. WORD_BOUNDARY): treat as always passing
break;
}
mv.visitLabel(anchorPassed);
// Anchor passed - add epsilon targets normally
for (NFA.NFAState target : state.getEpsilonTransitions()) {
Label alreadyVisited = new Label();
checkStateInSetConst(mv, statesVar, target.id, allocator);
mv.visitJumpInsn(IFNE, alreadyVisited);
addStateToSet(mv, statesVar, target.id, allocator);
mv.visitVarInsn(ALOAD, worklistVar);
mv.visitVarInsn(ILOAD, worklistSizeVar);
pushInt(mv, target.id);
mv.visitInsn(IASTORE);
mv.visitIincInsn(worklistSizeVar, 1);
mv.visitLabel(alreadyVisited);
}
} else {
// No assertion - add epsilon targets normally
for (NFA.NFAState target : state.getEpsilonTransitions()) {
Expand Down Expand Up @@ -5233,13 +5312,13 @@ private boolean tryInlineEpsilonClosure(
}
}

// Don't inline if closure contains assertion states (they need runtime checking)
// Don't inline if closure contains assertion or anchor states (they need runtime checking)
if (!followThroughAssertions) {
for (Integer stateId : completeClosure) {
NFA.NFAState state =
nfa.getStates().stream().filter(s -> s.id == stateId).findFirst().orElse(null);
if (state != null && state.assertionType != null) {
return false; // Can't inline - need runtime assertion checking
if (state != null && (state.assertionType != null || state.anchor != null)) {
return false; // Can't inline - need runtime assertion/anchor checking
}
}
}
Expand Down Expand Up @@ -6824,14 +6903,15 @@ private void generateEpsilonClosureWithGroups(
addStateToSetVar(mv, processedVar, stateIdVar);

// Generate switch for O(log N) state lookup
// Collect all states that need processing (have groups, backrefs, assertions, or epsilon
// transitions)
// Collect all states that need processing (have groups, backrefs, assertions, anchors, or
// epsilon transitions)
List<NFA.NFAState> statesToProcess = new ArrayList<>();
for (NFA.NFAState state : nfa.getStates()) {
if (state.enterGroup != null
|| state.exitGroup != null
|| state.backrefCheck != null
|| state.assertionType != null
|| state.anchor != null
|| !state.getEpsilonTransitions().isEmpty()) {
statesToProcess.add(state);
}
Expand Down Expand Up @@ -6969,6 +7049,92 @@ else if (state.assertionType != null) {
groupStartsVar,
groupEndsVar,
allocator);
} else if (state.anchor != null) {
// Inline position guard: only follow epsilon transitions if anchor passes
Label anchorPassedWG = new Label();
switch (state.anchor) {
case START:
case STRING_START:
mv.visitVarInsn(ILOAD, posVar);
mv.visitJumpInsn(IFNE, worklistLoop);
break;
case START_MULTILINE:
mv.visitVarInsn(ILOAD, posVar);
mv.visitJumpInsn(IFEQ, anchorPassedWG);
mv.visitVarInsn(ALOAD, inputVar);
mv.visitVarInsn(ILOAD, posVar);
mv.visitInsn(ICONST_1);
mv.visitInsn(ISUB);
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/String", "charAt", "(I)C", false);
pushInt(mv, '\n');
mv.visitJumpInsn(IF_ICMPNE, worklistLoop);
break;
case END:
case STRING_END_ABSOLUTE:
mv.visitVarInsn(ILOAD, posVar);
mv.visitVarInsn(ALOAD, inputVar);
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/String", "length", "()I", false);
mv.visitJumpInsn(IF_ICMPNE, worklistLoop);
Comment on lines +7074 to +7077

@chatgpt-codex-connector chatgpt-codex-connector Bot May 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use the bounded region end for NFA end anchors

When an OPTIMIZED_NFA pattern with $, \Z, or \z is evaluated through matchesBounded/matchBounded, this closure is called from generateMatchBoundedMethod with only posVar and the full input string, so these new guards compare against input.length() instead of the end parameter for the active region. That makes a bounded match that should be equivalent to matching input.subSequence(start, end) fail whenever the region ends before the full string, e.g. an NFA-fallback pattern ending in $ over a slice inside a larger input; pass the bound into the closure or use a bounded-specific anchor check.

Useful? React with 👍 / 👎.

break;
case STRING_END:
// \Z: pos == length || (pos == length-1 && charAt(pos) == '\n')
mv.visitVarInsn(ILOAD, posVar);
mv.visitVarInsn(ALOAD, inputVar);
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/String", "length", "()I", false);
mv.visitJumpInsn(IF_ICMPEQ, anchorPassedWG);
mv.visitVarInsn(ALOAD, inputVar);
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/String", "length", "()I", false);
mv.visitInsn(ICONST_1);
mv.visitInsn(ISUB);
mv.visitVarInsn(ILOAD, posVar);
mv.visitJumpInsn(IF_ICMPNE, worklistLoop);
mv.visitVarInsn(ALOAD, inputVar);
mv.visitVarInsn(ILOAD, posVar);
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/String", "charAt", "(I)C", false);
pushInt(mv, '\n');
mv.visitJumpInsn(IF_ICMPNE, worklistLoop);
break;
case END_MULTILINE:
mv.visitVarInsn(ILOAD, posVar);
mv.visitVarInsn(ALOAD, inputVar);
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/String", "length", "()I", false);
mv.visitJumpInsn(IF_ICMPEQ, anchorPassedWG);
mv.visitVarInsn(ALOAD, inputVar);
mv.visitVarInsn(ILOAD, posVar);
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/String", "charAt", "(I)C", false);
pushInt(mv, '\n');
mv.visitJumpInsn(IF_ICMPNE, worklistLoop);
break;
default:
// Other anchor types (e.g. WORD_BOUNDARY): treat as always passing
break;
}
mv.visitLabel(anchorPassedWG);
// Anchor passed - follow epsilon transitions
for (NFA.NFAState target : state.getEpsilonTransitions()) {
Label alreadyVisitedA = new Label();
checkStateInSetConst(mv, statesVar, target.id, allocator);
mv.visitJumpInsn(IFNE, alreadyVisitedA);
addStateToSet(mv, statesVar, target.id, allocator);
mv.visitVarInsn(ALOAD, worklistVar);
mv.visitVarInsn(ILOAD, worklistSizeVar);
pushInt(mv, target.id);
mv.visitInsn(IASTORE);
mv.visitIincInsn(worklistSizeVar, 1);
mv.visitLabel(alreadyVisitedA);
// Per-config tracking for POSIX last-match semantics (same as normal epsilon path)
if (usePosixLastMatch && configGroupStartsVar >= 0) {
int groupCount = nfa.getGroupCount();
generateCopyConfigArray(mv, configGroupStartsVar, stateIdVar, target.id, groupCount);
generateCopyConfigArray(mv, configGroupEndsVar, stateIdVar, target.id, groupCount);
}
if (usePosixLastMatch && parentStateMapVar >= 0) {
mv.visitVarInsn(ALOAD, parentStateMapVar);
pushInt(mv, target.id);
mv.visitVarInsn(ILOAD, stateIdVar);
mv.visitInsn(IASTORE);
}
Comment thread
jbachorik marked this conversation as resolved.
}
} else if (!state.getEpsilonTransitions().isEmpty()) {
for (NFA.NFAState target : state.getEpsilonTransitions()) {
Label alreadyVisited = new Label();
Expand Down
2 changes: 1 addition & 1 deletion ...-codegen/src/main/java/com/datadoghq/reggie/codegen/codegen/OnePassBytecodeGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public class OnePassBytecodeGenerator {
public OnePassBytecodeGenerator(NFA nfa) {
this.nfa = nfa;
this.groupCount = nfa.getGroupCount();
this.hasMultilineStart = nfa.hasMultilineStartAnchor();
this.hasMultilineStart = nfa.requiresMultilineStartAnchor();
this.hasStartAnchor = nfa.hasStartAnchor();
this.hasStringStartAnchor = nfa.hasStringStartAnchor();
this.hasEndAnchor = nfa.hasEndAnchor();
Expand Down
Loading
Loading