diff options
| -rw-r--r-- | TODO | 41 | ||||
| -rw-r--r-- | idatui/app.py | 25 | ||||
| -rw-r--r-- | tests/test_trace_ui.py | 21 |
3 files changed, 63 insertions, 24 deletions
@@ -115,29 +115,24 @@ cursor's function), the trace one on the decompiler's loaded function. That is how they ended up describing different functions. There is now one index, keyed to what the decompiler HOLDS, and both read it. -## A stray navigation to the entry point during trace seeking +## Stale navigations (fixed for seeks; general case left alone) -Seen while building M3. After a trace seek, a SECOND _open_at arrives from a -worker callback and re-points the view at the entry function: +A navigation runs in a worker and its result is applied when it lands. The trace's +OPENING seek goes to t=0, which for a normal binary is _start, and that +navigation is slow — so it used to arrive after later seeks and drag the cursor +back to _start while the trace was elsewhere. It never settled (measured stable +for 3+ seconds), and anything cursor-based done just after a seek then acted on +the wrong address. - ('_goto_ea', ['0x3160']) <- the seek's own navigation - ('_open_at', ['0x3160', 'main']) <- correct - ('_open_at', ['0x34d0', 'start']) <- stray, and it wins +The listing path had no staleness guard at all; the decompiler path got one in +756589a. Fixed by giving the listing completion the same check +(_open_at_if_current) and bumping _nav_seq on each SEEK. -The cursor and _cur then sit on 'start' while the trace's pc is elsewhere, and it -does not settle — measured stable for 3+ seconds. Consequence: a cursor-based -action taken right after a seek (`>` seeks on the address under the cursor) acts -on the wrong address. - -Not _auto_land: 'start' is not in ENTRY_NAMES, and its guard was set. The -traceback only shows the Textual callback frame, so the origin is a -call_from_thread from some worker — most likely a navigation queued earlier -arriving late, which is the same shape as the stale-result bug fixed in 756589a -for _open_decomp_entry (that one got a sequence guard; the listing path did not). - -To reproduce: seek to an address executed twice, wait for the cursor to reach it, -then seek again and watch _cur. - -Workaround in place: nothing. The M3 tests place the cursor explicitly rather -than relying on where a seek left it, which is what a user does anyway (you point -at a line and ask about it) — but the drift is real and worth fixing. +Deliberately NOT bumped in _goto_ea for every navigation. That is the more +general rule — "the last thing you asked for wins" — and I tried it, but it also +means an ordinary follow can be dropped by whatever navigates next, and a full +suite run turned up a follow_xrefs failure with it in place (the same check has +flaked before, so it is not proof, but the mechanism is real and the evidence I +have is only about seeks). If rapid follows ever show the same drift, the general +bump is the fix — with a test that a follow in flight survives an unrelated +navigation. diff --git a/idatui/app.py b/idatui/app.py index 55f21ef..2f19ffe 100644 --- a/idatui/app.py +++ b/idatui/app.py @@ -5654,6 +5654,16 @@ class IdaTui(App): t = self._trace if t is None or not t.length: return + # A seek invalidates any navigation still in flight. They run in workers + # and finish out of order: the trace's opening seek lands on the entry + # point, takes a while, and used to arrive AFTER later seeks — dragging + # the cursor back to _start while the trace was elsewhere, permanently. + # + # Bumped HERE and not in _goto_ea. Doing it for every navigation is the + # more general rule ("the last thing you asked for wins") but it also + # lets an ordinary follow be dropped by whatever navigates next, and the + # only evidence I have is about seeks. Narrow fix for the measured bug. + self._nav_seq += 1 self._t = max(0, min(int(idx), t.length - 1)) self.query_one(TraceDock).show(t, self._t) self._paint_trail() @@ -6029,7 +6039,8 @@ class IdaTui(App): idx = max(lm.ensure_ea(ea), 0) if lm is not None else 0 name = fn.name if fn is not None else self.program.region_label(ea) self.app.call_from_thread( - self._open_at, ea, name, idx, push, -1, 0, fn is None, focus_name) + self._open_at_if_current, seq, ea, name, idx, push, fn is None, + focus_name) def _open_decomp_entry(self, fn_addr: int, fn_name: str, dec_idx: int, dec_cursor_x: int, push: bool, @@ -6202,6 +6213,18 @@ class IdaTui(App): cur = row_of(fallback_ea) return (cur, row_of(a.top_ea)) + def _open_at_if_current(self, seq: int, ea: int, name: str, cursor: int, + push: bool, is_region: bool, + focus_name: str | None) -> None: + """Apply a navigation result only if it's still the one being awaited. + + The decompiler path has had this since 756589a; the listing path hadn't, + so a slow navigation could still land on top of a newer one. + """ + if seq != self._nav_seq: + return + self._open_at(ea, name, cursor, push, -1, 0, is_region, focus_name) + def _open_at(self, ea: int, name: str, cursor: int, push: bool, dec_cursor: int = -1, dec_cursor_x: int = 0, is_region: bool = False, focus_name: str | None = None, diff --git a/tests/test_trace_ui.py b/tests/test_trace_ui.py index b08ca02..32977ad 100644 --- a/tests/test_trace_ui.py +++ b/tests/test_trace_ui.py @@ -286,6 +286,27 @@ async def run() -> int: mapped > 3 and missed == 0, f"{mapped} mapped, {missed} not followed") + # -- a late navigation must not drag the view back --------------- # + # Navigations run in workers and finish out of order. The trace's + # OPENING seek goes to the entry point (t=0 is _start), takes a + # while, and used to arrive after later seeks — leaving the cursor + # on _start while the trace was elsewhere, and it never settled. + # Anything cursor-based done right after a seek then acted on the + # wrong address. + two = [a for a in t.by_ip if len(t.by_ip[a]) > 1] + if two: + a = max(two, key=lambda x: len(t.by_ip[x])) + s0, s1 = list(t.by_ip[a])[:2] + dbaddr = a + t.slide + app._seek(s0) + await wait(lambda: lst._cursor_ea() == dbaddr, pilot, 60) + app._seek(s1) + await pilot.pause(3.0) # long enough for a stale one to land + check("a stale navigation doesn't drag the cursor away", + lst._cursor_ea() == dbaddr and app._cur.ea == dbaddr, + f"cursor={lst._cursor_ea():#x} cur={app._cur.ea:#x} " + f"want {dbaddr:#x}") + # -- seeking, as opposed to stepping ---------------------------- # # "When else did this instruction run?" — the question that makes a # trace more than a very long single-step log. |