Harden Dart snapshot discovery and drop bogus instruction-blob classification

* Only data snapshots carry the 0xf5f5dcdc magic
* A stray f5f5dcdc byte run could poison vm_instr/iso_instr on symbol-stripped Mach-O
* Preserve symbol-derived addresses instead of letting scan guesses overwrite them

COMMIT2.md

@@ -0,0 +1,77 @@
+# COMMIT 2 — Locate Dart instruction images without symbols (stripped Mach-O)
+
+Follow-up to commit 1 (`Harden Dart snapshot discovery...`). Commit 1 removed the
+junk-`iso_instr` risk and hardened `find_snapshots`, but on a genuinely
+symbol-less binary it deliberately leaves `vm_instr`/`iso_instr = 0` (instruction
+images carry no magic, so the scan cannot find them). This file records what
+commit 2 must do and everything we learned so we don't re-derive it.
+
+## Goal
+When the four `kDart*Snapshot*` symbols are absent, still recover `vm_instr` and
+`iso_instr` structurally so InstructionTable-based name recovery works (it is
+gated behind `if (!ctx->iso_instr) return 0;` in `dart_pool_parse.c`).
+
+## Key findings from the commit-1 investigation (don't redo these)
+- **Only data snapshots carry the magic `0xf5f5dcdc`.** Instruction images do
+  not, so magic-scanning can only ever locate `vm_data` / `iso_data`. Confirmed:
+  `test/bins/ios/Runner.app/.../App` has exactly 2 magic hits (both data).
+- **Layout (Runner.app, ground truth from the symbol table):**
+  - `_kDartVmSnapshotInstructions`      @ `0x4000`   — start of `__TEXT.__text` (`r-x`)
+  - `_kDartIsolateSnapshotInstructions` @ `0xe700`   — inside `__TEXT.__text`
+  - `_kDartVmSnapshotData`              @ `0x1b0900` — start of `__TEXT.__const`
+  - `_kDartIsolateSnapshotData`         @ `0x1b9600` — inside `__TEXT.__const`
+  So instruction images live consecutively at the start of the first executable
+  section: `[VmInstructions][IsolateInstructions]`.
+- **The Dart snapshot symbols are load-bearing and survive `strip`.** They are
+  external/global (the Flutter engine looks them up at runtime), so `llvm-strip`,
+  `llvm-strip -x`, and `strip` all leave them in place (`strip` even returns rc=1).
+  Consequence: a *functioning* Flutter app almost always keeps these symbols, so
+  the symbol path already works for normal apps. The symbol-less case is an
+  **edge case** (obfuscators, repackagers, hand-stripped LC_SYMTAB), which is why
+  this is a lower-frequency follow-up, not urgent.
+
+## Proposed approach
+1. `vm_instr` = start (`vaddr`) of the first executable (`perm & R_PERM_X`) section
+   that is not a segment. On the samples this is `__TEXT.__text`.
+2. Read the Dart **instructions-image header** at `vm_instr` to get the VM
+   instructions payload length, then `iso_instr = vm_instr + align_up(len, alignment)`.
+   - Need to nail the instructions-image header format (`Image::kHeaderSize` /
+     the leading length/size field) from `third_party/dart-sdk/runtime/vm/image_snapshot.{h,cc}`.
+     Cross-check the computed `iso_instr` against the ground-truth `0x4000`→`0xe700`
+     gap for Runner (i.e. VM-instructions length ≈ `0xe700 - 0x4000 = 0xa700`).
+   - Fallback if the header can't be parsed: leave `iso_instr = 0` (current safe
+     behavior) rather than guess.
+3. Only run this when `vm_instr`/`iso_instr` are still 0 after symbols + data scan,
+   so it never overrides symbol-derived values.
+
+## Test / fixture strategy (the hard part)
+- `strip`/`llvm-strip` do NOT remove the global Dart symbols, so we cannot make a
+  fixture with the CLI strip tools. Options, in order of preference:
+  1. Obtain a **real** obfuscated/stripped Flutter iOS `App` where the symbols are
+     actually gone (best signal). **TODO: collect more iOS Flutter binaries** —
+     especially release/obfuscated ones — into `test/bins/ios/` for coverage.
+  2. Craft a fixture by rewriting a copy of an existing `App` to drop the four
+     symbols from `LC_SYMTAB` (e.g. an r2 script that zeroes/removes the nlist
+     entries), keeping the snapshot bytes intact.
+- Regression assertion: on the symbol-less fixture, `-H -j` yields
+  `vm_data`/`iso_data` from the scan AND `vm_instr=0x4000` / `iso_instr=0xe700`
+  from structural location, and `-i`/`-f` recover IT entries/names.
+- Also add a scan-path assertion for commit 1 itself once a fixture exists:
+  data found, `instr` cleanly 0 (no junk), no crash.
+
+## Also deferred here (small, separate robustness items from the audit §3.1)
+- **Single-data-snapshot aliasing** (`pick_vm_iso_by_size`, count==1 sets
+  `vm_data == iso_data`). Visible on the synthetic bin (`vm_data==iso_data==4`).
+  The `sbom-synthetic` golden currently *encodes* this (`"vm_data":4,"iso_data":4`).
+  Fixing it (assign the lone snapshot to `iso_data`, leave `vm_data=0`) requires
+  updating that golden — do it alongside commit 2 and decide the correct semantics.
+- Raw-file fallback scan (`find_snapshots`, the `r_io_size` branch) conflates
+  physical offsets with IO addresses when a bin is loaded with maps but has
+  zero/unnamed sections; only fully correct under `r2 -n`. Low priority.
+
+## Files likely touched
+- `src/lib/dart_pool_discovery.c` — structural instruction-image location.
+- possibly a small helper to read the instructions-image header (new, or in
+  `dart_pool_snapshot.c`).
+- `test/bins/ios/...` — new stripped/obfuscated fixture(s).
+- `test/db/cmd/` — new discovery test(s).

