Remove legacy cluster walker

TODO.md

@@ -23,8 +23,10 @@ rank; each item states the concrete failure it causes.
 | `cd6b655` | Discovery hardening: removed the **bogus instruction-blob classification** (only data snapshots carry magic `0xf5f5dcdc`; a stray hit could poison `iso_instr`); symbol-derived addresses are no longer clobbered by scan guesses; `vaddr==0` duplicates can't erase a good address; magic hits deduped; Mach-O segments skipped. |
 | `0716a95` | **Structural instruction-image location** for symbol-less Mach-O: `vm_instr` = first executable section, `iso_instr = align_up(vm_instr + ImageSize, 64)` from the Dart Image header, with cross-checks that fall back to `0` rather than emit junk. Verified on stripped Runner (`0x4000`/`0xe700`, full 7671 IT entries) and AuthPass (`0x5a80`/`0xfdc0`). Adds `scripts/strip_macho_symbols.py` + `discovery-stripped-ios`. |
 | `d11ebc3` | **P0.1**: `modern_get_fill_spec`'s fallback `rules[]` is keyed by `DartCidKind` and resolved through a per-layout `by_kind[]` table built once in `modern_cid_cache_init`, instead of literal 3.6-era CIDs. Fixes a whole-block off-by-one on 3.2–3.5 layouts (36 CIDs from `SENTINEL` upward got the neighbouring class's ref/scalar layout) and `MonomorphicSmiableCall`/`UnlinkedCall` on 3.9+, where the two swapped places. Abstract classes that never form a cluster (`CallSiteData`, `AbstractType`, `TypedDataBase`) no longer claim a spec. |
-| *(uncommitted)* | **P2.5 + part of P0.2**: one shared cluster walker (`ModernWalk`) replaces the five hand-rolled tag→cid→alloc→fill loops. Consumers register readers in a `DartCidKind`-keyed table and the walker skips every class they don't claim, so tag decoding, alloc/fill skipping, `start_ref` bookkeeping and the inline-string decision each exist **once** (were 4–5× each). `modern_walk_fill_recorded` serves the resolver, which reads each cluster from its own recorded offset. Verified behaviour-preserving: output byte-identical on 5 samples × 21 actions. Also lands the **P0.2 read-only-data rule** (`modern_walk_is_rodata`): the six classes Dart moves into the data image when pointer compression is off now correctly have no fill records. Inert until the gate opens. |
-| *(uncommitted)* | **P1.1**: one `dart_cid_from_tags` helper now decodes `CID_INT32`, `CID_SHIFT1`, and `OBJECT_HEADER` tags for object inspection, modern and legacy cluster walks, data-image scanners, and InstructionTable discovery. This removes the duplicate decoders and fixes `CID_INT32` being treated as `OBJECT_HEADER` in two paths. |
+| `33bb19b` | **P2.5**: one shared cluster walker (`ModernWalk`) replaces the five hand-rolled tag→cid→alloc→fill loops. Consumers register readers in a `DartCidKind`-keyed table and the walker skips every class they don't claim, so tag decoding, alloc/fill skipping, `start_ref` bookkeeping and the inline-string decision each exist **once** (were 4–5× each). `modern_walk_fill_recorded` serves the resolver, which reads each cluster from its own recorded offset. Verified behaviour-preserving: output byte-identical on 5 samples × 21 actions. |
+| `e9fa905` | **Part of P0.2**: the read-only-data rule (`modern_walk_is_rodata`) — the six classes Dart moves into the data image when pointer compression is off (exactly `Serializer::ReadOnlyObjectType`) now correctly emit no fill records. Inert until the gate opens; see P0.2 for what still blocks it. |
+| `897a98d` | **P1.1**: one `dart_cid_from_tags` helper now decodes `CID_INT32`, `CID_SHIFT1`, and `OBJECT_HEADER` tags for object inspection, modern and legacy cluster walks, data-image scanners, and InstructionTable discovery. This removes the duplicate decoders and fixes `CID_INT32` being treated as `OBJECT_HEADER` in two paths. |
+| *(uncommitted)* | **P2.1**: deleted the legacy cluster walker — `deserialize_clusters`, `decode_class_cluster`, `decode_function_cluster`, `resolve_names`, the `DartClass`/`DartPoolFunction` types and the `DartCtx.classes`/`.functions` fields that only it wrote (244 lines). It had been running on every `-f`/`-A` alongside the modern scanner while contributing nothing: verified before removal by stubbing it out (50/50 outputs identical), and after by diffing 95 sample×action outputs against the previous commit. `decode_string_cluster`/`skip_generic_cluster`/`free_dart_string` stay — the legacy **class** parser in `dart_pool_classes.c` still uses them for CID_SHIFT1 samples. |
 
 Corrections to earlier assumptions, worth remembering:
 - **The CID tables are not the problem.** Flooring is exactly correct 2.10→3.12,
