Skip to content

decomp+ghidra: fix FUN_0000f7b0 lift for issue #225#228

Merged
kumarak merged 2 commits into
mainfrom
kumarak/issue225
May 13, 2026
Merged

decomp+ghidra: fix FUN_0000f7b0 lift for issue #225#228
kumarak merged 2 commits into
mainfrom
kumarak/issue225

Conversation

@kumarak
Copy link
Copy Markdown
Collaborator

@kumarak kumarak commented May 13, 2026

Stack locals that Ghidra mistypes (e.g., unsigned char[2] for a 16 KB recv buffer) caused a SIGABRT in clang Sema at INT_ZEXT.

Three layers, matching Ghidra-decompile C:

  • Producer (PcodeSerializer.java): infer stack-local size from buffer-shaped ABI calls (memset/memcpy/recv/...) and widen the DECLARE_LOCAL type, gated by defensive guards (sibling-view, frame-budget, no-overlap-with-other-locals, zero-offset, size cap, thunk-walk, COPY recursion bound, positive-offset refusal, VariableStorage identity for self-skip).
  • Resolution (OpBuilder::create_varnode): when the truthful per-varnode size is narrower than the resolved storage, emit arr[0] or *(uintN_t*)&arr at use-site resolution. Helper uses unsigned arithmetic end-to-end, caps reinterpret width at storage size, requires non-zero array length for subscript path.
  • Write side (create_assign_operation): when a scalar→array width matches one element, emit arr[0] = input instead of refusing; requires non-zero element count.

LIT fixture: test/patchir-decomp/issue_225_fun_0000f7b0.json pins the post-fix shape — the widening guard refuses here (Ghidra placed overlapping sub-locals), keeping unsigned char local_4018[2] and showing memset(&local_4018, 0, 16384U) as a visible OOB for downstream verifiers; element-subscript paths preserve byte read/write semantics.

Stack locals that Ghidra mistypes (e.g. `unsigned char[2]` for a
16 KB recv buffer) caused a SIGABRT in clang Sema at INT_ZEXT.

Three layers, matching Ghidra-decomp / IDA Hex-Rays / BN HLIL:
- Producer (PcodeSerializer.java): infer stack-local size from
  buffer-shaped ABI calls (memset/memcpy/recv/...) and widen the
  DECLARE_LOCAL type, gated by defensive guards (sibling-view,
  frame-budget, no-overlap-with-other-locals, zero-offset, size
  cap, thunk-walk, COPY recursion bound, positive-offset refusal,
  VariableStorage identity for self-skip).
- Resolution (OpBuilder::create_varnode): when the truthful
  per-varnode `size` is narrower than the resolved storage, emit
  `arr[0]` or `*(uintN_t*)&arr` at use-site resolution.  Helper
  uses unsigned arithmetic end-to-end, caps reinterpret width at
  storage size, requires non-zero array length for subscript path.
- Write side (create_assign_operation): when a scalar→array width
  matches one element, emit `arr[0] = input` instead of refusing;
  requires non-zero element count.

LIT fixture: test/patchir-decomp/issue_225_fun_0000f7b0.json pins
the post-fix shape — the widening guard refuses here (Ghidra placed
overlapping sub-locals), keeping `unsigned char local_4018[2]` and
showing `memset(&local_4018, 0, 16384U)` as a visible OOB for
downstream verifiers; element-subscript paths preserve byte
read/write semantics.

LIT: patchir-decomp 72/72; full suite 179/181 (two unresolved are
pre-existing veribin infra).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@kumarak
Copy link
Copy Markdown
Collaborator Author

kumarak commented May 13, 2026

@claude

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 13, 2026

PR Review in Progress

  • Read patchir-review skill definition
  • Read changed files
  • Analyze C/C++ changes (3 in-scope files: OperationBuilder.hpp, OperationBuilder.cpp, OperationStmt.cpp)
  • Post inline findings
  • Post final verdict

View job run

Comment thread lib/patchestry/AST/OperationBuilder.cpp
Comment thread lib/patchestry/AST/OperationStmt.cpp
Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

No blocking issues found.

When the helper returns null (unsupported width, or over-read past
storage), create_varnode falls back to the unnarrowed aggregate.  A
downstream INT_ZEXT/INT_SEXT/CAST then trips
  assert(Diagnosed && "failed to diagnose bad conversion")
in clang Sema, which is indistinguishable from the original #225
SIGABRT.  Emit a WARNING naming storage type, requested size, and
upstream op key so the unhandled width is one grep away.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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