Fix mmap accounting under partial munmap churn

apps/browser-demos/pages/sqlite-test/main.ts

@@ -95,6 +95,98 @@ function collectArtifacts(fs: MemoryFileSystem): SqliteTestResult["artifacts"] {
   return artifacts.length > 0 ? artifacts : undefined;
 }
 
+const testrunnerPlatformShim = [
+  "# Kandelo platform shim for child testrunner jobs.",
+  "# SQLite all-mode reruns config variants by invoking test/testrunner.tcl",
+  "# directly, so the platform override has to live in that file too.",
+  "set ::tcl_platform(os) OpenBSD",
+  "set ::tcl_platform(platform) unix",
+].join("\n");
+
+const testrunnerGuestPathShim = [
+  "# Kandelo guest path shim for all-mode child jobs.",
+  "# testrunner.tcl builds child run.sh files from host-normalized paths;",
+  "# convert workdir-local paths back to paths relative to each testdirN",
+  "# directory, because SQLite runs the script after cd-ing into it.",
+  "proc kandelo_guest_path {path} {",
+  "  if {[file pathtype $path] != \"absolute\" && [string equal $path [info nameofexec]]} {",
+  "    return $path",
+  "  }",
+  "  set normalized [file normalize $path]",
+  "  set topdir [file normalize [file dirname $::testdir]]",
+  "  set script [file normalize [info script]]",
+  "  if {[string equal $normalized $script]} { return \"../test/testrunner.tcl\" }",
+  "  if {[string equal $normalized $topdir]} { return \"..\" }",
+  "  set prefix \"${topdir}/\"",
+  "  if {[string first $prefix $normalized] == 0} {",
+  "    return \"../[string range $normalized [string length $prefix] end]\"",
+  "  }",
+  "  return $path",
+  "}",
+  "set ::kandelo_inline_run_sh 1",
+].join("\n");
+
+function patchTestrunnerForKandelo(runner: string): string {
+  let patched = runner;
+
+  if (!patched.includes("Kandelo platform shim for child testrunner jobs")) {
+    const lines = patched.split("\n");
+    lines.splice(3, 0, "", testrunnerPlatformShim);
+    patched = lines.join("\n");
+  }
+
+  if (!patched.includes("Kandelo guest path shim for all-mode child jobs")) {
+    patched = patched.replace("cd $dir\n", `cd $dir\n\n${testrunnerGuestPathShim}\n`);
+    patched = patched.replace(
+      "    set displayname [string map [list $topdir/ {}] $f]\n",
+      [
+        "    set displayname [string map [list $topdir/ {}] $f]",
+        "    set testfixture_guest [kandelo_guest_path $testfixture]",
+        "    set testrunner_tcl_guest [kandelo_guest_path $testrunner_tcl]",
+        "    set f_guest [kandelo_guest_path $f]",
+        "",
+      ].join("\n"),
+    );
+    patched = patched
+      .replace("      set cmd \"$testfixture $f\"", "      set cmd \"$testfixture_guest $f_guest\"")
+      .replace(
+        "      set cmd \"$testfixture $testrunner_tcl $config $f\"",
+        "      set cmd \"$testfixture_guest $testrunner_tcl_guest $config $f_guest\"",
+      )
+      .replace(
+        "    set set_tmp_dir \"export SQLITE_TMPDIR=\\\"[file normalize $dir]\\\"\"",
+        "    set set_tmp_dir \"export SQLITE_TMPDIR=.\"",
+      )
+      .replace(
+        "    set fd [open \"|$TRG(runcmd) 2>@1\" r]",
+        [
+          "    if {[info exists ::kandelo_inline_run_sh] && $::kandelo_inline_run_sh} {",
+          "      set inline_cmd \"$set_tmp_dir\\n$job(cmd)\"",
+          "      set fd [open \"|sh -c [list $inline_cmd] 2>@1\" r]",
+          "    } else {",
+          "      set fd [open \"|$TRG(runcmd) 2>@1\" r]",
+          "    }",
+        ].join("\n"),
+      );
+  }
+
+  return patched;
+}
+
+function installTestrunnerPatches(fs: MemoryFileSystem): void {
+  const runnerPath = "/sqlite/test/testrunner.tcl";
+  const decoder = new TextDecoder();
+  const runner = decoder.decode(readVfsFile(fs, runnerPath));
+  writeVfsFile(fs, runnerPath, patchTestrunnerForKandelo(runner), 0o644);
+
+  writeVfsFile(fs, "/sqlite/kandelo-testrunner.tcl", [
+    testrunnerPlatformShim,
+    "set argv0 test/testrunner.tcl",
+    "source $argv0",
+    "",
+  ].join("\n"), 0o644);
+}
+
 function createFs(): MemoryFileSystem {
   if (!vfsImageBytes) throw new Error("SQLite test VFS image not loaded");
   const fs = MemoryFileSystem.fromImage(vfsImageBytes, {
@@ -164,13 +256,7 @@ async function init() {
     };
     const artifactTimer = window.setInterval(publishArtifactSnapshot, 5000);
     if (argv[1] === "kandelo-testrunner.tcl") {
-      writeVfsFile(fs, "/sqlite/kandelo-testrunner.tcl", [
-        "set ::tcl_platform(os) OpenBSD",
-        "set ::tcl_platform(platform) unix",
-        "set argv0 test/testrunner.tcl",
-        "source $argv0",
-        "",
-      ].join("\n"), 0o644);
+      installTestrunnerPatches(fs);
     }
     const kernel = new BrowserKernel({
       memfs: fs,

crates/kernel/src/memory.rs

@@ -92,7 +92,8 @@ impl MemoryManager {
     }
 
     /// Restore mmap mappings from fork (used by deserialize_fork_state).
-    pub fn set_mappings(&mut self, mappings: Vec<MappedRegion>) {
+    pub fn set_mappings(&mut self, mut mappings: Vec<MappedRegion>) {
+        mappings.sort_by_key(|m| m.addr);
         self.mappings = mappings;
     }
 
@@ -159,31 +160,44 @@ impl MemoryManager {
     /// Find the first gap in [mmap_base, max_addr) that can fit `needed` bytes.
     fn find_gap(&self, needed: usize) -> Option<usize> {
         let mut cursor = self.mmap_base.max(self.program_break);
-        let mut occupied: Vec<(usize, usize)> =
-            Vec::with_capacity(self.mappings.len() + self.reserved_regions.len());
-        occupied.extend(self.mappings.iter().map(|m| (m.addr, m.len)));
-        occupied.extend(self.reserved_regions.iter().map(|r| (r.addr, r.len)));
-        occupied.sort_by_key(|(addr, _)| *addr);
-
-        for (addr, len) in occupied {
-            if addr < cursor {
-                let end = addr.saturating_add(len);
-                if end > cursor {
-                    cursor = end;
+        let mut mapping_idx = 0;
+        let mut reserved_idx = 0;
+
+        while mapping_idx < self.mappings.len() || reserved_idx < self.reserved_regions.len() {
+            let next_mapping = self.mappings.get(mapping_idx).map(|m| (m.addr, m.len));
+            let next_reserved = self
+                .reserved_regions
+                .get(reserved_idx)
+                .map(|r| (r.addr, r.len));
+            let (addr, len, is_mapping) = match (next_mapping, next_reserved) {
+                (Some(mapping), Some(reserved)) => {
+                    if mapping.0 <= reserved.0 {
+                        (mapping.0, mapping.1, true)
+                    } else {
+                        (reserved.0, reserved.1, false)
+                    }
                 }
-                continue;
+                (Some(mapping), None) => (mapping.0, mapping.1, true),
+                (None, Some(reserved)) => (reserved.0, reserved.1, false),
+                (None, None) => break,
+            };
+
+            if is_mapping {
+                mapping_idx += 1;
+            } else {
+                reserved_idx += 1;
             }
-            if addr >= cursor {
-                let gap = addr - cursor;
-                if gap >= needed {
-                    return Some(cursor);
-                }
+
+            if addr >= cursor && addr - cursor >= needed {
+                return Some(cursor);
             }
+
             let end = addr.saturating_add(len);
             if end > cursor {
                 cursor = end;
             }
         }
+
         // Check gap after last mapping
         if cursor.saturating_add(needed) <= self.max_addr {
             Some(cursor)
@@ -267,43 +281,64 @@ impl MemoryManager {
         if len == 0 {
             return false;
         }
-        let unmap_end = addr.saturating_add(len);
+        let len = match len.checked_add(0xFFFF) {
+            Some(v) => v & !0xFFFF,
+            None => return false,
+        };
+        let unmap_end = match addr.checked_add(len) {
+            Some(end) => end,
+            None => return false,
+        };
         let mut found = false;
-        let mut new_mappings: Vec<MappedRegion> = Vec::new();
+        let mut i = 0;
 
-        for m in self.mappings.drain(..) {
+        while i < self.mappings.len() {
+            let m = self.mappings[i].clone();
             let m_end = m.addr.saturating_add(m.len);
 
             // No overlap — keep as is
             if m_end <= addr || m.addr >= unmap_end {
-                new_mappings.push(m);
+                i += 1;
                 continue;
             }
 
             found = true;
-
-            // Left remnant: mapping starts before unmap region
-            if m.addr < addr {
-                new_mappings.push(MappedRegion {
-                    addr: m.addr,
-                    len: addr - m.addr,
-                    prot: m.prot,
-                    flags: m.flags,
-                });
-            }
-
-            // Right remnant: mapping extends past unmap region
-            if m_end > unmap_end {
-                new_mappings.push(MappedRegion {
-                    addr: unmap_end,
-                    len: m_end - unmap_end,
-                    prot: m.prot,
-                    flags: m.flags,
-                });
+            let left_len = if m.addr < addr { addr - m.addr } else { 0 };
+            let right_len = if m_end > unmap_end {
+                m_end - unmap_end
+            } else {
+                0
+            };
+
+            match (left_len > 0, right_len > 0) {
+                (false, false) => {
+                    self.mappings.remove(i);
+                }
+                (true, false) => {
+                    self.mappings[i].len = left_len;
+                    i += 1;
+                }
+                (false, true) => {
+                    self.mappings[i].addr = unmap_end;
+                    self.mappings[i].len = right_len;
+                    i += 1;
+                }
+                (true, true) => {
+                    self.mappings[i].len = left_len;
+                    self.mappings.insert(
+                        i + 1,
+                        MappedRegion {
+                            addr: unmap_end,
+                            len: right_len,
+                            prot: m.prot,
+                            flags: m.flags,
+                        },
+                    );
+                    i += 2;
+                }
             }
         }
 
-        self.mappings = new_mappings;
         found
     }
 
@@ -519,13 +554,14 @@ impl MemoryManager {
 
     /// Extend an existing mapping at `addr` from `old_len` to `new_len`.
     /// The caller must ensure the space is free (via `can_grow_at`).
-    pub fn extend_mapping(&mut self, addr: usize, old_len: usize, new_len: usize) {
-        for m in &mut self.mappings {
-            if m.addr == addr && m.len == old_len {
-                m.len = new_len;
-                return;
+    pub fn extend_mapping(&mut self, addr: usize, old_len: usize, new_len: usize) -> bool {
+        for mapping in &mut self.mappings {
+            if mapping.addr == addr && mapping.len == old_len {
+                mapping.len = new_len;
+                return true;
             }
         }
+        false
     }
 }
 
@@ -558,6 +594,67 @@ mod tests {
         assert_eq!(addr2 - addr1, 0x10000);
     }
 
+    #[test]
+    fn test_adjacent_compatible_mmaps_preserve_boundaries() {
+        let mut mm = MemoryManager::new();
+        let rw = PROT_READ | PROT_WRITE;
+        let anon = MAP_PRIVATE | MAP_ANONYMOUS;
+
+        let addr1 = mm.mmap_anonymous(0, 0x10000, rw, anon);
+        let addr2 = mm.mmap_anonymous(0, 0x20000, rw, anon);
+
+        assert_eq!(addr2, addr1 + 0x10000);
+        assert_eq!(mm.mappings.len(), 2);
+        assert_eq!(mm.mappings[0].addr, addr1);
+        assert_eq!(mm.mappings[0].len, 0x10000);
+        assert_eq!(mm.mappings[1].addr, addr2);
+        assert_eq!(mm.mappings[1].len, 0x20000);
+    }
+
+    #[test]
+    fn test_find_gap_respects_reserved_regions_without_temp_vec() {
+        let mut mm = MemoryManager::new();
+        let rw = PROT_READ | PROT_WRITE;
+        let anon = MAP_PRIVATE | MAP_ANONYMOUS;
+        let base = MemoryManager::MMAP_BASE;
+
+        let first = mm.mmap_anonymous(0, 0x10000, rw, anon);
+        assert_eq!(first, base);
+        assert_eq!(
+            mm.reserve_host_region_at(base + 0x10000, 0x10000),
+            base + 0x10000
+        );
+
+        let second = mm.mmap_anonymous(0, 0x10000, rw, anon);
+        assert_eq!(second, base + 0x20000);
+    }
+
+    #[test]
+    fn test_set_mappings_restores_sorted_gap_invariant() {
+        let mut mm = MemoryManager::new();
+        let rw = PROT_READ | PROT_WRITE;
+        let anon = MAP_PRIVATE | MAP_ANONYMOUS;
+        let base = MemoryManager::MMAP_BASE;
+
+        mm.set_mappings(vec![
+            MappedRegion {
+                addr: base + 0x20000,
+                len: 0x10000,
+                prot: rw,
+                flags: anon,
+            },
+            MappedRegion {
+                addr: base,
+                len: 0x10000,
+                prot: rw,
+                flags: anon,
+            },
+        ]);
+
+        let addr = mm.mmap_anonymous(0, 0x10000, rw, anon);
+        assert_eq!(addr, base + 0x10000);
+    }
+
     #[test]
     fn test_munmap() {
         let mut mm = MemoryManager::new();
@@ -568,6 +665,20 @@ mod tests {
         assert!(!mm.is_mapped(addr));
     }
 
+    #[test]
+    fn test_munmap_rounds_length_up_to_page() {
+        let mut mm = MemoryManager::new();
+        let addr = mm.mmap_anonymous(
+            0,
+            0x30000,
+            PROT_READ | PROT_WRITE,
+            MAP_PRIVATE | MAP_ANONYMOUS,
+        );
+
+        assert!(mm.munmap(addr, 0x29000));
+        assert_eq!(mm.mappings.len(), 0);
+    }
+
     #[test]
     fn test_munmap_nonexistent() {
         let mut mm = MemoryManager::new();
@@ -1035,7 +1146,7 @@ mod tests {
         let rw = PROT_READ | PROT_WRITE;
         let anon = MAP_PRIVATE | MAP_ANONYMOUS;
         let addr = mm.mmap_anonymous(0, 0x10000, rw, anon);
-        mm.extend_mapping(addr, 0x10000, 0x20000);
+        assert!(mm.extend_mapping(addr, 0x10000, 0x20000));
         assert!(mm.is_mapped(addr + 0x10000)); // extended area is now mapped
     }