Repository navigation
merge queue: queuing main (ddd9b0f) and #177 together - #180
Closed
mergify[bot] wants to merge 33 commits into
Closed
mergify[bot] wants to merge 33 commits into
mergify[bot] wants to merge 33 commits into
Conversation
Extend SymbolDemangler to demangle MSVC-mangled symbols (?-prefixed) found in Windows PE binaries, closing the gap where the pipeline already detected ?-prefixed symbols but the demangler silently passed them through. - Add msvc-demangler 0.11 (MIT/NCSA) and allow NCSA in deny.toml - is_mangled() now returns true for ?-prefixed symbols - try_demangle_internal() routes ? symbols to a new try_msvc_demangle() helper using DemangleFlags::llvm(), mirroring the Rust/C++ helpers - Demangled MSVC symbols preserve the original in original_text and receive Tag::DemangledSymbol via the existing demangle() flow - Add 9 unit tests (plain fn, constructor, operator, invalid fallback, tag preservation) plus doctest coverage; no pipeline changes needed Closes #19 Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Address code-review findings on the MSVC demangling change: - P1: a crafted ?-prefixed symbol with deeply nested type modifiers drives msvc-demangler's recursive parser into a stack overflow that aborts the process (uncaught by catch_unwind, which does not catch stack overflow). Reject symbols longer than MSVC_MAX_SYMBOL_LEN (4096) before parsing -- cheap defense-in-depth for untrusted binary input. Add a regression test with a 60KB nested-pointer symbol. - P2: correct try_demangle_internal doc comment to mention MSVC. - P2: list msvc-demangler in AGENTS.md Key Dependencies. - Clarify the DemangleFlags::llvm() rationale comment (simplify pass). Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Initialize docs/adr/ with the Michael Nygard ADR format and capture the two architectural decisions from the MSVC symbol demangling work: - ADR-0001: length-cap guard (MSVC_MAX_SYMBOL_LEN) chosen over thread isolation, upstream fix, or source restriction to contain the stack-overflow DoS that catch_unwind cannot catch. - ADR-0002: selecting the msvc-demangler crate and allowing the NCSA license, over hand-rolling, shelling to undname.exe, or broadening deny.toml. Adds README.md index and a blank template.md for future records. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…cy test Addresses two non-blocking review follow-ups from PR #177: - symbols.rs exceeded the 500-line soft limit (674 lines, mostly test growth). Split into symbols/mod.rs (production, 306 lines) and symbols/tests.rs (390 lines), mirroring the existing patterns/ module directory convention in classification/. - Add test_demangle_msvc_idempotent covering the MSVC double-demangle no-op, matching the existing Rust-path test_demangle_idempotent. The no-op was mechanically guaranteed by the is_mangled() early-exit but not directly tested. No behavior change. fmt + clippy clean; all symbol tests pass. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
- docs/plans: replace 32 em-dashes (—) with ASCII -- per AGENTS.md ASCII-only rule (Copilot review, 3 threads) - symbols/tests.rs: remove test_is_mangled_msvc_not_triggered_for_plain; its printf/CreateFileW/empty cases are already covered by test_is_mangled_not_mangled (Copilot DRY review) Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…tails Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…kflow Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…workflow Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
- symbols/tests.rs: build FoundString via new() instead of a struct literal, so the #[non_exhaustive] type stays forward-compatible (Copilot); behavior-identical (new() defaults match the old literal) - symbols/tests.rs: shrink the oversized-symbol regression input from ~60KB to ~4.2KB (just over MSVC_MAX_SYMBOL_LEN) -- the length guard early-returns before parsing, so the extra size was wasted (Copilot) - docs/plans: update src/classification/symbols.rs references to symbols/mod.rs after the module split (Copilot) - remove .zig-version: redundant with mise.toml (zig 0.16.0) and had drifted to 0.15.2; mise.toml is the single source of truth (Copilot) Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…t standards - Deleted work_next_task.md as it was no longer relevant to the workflow. - Removed mcp.json configuration file for MCP server setup. - Eliminated cargo-toml.mdc, configuration-management.mdc, error-handling-patterns.mdc, error-handling.mdc, linting-rules.mdc, performance-optimization.mdc, and rust-standards.mdc as they were outdated or redundant. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…ge ecosystems Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…s and guidelines Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
dist-workspace.toml's github-action-commits entry for actions/upload-artifact had a trailing space inside the quoted SHA, which cargo-dist templated into release.yml at 5 uses: lines. GitHub Actions treats the whole string as the ref, so it would fail to resolve. Fixed the source config and the generated workflow; actionlint clean. (Copilot review, 6 threads) Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
6 of 7 tasks
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎉 This pull request has been checked successfully and will be merged soon. 🎉
Branch main (ddd9b0f) and #177 are queued together for merge.
This pull request has been created by Mergify to check the mergeability of #177.
You don't need to do anything. Mergify will close this pull request automatically when it is complete.
Required conditions of queue rule
defaultfor merge:check-success = coveragecheck-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 = qualitycheck-success = testcheck-success = test-cross-platform (macos-latest, macOS)check-success = test-cross-platform (ubuntu-latest, Linux)check-success = test-cross-platform (windows-latest, Windows)#commits-behind <= 10title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.+\))?:check-success = coveragecheck-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 = qualitycheck-success = testcheck-success = test-cross-platform (macos-latest, macOS)check-success = test-cross-platform (ubuntu-latest, Linux)check-success = test-cross-platform (windows-latest, Windows)main]:check-neutral = Mergify Merge Protectionscheck-skipped = Mergify Merge Protectionscheck-success = Mergify Merge ProtectionsRequired conditions to stay in the queue:
author != dependabot[bot]author != dosubot[bot]base = main#commits-behind <= 10title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.+\))?:check-success = coveragecheck-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 = qualitycheck-success = testcheck-success = test-cross-platform (macos-latest, macOS)check-success = test-cross-platform (ubuntu-latest, Linux)check-success = test-cross-platform (windows-latest, Windows)main]:check-success = Mergify Merge Protectionscheck-neutral = Mergify Merge Protectionscheck-skipped = Mergify Merge Protections