diff options
| -rw-r--r-- | idatui/app.py | 88 | ||||
| -rw-r--r-- | idatui/domain.py | 14 | ||||
| -rw-r--r-- | server/patch_server.py | 41 | ||||
| -rw-r--r-- | tests/test_blob_ui.py | 33 | ||||
| -rw-r--r-- | tests/test_thumb_ui.py | 20 |
5 files changed, 180 insertions, 16 deletions
diff --git a/idatui/app.py b/idatui/app.py index ec55911..1ea630e 100644 --- a/idatui/app.py +++ b/idatui/app.py @@ -149,6 +149,8 @@ class ViewAnchor: top_ea: int | None = None # first visible address cursor_x: int = 0 flash: str | None = None + #: The edit changed which functions exist, so the index must be rebuilt. + refresh_functions: bool = False @dataclass @@ -3253,8 +3255,9 @@ class IdaTui(App): n = len(self._func_index) if self._func_index else 0 note = ("this image has no functions, so nothing is lost" if n == 0 else - f"discards the database for this binary \u2014 {n} functions, " - f"plus any names and comments you've added") + f"discards the database for this binary \u2014 {n} " + f"function{'s' if n != 1 else ''}, plus any names and comments " + f"you've added") self.push_screen(ConfirmScreen("Reload with different options?", note), self._on_reload_confirmed) @@ -3419,6 +3422,11 @@ class IdaTui(App): # An image with no functions at all is nearly always a blob described # wrongly, and that stays true as you scroll around — so it belongs in # the status bar, not in a one-off message the next write clobbers. + # It stops being true the moment a function exists, though: latching it + # meant the warning survived defining one with `p` and kept telling you + # the load was wrong when it no longer was. + if self._no_functions and self._func_index is not None and len(self._func_index): + self._no_functions = False if self._no_functions: text += " \u2014 no functions: wrong processor/base? Ctrl+L to reload" try: @@ -5202,6 +5210,7 @@ class IdaTui(App): # anchor restore, same flash. That is the whole point of having # one path. elif kind == "func": + anchor.refresh_functions = True r = self.program.define_func(ea) if r.get("start") and r.get("end"): verb = (f"created function {r['start']}\u2013{r['end']}" @@ -5211,6 +5220,8 @@ class IdaTui(App): s = self.program.make_string(ea) verb = f"made string ({s[:24]!r})" if s else verb else: + # Undefining can destroy a function as easily as `p` creates one. + anchor.refresh_functions = True self.program.undefine(ea) except Exception as e: # noqa: BLE001 -- surface soft/hard tool errors self.app.call_from_thread(self._status, f"{kind}: {e}") @@ -5248,7 +5259,35 @@ class IdaTui(App): self._dirty = True self._flash = anchor.flash if anchor.flash: - self._status(anchor.flash) + self._status(anchor.flash) # and again from the reload, via _flash + if anchor.refresh_functions: + # Creating (or destroying) a function changes the index that the + # names pane, Ctrl+N and the "no functions" hint all read. Without + # this, `p` gave you a function the rest of the app couldn't see. + self._reindex_functions() + + @work(thread=True, exclusive=True, group="load-funcs") + def _reindex_functions(self) -> None: + """Rebuild the function index in place after an edit changed it. + + Deliberately not _load_functions(): that one is the BOOT path — it + clears the table, streams progress and then auto-lands, which would + yank the view away from the function you just made. + """ + if self.program is None: + return + idx = self.program.functions() + idx.load_all() + self._func_index = idx + self.app.call_from_thread(self._after_reindex) + + def _after_reindex(self) -> None: + idx = self._func_index + if idx is None: + return + if len(idx): + self._no_functions = False + self._apply_filter(self._filter_term) # repopulate the names pane @work(thread=True, exclusive=True, group="save") def _save(self) -> None: @@ -5526,6 +5565,8 @@ class IdaTui(App): view.focus() def on_key(self, event) -> None: # type: ignore[no-untyped-def] + # Any keypress means the result of the last edit has been read. + self._flash = None if event.key != "escape": return if self.query_one("#search", Input).display: @@ -5895,9 +5936,12 @@ class IdaTui(App): self._goto_ea(msg.va, push=True) def _status_for_cur(self, mode: str) -> None: - flash, self._flash = self._flash, None - if flash: - self._status(flash) # the edit that caused this reload wins + if self._flash: + # Shown, NOT consumed. A reload emits several of these (prime, then + # cursor), so consuming on the first one meant the second erased the + # message the user was meant to read. It clears on the next keypress + # instead — i.e. when they've moved on. + self._status(self._flash) return if self._cur is not None: self._status(f"{self._cur.name} @ {self._cur.ea:#x} [{mode}]") @@ -5906,12 +5950,22 @@ class IdaTui(App): def _load_decomp(self, ea: int, name: str) -> None: assert self.program is not None dec = self.program.decompile(ea) - self.app.call_from_thread(self._apply_decomp, ea, name, dec) + why = "" + if dec.failed: + # Ask Hex-Rays why, in the same worker: the plain tool reports + # "Decompilation failed at 0x0" and drops the only useful part. + # "Decompile failed" with no reason is indistinguishable from a bug + # in this app, and for the common cause (a 32-bit function in a + # 64-bit database) the user cannot even guess the fix. + why = self.program.decomp_error(ea) + self.app.call_from_thread(self._apply_decomp, ea, name, dec, why) - def _apply_decomp(self, ea: int, name: str, dec) -> None: # type: ignore[no-untyped-def] + def _apply_decomp(self, ea: int, name: str, dec, # type: ignore[no-untyped-def] + why: str = "") -> None: view = self.query_one(DecompView) view.loading = False if dec.failed: + detail = f" \u2014 {why}" if why else "" # No pseudocode for this function: fall back to the code view rather # than an error panel. If we came from the continuous listing (F5), # return there; otherwise show the disassembly. @@ -5922,13 +5976,20 @@ class IdaTui(App): self._decomp_return = None self._cur = ret self._active = "listing" + # Hand the reason over as a flash BEFORE reopening: going back + # to the listing reloads it, and the reload writes its own + # status afterwards — which is precisely how "F5 does nothing" + # looked like nothing at all. + msg = f"{name}: cannot decompile{detail}" + self._flash = msg self._open_entry(ret, push=False) - self._status(f"{name} — decompile failed; back to the listing") + self._status(msg) return self._active = "disasm" + msg = f"{name}: cannot decompile{detail}" + self._flash = msg self._show_active() - self._status( - f"{name} — no pseudocode (decompile failed); showing disassembly") + self._status(msg) return if self._active == "decomp": view.focus() # loading cover had blurred it; restore focus @@ -6138,9 +6199,8 @@ class IdaTui(App): # A pending flash (the result of an edit that caused this reload) # outranks the idle hint: the cursor lands here as part of the # reload, so this handler would otherwise always have the last word. - flash, self._flash = self._flash, None - if flash: - self._status(flash) + if self._flash: + self._status(self._flash) return sec = self.program.section_of(ea) if self.program else None self._status(f"{sec or '?'} @ {ea:#x} [listing] " diff --git a/idatui/domain.py b/idatui/domain.py index 9047d82..a92ea49 100644 --- a/idatui/domain.py +++ b/idatui/domain.py @@ -1373,6 +1373,20 @@ class Program: if res.get("error"): raise IDAToolError("define_code", f"@ {ea:#x}: {res['error']}") + def decomp_error(self, ea: int) -> str: + """Hex-Rays' own reason for refusing ``ea``, or "" if it won't say.""" + try: + r = self.client.call("decomp_error", addr=hex(ea)) + except IDAToolError: + return "" + if not isinstance(r, dict): + return "" + reason = str(r.get("reason") or "") + if reason and r.get("bitness") == 64 and "64-bit" in reason: + # Unfixable in place: the database's bitness is decided at load. + reason += " \u2014 Ctrl+L and pick arm:ARMv7-A" + return reason + def set_thumb(self, ea: int, mode: str = "toggle") -> dict: """Switch ARM/Thumb decoding at ``ea``. Returns the resulting state.""" r = self.client.call("set_thumb", addr=hex(ea), mode=mode) diff --git a/server/patch_server.py b/server/patch_server.py index ee7600a..2de852f 100644 --- a/server/patch_server.py +++ b/server/patch_server.py @@ -1083,6 +1083,47 @@ def define_func_run( f = idaapi.get_func(ea) return {"addr": hex(ea), "ok": True, "start": hex(f.start_ea), "end": hex(f.end_ea), "how": "explicit-end"} + +@tool +@idasync +def decomp_error( + addr: Annotated[str, "Address of the function that failed to decompile"], +) -> dict: + """Why Hex-Rays refused this function, in its own words. + + The plain decompile tool reports "Decompilation failed at 0x0" and drops the + reason, which is the only useful part. Hex-Rays fills in a hexrays_failure_t + saying things like "only 64-bit functions can be decompiled in the current + database" — that one is unfixable in place (the database's bitness is set at + load), so a user who can't see it has no way to know they must reload.""" + import ida_funcs + import ida_hexrays + import ida_ida + + try: + ea = parse_address(addr) + except Exception as e: + return {"addr": str(addr), "error": str(e)} + out = {"addr": hex(ea), "bitness": ida_ida.inf_get_app_bitness()} + fn = ida_funcs.get_func(ea) + if fn is None: + out["reason"] = "no function here" + return out + try: + if not ida_hexrays.init_hexrays_plugin(): + out["reason"] = "the decompiler is not available for this processor" + return out + hf = ida_hexrays.hexrays_failure_t() + cf = ida_hexrays.decompile_func(fn, hf) + if cf is not None: + out["reason"] = "" # it decompiles now + return out + out["reason"] = hf.desc() or f"error {hf.code}" + out["code"] = int(hf.code) + out["errea"] = hex(hf.errea) + except Exception as e: # noqa: BLE001 + out["reason"] = f"{type(e).__name__}: {e}" + return out ''' SNIPPET = f"{BEGIN}\n{BODY.strip()}\n{END}\n" diff --git a/tests/test_blob_ui.py b/tests/test_blob_ui.py index f930401..b30bba5 100644 --- a/tests/test_blob_ui.py +++ b/tests/test_blob_ui.py @@ -214,14 +214,43 @@ async def run() -> int: lst._cursor_ea() == cur_before, f"{cur_before:#x} -> {lst._cursor_ea():#x}") + # -- `p` after carving: the rest of the app must notice ------ # + # The "no functions" hint was latched at load and only cleared on a + # reload, so it kept telling you the processor/base were wrong long + # after you'd defined a function. The function index was never + # rebuilt either, which meant Ctrl+N couldn't find what `p` made. + check("no functions yet, and the hint says so", + len(app._func_index) == 0 + and "no functions" in str(app.query_one("#status", Static).render()), + f"n={len(app._func_index)}") + lst.cursor = lst.model.index_of_ea(target) + lst._scroll_cursor_into_view() + await pilot.pause(0.3) + mp = lst.model + await pilot.press("p") + await wait(lambda: lst.model is not mp and lst.model is not None, + pilot, 40) + await wait(lambda: app._func_index is not None + and len(app._func_index) > 0, pilot, 60) + check("`p` creates a function the index can see", + len(app._func_index) == 1, f"n={len(app._func_index)}") + status = str(app.query_one("#status", Static).render()) + check("and the stale 'no functions' hint is gone", + "no functions" not in status, status[:90]) + check("the status names the function it made", + "created function" in status, status[:90]) + await pilot.press("ctrl+l") opened = await wait(lambda: isinstance(app.screen, ConfirmScreen), pilot, 20) check("Ctrl+L offers to reload with different options", opened, f"screen={type(app.screen).__name__}") if opened: note = str(app.screen.query_one("#confirm-note", Static).render()) - check("and says nothing is lost when there's nothing to lose", - "nothing is lost" in note, note[:70]) + # We just made a function, so it must NOT claim nothing is lost — + # reloading throws the database away and that is now a real cost. + check("the confirmation counts what would be lost", + "1 function," in note and "nothing is lost" not in note, + note[:80]) await pilot.press("escape") await pilot.pause(0.3) check("declining leaves the binary open", diff --git a/tests/test_thumb_ui.py b/tests/test_thumb_ui.py index 090dc2a..5b6f18e 100644 --- a/tests/test_thumb_ui.py +++ b/tests/test_thumb_ui.py @@ -146,6 +146,26 @@ async def run() -> int: "64-bit" in status and "decompile" in status, status[:120]) check("and names the fix", "ARMv7-A" in status, status[:120]) + # And if you ignore that and carry on, the failure has to say WHY. The + # plain decompile tool reports "Decompilation failed at 0x0" and drops + # the reason, which is the only part that tells you what to do — with it + # missing, F5 doing nothing is indistinguishable from a bug in the TUI. + lst.cursor = lst.model.index_of_ea(0) + await pilot.pause(0.2) + mp = lst.model + await pilot.press("p") + await wait(lambda: lst.model is not mp and lst.model is not None, pilot, 60) + await wait(lambda: app._func_index is not None + and len(app._func_index) > 0, pilot, 60) + await pilot.press("tab") + await wait(lambda: "cannot decompile" in + str(app.query_one("#status", Static).render()), pilot, 90) + status = str(app.query_one("#status", Static).render()) + check("a failed decompile says why, in Hex-Rays' own words", + "only 64-bit functions" in status, status[:130]) + check("and the reason survives the view reloading under it", + "cannot decompile" in status, status[:130]) + # -- the whole point: a 32-bit database decompiles ---------------------- # for ext in (".i64", ".id0", ".id1", ".id2", ".nam", ".til"): try: |