@@ -177,22 +179,21 @@ snapshot is the isolate, leave `vm_data = 0`) and updating that test.
 
 ## P2 — duplication, dead code, structural debt
 
-### P2.1 Two cluster decoders, both run on every enumeration
-`decode_pool_and_emit` (`dart_pool_parse.c:323, :386`) runs the legacy
-`deserialize_clusters` **and** the modern scanner over the same stream, then scans
-the data image again (`scan_code_names`, `collect_data_names`, + optional r2 pass).
-The legacy walker compares raw cluster CIDs against the **fake** `kXxxCid` enum
-(P2.2), so it silently never decodes strings (`kStringCid=72` vs real ~92) and
-misreads the cluster CID as a tagged `>>12` header.
-**Fix:** once P0.2 widens the modern gate, delete the legacy walker (or reduce it
-to a documented fallback). Also the biggest **perf** win — see P3.1.
-
 ### P2.2 Delete the fake `kXxxCid` enum
 `dart_pool_parse_priv.h:20-52` hardcodes a CID set matching **no real Dart
 version** (`kStringCid=72`, `kOneByteStringCid=73`, `kCodeCid=40`,
 `kTypeCid=110`; real ≈ 92/94/17/48), plus `dart_pool_classes.c:51-52` adds
 `kFieldCid_extract=10`, `kLibraryCid_extract=12`. Used ~35× in the legacy paths.
-**Fix:** route every consumer through `dart_cid_get(layout, …)` and remove the enum.
+Worse than dead: two sites use a fake CID as the **fallback** when real
+resolution fails — `dart_pool_data_image.c:161` (`resolved_function_cid >= 0?
+resolved_function_cid: kFunctionCid`) and `dart_pool_it.c:152` (same shape with
+`kOneByteStringCid`). Falling back to a fabricated CID silently compares against
+a value wrong for every Dart version; these should fail closed instead.
+**Fix:** route every consumer through `dart_cid_get(layout, …)`, make the two
+fallbacks bail, and remove the enum. P2.1 removed the legacy *cluster* walker,
+which cut the uses from ~35 to 27; the rest sit in the legacy **class** parser
+(`dart_pool_classes.c`), which is still live for CID_SHIFT1 samples like
+AuthPass, so they must be converted rather than deleted.
 
 ### P2.3 Two different "newest" baselines (introduced by `d387c0b`)
 `dart_pick_layout_by_hash` now fingerprints unknown hashes to
@@ -227,9 +228,14 @@ Read caching (`read_mem`, `dart_pool_snapshot.c:10-44`) already turns per-byte
 varint reads into a 1 MB sliding window. Remaining hotspots, largest first:
 
 ### P3.1 Redundant full passes over the same regions
-Per P2.1 a single `-f`/`-A` walks the cluster stream twice and the data image 2–3
-times. Collapsing to one cluster walk + one data-image scan is the biggest win on
-large inputs (`mafia/libapp.so` ~15 MB, `AuthPass.app` ~38 MB). Falls out of P2.1.
+A single `-f`/`-A` walks the cluster stream twice and the data image 2–3 times.
+
+**This was claimed to be the biggest perf win; measured, it is not.** Removing
+the whole legacy cluster walk changes runtime by ~1%, inside noise (mafia `-f`
+993 → 1005 ms; AuthPass `-f` 616 → 618 ms) — it bails out early rather than
+re-walking the stream. Collapsing the remaining data-image scans may still help,
+but **profile before spending time here**: the real cost is elsewhere, and P3.2's
+brute-force InstructionTable search is the better first suspect.
 
 ### P3.2 Brute-force InstructionTable header search
 `dart_pool_it.c:233-330` probes `{16,0,8,12} × delta -64..64 step 4`, then scans
@@ -275,16 +281,21 @@ repeatedly. Consider a small multi-window/LRU cache and a short-read-tolerant re
 
 ## Suggested execution order
 
