Four places the runtime answered with something other than the truth

The argument vector's malloc was unchecked, and a failure there would have
published a null pointer with a length beside it. It now dies naming what it
was building, because argv has no allocation site for a condition to hang on.

flan_slurp_into read a capacity of elements as a capacity of bytes and skipped
the epoch check every other container operation runs. The element size is now
a parameter and the length it publishes counts whole elements, so the day slurp
answers something other than (Vec u8) it does not answer with bytes nobody
wrote.

A string with a NUL in it is refused at the C boundary, which is the policy
flan_path_cstr has always had for a path: C reads to the first NUL, so what
crosses is a prefix of what was passed, and a window title is no different from
a filename in that respect. The refusal names the declare-c, which is the name
the program's author wrote.

The runtime's two translation units are compiled with -Wall -Wextra. They were
already clean under both; the flag is there so the next one is caught rather
than read.

The generation word keeps its place and loses its "yet": a reader for it is a
third word on every slice in the language, which is a spec amendment rather
than a runtime patch, and the comment now says so where someone deciding to
trust the word would read it.

docs/BUILT.md

@@ -2561,6 +2561,11 @@ nothing marked it — which is precisely what a static rule cannot see.
 exists for is not implemented: a slice is ptr+len and has nowhere to carry the Vec's identity or its generation. Said
 plainly here rather than implied by the word's presence in the header.
 
+It is not "not yet", either, and the runtime's own comment now says so. A reader for that word is a third word on every
+slice in the language — a layout `spec-memory.md` fixes — so implementing the trap is a spec amendment and an ABI
+change, not a runtime patch. The two live options are that amendment, or dropping the word from the header and from the
+spec together; neither is a cleanup, and until one is taken the word is carried and trusted by nothing.
+
 ### What this leaves for steps 5 to 7
 
 `(Map K V)` is built — see below. What is left: `drop` and with it the transitive move-only rule, recursive teardown,

lib/build.ml

@@ -645,7 +645,17 @@ let clang_stamp = lazy (stamp_of clang)
    the source text, the compiler and the flags are all unchanged. The key has
    to carry [opt] and [target]: the acceptance table builds the same programs
    at -O0 and -O2, and an -O2 object must not serve an -O0 build. *)
-let compile_c ~opts ?tflags ~src ~name () =
+(* Warnings for the translation units this project *owns*, which is the runtime
+   and the dev half of it. They are not on for a package's C or for the
+   generated shim: a package's sources are someone else's code, and a warning
+   nobody in this repository can fix is noise on every build that imports it.
+   The runtime is the opposite case — an unused result, a sign compare or a
+   conversion that narrows is a bug report here, and the file had none of this
+   coverage before. [-Werror] is deliberately absent: a clang upgrade must not
+   stop a user's build over a new diagnostic in code they did not write. *)
+let runtime_warnings = [ "-Wall"; "-Wextra" ]
+
+let compile_c ~opts ?tflags ?(warn = []) ~src ~name () =
   (* The whole flag list, not just the triple: on wasm32 the sysroot and the
      resource directory decide which headers and which builtins an object was
      built against, so repointing either must not serve a stale .o. *)
@@ -657,7 +667,8 @@ let compile_c ~opts ?tflags ~src ~name () =
          (String.concat "\000"
             [ name; src; stamp_of cc; opts.opt;
               String.concat " " (cflags opts);
-              String.concat " " tflags ]))
+              String.concat " " tflags;
+              String.concat " " warn ]))
   in
   let obj = Filename.concat (cachedir ()) (key ^ ".o") in
   if not (Sys.file_exists obj) then begin
@@ -671,6 +682,7 @@ let compile_c ~opts ?tflags ~src ~name () =
       String.concat " "
         ([ Filename.quote cc; opts.opt ]
          @ cflags opts
+         @ warn
          @ [ "-c" ] @ tflags
          @ [ Filename.quote c; "-o"; Filename.quote tmp ])
     in
@@ -796,13 +808,13 @@ let executable ?(opts = default) ?(csrcs = []) ?(lflags = []) ?(pnames = [])
      rather than as a missing flag. The table is BSS, so this costs address
      space and not binary size, and [-rdynamic] and the cells are still what
      [--dev] means. *)
