Skip to content

Mitigation: no Regex on the first managed report (Mono GC crash trigger) - #271

Merged
krassx merged 1 commit into
mainfrom
fix/no-regex-first-report
Nov 3, 2026
Merged

krassx merged 1 commit into
mainfrom
fix/no-regex-first-report

Conversation

@krassx

@krassx krassx commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

What

ManagedAddressPairing and ManagedSignatureRecipe built four Regex objects (three distinct patterns) in static constructors, so the first managed exception report constructed them. They are replaced by hand-written parsers in the new ManagedFrameLocation (" in FILE:LINE", "(at FILE:LINE)", "libX.so"), with the same results. No public API change.

Why

On Unity 2021.3.45f1 Mono (ARMv7 Android, HONOR WOD-LX1) a SIGSEGV in libmonobdwgc GC_mark_from was seen in 2 of 28 fresh-install cold starts (plus one earlier one-off). The managed stack of all 3 crashes: RegexCharClass:.cctor <- Regex:.ctor <- ManagedAddressPairing:.cctor <- ManagedExceptionPayload.FromException <- ExceptionPipeline.Report <- LogException. The suspected cause is a Mono/Boehm GC defect; the SDK's first report is where it was triggered. This PR removes the Regex construction from that first report.

This is a mitigation, not a fix. The suspected runtime defect is not addressed.

Equivalence evidence

  • The host tests keep the original three patterns as the reference and compare them with the new parsers and with the public methods (HasSourceLocation, TryFileLine, IsEngineFrame): real device frames (Mono Release/Development, dev matrix, Unity-log (at ...) lines and native lib*.so lines), about 80 handwritten edge cases, 200k generated strings plus a frame-shaped grid, and the \s / \d vs char.IsWhiteSpace / char.IsDigit assumption over all 65,536 chars.
  • Full dotnet test in Tests~/WrapperPolicy: 2176 passed. compile-check.sh ok.

Device evidence

Unity 2021.3.45f1 Mono Release build with this change, fresh install before every cold start, BugseeMiniGame + lx_scn main-sync probe, same flow as the sessions that reproduced the crash: 0 SIGSEGV in 41 valid runs (earlier: 2 of 28 fresh-install runs). Two further runs died at the Android 15 32-bit ABI dialog before Unity started and are excluded. Reports still reach staging AU (AU-246, frames in the usual form).

Statistics caveat: if the true rate were still about 7%, P(0 of 41) is about 5%, so 0/41 is only suggestive, not conclusive.

Known residual risk / possible follow-up (not in this PR)

Other first-use initialisations remain on the first-report path: SHA256.Create() in ManagedSignatureRecipe (CryptoConfig first use), new StackTrace(false).ToString() in ManagedExceptionPayload, the first Exception.StackTrace formatting, and JIT. A possible follow-up is warming the signature path at launch next to WarmModuleUuid; it needs its own device loop.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SfQe6YfjwEeceEeuf3jS6x

…ten parsing.

ManagedAddressPairing and ManagedSignatureRecipe built four Regex objects (three
distinct patterns) in static constructors, so the first managed exception report
constructed them. On Unity 2021.3.45f1 Mono (ARMv7 Android) the stack of a SIGSEGV in
libmonobdwgc GC_mark_from (2 of 28 fresh-install cold starts) showed that Regex
construction running when the fault happened. New ManagedFrameLocation parses the
same three shapes (" in FILE:LINE", "(at FILE:LINE)", "libX.so") with no Regex.

Behaviour is unchanged: the host tests keep the original patterns as the reference and
compare over real device frames, handwritten edge cases and 200k generated inputs, and
pin the \s / \d assumptions over all 65,536 chars.
Mitigation only: removes this allocation burst from the first report; the suspected
runtime defect is not fixed.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SfQe6YfjwEeceEeuf3jS6x
@coderabbitai

coderabbitai Bot commented Nov 3, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 10 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: cc0d3a58-8a14-4eff-b0c0-cd32903672b9
Commits

Reviewing files that changed from the base of the PR and between ab90c53 and 2db1cc8.

Files selected for processing: 7
  • CHANGELOG.md
  • Runtime/WrapperPolicy/ManagedAddressPairing.cs
  • Runtime/WrapperPolicy/ManagedFrameLocation.cs
  • Runtime/WrapperPolicy/ManagedFrameLocation.cs.meta
  • Runtime/WrapperPolicy/ManagedSignatureRecipe.cs
  • Tests~/WrapperPolicy/Fixtures/frame-lines-corpus.txt
  • Tests~/WrapperPolicy/ManagedFrameLocationTests.cs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.

@claude

claude Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Claude finished @krassx's task in 1m 32s —— View job


Review of PR #271: no Regex on the first managed report

  • Read the diff, the callers and the host tests
  • Checked the parsers against the three original patterns, case by case
  • Ran the PR's tests and my own fuzz against the original Regexes

No bugs found, so there are no inline comments.

What I checked

Equivalence of ManagedFrameLocation with the original patterns

  • TryLocation (in/at forms):
    • Trailing whitespace is trimmed first, then the optional ) and the digit run, which must be preceded by :. That matches the greedy .+ and \d+\s*$ semantics: the colon is the one before the final digit run.
    • The head scan walks i leftmost-first. Greedy \s+ is handled with wsEnd, then Math.Min(wsEnd, colon - 1) leaves one file character, as the regex backtracking would.
    • The . that excludes \n is handled by start <= lastLineFeed → continue. A shorter \s+ could only start the file earlier and so still contain the \n, so skipping is correct.
    • CompareOrdinal and text[after] stay in bounds because of the after + 1 >= colon guard.
  • IsSharedLibrary: The loop equals ^lib[^\s/()]*\.so(?![A-Za-z0-9_]). It tries each .so while every earlier character is still in the class, and the lookahead is ASCII-only, like the explicit ranges in the pattern.
  • \s and \d assumption: char.IsWhiteSpace and char.IsDigit equal \s and \d. ParserAssumptionsHoldForEveryChar checks this over all 65,536 chars on the host runtime.
  • Callers: HasSourceLocation and TryFileLine keep the same int.TryParse and <= 0 rules, and Trim() on the file. IsEngineFrame keeps the same ordering. Non-ASCII digits are still rejected by int.TryParse, as before.
  • Complexity: Each i does O(1) work plus a whitespace-run scan, so there is no catastrophic or quadratic blow-up on long malformed lines.