src/lib/dart_pool_discovery.c

@@ -73,8 +73,22 @@ static int collect_snapshot_magics_in_range(DartCtx *ctx, ut64 start, ut64 size,
 		}
 		for (int j = 0; j <= toread - 4; j += 4) {
 			uint32_t val = r_read_le32 (buf + j);
-			if (val == DART_SNAPSHOT_MAGIC) {
-				found_addrs[found_cnt++] = addr + j;
+			if (val != DART_SNAPSHOT_MAGIC) {
+				continue;
+			}
+			const ut64 hit = addr + j;
+			// The read window overlaps the previous chunk by 16 bytes so a magic
+			// straddling a boundary is not missed; dedup so those overlap bytes
+			// (and Mach-O segment/section double-coverage) don't count twice.
+			bool dup = false;
+			for (int d = 0; d < found_cnt; d++) {
+				if (found_addrs[d] == hit) {
+					dup = true;
+					break;
+				}
+			}
+			if (!dup) {
+				found_addrs[found_cnt++] = hit;
 				if (found_cnt >= found_cap) {
 					return found_cnt;
 				}
@@ -118,9 +132,12 @@ int find_snapshots(DartCtx *ctx) {
 					continue;
 				}
 				for (int k = 0; k < 8; k++) {
-					if (!strcmp (nm, names[k])) {
+					// Only assign when this symbol carries an address: Mach-O
+					// often lists a name twice and a later vaddr==0 duplicate
+					// must not erase the good value we already resolved.
+					if (!strcmp (nm, names[k]) && sym->vaddr) {
 						int idx = k / 2;
-						*outs[idx] = sym->vaddr? rebase_bin_addr (ctx, sym->vaddr): 0;
+						*outs[idx] = rebase_bin_addr (ctx, sym->vaddr);
 					}
 				}
 			}
@@ -139,6 +156,11 @@ int find_snapshots(DartCtx *ctx) {
 			if (!sec || !sec->vaddr || !sec->vsize) {
 				continue;
 			}
+			// r_bin_get_sections_vec returns both segments and their child
+			// sections on Mach-O; scanning both covers the same bytes twice.
+			if (sec->is_segment) {
+				continue;
+			}
 			ut64 vaddr = rebase_bin_addr (ctx, sec->vaddr);
 			ut64 size = sec->vsize;
 			if (ctx->verbose > 0) {
@@ -160,39 +182,35 @@ int find_snapshots(DartCtx *ctx) {
 		}
 	}
 	if (found_cnt >= 1) {
+		// Only data snapshots carry the snapshot magic; instruction images do
+		// not. The old code classified every non-data magic hit as an
+		// "instructions" snapshot and fed its bogus total_len into the vm/iso
+		// picker, so a stray f5f5dcdc byte run could poison vm_instr/iso_instr.
+		// Derive only the data snapshots here; instruction images come from
+		// symbols (and, once implemented, structural section location).
 		ut64 data_addrs[4];
 		ut64 data_lens[4];
 		int data_cnt = 0;
-		ut64 instr_addrs[4];
-		ut64 instr_lens[4];
-		int instr_cnt = 0;
-		for (int i = 0; i < found_cnt; i++) {
-			ut8 hdr2[16];
-			if (!read_mem (ctx, found_addrs[i] + 4, hdr2, sizeof (hdr2))) {
-				continue;
-			}
-			ut64 total_len = r_read_le64 (hdr2) + 4;
-			ut64 classified_len = 0;
-			bool is_data = looks_like_data_snapshot (ctx, found_addrs[i], &classified_len);
-			if (is_data) {
-				if (data_cnt < 4) {
-					data_addrs[data_cnt] = found_addrs[i];
-					data_lens[data_cnt] = total_len;
-					data_cnt++;
-				}
-			} else {
-				if (instr_cnt < 4) {
-					instr_addrs[instr_cnt] = found_addrs[i];
-					instr_lens[instr_cnt] = total_len;
-					instr_cnt++;
-				}
+		for (int i = 0; i < found_cnt && data_cnt < 4; i++) {
+			ut64 total_len = 0;
+			if (looks_like_data_snapshot (ctx, found_addrs[i], &total_len)) {
+				data_addrs[data_cnt] = found_addrs[i];
+				data_lens[data_cnt] = total_len;
+				data_cnt++;
 			}
 		}
 		if (data_cnt >= 1) {
-			pick_vm_iso_by_size (data_addrs, data_lens, data_cnt, &ctx->vm_data, &ctx->iso_data);
-		}
-		if (instr_cnt >= 1) {
-			pick_vm_iso_by_size (instr_addrs, instr_lens, instr_cnt, &ctx->vm_instr, &ctx->iso_instr);
+			// Preserve any addresses already resolved from symbols; only fill
+			// the ones still unset so scan guesses cannot overwrite good values.
+			ut64 scan_vm_data = 0;
+			ut64 scan_iso_data = 0;
+			pick_vm_iso_by_size (data_addrs, data_lens, data_cnt, &scan_vm_data, &scan_iso_data);
+			if (!ctx->vm_data) {
+				ctx->vm_data = scan_vm_data;
+			}
+			if (!ctx->iso_data) {
+				ctx->iso_data = scan_iso_data;
+			}
 		}
 		if (ctx->vm_data || ctx->iso_data || ctx->vm_instr || ctx->iso_instr) {
 			return 0;