-1. **P0.2** — widen the modern decoder to uncompressed snapshots. Biggest quality
+1. **P2.2** — the fake `kXxxCid` enum and the two fabricated-CID fallbacks.
+   P2.1 is done; the 27 remaining uses are in the legacy class parser, which is
+   still live, so convert them to `dart_cid_get` rather than deleting.
+2. **P2.3** — one-liner: make the CID fallback use `dart_newest_profile()` rather
+   than the literal `"3.9.2"`. Maintainer priority #1.
+3. **P0.2** — widen the modern decoder to uncompressed snapshots. Biggest quality
    win; makes most iOS/macOS samples use the good path. The RO-data half is
    already implemented and the alloc phase is proven correct; what remains is
    version-dependent *fill* shapes — see the P0.2 notes before starting.
-2. **P0.3** — grow the corpus alongside 1 so it is actually verified, and to give
+4. **P0.3** — grow the corpus alongside 3 so it is actually verified, and to give
    the landed P0.1 fix an end-to-end regression test.
-3. **P2.1 + P3.1** — delete the legacy walker; one cluster walk, one data scan.
-4. **P1.3** — shared frontend parser (fixes the `-r`/`r2 -n` AGENTS.md violations).
-5. **P1.2** — arch gating.
-6. **P3.2–P3.4**, remaining P1/P2 cleanups, test hardening.
+5. **P1.3** — shared frontend parser (fixes the `-r`/`r2 -n` AGENTS.md violations).
+6. **P1.2** — arch gating.
+7. **P3.2** (profile first — see the P3.1 note), remaining P2 cleanups, test
+   hardening.
 
 `COMMIT2.md` tracks the narrower follow-ups left over from the discovery work
 (gapped layouts, more iOS fixtures, stripped ELF, P1.5 semantics).

include/r2flutter/dart_r2.h

@@ -56,8 +56,6 @@ typedef struct {
 	RList *name_pool;
 	int name_pool_idx;
 	RList *strings;
-	RList *classes;
-	RList *functions;
 	void **refs;
 	ut64 refs_count;
 	ut64 num_base_objects;

src/lib/dart_pool_clusters.c

@@ -16,22 +16,6 @@ void free_dart_string(void *p) {
 	}
 }
 
