Skip to content

Fix #22: partial signature on cancel (opt-in)#25

Merged
mahmoudimus merged 9 commits into
mainfrom
fix/issue-22-partial-signature-on-cancel
May 27, 2026
Merged

Fix #22: partial signature on cancel (opt-in)#25
mahmoudimus merged 9 commits into
mainfrom
fix/issue-22-partial-signature-on-cancel

Conversation

@mahmoudimus

Copy link
Copy Markdown
Owner

Closes #22.

Background

From @OshidaBCF on issue thread #22:

The search has been going for 10 minutes now, and it's at least 200 byte long, while i don't mind waiting, when i inevitably decide to cancel it, i'd like to get a "partial" signature

Today, when a unique-signature search is cancelled, the in-progress signature lives only as a local variable inside UniqueSignatureGenerator.generate and gets thrown away. The user sees Operation cancelled by user and nothing else. This PR keeps the partial around when the user opts in.

Algorithm note

The generator already calls is_unique(...) on every iteration, implemented as len(find_all(...)) == 1. That is, every byte we append, we do a full database scan and then throw the count away. So the match count we want to show on cancel was already being computed every iteration in the loop above; we just weren't keeping it.

I expose the count via a new SignatureSearcher.count_matches, rewrite is_unique on top of it, and have the generator hold onto the last value it observed. On cancel, the most recent count is exactly the right number for the bytes currently in the partial signature, by construction (the cancel check is at the top of the loop, count_matches is at the bottom). Zero extra search work.

Net behavior

  • Default. Cancel a unique-signature search and the output reads Operation cancelled by user. No change from current behavior.
  • Opt-in. Tick "Output partial signature on cancel (opt-in)" on the main form. Cancel mid-run prints something like Partial signature (NOT unique, 47 matches) for 0x140001234: E8 ?? ?? ?? ?? 45 33 F6 .... The clipboard is not touched, so an accidental cancel cannot clobber what you had copied before.
  • Cancel before any byte is appended. Still reads Operation cancelled by user, so no zero-byte garbage printed.
  • XREF and range paths are untouched.

Verification

Suite Before After
Host unit (tests/unit_test_sigmaker.py) 118 OK 134 OK (+16 new)
Docker idapro-tests (9.0/9.1) 134 OK 150 OK
Docker idapro-tests-9.2 134 OK 150 OK

Zero regressions. New test coverage:

  • TestSignatureSearcherCountMatches: count_matches equals len(find_all(...)); is_unique stays True only at exactly 1.
  • TestGenerationStatusAndPolicy: enum members and policy classmethods.
  • TestGeneratedSignatureDisplay: UNIQUE prints and writes clipboard; PARTIAL_ON_CANCEL prints with match count and does not write clipboard; partials never recompute the count.
  • TestUniqueSignatureGeneratorPartialOnCancel: strict cancel raises; permissive cancel returns a partial with the last-observed match count; permissive + unique returns UNIQUE; permissive cancel before any byte appended still raises.
  • TestSigMakerConfigDefaults: new field defaults False.

…n it

Clarity refactor with zero behavior change. is_unique was already
implemented as len(find_all(...)) == 1, throwing away the count.
Exposing it gives UniqueSignatureGenerator a clean hook to reuse
the search work it was already doing per iteration.
Introduces the opt-in vocabulary for the partial-on-cancel feature.
GenerationStatus.UNIQUE preserves existing semantics; GenerationPolicy
defaults to strict (raise on cancel) so existing callers see no change.
Adds the result-object fields the generator will populate in the next
task. display() prints the partial-format message with the count and
intentionally skips Clipboard.set_text for partials so an accidental
cancel cannot clobber the user's clipboard.
UniqueSignatureGenerator.generate now returns a GeneratedSignature
directly (carrying status + match_count) so partial-on-cancel can
thread cleanly. Match count is reused from the per-iteration
uniqueness check (rebuilt on count_matches in the previous commit)
with zero extra search work. Strict policy (default) preserves the
legacy contract.

