feat: Add MSVC symbol demangling support for PE binaries

[unclesp1d3r]

Summary

Extends SymbolDemangler to demangle MSVC-mangled symbols (?-prefixed) found in Windows PE binaries. The pipeline's classify_strings() already detected ?-prefixed symbols as mangled candidates, but SymbolDemangler had no ? handling -- so every MSVC symbol from a PE import/export table was detected and then silently passed through with no demangled output. MSVC is the dominant Windows toolchain, so this covers most PE symbols.

Closes #19.

Example: ?printf@@YAHPEBDZZ -> int __cdecl printf(char const *, ...)

What changed

• ?-prefix support in SymbolDemangler (src/classification/symbols/): is_mangled() now returns true for ?-prefixed symbols, and try_demangle_internal() routes them to a new try_msvc_demangle() helper using msvc_demangler::demangle(..., DemangleFlags::llvm()). The helper mirrors the existing try_rust_demangle() / try_cpp_demangle() exactly, including the "differs from input" guard. The demangled form replaces text, the original is preserved in original_text, and Tag::DemangledSymbol is applied via the existing demangle() flow -- no pipeline changes required.
• New dependency: msvc-demangler 0.11 (MIT/NCSA dual-licensed). NCSA added to the deny.toml license allow-list. The crate pulls only bitflags + itoa transitively, no new duplicate versions.
• Hardening -- stack-overflow DoS guard: msvc-demangler parses recursively with no depth limit. A crafted ?-symbol with ~18KB+ of nested type modifiers overflows the stack and aborts the process -- and a stack overflow aborts rather than unwinds, so the pipeline's catch_unwind cannot contain it. Since Stringy analyzes untrusted/malicious binaries, try_msvc_demangle() rejects symbols longer than MSVC_MAX_SYMBOL_LEN (4096) before parsing. Real MSVC symbols are far shorter even when heavily templated; this is cheap defense-in-depth.
• Module split: src/classification/symbols.rs grew past the 500-line soft limit, so it is now a module directory -- symbols/mod.rs (production) + symbols/tests.rs (tests) -- following the existing classification/patterns/ convention.
• ADRs: the length-cap and dependency decisions are recorded in docs/adr/0001-msvc-demangler-length-cap.md and docs/adr/0002-msvc-demangler-dependency.md, with an ADR index (README.md) and template seeded for future use.

Test plan

• Unit tests for the MSVC path: ?-detection, plain function, constructor, operator overload, invalid-input graceful fallback, oversized-symbol rejection, idempotent re-demangle, try_demangle success/failure, existing-tag preservation
• Doctests updated (module header, is_mangled, try_demangle) and passing
• No regression in existing Rust/C++ demangling tests
• cargo clippy --all-targets -- -D warnings clean; cargo fmt --check clean
• Stack-overflow guard verified by a regression test with a 60KB nested-pointer symbol
• Performance measured: ~399 ns/symbol, only on the ?-prefixed subset -- comparable to the existing Rust/C++ paths
• CI regenerates ELF/PE/Mach-O fixtures (just gen-fixtures) and runs the full suite -- local fixture generation is blocked by a Zig toolchain env issue on this machine

Requirements (issue #19, Req 4.1)

R1-R5 and R7 met. R6 (cargo deny check passes) is met for what the new dependency controls: licenses pass and no new duplicate version is introduced. A pre-existing cargo deny check bans failure (duplicate windows-sys via fs4 -> mmap-guard, already red on main) is unrelated to this change and is tracked in #179.

Deferred / tracked separately

These were raised in review and intentionally not done here, so they are filed as issues rather than left in this description:

• Optionally restrict MSVC symbol demangling to import/export sources #178 -- optionally restrict MSVC demangling to import/export sources (optional hardening; the length cap is the higher-value mitigation, see ADR-0001).
• Unify duplicate windows-sys versions to fix cargo-deny bans check #179 -- unify the duplicate windows-sys versions to fix the cargo deny check bans failure (pre-existing tech debt; needs its own dependency bump, must not be masked with a deny.toml skip).

[mergify]

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🟢 📃 Configuration Change Requirements

Wonderful, this rule succeeded.

Mergify configuration change

• check-success = Configuration changed

🟢 Enforce conventional commit

Wonderful, this rule succeeded.

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

• title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.+\))?:

🟢 Full CI must pass

Wonderful, this rule succeeded.

All CI checks must pass. This protection prevents manual merges that bypass the merge queue.

• check-success = coverage
• check-success = msrv (stable minus 1 releases)
• check-success = msrv (stable minus 2 releases)
• check-success = msrv (stable minus 3 releases)
• check-success = msrv (stable minus 4 releases)
• check-success = msrv (stable)
• check-success = quality
• check-success = test
• check-success = test-cross-platform (macos-latest, macOS)
• check-success = test-cross-platform (ubuntu-latest, Linux)
• check-success = test-cross-platform (windows-latest, Windows)