-static void free_dart_class(void *p) {
-	DartClass *dc = (DartClass *)p;
-	if (dc) {
-		free (dc->name);
-		free (dc);
-	}
-}
-
-static void free_dart_func(void *p) {
-	DartPoolFunction *df = (DartPoolFunction *)p;
-	if (df) {
-		free (df->name);
-		free (df);
-	}
-}
-
 int decode_string_cluster(ClusterStream *s, DartCtx *ctx, ut64 *ref_counter, bool is_canonical) {
 	(void)is_canonical;
 	ut64 count = 0;
@@ -98,88 +82,6 @@ int decode_string_cluster(ClusterStream *s, DartCtx *ctx, ut64 *ref_counter, boo
 	return 0;
 }
 
-static int decode_class_cluster(ClusterStream *s, DartCtx *ctx, ut64 *ref_counter, bool is_canonical) {
-	(void)is_canonical;
-	ut64 count = 0;
-	if (!cs_read_unsigned (s, &count)) {
-		return -1;
-	}
-	if (count == 0 || count > 50000) {
-		return 0;
-	}
-	if (ctx->verbose > 1) {
-		fprintf (stderr, "[r2flutter] Class cluster: count=%" PRIu64 "\n", count);
-	}
-	for (ut64 i = 0; i < count; i++) {
-		DartClass *dc = R_NEW0 (DartClass);
-		dc->ref_id = (*ref_counter)++;
-		uint32_t instance_size = 0;
-		cs_read_u32 (s, &instance_size);
-		dc->instance_size = (int)instance_size;
-		ut64 name_ref = 0;
-		cs_read_ref_id (s, &name_ref);
-		dc->name_ref = name_ref;
-		ut64 library_ref = 0;
-		cs_read_ref_id (s, &library_ref);
-		dc->library_ref = library_ref;
-		ut64 skip_count = 6;
-		for (ut64 j = 0; j < skip_count; j++) {
-			ut64 dummy = 0;
-			cs_read_ref_id (s, &dummy);
-		}
-		if (ctx->classes) {
-			r_list_append (ctx->classes, dc);
-		}
-		if (ctx->refs && dc->ref_id < ctx->refs_count) {
-			ctx->refs[dc->ref_id] = dc;
-		}
-	}
-	return 0;
-}
-
-static int decode_function_cluster(ClusterStream *s, DartCtx *ctx, ut64 *ref_counter, ut64 iso_instr, bool is_canonical) {
-	(void)is_canonical;
-	ut64 count = 0;
-	if (!cs_read_unsigned (s, &count)) {
-		return -1;
-	}
-	if (count == 0 || count > 100000) {
-		return 0;
-	}
-	if (ctx->verbose > 1) {
-		fprintf (stderr, "[r2flutter] Function cluster: count=%" PRIu64 "\n", count);
-	}
-	for (ut64 i = 0; i < count; i++) {
-		DartPoolFunction *df = R_NEW0 (DartPoolFunction);
-		df->ref_id = (*ref_counter)++;
-		ut64 name_ref = 0;
-		cs_read_ref_id (s, &name_ref);
-		df->name_ref = name_ref;
-		ut64 owner_ref = 0;
-		cs_read_ref_id (s, &owner_ref);
-		df->owner_ref = owner_ref;
-		ut64 code_idx = 0;
-		cs_read_unsigned (s, &code_idx);
-		if (code_idx > 0 && iso_instr > 0) {
-			df->entry_point = iso_instr + (code_idx - 1) * 4;
-		}
-		ut64 skip_refs = 4;
-		for (ut64 j = 0; j < skip_refs; j++) {
-			ut64 dummy = 0;
-			cs_read_ref_id (s, &dummy);
-		}
-		uint32_t kind_tag = 0;
-		cs_read_u32 (s, &kind_tag);
-		if (ctx->functions) {
-			r_list_append (ctx->functions, df);
-		}
-		if (ctx->refs && df->ref_id < ctx->refs_count) {
-			ctx->refs[df->ref_id] = df;
-		}
-	}
-	return 0;
-}
-
 void skip_generic_cluster(ClusterStream *stream) {
 	ut64 count = 0;
 	if (cs_read_unsigned (stream, &count)) {
@@ -199,98 +101,3 @@ void skip_generic_cluster(ClusterStream *stream) {
 	}
 }
 
-int deserialize_clusters(DartCtx *ctx, ut64 cluster_start, ut64 cluster_end, ut64 num_clusters, ut64 iso_instr) {
-	if (!ctx || !ctx->core || cluster_start >= cluster_end) {
-		return -1;
-	}
-	ctx->strings = r_list_newf (free_dart_string);
-	ctx->classes = r_list_newf (free_dart_class);
-	ctx->functions = r_list_newf (free_dart_func);
-	ut64 total_refs = ctx->num_base_objects + ctx->num_objects + 16;
-	ctx->refs_count = total_refs;
-	ctx->refs = (void **)calloc (total_refs, sizeof (void *));
-	ClusterStream stream = {
-		.ctx = ctx,
-		.cursor = cluster_start,
-		.end = cluster_end
-	};
-	ut64 ref_counter = ctx->num_base_objects + 1;
-	for (ut64 ci = 0; ci < num_clusters && stream.cursor < stream.end; ci++) {
-		uint32_t tags = 0;
-		if (!cs_read_u32 (&stream, &tags)) {
-			if (ctx->verbose > 0) {
-				fprintf (stderr, "[r2flutter] Failed to read cluster tag at %" PRIu64 "\n", ci);
-			}
-			break;
-		}
-		const uint32_t cid = dart_cid_from_tags (ctx, tags);
-		bool is_canonical = ((tags >> 1) & 1) != 0;
-		if (ctx->verbose > 1) {
-			fprintf (stderr, "[r2flutter] Cluster %" PRIu64 ": cid=%u canonical=%d cursor=0x%" PFMT64x "\n", ci, cid, is_canonical, stream.cursor);
-		}
-		int rc = 0;
-		switch (cid) {
-		case kOneByteStringCid:
-		case kTwoByteStringCid:
-		case kStringCid:
-			rc = decode_string_cluster (&stream, ctx, &ref_counter, is_canonical);
-			break;
-		case kClassCid:
-			rc = decode_class_cluster (&stream, ctx, &ref_counter, is_canonical);
-			break;
-		case kFunctionCid:
-			rc = decode_function_cluster (&stream, ctx, &ref_counter, iso_instr, is_canonical);
-			break;
-		default:
-			skip_generic_cluster (&stream);
-			break;
-		}
-		if (rc < 0) {
-			if (ctx->verbose > 0) {
-				fprintf (stderr, "[r2flutter] Error decoding cluster cid=%u\n", cid);
-			}
-		}
-	}
-	if (ctx->verbose > 0) {
-		fprintf (stderr, "[r2flutter] Decoded: strings=%d classes=%d functions=%d\n", ctx->strings? r_list_length (ctx->strings): 0, ctx->classes? r_list_length (ctx->classes): 0, ctx->functions? r_list_length (ctx->functions): 0);
-	}
-	return 0;
-}
-
-void resolve_names(DartCtx *ctx) {
-	if (!ctx || !ctx->refs) {
-		return;
-	}
-	if (ctx->classes) {
-		RListIter *it;
-		DartClass *dc;
-		r_list_foreach (ctx->classes, it, dc) {
-			if (!dc || dc->name) {
-				continue;
-			}
-			if (dc->name_ref > 0 && dc->name_ref < ctx->refs_count) {
-				DartString *ds = (DartString *)ctx->refs[dc->name_ref];
-				if (ds && ds->value) {
-					dc->name = strdup (ds->value);
-					dart_obf_apply (ctx, &dc->name);
-				}
-			}
-		}
-	}
-	if (ctx->functions) {
-		RListIter *it;
-		DartPoolFunction *df;
-		r_list_foreach (ctx->functions, it, df) {
-			if (!df || df->name) {
-				continue;
-			}
-			if (df->name_ref > 0 && df->name_ref < ctx->refs_count) {
-				DartString *ds = (DartString *)ctx->refs[df->name_ref];
-				if (ds && ds->value) {
-					df->name = strdup (ds->value);
-					dart_obf_apply (ctx, &df->name);
-				}
-			}
-		}
-	}
-}

src/lib/dart_pool_parse.c

@@ -319,36 +319,6 @@ static int decode_pool_and_emit(DartItEmitRequest *req) {
 
 	const ut64 cluster_start = sh.cluster_start;
 	const ut64 cluster_end = base + total_len;
-	if (nc > 0 && nc < 5000 && no < 500000 && cluster_start < cluster_end) {
-		const int deser_rc = deserialize_clusters (ctx, cluster_start, cluster_end, nc, ctx->iso_instr);
-		if (deser_rc == 0) {
-			resolve_names (ctx);
-			if (ctx->functions && req->on_fn) {
-				RListIter *fit;
-				DartPoolFunction *df;
-				r_list_foreach (ctx->functions, fit, df) {
-					if (!df || df->entry_point == 0) {
-						continue;
-					}
-					char *clean_name = r_name_filter_dup (R_STR_ISEMPTY (df->name)? "method.unknown": df->name);
-					req->on_fn (clean_name, df->entry_point, 0, req->fn_user);
-					free (clean_name);
-				}
-			}
-			if (ctx->verbose > 1 && ctx->strings) {
-				RListIter *sit;
-				DartString *ds;
-				int str_count = 0;
-				r_list_foreach (ctx->strings, sit, ds) {
-					if (ds && ds->value && str_count < 50) {
-						fprintf (stderr, "[r2flutter] String[%" PRIu64 "]: %s\n", ds->ref_id, ds->value);
-						str_count++;
-					}
-				}
-			}
-		}
-	}
-
 	HtUP *sym_by_addr = ht_up_new0 ();
 	if (ctx->core && ctx->core->bin && sym_by_addr) {
 		RVecRBinSymbol *v = r_bin_get_symbols_vec (ctx->core->bin);

src/lib/dart_pool_parse_priv.h

@@ -65,23 +65,6 @@ typedef enum {
 	kNumPredefinedCids = 128
 } DartCid;
 
-typedef struct {
-	ut64 ref_id;
-	ut64 name_ref;
-	ut64 library_ref;
-	char *name;
-	int instance_size;
-} DartClass;
-
-typedef struct {
-	ut64 ref_id;
-	ut64 name_ref;
-	ut64 owner_ref;
-	ut64 code_ref;
-	ut64 entry_point;
-	char *name;
-} DartPoolFunction;
-
 #define DART_SNAPSHOT_MAGIC 0xdcdcf5f5
 #define DART_SNAPSHOT_FIXED_SIZE (4 + 8 + 8)
 #define DART_SNAPSHOT_HASH_SIZE 32
@@ -290,8 +273,6 @@ bool cs_read_bytes(ClusterStream *s, ut8 *buf, int len);
 void free_dart_string(void *p);
 int decode_string_cluster(ClusterStream *s, DartCtx *ctx, ut64 *ref_counter, bool is_canonical);
 void skip_generic_cluster(ClusterStream *stream);
-int deserialize_clusters(DartCtx *ctx, ut64 cluster_start, ut64 cluster_end, ut64 num_clusters, ut64 iso_instr);
-void resolve_names(DartCtx *ctx);
 
 bool modern_skip_n_bytes(ClusterStream *s, ut64 len);
 bool dart_modern_is_supported_snapshot(DartCtx *ctx);