-  let cc src name = compile_c ~opts ~tflags ~src ~name () in
+  let cc ?(warn = []) src name = compile_c ~opts ~tflags ~warn ~src ~name () in
   (* The runtime's own C wants -g too, or a backtrace that passes through
      flan_error lands in a frame with no line. The flag is part of the object
      cache key via [compile_c]'s [opt]/[tflags] digest — see [cflags]. *)
   let objs =
-    cc Runtime_src.source "flan_rt.c"
+    cc ~warn:runtime_warnings Runtime_src.source "flan_rt.c"
-    :: [ cc Runtime_src.dev_source "flan_dev.c" ]
+    :: [ cc ~warn:runtime_warnings Runtime_src.dev_source "flan_dev.c" ]
     (* wasi-libc's entry point, which is not [main]. See [wasm_main_source].
        Not the browser's: emscripten's start code calls [main] under that name,
        so the .ll's @main is already the entry point and the shim would be a
@@ -1053,10 +1065,10 @@ let macro_module ?(opts = default) ?(csrcs = []) ?(lflags = []) ~macros
      link time, not at codegen, which is the same trap [shared] meets and
      answers with -relocation-model=pic. *)
   let tflags = target_flags opts @ [ "-fPIC" ] in
-  let cc src name = compile_c ~opts ~tflags ~src ~name () in
+  let cc ?(warn = []) src name = compile_c ~opts ~tflags ~warn ~src ~name () in
   let objs =
-    cc Runtime_src.source "flan_rt.c"
+    cc ~warn:runtime_warnings Runtime_src.source "flan_rt.c"
-    :: [ cc Runtime_src.dev_source "flan_dev.c" ]
+    :: [ cc ~warn:runtime_warnings Runtime_src.dev_source "flan_dev.c" ]
     @ (match p.Tast.cshim with
        | [] -> []
        | parts ->

lib/check.ml

@@ -4736,7 +4736,8 @@ and named_call ctx ~want loc name args =
            (* Fills the Vec the line above sized. A file that grew since the
               measurement is truncated to the buffer; one that shrank leaves a
               shorter Vec. Both are successful reads of what was there. *)
-           try_ (rt loc (Types.Int Types.I8) "flan_slurp_into" [ vv (); psv () ]) ]
+           try_ (rt loc (Types.Int Types.I8) "flan_slurp_into"
+                   [ vv (); psv (); size_of loc u8; here loc ]) ]
        in
        expect loc ~want
          (mk loc vt

lib/emit.ml

@@ -2810,7 +2810,7 @@ declare i64 @flan_hash_combine(i64, i64)
 declare i8 @flan_file_size(ptr, i64, ptr)
 declare i8 @flan_file_write(ptr, i64, ptr, i64)
 declare i64 @flan_file_fail_reason()
-declare i8 @flan_slurp_into(ptr, ptr, i64)
+declare i8 @flan_slurp_into(ptr, ptr, i64, i64, ptr, i64)
 |}
 
 (* C's main, adapting to whichever of the four shapes Flan's main has: argv and

lib/shim.ml

@@ -306,11 +306,21 @@ let header =
    what keeps that allocation-free in the overwhelmingly common case, and it is
    256 because that covers a title, a path and a line of text without making
    every foreign call carry a page of stack. The only truncation left is when
-   malloc itself fails, where the alternative is handing C a null pointer. *)
+   malloc itself fails, where the alternative is handing C a null pointer.
+
+   An embedded NUL is refused rather than copied, which is the policy
+   flan_path_cstr has always had for a path and which a title, a name or a
+   query needs for the same reason: C reads to the first NUL, so what crosses
+   would be a prefix of the string the program passed and the function would
+   act on a value nobody wrote. The refusal is the runtime's — the shim has no
+   condition channel — and it names the declare-c it came from. *)
 let cstr_helpers =
