diff options
| -rw-r--r-- | .auto/check_edit.py | 126 | ||||
| -rwxr-xr-x | .auto/checks.sh | 13 | ||||
| -rw-r--r-- | .auto/ideas.md | 20 | ||||
| -rw-r--r-- | .auto/log.jsonl | 1 | ||||
| -rw-r--r-- | idatui/domain.py | 74 | ||||
| -rw-r--r-- | idatui/edit_ctl.py | 10 |
6 files changed, 234 insertions, 10 deletions
diff --git a/.auto/check_edit.py b/.auto/check_edit.py new file mode 100644 index 0000000..2472c84 --- /dev/null +++ b/.auto/check_edit.py @@ -0,0 +1,126 @@ +#!/usr/bin/env python3 +"""Correctness gate for the listing after an item edit (run by .auto/checks.sh). + +An item edit (`c`/`d`/`u`/`p`) changes structure, but only locally: every head +in front of it keeps its address and its row number. So `Program.bump_items(ea)` +keeps the walk up to there instead of discarding the model — worth 257x on a big +binary (4.9s to make one byte into data, against 19ms). + +Keeping *anything* across a structural edit is the risky half of that, and it +fails silently: the pane shows rows that are no longer what the database says. +So this drives real edits and compares the kept model against one built from +scratch, row for row — narrow reads (what painting does) and wide ones (what +building the search body does). + +The staged database is a throwaway copy and is never saved, so the edits here do +not need undoing and can be as destructive as they like. + + ~/ida-venv/bin/python .auto/check_edit.py [targets/echo] +""" +from __future__ import annotations + +import os +import shutil +import sys + +HERE = os.path.dirname(os.path.abspath(__file__)) +ROOT = os.path.dirname(HERE) +sys.path.insert(0, ROOT) +sys.path.insert(0, HERE) + +from bench import stage # noqa: E402 +from idatui.domain import Program # noqa: E402 +from idatui.worker_client import WorkerClient # noqa: E402 + + +def snapshot(model, total: int, wide: bool): + if wide: + rows = [] + for base in range(0, total, 4096): + rows.extend(model.window(base, min(4096, total - base))) + else: + rows = [model.get(i) for i in range(total)] + return [(h.ea, h.kind, h.size, h.text, h.name) if h else None for h in rows] + + +def main() -> int: + target = sys.argv[1] if len(sys.argv) > 1 else "targets/echo" + d, path = stage(os.path.join(ROOT, target)) + fails: list[str] = [] + client = WorkerClient(path) + try: + prog = Program(client) + idx = prog.functions() + idx.load_all() + funcs = sorted(idx.all_loaded(), key=lambda f: f.addr) + if len(funcs) < 8: + print(f"{target}: too few functions to check") + return 1 + seg = funcs[len(funcs) // 2].addr + model = prog.listing(seg) + model.load_all() + total = len(model) + + # Edits at a spread of positions: near the end (where truncation keeps + # nearly everything), the middle, and early on (where it must give up). + spots = [funcs[int(len(funcs) * f)].addr for f in (0.9, 0.5, 0.05)] + for k, ea in enumerate(spots): + # Undefine, at three sizes. This is the edit that can coalesce + # BACKWARDS into the undefined run in front of it, which is the + # reason truncate_from drops two pages rather than one. + size = (1, 4, 16)[k % 3] + kind = f"undefine {size}B" + try: + prog.undefine(ea, size) + except Exception as e: # noqa: BLE001 + fails.append(f"{kind} at {ea:#x} failed: {e}") + continue + prog.bump_items(ea) + + kept = prog.listing(seg) + kept_total = None + got_wide = None + if kept is not None: + kept.load_all() + kept_total = len(kept) + got_wide = snapshot(kept, kept_total, wide=True) + got_narrow = snapshot(kept, kept_total, wide=False) + + # ... against a model that knows nothing about what came before. + prog._listings.clear() + fresh = prog.listing(seg) + fresh.load_all() + want = snapshot(fresh, len(fresh), wide=False) + + if kept_total != len(fresh): + fails.append(f"{kind} at {ea:#x}: kept model has {kept_total} " + f"rows, a rebuild has {len(fresh)}") + elif got_narrow != want or got_wide != want: + which = "narrow" if got_narrow != want else "wide" + bad = next((i for i, (a, b) in + enumerate(zip(got_narrow if which == "narrow" + else got_wide, want)) if a != b), None) + fails.append( + f"{kind} at {ea:#x}: {which} read differs from a rebuild at " + f"row {bad}: {(got_narrow if which == 'narrow' else got_wide)[bad]}" + f" vs {want[bad]}") + + model = prog.listing(seg) + model.load_all() + total = len(model) + + print(f"item edits: {len(spots)} edits checked against a rebuild " + f"({total} rows), {len(fails)} problems") + for f in fails: + print(" FAIL", f) + return 1 if fails else 0 + finally: + try: + client.close() + except Exception: # noqa: BLE001 + pass + shutil.rmtree(d, ignore_errors=True) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.auto/checks.sh b/.auto/checks.sh index d4120b0..4b9778a 100755 --- a/.auto/checks.sh +++ b/.auto/checks.sh @@ -9,7 +9,11 @@ # the old name, which nothing else notices. Checks a NARROW read (what # painting does) and a WIDE one (what building the search body does), and # the whole model against a rebuild. -# 3. tests/run.py -- the project's own front door: every suite, pure and IDA, +# 3. .auto/check_edit.py -- the listing after an ITEM edit. bump_items(ea) +# keeps the walk in front of the edit rather than discarding it; keeping +# anything across a structural change is the risky half and it fails +# silently, so the kept model is compared against a rebuild row for row. +# 4. tests/run.py -- the project's own front door: every suite, pure and IDA, # ~185s. It used to be only the scenario suite here, and that gap cost a # real regression: a faster worker connect left the loading overlay up a # moment longer relative to the index finishing, and project mode's first @@ -40,6 +44,13 @@ rn=$("$PY" .auto/check_rename.py targets/echo 2>&1) || { } echo "$rn" | tail -1 +ed=$("$PY" .auto/check_edit.py targets/echo 2>&1) || { + echo "--- the listing is wrong after an item edit ---" + echo "$ed" | tail -20 + exit 1 +} +echo "$ed" | tail -1 + out=$(python3 tests/run.py 2>&1) || true echo "$out" | tail -2 diff --git a/.auto/ideas.md b/.auto/ideas.md index 98804f0..e77a1fa 100644 --- a/.auto/ideas.md +++ b/.auto/ideas.md @@ -152,3 +152,23 @@ not trivial. Probe: `/tmp/traceprof.py`. differential against Tenet's own reference reader and it is currently skipped here, which leaves `tests/test_trace.py`'s 35 synthetic checks as the only guard on a subtle indexing change. + +## A bench phase for item edits hangs (attempted, reverted) + +`bump_items(ea)` keeping the listing's walk is worth 257x (4890ms → 19ms on +bash) but is **not visible in total_ms**, because the bench has no item-edit +phase. One was written and reverted: driving `undefine` from inside the pilot +hangs the run (no output, killed at the timeout), while the identical sequence +against `Program` directly is fine, and the same sequence with a `print` between +`prog.listing(ea)` and `ensure_ea` is also fine. + +That points at the app's background threads rather than at the edit: after +`bump_items` deletes the segment model, a `_prime`/`_grow` worker still in +flight can re-register a fresh one and hold its `_load_lock` — and this is the +same area as the unexplained streaming-responsiveness item above (a navigation +leaves extra `_grow` threads behind, and `@work(exclusive=True)` does not stop a +thread worker that is already running). **Understand that first**; a bench phase +built on top of it would be flaky. + +Until then the win is carried by `/tmp/itemedit.py` (direct measurement) and +`.auto/check_edit.py` (correctness, in the gate). diff --git a/.auto/log.jsonl b/.auto/log.jsonl index bb217ce..fea1e26 100644 --- a/.auto/log.jsonl +++ b/.auto/log.jsonl @@ -45,3 +45,4 @@ {"run":38,"commit":"df88ece","metric":27552.6,"metrics":{"lg_boot_ms":759.2,"lg_decomp_ms":2754.1,"lg_graph_ms":1207.2,"lg_hex_ms":448,"lg_index_ms":69.6,"lg_listing_cold_ms":434.5,"lg_listing_warm_ms":442.9,"lg_nav_ms":6642.3,"lg_palette_ms":4.7,"lg_rename_ms":730.6,"lg_render_ms":223.6,"lg_search_ms":5627.5,"lg_split_ms":2268,"pure_graph_ms":218.7,"sm_boot_ms":465,"sm_decomp_ms":1290.8,"sm_graph_ms":720.7,"sm_hex_ms":436.1,"sm_index_ms":2.4,"sm_listing_cold_ms":267.2,"sm_listing_warm_ms":266.5,"sm_nav_ms":292.9,"sm_palette_ms":0.3,"sm_rename_ms":380.1,"sm_render_ms":255.5,"sm_search_ms":68.1,"sm_split_ms":1276.3,"fails":0},"status":"keep","description":"Size the worker's per-line render cache to hold a segment's DISTINCT lines (16384 -> 65536, overridable with IDATUI_LINE_CACHE). This was a recorded dead end — it does nothing for a cold sweep — but the rename fix created a second-sweep workload, and re-rendering after a rename is now 21% cheaper. lg_search 7123 -> 5628.","timestamp":1786076605767,"segment":6,"confidence":8.431402690090582,"asi":{"hypothesis":"the biggest remaining term (lg_search 7.1s) is entirely the post-rename re-render, and a re-render is a SECOND sweep over the same lines -- which is exactly the case the line cache was measured to help and then filed as a dead end because nothing did it","why_it_stopped_being_a_dead_end":"experiment v1 #6 sized the cache at 16384 and noted that growing it 'does nothing for a cold sweep, only for a second sweep'. Nothing did a second sweep -- until v7 #2 made a rename keep the listing's walk and re-render its text on demand. The structural change created the workload the dead end was waiting for.","sizing_measurement":{"16384":[17.18,16.88,"+29MB"],"32768":[16.99,17.23,"+52MB"],"65536":[17.01,11.13,"+75MB"],"131072":[16.91,11.2,"+75MB"],"note":"bash .text, first sweep then second, in the worker"},"why_65536":"bash's .text is 228659 rows but only 53363 DISTINCT lines; 32768 still thrashes and 131072 buys nothing because the working set already fits. It is a bound, not a proportion -- a bigger binary fills it and stops, so the cost is capped at ~56MB whatever is open. IDATUI_LINE_CACHE lowers it for a pool of workers competing for memory.","direct_ab_on_the_real_path":"/tmp/refreshcost.py -- load the whole listing, rename, re-read every row through window(): 6445ms -> 5115ms (-21%), worker RSS 164MB -> 211MB. Initial load_all unchanged (5645 vs 5585).","gains":"total 28651 (previous best) -> 27553; lg_search 7123 -> 5628 (-21%); lg_split 2673 -> 2268","work_unchanged":"decomp_lines 3436, split_mapped 2070, search_hits 91783, graph_blocks 1000, rename_ok 6/6 -- all identical","verified":"830-check gate plus check_search and check_rename green; spans still byte-identical vs 2b0ae8d","lesson_recorded_in_prompt":"re-read the dead-end list after any structural change -- a rejected idea can become the right one when the workload around it moves","next_action_hint":"lg_nav 6642 is now the largest term again and is at the per-row floor. lg_search 5628 is still the post-rename re-render; the only way further down is to re-render fewer rows (see the xrefs-driven invalidation idea in .auto/ideas.md, which is risky because a name can reach a row without a direct xref)."}} {"run":39,"commit":"df88ece","metric":28376.5,"metrics":{"lg_boot_ms":762.1,"lg_decomp_ms":2374.3,"lg_graph_ms":938.1,"lg_hex_ms":452.4,"lg_index_ms":70.2,"lg_listing_cold_ms":419.6,"lg_listing_warm_ms":464.1,"lg_nav_ms":6941.9,"lg_palette_ms":5,"lg_rename_ms":1973.2,"lg_render_ms":221.9,"lg_search_ms":4117.3,"lg_split_ms":2333.3,"pure_graph_ms":214.6,"sm_boot_ms":453.7,"sm_decomp_ms":1270.5,"sm_graph_ms":726.9,"sm_hex_ms":440.6,"sm_index_ms":2.4,"sm_listing_cold_ms":283,"sm_listing_warm_ms":284.3,"sm_nav_ms":310.4,"sm_palette_ms":0.3,"sm_rename_ms":1575.9,"sm_render_ms":257.6,"sm_search_ms":59.6,"sm_split_ms":1423.7,"fails":0},"status":"checks_failed","description":"Digest mode for `heads`: the worker answers \"does this page still render exactly as it did?\" for the cost of the render alone, so a post-rename refresh skips shipping, unpickling and rebuilding pages that did not change. lg_search 5628 -> 4117 — but it BREAKS 'O' cycles back, and it made rename 2-4x slower. Both causes understood.","timestamp":1786077531326,"segment":6,"confidence":10.35869418588966,"asi":{"hypothesis_first_tested_empirically":"before building anything I checked the parked xrefs-driven idea: /tmp/whatchanges.py rebuilds the whole segment before and after a rename and diffs every row. On echo, all 19 changed rows over 4 renames were covered by (function extent + xrefs_to). On ls_ttl, 53 of 54 were -- the one that was not is 'lea rcx, unk_1D7A0' -> 'byte_1D7A0', which the RENAME DID NOT CAUSE: IDA's own analysis defined that byte. An address-predicted invalidation would leave that row stale for good, so the idea is now measured-and-rejected rather than assumed-risky.","what_i_built_instead":"an exact check: `heads(..., digest=True)` builds the rows as usual but returns only hash+count instead of the rows. The client stores the digest each page came back with and asks 'still the same?' before re-fetching. Uses the interpreter's own hash deliberately -- it never has to mean anything outside the worker process, the client is only a courier.","measured_win":"post-rename whole-segment re-read 5115ms -> 3833ms (-25%); lg_search 5628 -> 4117","BUG_1_correctness":"the stored digest describes the page AS LOADED, not as the client currently holds it. After a full refetch the client's rows change but _page_digest is not updated -- so when a literal format cycles hex -> dec -> ... -> hex, the worker's digest matches the ORIGINAL stored one, the page is declared unchanged, and the row keeps the intermediate decimal text. That is exactly the failure: opfmt_listing ''O' cycles back' got 'sub rsp, 184' wanting 'sub rsp, 0B8h'.","BUG_2_performance":"_ensure_text_from tests page freshness with all(_head_gen[k] == gen for k in the page), and get() calls _ensure_text per ROW -- so every row scanned a whole page's gen array. rename went 380 -> 1576ms (sm) and 730 -> 1973ms (lg).","fix_for_both":"refresh at PAGE granularity end to end instead of the snapped TEXT_BLOCK: a page is exactly what the tool produced from (addr, count=PAGE), so refetching with the same parameters reproduces the same sequence with no snapping, the stored digest can be updated whenever the page's rows are replaced, and every head in a page shares one gen value so freshness is a single probe rather than a scan.","work_preserved":".auto/wip-digest.patch","gate_worked":"check_rename and check_search both passed -- neither exercises a format cycle. The scenario suite caught it. That is the third time a cache-shaped change failed in a way only one specific test could see."}} {"run":40,"commit":"d9e8fdb","metric":26491.7,"metrics":{"lg_boot_ms":808.6,"lg_decomp_ms":2600.2,"lg_graph_ms":901.2,"lg_hex_ms":448.7,"lg_index_ms":70.3,"lg_listing_cold_ms":432.8,"lg_listing_warm_ms":445.5,"lg_nav_ms":7004.8,"lg_palette_ms":4.8,"lg_rename_ms":758.6,"lg_render_ms":231.3,"lg_search_ms":3959.2,"lg_split_ms":2659.2,"pure_graph_ms":214.7,"sm_boot_ms":432.6,"sm_decomp_ms":1305.1,"sm_graph_ms":758.3,"sm_hex_ms":431.8,"sm_index_ms":2.3,"sm_listing_cold_ms":275.4,"sm_listing_warm_ms":289.4,"sm_nav_ms":295.6,"sm_palette_ms":0.3,"sm_rename_ms":424.3,"sm_render_ms":263.6,"sm_search_ms":61.1,"sm_split_ms":1411.9,"fails":0},"status":"keep","description":"heads(digest=True): the worker answers \"does this page still render exactly as you hold it?\" with a hash and a count instead of the page. After a rename nearly every page is unchanged, so the pickling, transfer, unpickling and Head rebuild are all skipped. Redone at PAGE granularity end to end, which fixes both bugs of the first attempt. lg_search 5628 -> 3959.","timestamp":1786077952004,"segment":6,"confidence":8.431402690090582,"asi":{"gains":"total 27553 (previous best) -> 26492 (-3.9%); lg_search 5628 -> 3959 (-30%); post-rename whole-segment re-read 5115 -> 3720ms measured directly","cost":"lg_rename 731 -> 759 and sm_rename 380 -> 424: a page now costs one extra round trip to probe before it is either accepted or refetched. Getting a viewport back after a rename on bash is 21.5ms instead of 10.3ms -- still 230x better than the 4941ms rebuild it replaced.","what_the_two_bugs_were":{"correctness":"the first attempt stored the digest a page LOADED with and never updated it when the page was refetched. A literal format cycling hex -> dec -> hex then hashed back to the original while the client still held the decimal text: opfmt_listing ''O' cycles back' got 'sub rsp, 184' wanting '0B8h'.","performance":"freshness was tested with all(_head_gen[k] == gen for k in the page) while get() called _ensure_text per ROW, so every row scanned a whole page's gen array -- rename went 2-4x slower."},"the_fix_was_one_idea":"refresh at PAGE granularity instead of the snapped TEXT_BLOCK. A page is exactly what heads(addr, count=PAGE) produced, so re-asking with the same arguments reproduces the same sequence -- no snapping to address groups needed, the stored digest can be updated whenever the rows are replaced, and every head in a page shares one generation marker so freshness is a single probe. TEXT_BLOCK is gone.","why_hash_and_not_a_stable_digest":"the value never has to mean anything outside the worker process -- the client stores what a page hashed to and hands the same number back. One worker, one process, one hash seed. It covers ea/kind/size/text/name AND the colour spans, so two lines that collapse to the same text but colour differently are not confused.","empirical_work_that_shaped_this":"before building anything I tested the parked xrefs-driven idea with /tmp/whatchanges.py (rebuild the segment before and after a rename, diff every row). It fails: on ls_ttl one changed row was 'lea rcx, unk_1D7A0' -> 'byte_1D7A0', which the rename did not cause -- IDA's own analysis defined that byte. Address-predicted invalidation would leave it stale forever. Recorded as measured-and-rejected in .auto/ideas.md.","work_unchanged":"decomp_lines 3436, split_mapped 2070, search_hits 91783, graph_blocks 1000, rename_ok 6/6, decomp_ok 12 -- identical","verified":"830-check gate, check_search (140 prefixes), check_rename on echo AND ls_ttl (5952 and 28807 rows, wide and narrow, against a rebuild), and the 19 opfmt scenarios that caught the first attempt","next_action_hint":"lg_nav 7005 is the largest term and is the cold walk at its per-row floor. lg_search 3959 is now mostly the _line_plain pass plus the pages that genuinely changed. Next best unexplored: why the digest probe costs a whole extra round trip per page -- it could ride along with the first refetch request rather than preceding it."}} +{"run":41,"commit":"7e4f086","metric":25814,"metrics":{"lg_boot_ms":714.9,"lg_decomp_ms":2376.2,"lg_graph_ms":948.8,"lg_hex_ms":458.6,"lg_index_ms":69.3,"lg_listing_cold_ms":448.5,"lg_listing_warm_ms":482.1,"lg_nav_ms":6808,"lg_palette_ms":4.7,"lg_rename_ms":752,"lg_render_ms":219.4,"lg_search_ms":4008.3,"lg_split_ms":2275.5,"pure_graph_ms":225.2,"sm_boot_ms":456.1,"sm_decomp_ms":1292.4,"sm_graph_ms":795.8,"sm_hex_ms":475.2,"sm_index_ms":2.5,"sm_listing_cold_ms":269.1,"sm_listing_warm_ms":272.2,"sm_nav_ms":320.2,"sm_palette_ms":0.3,"sm_rename_ms":446.3,"sm_render_ms":264.8,"sm_search_ms":60.6,"sm_split_ms":1366.9,"fails":0},"status":"keep","description":"Three redundancies in the heads walk: item flags were fetched three times per head (row builder, _is_unknown via _advance, and _rows_for), get_func was called per head where a head is nearly always in the same function as the one before it, and the page digest rebuilt a tuple-of-tuples per row where one spans list is shared by ~45% of them. Cold heads 18.62 -> 17.77 us/row, warm 11.53 -> 10.87.","timestamp":1786078727527,"segment":6,"confidence":7.765877831715214,"asi":{"gains":"total 26492 (previous best) -> 25814 (-2.6%); lg_split 2659 -> 2276; sm_nav_worst 158 -> 151. Microbenchmarked rather than trusted to total_ms, since ~0.8us/row over ~500k row-renders is 0.4s and the box's spread is ~500ms: cold heads 18.62 -> 17.77 us/row, warm 11.53 -> 10.87, digest 0.75 -> 0.62.","equivalence":"/tmp/headsdump.py runs the tool under both code versions in separate processes and compares whole payloads: 109 004 rows over 294 payloads (bash + ls_ttl, BOTH annotate modes, plus the offset/end/back/count variants that share the walk) -- 0 mismatches. diff_spans still byte-identical vs 2b0ae8d on three binaries. 830-check gate, check_search and check_rename green.","get_func_cache_was_validated_not_assumed":"I had rejected this earlier on a hunch about IDA function CHUNKS -- a tail chunk of B inside A's [start_ea, end_ea) would be misattributed. /tmp/funccache.py walks every head of bash/ls_ttl/echo comparing the cached answer against get_func: 437 324 heads, 0 disagreements. Hit rate is only ~50% across all segments (a head outside any function cannot be cached by range) but much higher inside .text, which is what the listing walks.","TWO_SCHEMES_MEASURED_AND_REJECTED_FIRST":"before micro-optimising I tested the two big parked ideas, both of which would have made a rename nearly free. Listing rows predicted from xrefs_to + function extent: ls_ttl had 1 of 54 changed rows uncovered, 'lea rcx, unk_1D7A0' -> 'byte_1D7A0', which the rename did not cause -- IDA's own analysis defined that byte. Decompilations predicted from 'the old name appears in the cached text': 16 misses over 4 renames, EVERY one Hex-Rays type inference moving (unsigned int a4 -> int a4) in functions unrelated to the rename. Both recorded in .auto/ideas.md.","generalised_lesson":"predicting the effect of an edit on a database that has its own opinions is unsound. Verify instead -- which is exactly why heads(digest=True) works: it asks what a row renders as NOW, not what should have changed. This probably also explains the lg_decomp_lines drift blamed on CPU starvation in v5 #4.","things_checked_and_found_not_worth_it":{"digest_on_the_normal_load_path":"suspected a regression from v7 #10; it is 0.62-0.75 us/row, not the 1.9 the noisy call timing suggested","folding_the_opcode_byte_read_into_heads":"the extra read_bytes round trip is 0.35 us/row, ~81ms over a whole bash segment","pipelining_client_parsing_with_worker_compute":"blocked -- cursor.next is inside the pickle, so the next request cannot be issued before unpickling, and _build_page itself needs the socket for opcode bytes"},"state":"generate_disasm_line is now 6.09 of the 10.87 us/row warm cost (56%) and is IDA's. What is left of ours is ~4.8 us/row spread thin across a dozen places.","next_action_hint":"lg_nav 6808 (26%) and lg_search 4008 (16%) are both at their per-row floors now. The unexplored areas are the ones .auto/ideas.md lists as blocked or unmeasured: the trace memory index (needs test_trace_vs_tenet runnable), the streaming-responsiveness question, and the features the bench still does not drive (xrefs dialog, strings browser, struct editor, make-code edits, history, traces, RPC)."}} diff --git a/idatui/domain.py b/idatui/domain.py index f4aa63c..2cfd9d9 100644 --- a/idatui/domain.py +++ b/idatui/domain.py @@ -906,6 +906,51 @@ class ListingModel: def __len__(self) -> int: return self.loaded() + def truncate_from(self, ea: int) -> bool: + """Drop the walk from the page an edit at ``ea`` could have moved. + + An item edit changes structure, but only *locally*: every head before it + keeps its address and its row number. Throwing the whole model away made + the reload re-walk the segment -- 4.9 seconds on bash to make one byte + into data, for an edit the user made at the row they were looking at. + + Two pages are dropped rather than one, because undefining can coalesce + backwards into the run in front of it. Beyond that the caller marks the + kept prefix text-stale, so every kept page is digest-checked on the next + read and a page that really did move fails its sequence check and forces + a rebuild. Safe by construction, not by argument. + + Returns False if nothing worth keeping is left. + """ + with self._lock: + if not (self.seg_start <= ea < self.seg_end): + return True # another segment; nothing moved here + if len(self._page_head) < 3: + return False # barely walked; a rebuild is cheaper + p = bisect.bisect_right(self._page_addr, ea) - 1 + p = max(p - 1, 0) + if p <= 0: + return False # the edit is in the first pages + keep = self._page_head[p] + if keep <= 0: + return False + for h in self._heads[keep:]: + self._by_ea.pop(h.ea, None) + del self._heads[keep:] + del self._head_eas[keep:] + del self._head_gen[keep:] + del self._row_at[keep:] + del self._page_head[p:] + self._next = self._page_addr[p] + del self._page_addr[p:] + del self._page_digest[p:] + del self._page_rows[p:] + last = self._heads[-1] + self._rows = self._row_at[-1] + self._span(last) + self._done = False + self._ubytes.clear() # undefined-run bytes behind the drop point + return True + def invalidate_text(self) -> None: """A rename changed how rows READ, not which rows exist. @@ -1589,22 +1634,41 @@ class Program: for lm in listings: lm.invalidate_text() - def bump_items(self) -> None: + def bump_items(self, ea: int | None = None) -> None: """Signal that item/function STRUCTURE changed (define code/data/func, undefine). Unlike a rename this can move instruction boundaries and - change function membership anywhere, so drop the disasm block caches, - the decompilation cache and the cached function indices outright, and - bump the name generation too (labels/names may appear or vanish).""" + change function membership, so drop the disasm block caches, the + decompilation cache and the cached function indices outright, and bump + the name generation too (labels/names may appear or vanish). + + Given the address that was edited, the segment listing keeps the walk in + front of it instead of being thrown away: the rows before an edit keep + their addresses and their row numbers. Without ``ea`` this falls back to + discarding the listings, as it always did. + """ with self._lock: self._name_gen += 1 self._indices.clear() self._decomp.clear() - self._listings.clear() self._pc_nums.clear() models = list(self._disasm.values()) self._disasm.clear() + listings = list(self._listings.items()) + if ea is None: + self._listings.clear() for m in models: m.invalidate() + if ea is None: + return + for start, lm in listings: + if lm.truncate_from(ea): + # Names can move too; the kept prefix is re-rendered on demand, + # and that is also what catches a page the edit really did move. + lm.invalidate_text() + else: + with self._lock: + if self._listings.get(start) is lm: + del self._listings[start] # -- item / function structure edits (IDA c/d/u/p) --------------------- # @staticmethod diff --git a/idatui/edit_ctl.py b/idatui/edit_ctl.py index 54f4d89..435fab9 100644 --- a/idatui/edit_ctl.py +++ b/idatui/edit_ctl.py @@ -288,7 +288,7 @@ class EditController: app.call_from_thread(app._status, err) return # The label shows in the listing's head rows -> invalidate + reopen. - app.program.bump_items() + app.program.bump_items(addr) # Naming the address *of a function start* is a function rename by any # other name. Without this the cached index kept the old name, so # `functions`/`names`/resolve/the palette all reported the rename had @@ -510,7 +510,7 @@ class EditController: diag.note(f"make_data({ea:#x}, {type_decl!r})", e) app.call_from_thread(app._status, f"make data: {e}") return - app.program.bump_items() + app.program.bump_items(ea) anchor = anchor or _M().ViewAnchor() anchor.flash = f"data ({type_decl}) @ {ea:#x} (Ctrl+S to save)" name = app.program.region_label(ea) @@ -707,8 +707,10 @@ class EditController: diag.note(f"edit_item({kind}, {ea:#x})", e) app.call_from_thread(app._status, f"{kind}: {e}") return - # Structure changed everywhere: drop all item/function/decomp caches. - app.program.bump_items() + # Structure changed: drop all item/function/decomp caches. The segment + # listing keeps its walk in front of `ea` -- rows before an edit keep + # their addresses and their row numbers. + app.program.bump_items(ea) # Re-resolve: a define_func upgrades the region to a real function view; # anything else re-reads the (still function-less) listing in place. anchor = anchor or _M().ViewAnchor() |