| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
I wrote this PR with an LLM, so this still needs careful review. From a glance, the solution here seems very over complicated. But it does pass tests and rebar benchmarks. |
Sorry, something went wrong.
... so that tools like ripgrep and fd will search it.
We're going to use it in more places and it was already getting pretty repetitive.
I'm still liking the infrastructure I set up, but the logic is pretty complicated and about to get even more complicated. So it's wise I think to finally have tests confirming a specific strategy for a particular pattern. I had been "testing" this previously through benchmarks. If a pattern was slower than expected, then it probably got the wrong strategy. However, this is annoying to do in practice and has much longer iteration times. Plus, the results require interpretation, which isn't terribly accessible to most folks.
And also in one example in `regex-syntax`. I'm not sure how this happened.
This matches what we do for the reverse inner optimization. And indeed, this is required for correctness given how the optimizations currently work.
Previously, we just returned the `Hir` prefix (the thing that needs to be compiled into a reverse automaton) and the actual prefilter itself. But in order to deal with #1354, we're going to want to do some analysis on the actual literals returned. So we expose that information.
This doesn't have any behavior changes. It just reads a bit more nicely.
Specifically, this bug surfaced because we are returning the `Core` engine in more cases. Basically, we were being a bit too eager to run a prefix literal scan when we already have a match in hand. When we do, we should not re-run the prefix scan or else we might miss the match we already have.
|
I think this is probably good to go. I need to do another round of review, but I significantly simplified the original LLM fix for this. There will likely unfortunately be some performance regressions here, but I tried to claw back what I could. |
Sorry, something went wrong.
A minimal reproducer of this bug is on a haystack of `zabb` with the regex `.bb|b`. The `regex` crate will report a match at `2..3`, but the correct match is `1..4`. While this seems like a simple regex, there are a pretty specific set of circumstances required to trigger the bug: 1. There are no prefix literals that activate a standard prefix literal scan. 2. There needs to be an extractable *suffix* or *inner* literal. 3. An actual match needs to be present in the haystack. 4. The regex and haystack need to be such that the first candidate (from a literal scan) corresponds to a match whose start offset is greater than the start offset of a match confirmed by a subsequent candidate. Crucially, note that because of (3), this bug will never lead to `Regex::is_match` providing a false positive *or* a false negative. This bug is strictly about leftmost-first match semantics being incorrect in some cases and will report an incorrect match span. (4) could do with a bit more explanation, since it's rather subtle. Let's trace the minimal example through the regex crate's "reverse suffix" optimization. During compilation, there is no prefix literal that can be extracted. The `.` defeats that class of optimization. Moreover, there is a suffix literal in the regex. That is, all matches for `.bb|b` must end with `b`. The regex crate sees this and will scan for matches of `b`. It will then attempt to match the regex in reverse at each candidate match of `b`. Let's see what happens: * Find first occurrence of `b` at offset `2` in `zabb`. * Start reverse confirmation step at offset `2`. * The second alternation branch, `b` in `.bb|b`, matches at `2..3`. * The second alternation branch is reported as the overall match. This happens because the first alternation branch, `.bb`, does _not_ have a match ending at offset `3`. The fundamental problem here is that there is an overlap between the reverse automaton for confirming the match and the literal scan. Small changes, even to the haystack, can result in the bug disappearing. For example, with a haystack of `zbb`, the correct match of `0..3` is reported. This occurs because there is a quadratic "trip wire" that triggers in this case that causes the search to bail out and fall back to a DFA without using any literal optimizations. This bug also applies to the "reverse inner" optimization. This can happen when the literal is extracted from inside the regex as opposed to it being a suffix literal. For example, the regex `(?:..acbb|b)a(?:c|d)` on the haystack `xzbacbbac` reported a match at `2..5`, but the correct match is `1..9`. Note that #1355 technically fixes this problem and is much simpler, but in so doing, makes the reverse suffix and inner optimizations completely ineffective. In other words, it nearly disables them entirely. This PR keeps them enabled, but does some work to determine whether the regex pattern combined with the literals exatracted could ever lead to (4) being true. This does unfortunately cause some regressions. For example, `(.*?,){13}z` no longer gets the reverse suffix optimization because it is difficult to prove that reverse scan of `(.*?,){13}` will never lead to a later match than the true leftmost-first match after finding an occurrence of `z`. Fixes #1354, Closes #1355
|
I'm doing one last run of the rebar benchmark suite first. |
Sorry, something went wrong.
|
rebar benchmarks diffed with current master: $ rebar diff x/master.csv x/change07.csv -t 1.2 benchmark engine x/master.csv x/change07.csv --------- ------ ------------ -------------- curated/06-cloud-flare-redos/original rust/regex 566.9 MB/s (1.00x) 314.9 MB/s (1.80x) curated/06-cloud-flare-redos/simplified-long rust/regex 84.7 GB/s (1.00x) 42.9 GB/s (1.97x) imported/leipzig/word-ending-nn rust/regex 29.2 GB/s (1.00x) 825.5 MB/s (36.19x) imported/leipzig/tom-sawyer-huckle-fin-prefix-short rust/regex 19.2 GB/s (1.00x) 823.7 MB/s (23.88x) imported/leipzig/tom-sawyer-huckle-fin-prefix-long rust/regex 18.4 GB/s (1.00x) 823.7 MB/s (22.90x) imported/leipzig/ing rust/regex 3.1 GB/s (1.00x) 765.5 MB/s (4.10x) imported/leipzig/bounded-strings-ending-z rust/regex 49.0 GB/s (1.00x) 824.2 MB/s (60.94x) imported/rsc/reallyhard0-1k rust/regex 10.9 GB/s (1.00x) 794.7 MB/s (14.00x) imported/rsc/reallyhard0-32k rust/regex 45.4 GB/s (1.00x) 825.6 MB/s (56.37x) imported/rsc/reallyhard0-1mb rust/regex 44.0 GB/s (1.00x) 826.5 MB/s (54.46x) imported/sherlock/word-ending-n rust/regex 835.8 MB/s (1.00x) 545.6 MB/s (1.53x) imported/sherlock/ing-suffix rust/regex 3.5 GB/s (1.00x) 694.6 MB/s (5.19x) opt/reverse-inner/factored-prefix rust/regex 9.1 GB/s (1.00x) 803.5 MB/s (11.64x) There ight be a way to claw some of these back, but it's hard and I've already been working on this bug fix for over a month now (off-and-on). So I wanted to ship a correctness fix before doing more work here. |
Sorry, something went wrong.
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [displaydoc](https://github.com/yaahc/displaydoc) | dependencies | patch | `0.2.6` → `0.2.7` | | [hybrid-array](https://github.com/RustCrypto/hybrid-array) | dependencies | patch | `0.4.13` → `0.4.14` | | [regex](https://github.com/rust-lang/regex) | dev-dependencies | patch | `1.13.0` → `1.13.1` | | [serde](https://serde.rs) ([source](https://github.com/serde-rs/serde)) | dependencies | patch | `1.0.228` → `1.0.229` | | [serde_json](https://github.com/serde-rs/json) | dev-dependencies | patch | `1.0.150` → `1.0.151` | --- ### Release Notes <details> <summary>yaahc/displaydoc (displaydoc)</summary> ### [`v0.2.7`](https://github.com/yaahc/displaydoc/blob/HEAD/CHANGELOG.md#027---2026-07-28) [Compare Source](yaahc/displaydoc@v0.2.6...v0.2.7) #### Added - Updated `syn` dependency to 3.0 #### Changed - Bumped MSRV to 1.71 </details> <details> <summary>RustCrypto/hybrid-array (hybrid-array)</summary> ### [`v0.4.14`](https://github.com/RustCrypto/hybrid-array/blob/HEAD/CHANGELOG.md#0414-2026-07-30) [Compare Source](RustCrypto/hybrid-array@v0.4.13...v0.4.14) ##### Added - Sizes for `sntrup` ([#​231]) [#​231]: RustCrypto/hybrid-array#231 </details> <details> <summary>rust-lang/regex (regex)</summary> ### [`v1.13.1`](https://github.com/rust-lang/regex/blob/HEAD/CHANGELOG.md#1131-2026-07-15) [Compare Source](rust-lang/regex@1.13.0...1.13.1) \=================== This is a release that fixes a bug where incorrect regex match offsets could be reported. Note that this doesn't impact whether a match occurs or not, just where it occurs. The match offsets are still valid for slicing, they just may not refer to the correct leftmost-first match. See [#​1364](rust-lang/regex#1364) for (many) more details. Bug fixes: - [#​1354](rust-lang/regex#1354): Fixes previously unsound reverse suffix and inner optimizations. </details> <details> <summary>serde-rs/serde (serde)</summary> ### [`v1.0.229`](https://github.com/serde-rs/serde/releases/tag/v1.0.229) [Compare Source](serde-rs/serde@v1.0.228...v1.0.229) - Update to syn 3 </details> <details> <summary>serde-rs/json (serde_json)</summary> ### [`v1.0.151`](https://github.com/serde-rs/json/releases/tag/v1.0.151) [Compare Source](serde-rs/json@v1.0.150...v1.0.151) - Add RawValue::from\_string\_unchecked ([#​1331](serde-rs/json#1331), thanks [@​WonderLawrence](https://github.com/WonderLawrence)) </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate CLI](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yODEuMSIsInVwZGF0ZWRJblZlciI6IjQ0LjExLjYiLCJ0YXJnZXRCcmFuY2giOiJtYXN0ZXIiLCJsYWJlbHMiOltdfQ==--> Reviewed-on: https://dev.unebaguette.fr/vexahub/voprf-vx/pulls/16 Co-authored-by: Renovate Bot <renovatebot@lunomail.com>
| Back | FazBrowse Home | New Git URL |
A minimal reproducer of this bug is on a haystack of zabb with the
regex .bb|b. The regex crate will report a match at 2..3, but the
correct match is 1..4.
While this seems like a simple regex, there are a pretty specific set
of circumstances required to trigger the bug:
literal scan.
(from a literal scan) corresponds to a match whose start offset
is greater than the start offset of a match confirmed by a
subsequent candidate.
Crucially, note that because of (3), this bug will never lead to
Regex::is_match providing a false positive or a false negative.
This bug is strictly about leftmost-first match semantics being
incorrect in some cases and will report an incorrect match span.
(4) could do with a bit more explanation, since it's rather subtle.
Let's trace the minimal example through the regex crate's "reverse
suffix" optimization.
During compilation, there is no prefix literal that can be extracted.
The . defeats that class of optimization. Moreover, there is a suffix
literal in the regex. That is, all matches for .bb|b must end with
b. The regex crate sees this and will scan for matches of b. It will
then attempt to match the regex in reverse at each candidate match of
b. Let's see what happens:
This happens because the first alternation branch, .bb, does not
have a match ending at offset 3.
The fundamental problem here is that there is an overlap between the
reverse automaton for confirming the match and the literal scan. Small
changes, even to the haystack, can result in the bug disappearing.
For example, with a haystack of zbb, the correct match of 0..3 is
reported. This occurs because there is a quadratic "trip wire" that
triggers in this case that causes the search to bail out and fall back
to a DFA without using any literal optimizations.
This bug also applies to the "reverse inner" optimization. This
can happen when the literal is extracted from inside the regex
as opposed to it being a suffix literal. For example, the regex
(?:..acbb|b)a(?:c|d) on the haystack xzbacbbac reported a match at
2..5, but the correct match is 1..9.
Note that #1355 technically fixes this problem and is much simpler, but
in so doing, makes the reverse suffix and inner optimizations completely
ineffective. In other words, it nearly disables them entirely.
This PR keeps them enabled, but does some work to determine whether the
regex pattern combined with the literals exatracted could ever lead to
(4) being true.
This does unfortunately cause some regressions. For example,
(.*?,){13}z no longer gets the reverse suffix optimization because it
is difficult to prove that reverse scan of (.*?,){13} will never lead
to a later match than the true leftmost-first match after finding an
occurrence of z.
Fixes #1354, Closes #1355