Range generation is unchanged: RangeSignatureGenerator still returns
a bare Signature and SignatureMaker.make_signature still wraps it.
Off by default per issue #22 discussion. Users opt in via the form
checkbox added in the next task.
Wires the new SigMakerConfig.output_partial_on_cancel field to a new
checkbox on the main form (bit 16, default OFF). SigMakerPlugin.run
reads it for the CREATE_UNIQUE branch and passes the matching policy
to make_signature. Range and XREF paths use the default strict policy
and are unaffected.
InstructionWalker had end_ea: int = idaapi.BADADDR as a dataclass
field default. Dataclass defaults are evaluated at class-definition
time, which under the unit-test mock of idaapi froze end_ea as a
MagicMock attribute. Real IDA always supplies a real int, so this
was unit-test-only, but it forced TestUniqueSignatureGeneratorPartialOnCancel
to mock the whole InstructionWalker rather than the real one.

Switch to default_factory=lambda: idaapi.BADADDR so the value is
resolved per-instance. Restore the cleaner test which now exercises
the real walker.
@OshidaBCF reported testing PR #25 and seeing 'Partial signature
(NOT unique, 0 matches)' even when the partial actually matched many
places. The cause: SignatureSearcher.find_all polls
idaapi_user_canceled inside its scan loop and bails early when set,
returning whatever partial count it had so far (often 0).
UniqueSignatureGenerator was recording that as last_match_count
without checking whether the call had been interrupted.

Check idaapi_user_canceled right after count_matches returns. If
set, mark last_match_count as None so the partial-on-cancel path
shows 'match count unavailable' rather than a falsely-low count.
@OshidaBCF pointed out in issue #22 that 'match count unavailable'
is less useful than it could be. The prior fix in e4f4fd4 detected
when count_matches was interrupted mid-scan and discarded the
unreliable value, but it ALSO discarded the previous iteration's
trustworthy count by overwriting last_match_count.

This commit changes the guard to a one-way assignment: only update
last_match_count when count_matches completed normally. On
interruption, last_match_count keeps its prior value (typically the
count from the iteration before the user clicked Cancel).

The reported number corresponds to a signature one instruction
shorter than the partial we emit, so it is an upper bound on the
partial's actual match count. That is dramatically more useful than
either a fake 0 or a 'match count unavailable' placeholder.
@mahmoudimus mahmoudimus merged commit 2f23e15 into main May 27, 2026
2 checks passed
@mahmoudimus mahmoudimus deleted the fix/issue-22-partial-signature-on-cancel branch May 27, 2026 15:43
mahmoudimus added a commit that referenced this pull request May 27, 2026
* docs: add CHANGELOG.md covering 1.7.0 (#24, #25)

Keep a Changelog format. Documents the wait-box cancel UX from
issue #18 (PR #24) and the opt-in partial-on-cancel output from
issue #22 (PR #25), including the user-visible behavior changes,
the new opt-in form controls, and the bug fixes that were folded
in along the way.

* fix: use US spelling in user-facing cancel message

The output line on UserCanceledError was 'Operation cancelled by
user' (UK), the only UK-spelled user-visible string left in the
plugin. Align with the rest of the codebase which uses 'canceled'
throughout (per the comment at line 87).

* chore: bump version to 1.7.0

Cuts the 1.7.0 release covering the wait-box cancel UX (#24) and
opt-in partial-on-cancel output (#25). See CHANGELOG.md for the
full set of user-visible and internal changes.
mahmoudimus added a commit that referenced this pull request May 27, 2026
Brings in PR #24 (issue #18 cancel), PR #25 (issue #22 partial on
cancel), and PR #28 (1.7.0 release). The auto-merge handled
src/sigmaker/__init__.py cleanly; tests/unit_test_sigmaker.py had
overlapping test-class insertions at the same anchor, resolved by
taking main's test file as the base and appending PR #26's three
new test classes (TestMinimalFunctionSignatureGenerator,
TestActionEnumAddsFunctionSig, TestGeneratedSignatureOrdering)
before the if __name__ block.
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.

[Feature Request] Cancelling search for any reason should return partial, even if incomplete signature instead of nothing, even if it isn't unique

1 participant