diff --git a/src/hexgraph/agent/agent_tools.py b/src/hexgraph/agent/agent_tools.py index 9618ea3..d7fe0e5 100644 --- a/src/hexgraph/agent/agent_tools.py +++ b/src/hexgraph/agent/agent_tools.py @@ -510,15 +510,29 @@ def _format_decomp(out: dict, label: str, *, limit: int = _MAX) -> str: focus = out.get("focus") if not focus: defined = out.get("functions", []) or [] - # "defined functions" (not "imports") — the list is the decompiler's DEFINED set, which - # for a stripped binary or a Ghidra/r2 inventory mismatch may not include the requested - # focus even though it exists. If the label is a function-NAME miss, point at the - # address-based path (decompile_at resolves the function CONTAINING a hex address from - # xrefs/strings, and r2 backs Ghidra when their inventories disagree). - hint = ("" if label.startswith("address ") - else " — if you have its address, try re_decompile_at()") - return (f"{label} not found among the {len(defined)} defined functions" - + (f": {', '.join(defined[:40])}" if defined else "") + hint) + # The TRUE whole-program count — not len(defined), which is a (possibly capped) returned slice. + # Reporting the slice length as the total is what made a large firmware read as "only 400 + # functions"; functions_total is the real inventory size. + total = out.get("functions_total") + if not isinstance(total, int) or total < len(defined): + total = len(defined) + # An ADDRESS that resolved to no function is a DIFFERENT failure than a missing NAME: the + # address isn't inside any defined function (unanalyzed region, data, a different segment, or + # simply not code). Dumping the function-name list can't help an address lookup — point at the + # raw-disassembly / deeper-analysis escape hatches instead. + if label.startswith("address "): + return (f"{label} is not inside any defined function (of {total} defined). It may be in an " + f"unanalyzed region, data, or a different segment. Try re_disassemble_range(, " + f"count=…) for a raw disassembly there regardless of function boundaries, or " + f"re_decompile_at(, reanalyze=True) to force a deeper analysis pass.") + # A NAME miss: the decompiler's DEFINED set may not include the requested focus even though it + # exists (a stripped binary, or a Ghidra/r2 inventory mismatch). Show a SAMPLE of the inventory + # (never the whole thing) with the true total, and point at the address-based path. + shown = defined[:40] + more = f" (+{total - len(shown)} more)" if total > len(shown) else "" + return (f"{label} not found among the {total} defined functions" + + (f": {', '.join(shown)}{more}" if shown else "") + + " — if you have its address, try re_decompile_at()") addr = f" @ {focus['address']}" if focus.get("address") else "" name = focus.get("name") or label promo = out.get("promotable_callees") or [] @@ -659,17 +673,20 @@ def _decomp(ctx: ToolContext, function: str | None, *, # call (that yielded no focus), not a list_functions call, so it must not pollute # the discoverability index under the wrong tool name. fns = out.get("functions", []) + ftotal = out.get("functions_total") + if not isinstance(ftotal, int) or ftotal < len(fns): + ftotal = len(fns) if focused: subj = function or address obs, _cached = _record_obs( ctx, tool=req_tool, args=req_args, result_kind="function_list", - payload={"functions": fns}, - summary=f"{subj!r} not found; {len(fns)} functions available") + payload={"functions": fns, "functions_total": ftotal}, + summary=f"{subj!r} not found; {ftotal} functions available") else: obs, _cached = _record_obs( ctx, tool=req_tool, args=req_args, result_kind="function_list", - payload={"functions": fns}, - summary=f"{len(fns)} functions" + (" (re-analyzed)" if reanalyze else "")) + payload={"functions": fns, "functions_total": ftotal}, + summary=f"{ftotal} functions" + (" (re-analyzed)" if reanalyze else "")) out["observation_id"] = obs.id if obs is not None else None return out @@ -923,7 +940,14 @@ def run_tool(ctx: ToolContext, name: str, args: dict) -> str: if out.get("error"): return out["error"] fns = out.get("functions", []) - return _clip(f"re-analyzed ({len(fns)} functions):\n" + "\n".join(fns[:300])) + # Report the TRUE whole-program count (functions_total), not len(fns) — the returned list is + # a bounded slice, and the inline body shows only the first 300 (same slice-vs-total fix as + # list_functions / the decompile fallback). + ftotal = out.get("functions_total") + if not isinstance(ftotal, int) or ftotal < len(fns): + ftotal = len(fns) + more = "; showing first 300" if ftotal > 300 else "" + return _clip(f"re-analyzed ({ftotal} functions{more}):\n" + "\n".join(fns[:300])) if name == "check_decompiler": from hexgraph.agent.mcp_tools import check_decompiler d = check_decompiler() @@ -1232,18 +1256,26 @@ def _list_functions(ctx: ToolContext, args: dict) -> str: if isinstance(out, dict) and out.get("error"): return out["error"] names = [str(f) for f in (out.get("functions", []) or [])] + # The TRUE whole-program count. `names` is the inventory the decompiler emitted (the full list up + # to a defensive cap); functions_total is how many functions actually exist. Reporting len(names) + # as the total is the bug that made a large firmware look like it had only 400 functions. + grand_total = out.get("functions_total") + if not isinstance(grand_total, int) or grand_total < len(names): + grand_total = len(names) + withheld = grand_total - len(names) # functions beyond the returned inventory cap (normally 0) pat = args.get("pattern") or "" use_regex = bool(args.get("regex")) match = _compile_grep(pat, regex=use_regex) matches = [n for n in names if match(n)] if pat else names - total = len(matches) + pageable = len(matches) # how many names we can actually page over + total = pageable if pat else grand_total # the count reported to the agent - offset = _bound_page(args.get("offset"), 0, 0, max(0, total)) + offset = _bound_page(args.get("offset"), 0, 0, max(0, pageable)) limit = _bound_page(args.get("limit"), _FUNCS_PAGE, 1, _FUNCS_PAGE_MAX) page = matches[offset:offset + limit] next_offset = offset + len(page) - more = next_offset < total + more = next_offset < pageable pat_note = f" matching {pat!r}" if pat else "" # Record the page actually returned (keyed by pattern+offset+limit so a different page is its @@ -1254,15 +1286,21 @@ def _list_functions(ctx: ToolContext, args: dict) -> str: args={k: v for k, v in (("pattern", pat), ("offset", offset), ("limit", limit)) if v}, result_kind="function_list_page", - payload={"functions": page, "total": total, "offset": offset, "limit": limit}, + payload={"functions": page, "total": total, "grand_total": grand_total, + "offset": offset, "limit": limit}, summary=f"{total} functions{pat_note}; page {offset}-{next_offset}") header = f"functions{pat_note} ({total} total, showing {offset}-{next_offset})" body = "\n".join(page) or "(none)" tail = "" if more: - tail = (f"\n…[{total - next_offset} more — re-call with offset={next_offset}" + tail = (f"\n…[{pageable - next_offset} more — re-call with offset={next_offset}" + (f", limit={limit}" if limit != _FUNCS_PAGE else "") + "]") + if withheld: + # The decompiler returned a bounded slice of a larger inventory — never let the page counts + # read as the whole program: name the true total and how many are beyond the returned set. + tail += (f"\n…[note: {grand_total} functions defined; {withheld} are beyond the returned " + f"inventory and not listed here — re_decompile_at reaches any function by address]") prefix = f"{header}:\n" budget = _MAX - len(prefix) - len(tail) if budget > 0 and len(body) > budget: diff --git a/src/hexgraph/engine/re/ghidra_bridge.py b/src/hexgraph/engine/re/ghidra_bridge.py index d5545a4..8665846 100644 --- a/src/hexgraph/engine/re/ghidra_bridge.py +++ b/src/hexgraph/engine/re/ghidra_bridge.py @@ -28,6 +28,10 @@ # (analyze-at-address), mirroring the headless probe; otherwise the focus is a name. _ADDR = re.compile(r"^0x[0-9a-fA-F]+$") +# Defensive upper bound on the returned function-name inventory (names are cheap; the full list rides +# with `functions_total`, the true count). Mirrors pyghidra_lib.MAX_FUNCTION_NAMES. +_MAX_FUNCTION_NAMES = 20000 + class BridgeUnavailable(RuntimeError): """The Ghidra Bridge client isn't installed or no server is reachable.""" @@ -86,7 +90,8 @@ def decompile(self, program: str | None, function: str | None) -> dict: if resolved: focus = {"name": resolved, "resolved": resolved, "pseudocode": pseudo, "disasm": "", "callees": []} - return {"functions": names[:400], "focus": focus, "tool": "ghidra_bridge"} + return {"functions": names[:_MAX_FUNCTION_NAMES], "functions_total": len(names), + "focus": focus, "tool": "ghidra_bridge"} def _decompile_one(self, focus: str) -> tuple[str, str]: """Decompile the function identified by `focus` over the bridge — an exact NAME, or a @@ -199,7 +204,8 @@ def decompile(self, program: str | None, function: str | None) -> dict: resp = self._rpc(req) # An error / not-found focus reads as no focus, mirroring the headless probe. focus = None if resp.get("error") else resp.get("focus") - return {"functions": (resp.get("functions") or [])[:400], "focus": focus, + return {"functions": (resp.get("functions") or [])[:_MAX_FUNCTION_NAMES], + "functions_total": resp.get("functions_total"), "focus": focus, "tool": "ghidra_bridge"} # The remaining Ghidra ops, each a single RPC to the resident program (the server runs the diff --git a/src/hexgraph/sandbox/decompiler.py b/src/hexgraph/sandbox/decompiler.py index d7d6f12..87416ec 100644 --- a/src/hexgraph/sandbox/decompiler.py +++ b/src/hexgraph/sandbox/decompiler.py @@ -17,6 +17,12 @@ log = logging.getLogger(__name__) +# A per-function decompilation payload keeps only a bounded SAMPLE of the whole-program name list (the +# full inventory lives in the separate `function_list` Observation). This stops the full function-name +# list — now returned in full, up to MAX_FUNCTION_NAMES — from bloating every per-function Observation. +_FOCUS_PAYLOAD_FUNCTION_SAMPLE = 400 + + def focus_only_payload(out: dict) -> dict: """Trim a decompiler result dict to a FOCUS-ONLY payload for a per-function `decompilation` Observation. @@ -27,8 +33,20 @@ def focus_only_payload(out: dict) -> dict: (`_extract_functions`) and `search_decompiled` read only `focus` (whole-program calls/structs are enriched from SEPARATE call_graph/structs Observations recorded by enrich_recon), so dropping them here loses nothing — the focus's own callees stay inside `focus`. The single - source of truth so the agent-tool path and the single-pass static_analysis path stay in sync.""" - return {"functions": (out or {}).get("functions", []), "focus": (out or {}).get("focus")} + source of truth so the agent-tool path and the single-pass static_analysis path stay in sync. + + The whole-program `functions` list is kept only as a bounded sample (the full inventory is its own + `function_list` Observation); `functions_total` preserves the true whole-program count.""" + fns = (out or {}).get("functions", []) or [] + # Guard against a present-but-None `functions_total` (a stale managed bridge returns + # `functions_total: resp.get(...)` = None), matching the isinstance guard at the user-facing sites — + # `.get(..., default)` would only default on a MISSING key and store null here. + ftotal = (out or {}).get("functions_total") + if not isinstance(ftotal, int) or ftotal < len(fns): + ftotal = len(fns) + return {"functions": fns[:_FOCUS_PAYLOAD_FUNCTION_SAMPLE], + "functions_total": ftotal, + "focus": (out or {}).get("focus")} class Decompiler(ABC): diff --git a/src/hexgraph/sandbox/probes/decompile_probe.py b/src/hexgraph/sandbox/probes/decompile_probe.py index d8b02c3..f034353 100644 --- a/src/hexgraph/sandbox/probes/decompile_probe.py +++ b/src/hexgraph/sandbox/probes/decompile_probe.py @@ -187,6 +187,10 @@ def _callees(disasm: str) -> list[str]: # the returned text separately (the no-silent-caps marker). Generous enough to read a whole # blind-spot region in one pass. _RANGE_DEFAULT_LENGTH = 256 +# The function-name inventory is returned in FULL (names are cheap) up to this defensive upper bound, +# always alongside `functions_total` (the true count). The old hard [:200] made a large firmware look +# like it had only 200 functions and hid the rest from list_functions. +_MAX_FUNCTION_NAMES = 20000 _RANGE_MAX_LENGTH = 8192 _RANGE_MAX_COUNT = 1024 @@ -555,8 +559,8 @@ def main() -> int: # `cached` mirrors ghidra_probe: True ⇒ served from a WARM persistent project (no `aaa` # this call), False ⇒ a cold analysis (or the uncached throwaway path). - print(json.dumps({"tool": "decompile_probe", "functions": functions[:200], - "focus": focus, "cached": warm})) + print(json.dumps({"tool": "decompile_probe", "functions": functions[:_MAX_FUNCTION_NAMES], + "functions_total": len(functions), "focus": focus, "cached": warm})) return 0 finally: r2.quit() diff --git a/src/hexgraph/sandbox/probes/pyghidra_lib.py b/src/hexgraph/sandbox/probes/pyghidra_lib.py index cc9dc78..660c5b1 100644 --- a/src/hexgraph/sandbox/probes/pyghidra_lib.py +++ b/src/hexgraph/sandbox/probes/pyghidra_lib.py @@ -298,6 +298,12 @@ def _apply_rename(program, flat, addr, new_name) -> bool: return ok +# The whole-program function-name inventory is returned in FULL (names are cheap) up to this defensive +# upper bound, always alongside `functions_total` (the true count). The old hard [:400] made a large +# firmware look like it had only 400 functions and hid the rest from list_functions entirely. +MAX_FUNCTION_NAMES = 20000 + + def decompile_core(program, flat, monitor, *, focus=None, rename=None) -> dict: """Ported POST_SCRIPT: whole-program inventory (functions/calls/structs) + a focused decompile with recovered facts. `focus` is a function NAME or hex ADDRESS; `rename` is (addr, new_name). @@ -309,8 +315,9 @@ def decompile_core(program, flat, monitor, *, focus=None, rename=None) -> dict: fm = program.getFunctionManager() funcs = list(fm.getFunctions(True)) - result = {"functions": [f.getName() for f in funcs][:400], "focus": None, - "calls": [], "structs": []} + all_names = [f.getName() for f in funcs] + result = {"functions": all_names[:MAX_FUNCTION_NAMES], "functions_total": len(all_names), + "focus": None, "calls": [], "structs": []} edges = [] for f in funcs[:600]: @@ -1158,7 +1165,8 @@ def bridge_dispatch(program, flat, monitor, req) -> dict: return {"ok": True, "functions_total": program.getFunctionManager().getFunctionCount()} if op == "list": names = [f.getName() for f in program.getFunctionManager().getFunctions(True)] - return {"functions": names[:400], "tool": "ghidra_bridge"} + return {"functions": names[:MAX_FUNCTION_NAMES], "functions_total": len(names), + "tool": "ghidra_bridge"} if op == "decompile": result = decompile_core(program, flat, monitor, focus=req.get("focus")) result["tool"] = "ghidra_bridge" diff --git a/tests/test_address_access.py b/tests/test_address_access.py index c74e34d..2ee71a8 100644 --- a/tests/test_address_access.py +++ b/tests/test_address_access.py @@ -83,7 +83,7 @@ def test_decompile_at_address_not_found_records_under_decompile_at(hg_home, monk with session_scope() as s: ctx, p, t = _ctx(s) out = run_tool(ctx, "decompile_at", {"address": "0xdeadbeef"}) - assert "not found" in out + assert "not inside any defined function" in out # the distinct address-miss message # no graph mutation, and the miss is attributed to the call actually made assert s.query(Node).filter(Node.node_type == "function").count() == 0 obs = s.query(Observation).filter(Observation.target_id == t.id, diff --git a/tests/test_decompiler_fallback.py b/tests/test_decompiler_fallback.py index a514f38..95299a1 100644 --- a/tests/test_decompiler_fallback.py +++ b/tests/test_decompiler_fallback.py @@ -15,7 +15,30 @@ from __future__ import annotations from hexgraph.agent.agent_tools import _format_decomp -from hexgraph.sandbox.decompiler import GhidraDecompiler, R2Decompiler +from hexgraph.sandbox.decompiler import ( + _FOCUS_PAYLOAD_FUNCTION_SAMPLE, + GhidraDecompiler, + R2Decompiler, + focus_only_payload, +) + + +def test_focus_only_payload_guards_none_functions_total(): + """focus_only_payload derives functions_total from a PRESENT-but-None value (a stale managed bridge + returns `functions_total: None`), not only a missing key — else the stored payload would hold null. + It also caps the whole-program name list to a bounded sample (the full inventory is its own + `function_list` Observation).""" + # present-but-None ⇒ falls back to len(functions), not stored as null + p = focus_only_payload({"functions": ["a", "b", "c"], "functions_total": None, + "focus": {"name": "a"}}) + assert p["functions_total"] == 3 + # a real total is preserved; the name list is a bounded sample, not the whole inventory + big = [f"f{i}" for i in range(1000)] + p2 = focus_only_payload({"functions": big, "functions_total": 9000, "focus": None}) + assert p2["functions_total"] == 9000 + assert len(p2["functions"]) == _FOCUS_PAYLOAD_FUNCTION_SAMPLE + # a missing key defaults to len too + assert focus_only_payload({"functions": ["x"], "focus": None})["functions_total"] == 1 # --- (1) the seam fallback: Ghidra missed the focus → radare2 resolves it ---------- @@ -137,9 +160,24 @@ def test_not_found_wording_says_defined_functions_and_suggests_decompile_at(): assert "re_decompile_at()" in msg # the address-based recovery hint for a NAME miss -def test_not_found_wording_no_addr_hint_for_an_address_miss(): - """An address miss already used the address path — don't tell it to use re_decompile_at.""" - out = {"functions": ["main"], "focus": None} +def test_not_found_wording_for_an_address_miss_points_at_raw_disasm(): + """An address that resolves to no function gets a DISTINCT message: it explains the address isn't + inside a defined function and points at re_disassemble_range (raw disasm) + a reanalyze pass — + NOT a dump of the function-name list, which can't help an address lookup.""" + out = {"functions": ["main", "helper"], "functions_total": 2, "focus": None} msg = _format_decomp(out, "address 0xdeadbeef") - assert "defined functions" in msg - assert "re_decompile_at" not in msg + assert "not inside any defined function" in msg + assert "re_disassemble_range" in msg + assert "reanalyze=True" in msg + assert "main" not in msg and "helper" not in msg # no useless name-list dump for an address + + +def test_not_found_wording_reports_true_total_not_capped_slice(): + """The reported count is the TRUE whole-program total (functions_total), not the length of the + returned (possibly capped) name slice — the fix for a large firmware reading as 'only 400 + functions'. A sample of the inventory is shown, marked '+N more' so it isn't read as the whole + set.""" + out = {"functions": ["f0", "f1", "f2"], "functions_total": 3812, "focus": None} + msg = _format_decomp(out, "function 'cgi_handler'") + assert "3812 defined functions" in msg # the true total, not 3 (the returned slice length) + assert "more" in msg # the sample is explicitly marked partial diff --git a/tests/test_list_functions.py b/tests/test_list_functions.py index 46165d6..9c7435b 100644 --- a/tests/test_list_functions.py +++ b/tests/test_list_functions.py @@ -92,6 +92,24 @@ def test_pagination_bounds_and_reports_next_offset(hg_home, monkeypatch): assert "sub_0009" not in out2 +def test_reports_true_total_when_inventory_capped(hg_home, monkeypatch): + """When the decompiler returns a bounded slice of a larger inventory (functions_total exceeds the + returned list), the tool reports the TRUE total — the fix for a large firmware reading as 'only + 400 functions' — and notes how many functions are beyond the returned inventory.""" + monkeypatch.setattr( + AT, "_decomp", + lambda ctx, function, **kw: {"functions": [f"sub_{i:04d}" for i in range(300)], + "functions_total": 8000}) + monkeypatch.setattr("hexgraph.engine.re.analysis.analysis_state", + lambda project, target: {"state": "analyzed", "detail": "(warm)"}) + with session_scope() as s: + ctx = _ctx(s) + out = run_tool(ctx, "list_functions", {}) + assert "8000 total" in out # the TRUE count, not 300 (the returned slice) + assert "8000 functions defined" in out # the capping note names the true inventory size + assert "7700 are beyond" in out # 8000 - 300 withheld from the returned inventory + + def test_limit_is_clamped_to_ceiling(hg_home, monkeypatch): """An over-large limit clamps to the 1000 ceiling and still says there's more.""" big = [f"fn_{i:05d}" for i in range(2500)]