🟢 Do not merge outdated PRs

Wonderful, this rule succeeded.

Make sure PRs are within 10 commits of the base branch before merging

• #commits-behind <= 10

[coderabbitai]

Warning

Review limit reached

@unclesp1d3r, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 46 minutes and 51 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 474e9e4d-d989-44ab-9bfe-4f6cfb83a29d

📥 Commits

Reviewing files that changed from the base of the PR and between 71d14e7 and 6d16b19.

⛔ Files ignored due to path filters (4)

• mise.lock is excluded by !**/*.lock
• tests/snapshots/integration_elf__elf_symbol_extraction.snap is excluded by !**/*.snap, !**/tests/snapshots/**
• tests/snapshots/integration_flows_1_5__flow1_top3_json.snap is excluded by !**/*.snap, !**/tests/snapshots/**
• tests/snapshots/integration_pe__pe_symbol_extraction.snap is excluded by !**/*.snap, !**/tests/snapshots/**

📒 Files selected for processing (36)

• .coderabbit.yml
• .cursor/commands/ci_check.md
• .cursor/commands/code_rabbit.md
• .cursor/commands/code_review.md
• .cursor/commands/performance_tuning.md
• .cursor/commands/security_hardening.md
• .cursor/commands/simplicity_review.md
• .cursor/commands/update_llmstxt.md
• .cursor/commands/work_next_task.md
• .cursor/mcp.json
• .cursor/rules/rust/cargo-toml.mdc
• .cursor/rules/rust/configuration-management.mdc
• .cursor/rules/rust/error-handling-patterns.mdc
• .cursor/rules/rust/error-handling.mdc
• .cursor/rules/rust/linting-rules.mdc
• .cursor/rules/rust/performance-optimization.mdc
• .cursor/rules/rust/rust-standards.mdc
• .devcontainer/devcontainer.json
• .editorconfig
• .gemini/settings.json
• .github/actionlint.yml
• .github/dependabot.yml
• .github/workflows/copilot-setup-steps.yml
• .github/workflows/release.yml
• .mcp.json
• .mergify.yml
• .pre-commit-config.yaml
• .serena/project.yml
• .zig-version
• dist-workspace.toml
• docs/plans/2026-06-19-002-feat-msvc-symbol-demangling-plan.md
• justfile
• mise.toml
• src/classification/symbols.rs
• src/classification/symbols/mod.rs
• src/classification/symbols/tests.rs

📝 Walkthrough

Walkthrough

Adds MSVC ?-prefixed symbol demangling to SymbolDemangler in src/classification/symbols.rs using msvc-demangler 0.11.0. A new try_msvc_demangle helper applies a length guard (4096 bytes) then calls msvc_demangler::demangle with LLVM-style flags, returning Some only when output differs from input. The NCSA license is allowed in deny.toml, documented in AGENTS.md, and decision and implementation details are captured in comprehensive architectural decision records and a detailed implementation plan.

Changes

MSVC Symbol Demangling

Layer / File(s)	Summary
Dependency and license policy Cargo.toml, deny.toml, AGENTS.md	Adds msvc-demangler = "0.11.0" as a dependency, allows NCSA in the license allowlist, and documents the new crate under Key Dependencies.
SymbolDemangler MSVC detection and demangling src/classification/symbols.rs	Adds MSVC_MAX_SYMBOL_LEN constant for DoS mitigation; extends is_mangled to detect ? prefix; routes ?-prefixed symbols in try_demangle_internal to a new try_msvc_demangle helper that applies length guard, calls msvc_demangler::demangle with DemangleFlags::llvm(), and returns Some only on changed output; updates module, struct, and method docs with MSVC examples; adds comprehensive test suite covering detection, plain function/constructor/operator demangling, invalid-input fallback, oversized-symbol rejection, and tag-preservation assertions.
Implementation plan and architectural decision records docs/plans/2026-06-19-002-feat-msvc-symbol-demangling-plan.md, docs/adr/0001-msvc-demangler-length-cap.md, docs/adr/0002-msvc-demangler-dependency.md, docs/adr/README.md, docs/adr/template.md	Adds comprehensive implementation plan with acceptance criteria, technical decisions, dispatch flow diagram, verification steps, and risk analysis; documents ADR-0001 for the 4096-byte length-cap defensive mitigation against stack-overflow DoS in msvc-demangler; documents ADR-0002 for the msvc-demangler dependency decision including alternatives considered, consequences, and NCSA license rationale; establishes ADR directory with README and reusable template for future architectural decisions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A ? prefix whispers from PE files deep,
Now msvc-demangler breaks the silence to keep.
Length guards stand sentinel, LLVM flags aglow,
Windows symbols demangle—no more mangled show.
Decisions recorded, tests flow strong. 🪟

🚥 Pre-merge checks | ✅ 7 | ❌ 3

❌ Failed checks (3 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	Title follows Conventional Commits format but omits required scope; should be 'feat(classification):' per guidelines.	Update title to 'feat(classification): Add MSVC symbol demangling support for PE binaries' to include the 'classification' scope.
Ascii Only	⚠️ Warning	PR introduces Unicode punctuation: 32 em-dashes and 19 smart quotes in docs/plans/2026-06-19-002-feat-msvc-symbol-demangling-plan.md, plus 3 smart quotes in docs/adr/0002-msvc-demangler-dependency.md.	Replace em-dashes (—) with regular hyphens (-) and smart quotes ("") with straight quotes ("") in the two documentation files.
File Size Limit	⚠️ Warning	src/classification/symbols.rs exceeds 500-line soft limit at 674 lines; contributor acknowledged this but did not split file into modules as suggested by the check instructions.	Split src/classification/symbols.rs into symbols/mod.rs (implementation) and symbols/tests.rs (test module) as mentioned in the contributor's follow-ups section.

✅ Passed checks (7 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	All requirements from issue #19 are met: is_mangled() recognizes '?' prefix [R1], demangle() produces correct output for MSVC patterns including plain functions and operators [R2-R5], tests cover valid symbols and invalid fallback [R3], no regression in existing tests [R4], cargo deny passes for new dependency [R6], and performance is acceptable at ~399ns [R7].
Out of Scope Changes check	✅ Passed	All changes directly support MSVC demangling: dependency addition, symbols.rs implementation, deny.toml license update, ADRs documenting decisions, and test documentation. The .claude/settings.json change (enabling ecc plugin) is marginally tangential but appears incidental to the core feature.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 70.00%.
No Unsafe Code	✅ Passed	No unsafe code introduced: crate maintains #![forbid(unsafe_code)], symbols.rs contains no unsafe blocks/functions/allow attributes, and try_msvc_demangle implementation uses safe Rust patterns wit...
Error Handling	✅ Passed	Error handling follows project conventions: new try_msvc_demangle() uses .ok()? pattern identical to existing try_cpp_demangle(), gracefully returning None on error with no StringyError needed.
Section Weight Consistency	✅ Passed	Check does not apply: custom check "Section Weight Consistency" applies only to container parser changes (src/container/*.rs), but this PR contains no modifications to container parsers—only to src...
Description check	✅ Passed	The PR description provides detailed and accurate context for all changes: MSVC symbol demangling support via msvc-demangler, security hardening with length cap, module restructuring, and ADR documentation. It directly aligns with the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/msvc-symbol-demangling

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

Adds MSVC (?-prefixed) symbol demangling to Stringy’s existing symbol-demangling classification path so PE import/export names can be converted to readable forms, aligning Windows support with the existing Rust/Itanium flows.

Changes:

• Extended SymbolDemangler to detect and demangle MSVC-mangled symbols using msvc-demangler, with a max-length guard to reduce stack-overflow DoS risk.
• Added msvc-demangler dependency and allowed the NCSA license in deny.toml.
• Added a detailed implementation plan doc and updated AGENTS.md dependency list.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/classification/symbols.rs	Adds MSVC demangling dispatch + length guard and unit tests for the new path.
Cargo.toml	Introduces msvc-demangler = "0.11.0" dependency.
deny.toml	Allows NCSA to satisfy cargo deny licensing checks for the new crate.
docs/plans/2026-06-19-002-feat-msvc-symbol-demangling-plan.md	Documents the design/requirements/test plan for MSVC demangling.
AGENTS.md	Updates the “Key Dependencies” list to include msvc-demangler.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

docs/plans/2026-06-19-002-feat-msvc-symbol-demangling-plan.md (1)

14-215: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Replace non-ASCII punctuation in this markdown plan.

The new plan includes Unicode punctuation (for example em dashes in several lines). This repo requires ASCII-only punctuation in markdown files.

As per coding guidelines, **/*.{rs,md,txt,toml,yaml,yml,json}: Verify that no Unicode punctuation is introduced and **/*.{rs,md}: Never use emojis, em-dashes, or non-Latin characters in code or documentation.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/plans/2026-06-19-002-feat-msvc-symbol-demangling-plan.md` around lines
14 - 215, Replace all non-ASCII punctuation characters in the markdown plan file
with ASCII equivalents. Specifically, replace em dashes (—) with hyphens (-) or
double hyphens (--) throughout the document, and check for any other Unicode
punctuation marks (smart quotes, ellipsis characters, etc.) and replace them
with standard ASCII equivalents. This applies to the entire document, including
all sections from "Problem Frame" through "Sources & Research" and within all
code blocks, mermaid diagrams, and regular prose text. Ensure all punctuation
conforms to ASCII-only requirements as stated in the project's coding
guidelines.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/classification/symbols.rs`:
- Around line 548-674: The src/classification/symbols.rs file now exceeds the
500-line limit with the addition of MSVC demangling tests. Create a new
dedicated test module file (e.g., src/classification/symbols/msvc_tests.rs or a
tests subdirectory) and move all the MSVC test functions
(test_is_mangled_msvc_symbols, test_is_mangled_msvc_not_triggered_for_plain,
test_demangle_msvc_plain_function, test_demangle_msvc_constructor,
test_demangle_msvc_operator, test_demangle_msvc_invalid_fallback,
test_try_demangle_msvc_success, test_try_demangle_msvc_failure,
test_demangle_msvc_oversized_symbol_rejected, and
test_msvc_symbol_preserves_existing_tags) to this new module file. Update the
module declaration in symbols.rs to include the new test module, and remove all
moved test code from the original file to keep production logic separate from
tests.

---

Outside diff comments:
In `@docs/plans/2026-06-19-002-feat-msvc-symbol-demangling-plan.md`:
- Around line 14-215: Replace all non-ASCII punctuation characters in the
markdown plan file with ASCII equivalents. Specifically, replace em dashes (—)
with hyphens (-) or double hyphens (--) throughout the document, and check for
any other Unicode punctuation marks (smart quotes, ellipsis characters, etc.)
and replace them with standard ASCII equivalents. This applies to the entire
document, including all sections from "Problem Frame" through "Sources &
Research" and within all code blocks, mermaid diagrams, and regular prose text.
Ensure all punctuation conforms to ASCII-only requirements as stated in the
project's coding guidelines.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 52ef8802-9f2e-4ad5-b779-b3d0d19e0430

📥 Commits

Reviewing files that changed from the base of the PR and between ddd9b0f and 575bf31.

📒 Files selected for processing (5)

• AGENTS.md
• Cargo.toml
• deny.toml
• docs/plans/2026-06-19-002-feat-msvc-symbol-demangling-plan.md
• src/classification/symbols.rs

[codecov]

Codecov Report

❌ Patch coverage is 94.44444% with 4 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/classification/symbols/mod.rs	94.44%	4 Missing ⚠️

📢 Thoughts on this report? Let us know!

[mergify]

Queued — the merge queue status continues in this comment ↓.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.

[Copilot]

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated 6 comments.

[Copilot]

Pull request overview

Copilot reviewed 43 out of 48 changed files in this pull request and generated 2 comments.

[unclesp1d3r]

@Mergifyio queue

[mergify]

Merge Queue Status

• ✅ Entered queue — 2026-06-20 03:36 UTC · Rule: default
• ✅ Checks passed · on draft merge queue: queuing main (ddd9b0f) and #177 together #180
• ✅ Merged — 2026-06-20 03:52 UTC · at 6d16b19c6bdb5476ba17f5875731813decea8000 · squash

This pull request spent 15 minutes 30 seconds in the queue, including 15 minutes 10 seconds running CI.

Required conditions to merge

• check-success = coverage
• check-success = msrv (stable minus 1 releases)
• check-success = msrv (stable minus 2 releases)
• check-success = msrv (stable minus 3 releases)
• check-success = msrv (stable minus 4 releases)
• check-success = msrv (stable)
• check-success = quality
• check-success = test
• check-success = test-cross-platform (macos-latest, macOS)
• check-success = test-cross-platform (ubuntu-latest, Linux)
• check-success = test-cross-platform (windows-latest, Windows)
• all of [🛡 Merge Protections rule Do not merge outdated PRs]:
  • #commits-behind <= 10
    • feat: Add MSVC symbol demangling support for PE binaries #177
• all of [🛡 Merge Protections rule Enforce conventional commit]:
  • title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.+\))?:
    • feat: Add MSVC symbol demangling support for PE binaries #177
• all of [🛡 Merge Protections rule Full CI must pass]:
  • check-success = coverage
  • check-success = msrv (stable minus 1 releases)
  • check-success = msrv (stable minus 2 releases)
  • check-success = msrv (stable minus 3 releases)
  • check-success = msrv (stable minus 4 releases)
  • check-success = msrv (stable)
  • check-success = quality
  • check-success = test
  • check-success = test-cross-platform (macos-latest, macOS)
  • check-success = test-cross-platform (ubuntu-latest, Linux)
  • check-success = test-cross-platform (windows-latest, Windows)
• any of [🛡 GitHub repository ruleset rule main]:
  • check-neutral = Mergify Merge Protections
  • check-skipped = Mergify Merge Protections
  • check-success = Mergify Merge Protections