What I ran

  • PR tests: dotnet test --filter ManagedFrameLocationTests in Tests~/WrapperPolicy gave 5 passed, 0 failed. I did not run the full 2176-test suite.
  • My own fuzz: In /tmp, outside the repo, I compiled ManagedFrameLocation.cs next to the original three Regexes. I ran 3M random strings up to 15 pieces long, from an alphabet that adds \u001c, ᠎, and to the PR's whitespace, newline and digit pieces. It compared Success, the file capture and the line capture for the in form and the at form, and the IsSharedLibrary result. There were 0 divergences.
  • Not run: Nothing on a device or under Unity/Mono. Device results (0/41) come from the PR description and I did not verify them.

Other Regex use

  • The remaining Regex uses in Runtime/WrapperPolicy (XcodeBuildPhaseList, FeedbackOptIn, Gradle*Injection) are build-time and Editor-side. None is on the report path, so the PR removes every Regex there.

Minor, non-blocking observations

  • P3, ManagedFrameLocation.cs:42: a trailing space after LastIndexOf('\n', colon - 1); . Cosmetic only.
  • Equivalence on the player: the equivalence proof runs on .NET 8 on the host, while the shipped code runs on Unity's Mono class library. char.IsWhiteSpace and char.IsDigit there should match, but the char-sweep test cannot show that. This does not need action, since the Mono device run produced correct frames.
  • Mitigation caveat: The PR is honest that this is a mitigation. SHA-256 CryptoConfig first use and StackTrace formatting remain on the first-report path. The proposed launch-time warm-up follow-up looks like the right next step, but it needs its own device loop.

Summary

  1. Overall risk: Low. The change is small and behaviour-preserving, and it is pinned to the original patterns by tests that I reproduced and extended.
  2. Merge recommendation: Approve and merge. I cannot formally approve.
  3. Most important issues to fix: None blocking.
  4. Positives:
    • Keeping the original Regexes as a reference oracle in the tests is a strong approach, and the real-device frame corpus and the all-65,536-chars check are thorough.
    • The CHANGELOG and PR text are candid about statistical weakness (0/41 at a 7% rate is about 5%) and about which first-use work remains.
    • The change has no public API impact.
      · Branch: fix/no-regex-first-report

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Deep review (HEAD 2db1cc8)

Intent: Mitigation, not a Mono GC fix. On Unity 2021.3.45f1 Mono ARMv7, 2 of 28 fresh-install cold starts died in GC_mark_from while the first managed report constructed Regex objects (RegexCharClass:.cctor ← Regex:.ctor ← ManagedAddressPairing:.cctor ← ExceptionPipeline.Report). This replaces those four objects (three patterns) with ManagedFrameLocation.

Checked against the surrounding path, not just the diff. ManagedFrameAssembly.FrameLines already touches ManagedAddressPairing on Mono (where Pair is skipped because there are no native IPs), so the old type initializer ran on the first report even without pairing. FromException then hits ManagedSignatureRecipe.Compute → TryFileLine / IsEngineFrame. Production frames are Split+Trim’d first (StackLines), so the parsers see single-line text. HasSourceLocation still only recognizes in FILE:LINE (same as the old regex); (at FILE:LINE) remains signature-only. No other new Regex remains on the player report path (Runtime/Internal has none).

Adversarial cases walked: leftmost in / (at , greedy file, last :digits, . not crossing , \s+ leaving one file character, lib*.so with a non-[A-Za-z0-9_] lookahead, truncated logcat lines, in <hash>:0, extra ), libfoo.sox vs libunity.so(+0x…). The host tests keep the original three patterns as the oracle (handwritten edges, 377-line device corpus, 200k generated strings plus a frame-shaped grid, and \s/\d vs char.IsWhiteSpace/char.IsDigit over all 65,536 BMP chars).

Findings

No P0–P3 issues. Equivalence holds for the production shapes; the first-report type initializer that constructed Regex is gone.

Residual (already disclosed, not a defect of this PR)

  • SHA256.Create(), new StackTrace(false).ToString(), first Exception.StackTrace formatting, and JIT still run on the first report. Warming the signature path at launch (next to WarmModuleUuid) is a separate device loop.
  • 0 SIGSEGV in 41 post-change cold starts is only suggestive at a ~7% prior rate (the changelog already says this).

CI

Observed on this SHA, not run locally (dotnet is not on PATH): Unity C# compile, wrapper-policy tests (2144 passed / 32 skipped), wrapper-policy macOS iOS scripts, Android wrapper, Windows Editor, iOS bridge, CodeQL — all SUCCESS.


  1. Overall risk: Low
  2. Merge recommendation: Approve
  3. Most important issues to fix: None
  4. Positive: Honest mitigation framing; the old patterns are kept as a live oracle rather than deleted; no public API change; parsers are allocation-bounded and have no shared mutable state.
Open in Web View Automation 

Sent by Cursor Automation: Bugsee code review

@krassx
krassx merged commit 8207791 into main Nov 3, 2026
14 checks passed
@krassx
krassx deleted the fix/no-regex-first-report branch November 3, 2026 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant