Fix 15 pre-release-audit issues (GH 201-244): preprocessor, lexer, VM, mem2reg, object writers, linker, driver, headers

README.md

@@ -335,6 +335,10 @@ with user identifiers:
     __linux__                            // Linux targets only
 ```
 
+Comparing the string-literal predefines with `#if X == "..."` is a c5
+extension over C99, which restricts a `#if` controlling expression to an
+integer constant expression (see `std-conformance.md`).
+
 The MSVC/MinGW mimicry surface (`_MSC_VER` / `__MINGW32__` / `__int64`
 / `__declspec` / etc.) lives in `headers/include/msvc_compat.h`
 and is opted into per translation unit with `-include msvc_compat.h`.

demos/python/build.py

@@ -192,16 +192,7 @@
     #     (test_strftime_y2k, test_strftime_y2k_c99).
     # TODO: a C99-conforming CRT or a math/time shim removes every entry here.
     "windows-x64": {"windows": True, "tier2_ignore": _WIN_CRT_IGNORE},
-    # arm64 also ignores test_gh_120161. The test helper spawns the interpreter
-    # as a child with a sanitized environment block (neither PATH nor SystemRoot
-    # present); the child's `import asyncio` then loads _overlapped, whose
-    # WSAStartup initializes the Winsock provider catalog. On Windows arm64 that
-    # init fails with WinError 10106 (WSAEPROVIDERFAILEDINIT) under the stripped
-    # environment, while x64 tolerates it. The byte-identical interpreter imports
-    # _overlapped in every normal invocation, so this is an arm64 Winsock-startup
-    # property of the spawn environment, not a code-generation defect.
-    "windows-arm64": {"windows": True,
-                      "tier2_ignore": _WIN_CRT_IGNORE + ["test_gh_120161"]},
+    "windows-arm64": {"windows": True, "tier2_ignore": _WIN_CRT_IGNORE},
 }
 
 

headers/include/stdint.h

@@ -70,8 +70,12 @@ typedef uint64_t            uint_fast64_t;
 
 #define INT8_MIN  (-128)
 #define INT16_MIN (-32768)
-#define INT32_MIN (-2147483648)
-#define INT64_MIN (-9223372036854775808)
+/* C99 7.18.2: written as `-MAX-1` so the positive operand of unary minus
+   stays in range for the macro's exact-width signed type (matching the
+   limits.h idiom). `2147483648` exceeds INT_MAX and `9223372036854775808`
+   is unrepresentable in any signed type. */
+#define INT32_MIN (-2147483647-1)
+#define INT64_MIN (-9223372036854775807LL-1)
 
 #define INT8_MAX  127
 #define INT16_MAX 32767

headers/include/stdlib.h

@@ -163,8 +163,10 @@
 #pragma binding(msvcrt::abort,   "abort")
 #pragma binding(msvcrt::system,  "system")
 #pragma binding(msvcrt::getenv,  "getenv")
-// msvcrt's `setenv` is the underscored `_putenv_s`. Same shape:
+// POSIX putenv is msvcrt's underscored `_putenv`; msvcrt's `setenv`
+// is `_putenv_s`. Same shapes: (string) -> int and
 // (name, value, overwrite) -> int.
+#pragma binding(msvcrt::putenv,    "_putenv")
 #pragma binding(msvcrt::setenv,    "_putenv_s")
 #pragma binding(msvcrt::_wputenv_s, "_wputenv_s")
 #pragma binding(msvcrt::qsort,     "qsort")
@@ -187,16 +189,42 @@
 #pragma binding(data msvcrt::_sys_nerr,    "_sys_nerr")
 #pragma binding(data msvcrt::_sys_errlist, "_sys_errlist")
 #if defined(__aarch64__)
-// The legacy arm64 msvcrt.dll does not export the `_wenviron` data
-// symbol; UCRT exposes the wide environment only via this accessor.
-#pragma dylib(ucrtbase, "ucrtbase.dll")
-#pragma binding(ucrtbase::__p__wenviron, "__p__wenviron")
-unsigned short ***__p__wenviron(void);
-#define _wenviron (*__p__wenviron())
+// arm64 msvcrt.dll exports no environment data symbols, and ucrtbase's
+// `__p__environ` / `__p__wenviron` accessors read UCRT's separate copy
+// that msvcrt's getenv / _putenv / _wgetenv never update. Reach the
+// vectors through msvcrt's own accessor functions; the wide vector
+// stays NULL until the first wide-environment use, matching the CRT's
+// initialize-on-demand behaviour.
+#pragma binding(msvcrt::_get_environ,  "_get_environ")
+#pragma binding(msvcrt::_get_wenviron, "_get_wenviron")
+int _get_environ(char ***out);
+int _get_wenviron(unsigned short ***out);
+static inline char **__badc_environ(void) {
+    char **v = 0;
+    _get_environ(&v);
+    return v;
+}
+static inline unsigned short **__badc_wenviron(void) {
+    unsigned short **v = 0;
+    _get_wenviron(&v);
+    return v;
+}
+#define environ (__badc_environ())
+#define _environ (__badc_environ())
+#define _wenviron (__badc_wenviron())
 #else
-#pragma binding(data msvcrt::_wenviron,    "_wenviron")
+// x64 msvcrt.dll exports the vectors as data: bind each as a
+// loader-filled import so reads see the CRT's live view (getenv /
+// putenv mutations included). POSIX `environ` aliases the `_environ`
+// export. No unit may define these locally -- a local definition
+// shadows the import with a slot nothing populates.
+#pragma binding(data msvcrt::_wenviron, "_wenviron")
+#pragma binding(data msvcrt::_environ,  "_environ")
+#pragma binding(data msvcrt::environ,   "_environ")
+extern unsigned short **_wenviron;
+extern char **_environ;
+extern char **environ;
 #endif
-#pragma binding(data msvcrt::_environ,     "_environ")
 // `_doserrno` is an SDK macro over the exported accessor `__doserrno`,
 // which returns a pointer to the thread's OS error code.
 #pragma binding(msvcrt::__doserrno, "__doserrno")
@@ -207,10 +235,6 @@ extern char *_sys_errlist[];
 // Deprecated non-underscore aliases the msvcrt headers still expose.
 #define sys_nerr _sys_nerr
 #define sys_errlist _sys_errlist
-#if !defined(__aarch64__)
-extern unsigned short **_wenviron;
-#endif
-extern char **_environ;
 // MSVC CRT size limits (_makepath / _splitpath / environment).
 #define _MAX_PATH   260
 #define _MAX_DRIVE  3
@@ -428,16 +452,4 @@ static inline void __clear_cache(void *begin, void *end) {
     FlushInstructionCache(GetCurrentProcess(), begin,
                           (long long)((char *)end - (char *)begin));
 }
-// msvcrt exposes the environment vector through the `_environ`
-// data symbol. Both `environ` and `_environ` live in
-// `lib/runtime.c` as single canonical definitions; user code
-// declares each `extern` here so every TU resolves through the
-// same slot rather than contributing a tentative def of its
-// own.
-// TODO: bind msvcrt's `_environ` directly via `#pragma binding`'s
-// data form. The form is wired for ELF (COPY relocation) and Mach-O
-// (GOT import); the PE writer has no data-import path yet, so the
-// local slot stays until that lands.
-extern char **environ;
-extern char **_environ;
 #endif

lib/runtime.c

@@ -32,10 +32,14 @@
 // multiple-definition collision.
 //
 // macOS binds `environ` as a GOT data import to libSystem's `_environ`
-// (see <unistd.h>), so the symbol must stay undefined here for the
-// reference to route through the import slot rather than a local cell.
-#ifndef __APPLE__
+// (see <unistd.h>) and Windows binds `environ` / `_environ` /
+// `_wenviron` to msvcrt's live vectors (see <stdlib.h>), so on both
+// the symbols must stay undefined here for references to route
+// through the import rather than a local cell nothing populates.
+#if !defined(__APPLE__) && !defined(_WIN32)
 char **environ;
+#endif
+#ifndef __APPLE__
 // POSIX `tzset` outputs. Like `environ`, the Linux bindings route these
 // through a COPY relocation against the C library's symbols so a read
 // after `tzset()` sees what the library wrote; the local slots here are
@@ -45,11 +49,6 @@ long timezone;
 int daylight;
 #endif
 
-// msvcrt's environment-vector alias on Windows. `<stdlib.h>`'s
-// `_WIN32` section declares it `extern char **_environ;` for the same
-// reason `environ` lives here.
-char **_environ;
-
 // `__c5_exit` runs libc's `exit` so the atexit chain (including the
 // stdio flush) executes before the kernel reaps the process. The
 // per-OS dylib name mirrors `<stdlib.h>`'s convention.

src/c5/codegen/mod.rs

@@ -854,11 +854,13 @@ pub(crate) struct ResolvedImports {
     pub dylibs: Vec<ResolvedDylib>,
     /// Data symbols bound to a host data object via `#pragma
     /// binding(data <lib>::<local>, "<host>")`. Each entry is
-    /// `(local_name, host_symbol)`: the image-side symbol the source
-    /// references and the dynamic symbol it resolves to. The ELF
-    /// writer turns each into a COPY relocation; unlike `imports`,
-    /// these are not call sites and carry no GOT/PLT slot.
-    pub data_bindings: Vec<(String, String)>,
+    /// `(local_name, host_symbol, dylib_index)`: the image-side symbol
+    /// the source references, the dynamic symbol it resolves to, and
+    /// the owning entry in `dylibs`. The ELF writer turns each into a
+    /// COPY relocation; PE / Mach-O route the reference through a
+    /// loader-filled import slot. Unlike `imports`, these are not call
+    /// sites and carry no GOT/PLT slot of their own.
+    pub data_bindings: Vec<(String, String, usize)>,
 }
 
 impl ResolvedImports {
@@ -1070,11 +1072,12 @@ impl ResolvedImports {
         // emits a COPY relocation only when the final image defines the
         // local symbol (the runtime supplies `environ` for every hosted
         // image, so the binding binds it to the host's data object).
-        let mut data_bindings: Vec<(String, String)> = Vec::new();
+        let mut data_bindings: Vec<(String, String, usize)> = Vec::new();
         for spec in &program.dylibs {
             for b in &spec.bindings {
                 if b.is_data {
-                    let entry = (b.local_name.clone(), b.real_symbol.clone());
+                    let dylib_index = dylibs.iter().position(|d| d.name == spec.name).unwrap_or(0);
+                    let entry = (b.local_name.clone(), b.real_symbol.clone(), dylib_index);
                     if !data_bindings.contains(&entry) {
                         data_bindings.push(entry);
                     }

src/c5/codegen/ssa/mem2reg.rs

@@ -844,6 +844,21 @@ pub(crate) fn run(func: &mut FunctionSsa) -> Vec<i64> {
     if promotable.is_empty() {
         return Vec::new();
     }
+    // A slot live-in to the entry block (block 0) is read on some path
+    // before any store defines it (C99 6.2.4: an indeterminate value).
+    // It has no reaching definition on that path, so promoting it would
+    // inject a join phi with an undefined predecessor edge -- a dead phi
+    // that still holds a register and drives a predecessor-exit move.
+    // Exclude such slots so they stay entirely in memory; this is the
+    // up-front form of the rename's `failed` detection.
+    let entry_live_in = slot_live_in_sets(func, &promotable)[0].clone();
+    let promotable: BTreeSet<i64> = promotable
+        .into_iter()
+        .filter(|s| !entry_live_in.contains(s))
+        .collect();
+    if promotable.is_empty() {
+        return Vec::new();
+    }
     // Write-only address-free slots: every store is dead per C99
     // 6.2.4p2 (the slot's value is never observed). Neutralise each
     // such store to `Inst::Imm(0)`; the dead-pure check in the emit
@@ -1958,6 +1973,74 @@ mod tests {
         );
     }
 
+    #[test]
+    #[cfg(feature = "std")]
+    fn read_before_write_slot_is_left_in_memory_without_orphan_phi() {
+        // `int f(int c){ int x; if(c) x=7; return x+1; }`: slot -1 is
+        // stored only on the taken arm but read unconditionally, so it
+        // is live-in to the entry block. It must not be promoted; no
+        // Inst::Phi may be injected, and its load/store stay resident.
+        let insts = alloc::vec![
+            Inst::Imm(0), // block 0: cond
+            Inst::Imm(7), // block 1
+            Inst::StoreLocal {
+                off: -1,
+                value: 1,
+                kind: StoreKind::I64,
+            },
+            Inst::LoadLocal {
+                off: -1,
+                kind: LoadKind::I64,
+            }, // block 2
+        ];
+        let blocks = alloc::vec![
+            Block {
+                start_pc: 0,
+                inst_range: 0..1,
+                terminator: Terminator::Bz {
+                    cond: 0,
+                    target: 2,
+                    fall_through: 1,
+                },
+                exit_acc: 0,
+            },
+            Block {
+                start_pc: 0,
+                inst_range: 1..3,
+                terminator: Terminator::Jmp(2),
+                exit_acc: NO_VALUE,
+            },
+            Block {
+                start_pc: 0,
+                inst_range: 3..4,
+                terminator: Terminator::Return(3),
+                exit_acc: 3,
+            },
+        ];
+        let mut f = func_with(insts, blocks);
+        let promoted = with_phi_promote_override(true, || run(&mut f));
+        assert!(
+            !promoted.contains(&-1),
+            "read-before-write slot must not be promoted"
+        );
+        assert!(
+            !f.insts.iter().any(|i| matches!(i, Inst::Phi { .. })),
+            "no orphan phi may remain for the failed slot"
+        );
+        assert!(
+            f.insts
+                .iter()
+                .any(|i| matches!(i, Inst::StoreLocal { off: -1, .. })),
+            "the store must stay resident in memory"
+        );
+        assert!(
+            f.insts
+                .iter()
+                .any(|i| matches!(i, Inst::LoadLocal { off: -1, .. })),
+            "the load must stay resident in memory"
+        );
+    }
+
     #[test]
     fn predecessors_invert_terminator_successors() {
         // Block 0 conditionally branches to 1 (target) / 2

src/c5/lexer.rs

@@ -624,10 +624,22 @@ impl Lexer {
                 }
                 if quote == b'"' {
                     // A wide string stores one code point per element at
-                    // the target's `wchar_t` width (4 bytes on Unix, 2
-                    // on Windows / UTF-16).
-                    for k in 0..self.wchar_bytes {
-                        data.push((val >> (k * 8)) as u8);
+                    // the target's `wchar_t` width (4 bytes on Unix, 2 on
+                    // Windows / UTF-16). A UTF-16 element cannot hold a
+                    // code point above U+FFFF; C99 6.4.5 requires the
+                    // surrogate-pair encoding (Unicode 3.9 D91).
+                    if self.wchar_bytes == 2 && (0x10000..=0x10FFFF).contains(&val) {
+                        let v = val - 0x10000;
+                        let hi = 0xD800 + (v >> 10);
+                        let lo = 0xDC00 + (v & 0x3FF);
+                        data.push(hi as u8);
+                        data.push((hi >> 8) as u8);
+                        data.push(lo as u8);
+                        data.push((lo >> 8) as u8);
+                    } else {
+                        for k in 0..self.wchar_bytes {
+                            data.push((val >> (k * 8)) as u8);
+                        }
                     }
                 } else {
                     char_value = val;
@@ -1155,6 +1167,15 @@ impl Lexer {
                         self.tk = Tok(Token::FloatNum as i64);
                         return self.end_number();
                     }
+                    // C99 6.4.4.1: at least one hex digit must follow
+                    // `0x`/`0X`. Checked after the hex-float branch so a
+                    // digit-less integer part of a hex float (`0x.5p0`)
+                    // stays valid.
+                    if self.pos == hex_body_start {
+                        return Err(C5Error::Compile(crate::c5::error::fmt_internal_err(
+                            &format!("{}: hex literal `0x` has no digits", self.line),
+                        )));
+                    }
                     // Hex literals can carry the standard integer suffix
                     // letters (u/U/l/L plus ll/LL combinations such as
                     // 0xFFFFULL). Per C99 6.4.4.1 record the longness
@@ -2161,6 +2182,42 @@ mod tests {
         );
     }
 
+    #[test]
+    fn wide_string_astral_point_encodes_utf16_surrogate_pair_on_windows() {
+        // U+1F600 with a 2-byte wchar_t -> D83D DE00 (LE) + 2-byte nul.
+        assert_eq!(
+            lex_string_literal_w("L\"\u{1F600}\"", 2),
+            vec![0x3D, 0xD8, 0x00, 0xDE, 0, 0]
+        );
+        // A 4-byte wchar_t keeps the single UTF-32 element (no surrogates).
+        assert_eq!(
+            lex_string_literal("L\"\u{1F600}\""),
+            vec![0x00, 0xF6, 0x01, 0x00, 0, 0, 0, 0]
+        );
+    }
+
+    #[test]
+    fn hex_prefix_without_digits_is_an_error() {
+        // C99 6.4.4.1: `0x`/`0X` must be followed by at least one digit.
+        for src in ["0x", "0X", "0xg", "int x = 0x;"] {
+            let mut lex = Lexer::new(src.to_string());
+            let mut symbols: Vec<Symbol> = Vec::new();
+            let mut index = SymbolIndex::new();
+            let mut data: Vec<u8> = Vec::new();
+            let mut saw_err = false;
+            loop {
+                if lex.next(&mut symbols, &mut index, &mut data).is_err() {
+                    saw_err = true;
+                    break;
+                }
+                if lex.tk == 0 {
+                    break;
+                }
+            }
+            assert!(saw_err, "expected error for {src:?}");
+        }
+    }
+
     #[test]
     fn wide_string_literal_absorbs_adjacent_wide_chunks() {
         // The lexer concatenates adjacent `L"..."` literals into one