-  "static char *flan_shim_cstr(const char *p, int64_t n, char *buf, size_t cap) {\n\
+  "_Noreturn void flan_shim_nul_fail(const char *site);\n\n\
+   static char *flan_shim_cstr(const char *p, int64_t n, char *buf, size_t cap,\n\
+  \                            const char *site) {\n\
   \  size_t len = n <= 0 ? 0 : (size_t)n;\n\
   \  char *d = buf;\n\
+  \  if (len != 0 && memchr(p, '\\0', len) != NULL) flan_shim_nul_fail(site);\n\
   \  if (len + 1 > cap) {\n\
   \    d = (char *)malloc(len + 1);\n\
   \    if (d == NULL) { d = buf; len = cap - 1; }  /* out of memory: truncate */\n\
@@ -386,9 +396,11 @@ let c_for (s : shim) =
        | Pstr ->
          let a = arg_name i in
          Printf.bprintf b "  char %s_b[%d];\n" a cstr_cap;
+         (* The Flan name travels with the copy so that a refusal names the
+            call the way every other runtime trap names its site. *)
          Printf.bprintf b
-           "  char *%s = flan_shim_cstr(%s_p, %s_n, %s_b, sizeof %s_b);\n" a a a
+           "  char *%s = flan_shim_cstr(%s_p, %s_n, %s_b, sizeof %s_b, %S);\n"
-           a a
+           a a a a a s.sflan
        | _ -> ())
     s.sargs;
   let call_args =

runtime/flan_rt.c

@@ -157,9 +157,28 @@ void flan_rt_init(int32_t argc, char **argv) {
   setvbuf(stdout, NULL, _IOLBF, 0);
 }
 
+/* Defined below with the rest of the non-local exits, and forward-declared
+ * here because the argument vector is built long before them. */
+static _Noreturn void rt_die(void);
+
+/* The one malloc in this file that is not an allocator's, because the argument
+ * vector belongs to the process rather than to any region a Flan program named.
+ * A failure here cannot be a condition: [argv] has no allocation site for the
+ * compiler to wrap in a restart, and answering with a shorter vector — or with
+ * a null pointer and a length — is the silently-wrong answer every other entry
+ * point in this file refuses to give. It cannot fire in practice: this is a
+ * handful of words asked for before the program has allocated anything. */
 void flan_argv(flan_slice *out) {
   if (rt_args == NULL && rt_argc > 0) {
     rt_args = (flan_slice *)malloc(sizeof(flan_slice) * (size_t)rt_argc);
+    if (rt_args == NULL) {
+      fflush(stdout);
+      fprintf(stderr,
+              "flan: out of memory building the argument vector for %d "
+              "arguments\n",
+              rt_argc);
+      rt_die();
+    }
     for (int i = 0; i < rt_argc; i++) {
       rt_args[i].ptr = (const uint8_t *)rt_argv[i];
       rt_args[i].len = (int64_t)strlen(rt_argv[i]);
@@ -1252,7 +1271,16 @@ _Noreturn void flan_region_only_fail(const uint8_t *loc, int64_t loclen) {
  *
  *   ptr len cap allocator      the release layout spec-memory.md fixes
  *   gen                        bumped on every reallocation — the stale-slice
- *                              word. It has no reader yet; see docs/BUILT.md.
+ *                              word spec-memory.md asks for. It has no reader
+ *                              and cannot have one as things stand, which is
+ *                              the part "not yet" used to hide: a slice is
+ *                              ptr+len, so it carries neither the Vec it came
+ *                              from nor the generation it was taken at, and
+ *                              the check has nothing to compare. Giving it a
+ *                              reader is a third word on every slice in the
+ *                              language, not a change to this file. Nothing
+ *                              here or anywhere else reads it; do not write
+ *                              code that trusts it. See docs/BUILT.md.
  *   epoch                      the allocator's epoch when this Vec last
  *                              touched it. Any operation on a container whose
  *                              recorded epoch has moved traps.
@@ -2720,6 +2748,26 @@ int64_t flan_file_fail_reason(void) { return flan_file_fail; }
  * silently opened, which is the failure this exists to avoid. */
 #define FLAN_PATH_MAX 4096
 
+/* The same policy at the other boundary, for the generated FFI shim.
+ *
+ * flan_path_cstr refuses an embedded NUL because the file opened would not be
+ * the file named; a string handed to any other C function is no different —
+ * the callee reads to the first NUL, so what crosses is a prefix of the value
+ * the program passed, and every C API that takes a name, a title or a query
+ * would act on the wrong one. The shim cannot signal: a foreign call has no
+ * allocation site for the compiler to wrap and no transfer channel of its own,
+ * so this traps naming the declare-c that was called, the way an out-of-bounds
+ * index traps naming its site. See lib/shim.ml, which emits the call. */
+_Noreturn void flan_shim_nul_fail(const char *site) {
+  fflush(stdout);
+  fprintf(stderr,
+          "%s: a string passed to C contains a NUL byte — C reads to the "
+          "first one, so the value this function would act on is a prefix of "
+          "the one passed. Remove the NUL before the call.\n",
+          site);
+  rt_die();
+}
+
 static int flan_path_cstr(const uint8_t *p, int64_t n, char *out) {
   if (n < 0 || n >= FLAN_PATH_MAX) return 0;
   if (n > 0) memcpy(out, p, (size_t)n);
@@ -2827,9 +2875,24 @@ int8_t flan_file_write(const uint8_t *path, int64_t n, const void *src,
  * allocate is StorageExhausted with retry, and a failure to read is FileError
  * with retry and use-value. Two failures, two conditions, neither swallowing
  * the other. */
-int8_t flan_slurp_into(flan_vec *v, const uint8_t *path, int64_t n) {
+int8_t flan_slurp_into(flan_vec *v, const uint8_t *path, int64_t n,
-  int64_t got = 0;
+                       int64_t size, const uint8_t *loc, int64_t loclen) {
-  if (!flan_file_read(path, n, v->ptr, v->cap, &got)) return 0;
+  int64_t got = 0, room;
-  v->len = got;
+  /* The same check every other operation on a container runs, and skipped here
+   * until now: the Vec was sized on this turn, but a handler between the
+   * sizing and the read can have released the region it lives in, and this is
+   * the one entry point that would have written into it anyway. */
+  flan_vec_check(v, loc, loclen);
+  /* A capacity is a count of elements and a read is a count of bytes. They
+   * were the same number while slurp answered only (Vec u8) — the checker
+   * still pins it to that — and the conflation was a byte count one element
+   * size away from being wrong. The product is the block flan_vec_init already
+   * allocated, so it is representable by construction; the guard is here
+   * because "by construction" is an argument and not a check. */
+  if (!flan_mul_bytes(v->cap, size, &room)) return 0;
+  if (!flan_file_read(path, n, v->ptr, room, &got)) return 0;
+  /* Whole elements only: a file that ends mid-element leaves the partial one
+   * out rather than publishing a length that covers bytes nobody wrote. */
+  v->len = size > 0 ? got / size : 0;
   return 1;
 }

test/programs/shim-nul.flan

@@ -0,0 +1,28 @@
+;;;; A string with a NUL in it, handed to C.
+;;;;
+;;;; A Flan string is ptr+len and a C string ends at its first NUL, so the two
+;;;; disagree about what the value *is* the moment one of those bytes is in the
+;;;; middle. The generated shim copies and terminates — string-of-bytes.flan
+;;;; pins that — and a copy of these bytes is a C string of length 1 where the
+;;;; program passed 5. The function would then act on a value nobody wrote.
+;;;;
+;;;; flan_path_cstr has always refused this for a path, on the grounds that the
+;;;; file opened would not be the file named. Nothing about a path is special:
+;;;; the same reasoning covers a window title, a shader name and a query, so
+;;;; the shim refuses it too, naming the declare-c that was called. It is a
+;;;; trap rather than a condition because a foreign call has no allocation site
+;;;; for the compiler to guard and no transfer channel of its own.
+(declare-c c-puts [s string] i32 "puts")
+
+(defn main [] i32
+  (let [v (vec-new u8)]
+    (push v 104)                        ; h
+    (push v 105)                        ; i
+    (push v 0)
+    (push v 104)                        ; h
+    (push v 105)                        ; i
+    (println "before")
+    (c-puts (string (as-slice v)))
+    (println "unreachable")
+    (free v))
+  0)

test/test_acceptance.ml

@@ -310,6 +310,26 @@ let () =
     outputs "string of bytes" "programs/string-of-bytes.flan" string_of_bytes_out;
     outputs ~opt:"-O0" "string of bytes, -O0" "programs/string-of-bytes.flan"
       string_of_bytes_out;
+
+    (* The other side of that boundary: bytes the copy cannot represent. A NUL
+       inside the string is where ptr+len and C's "ends at the first NUL" stop
+       describing the same value, so the shim refuses instead of handing C a
+       prefix — the policy flan_path_cstr has always had for a path. The
+       refusal names the declare-c, which is the only name the program's author
+       wrote. *)
+    let exe = compile "programs/shim-nul.flan" in
+    let code, text = run exe None in
+    if code <> 134 || not (contains text "before")
+       || not (contains text "c-puts: a string passed to C contains a NUL byte")
+       || contains text "unreachable"
+    then begin
+      incr failures;
+      Printf.printf
+        "FAIL  a string with a NUL in it is refused at the C boundary\n\
+        \        got: %S (exit %d)\n     wanted: exit 134, naming the call\n"
+        text code
+    end;
+    (try Sys.remove exe with Sys_error _ -> ());
     (* handler-bind and signal, spec-conditions.md §1 and §2: signal returns
        Unit and carries on, an unhandled one is a no-op, a nested frame does
        not displace the one outside it, and the stack is restored after. *)
@@ -2238,10 +2258,12 @@ ERR@7 unexpected token: not the kind the caller was reading
        The buffer is sized here and not per call site, because a generator has
        no call site to look at: 256 on the stack, the heap past that, and the
        copy is freed after the call rather than before the return value is
-       computed. *)
+       computed. The Flan name travels with the copy so that the refusal a NUL
+       in the bytes raises can name the call — see programs/shim-nul.flan. *)
     shim_case "declare-c: a string is copied, NUL-terminated and freed"
       "(declare-c open-it [path string] bool \"OpenIt\")"
-      [ "char a0_b[256];"; "flan_shim_cstr(a0_p, a0_n, a0_b, sizeof a0_b)";
+      [ "char a0_b[256];";
+        "flan_shim_cstr(a0_p, a0_n, a0_b, sizeof a0_b, \"open-it\")";
         "bool r = OpenIt(a0);"; "flan_shim_cstr_free(a0, a0_b);";
         "  return r;\n" ];
     shim_case "declare-c: two strings get two buffers"