diff --git a/CHANGELOG.md b/CHANGELOG.md index c970c88be..fb3ac44b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,32 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `main`, the release pipeline automatically replaces `[current]` with the next version number before tagging the release. +## [current] + +### Fixed + +- **A leading-underscore function name no longer collides with the C runtime.** + `_write(path, content)` was emitted as a C function of the same name + verbatim, landing in the namespace C11 §7.1.3 reserves for the + implementation — which MSVCRT/UCRT populate heavily (`_write`, `_read`, + `_open`, `_close`, `_access`, …). On Windows the build then failed with + `conflicting types for '_write'`, pointing at generated C rather than at the + function name; on Linux the identical program built clean, because glibc + declares none of them. + + Codegen now renames such a function to an `ae`-prefixed symbol (`_write` → + `ae_write`) and rewrites its call sites — the same mechanism the #1366 extern + collision already uses. The Aether-level name is unchanged, so this is + invisible to callers. + + `static` alone would not have fixed it: a file-scope static whose name + matches a declared CRT prototype is still a conflicting-types error at + compile time. The trailing-underscore file-local convention (#279) is + untouched. + + Reported from the aeb line, where one shared `_write` test fixture broke 10 + of 118 tests on Windows only. + ## [0.515.0] ### Changed diff --git a/asks/leading-underscore-fn-collides-with-c-runtime.md b/asks/leading-underscore-fn-collides-with-c-runtime.md new file mode 100644 index 000000000..249c42d27 --- /dev/null +++ b/asks/leading-underscore-fn-collides-with-c-runtime.md @@ -0,0 +1,163 @@ +# A leading-underscore function name is emitted verbatim and collides with the C runtime (Windows) + +**From:** the aeb line (2026-08-10) · **Where it bit:** winbaz (Windows 11 / +MSYS2 MINGW64, gcc 16.1.0) — **10 of aeb's 118 tests fail to compile**, all with +the same error, all because a test helper is called `_write`. + +**Affects:** v0.513.0 / v0.515.0. Reproduced on `d104ba17`. +**Windows-only** — the identical program builds and runs on Linux. + +## Symptom + +```console +$ ae build tests/test_java_cache.ae -o tjc --lib lib --lib tools +tjc.c:319:6: error: conflicting types for '_write'; have 'void(const char *, const char *)' +In file included from tjc.c:18: +C:/msys64/mingw64/include/io.h:247:23: note: previous declaration of '_write' + with type 'int(int, const void *, unsigned int)' + 247 | _CRTIMP int __cdecl _write(int _FileHandle,const void *_Buf,unsigned int _MaxCharCount); +``` + +## Minimal repro + +Ten lines, no imports, no aeb involved: + +```aether +_write(p: string, c: string) { + println("leading underscore: ${p} ${c}") +} +main() { + _write("a", "b") + return 0 +} +``` + +```console +$ ae build us.ae -o us # Windows / MINGW64 +us.c:232:6: error: conflicting types for '_write'; ... +us.ae:1:6: error: conflicting types for '_write'; ... +Build failed. + +$ ae build lu.ae -o lu # Linux, same source +Built: lu +$ ./lu +ab +``` + +Linux is fine because glibc does not declare `_write`. The MSVCRT/UCRT headers +do, so every Windows build that includes `io.h` (which the generated C does, +transitively) sees the clash. + +## Cause + +A top-level Aether function is emitted as a C function with **the same name, +verbatim**. `_write` in Aether becomes `void _write(const char*, const char*)` +in the generated C — landing squarely in the C implementation's **reserved +identifier namespace**. + +Per C11 §7.1.3, identifiers beginning with an underscore at file scope are +reserved *for the implementation*. MSVCRT uses that namespace heavily and +declares real functions there: `_write`, `_read`, `_open`, `_close`, `_access`, +`_aligned_malloc`, … A sample of just four MinGW headers (`io.h`, `stdio.h`, +`stdlib.h`, `string.h`) turns up dozens; the first twenty alphabetically are + +``` +_abs64 _access _access_s _aligned_free _aligned_malloc _aligned_msize +_aligned_offset_malloc _aligned_offset_realloc _aligned_offset_recalloc +_aligned_realloc _aligned_recalloc _atodbl _atodbl_l _atof_l _atoflt +_atoflt_l _atoi_l _atoi64 _atoi64_l _atol_l +``` + +`_write`, `_read`, `_open`, `_close` are exactly the names a programmer reaches +for when writing a private I/O helper, which is what makes this likely to +recur rather than a one-off. + +## Why this is worth fixing rather than documenting + +Aether **already treats a decorated underscore as a linkage signal** — issue +**#279** gives *trailing*-underscore names (`helper_`) internal linkage: + +```c +/* compiler/codegen/codegen_func.c:930 */ +// Trailing-underscore convention `foo_` marks a function as +// file-local — ... Emit as `static` so two .ae files in the same +// namespace bundle / [[bin]] can each declare their own +// `record_start_` / `helper_` without the generated C colliding at +// link time. Closes #279. +if (fn_has_internal_linkage(func)) { + fprintf(gen->output, "static "); +} +``` + +So the language has a convention for "this is private, don't export it" — and +it is spelled with the underscore on the **wrong end** for the instinct most +people have. A leading underscore reads as private in Python, C#, JavaScript +and much C; in Aether it is the one spelling that gets *no* protection and +maximum collision risk. + +Confirmed the trailing form is unaffected: + +```console +$ cat us2.ae +write_(p: string, c: string) { println("trailing underscore ok: ${p} ${c}") } +main() { write_("c", "d") return 0 } +$ ae build us2.ae -o us2 && ./us2.exe +Built: us2.exe +trailing underscore ok: c d +``` + +## Ask, in preference order + +1. **Give leading-underscore functions internal linkage too** — the same + `static` treatment `fn_has_internal_linkage` already applies to trailing + underscore. A `static void _write(...)` still shadows nothing at link time, + and a file-scope `static` whose name matches a declared CRT prototype is + still a conflicting-types error at *compile* time, so on its own this is + necessary but not sufficient. It is the cheapest half. + +2. **Prefix or mangle the emitted symbol** so user code cannot land in the + implementation's namespace at all — e.g. emit `_write` as + `aeuser__write`, or as `aether_fn__write`, keeping the Aether-level name + unchanged. This closes the class rather than the instance, and it also + removes the (rarer, but real) risk for names like `_read` that a future + libc adds. Callers are all compiler-generated, so the rename is internal. + +3. **Failing either, reject it at parse time** with a diagnostic that names the + convention: "function name `_write` is reserved — identifiers beginning with + `_` are reserved for the C implementation; use `write_` for a file-local + helper (#279)". A loud, portable, early error beats a Windows-only wall of + gcc output pointing at generated C. + +Option 3 alone would have saved this entirely: the failure surfaces as a +`conflicting types` error in a `.c` file the user never wrote, on one platform +only, with no hint that the function *name* is the problem. + +## Impact on the aeb line + +10 of 118 aeb tests do not compile on Windows — every `*_cache` test, since +they share a `_write(path, content)` fixture helper: +`test_java_cache`, `test_go_cache`, `test_rust_cache`, `test_ts_cache`, +`test_clojure_cache`, `test_kotlin_cache`, `test_scala_cache`, +`test_dotnet_cache`, `test_groovy_cache`, `test_aether_cache`. All fail +identically. Verified on the box that they share the one cause. + +aeb can and will rename its helper — that is a one-line workaround per file and +we are not blocked. Filing because the *next* person to write `_read` or +`_close` will lose the same afternoon, on Windows only, with a diagnostic that +points at generated C rather than at their function name. + +## Not being asked + +- No change to the trailing-underscore convention (#279); it works and this ask + depends on it as precedent. +- Not asking for a general C-keyword/identifier blocklist. The specific, + bounded rule "leading underscore at file scope is reserved" is C11 §7.1.3 and + is worth honouring on its own. +- Not asking for anything at the `--emit=lib` alias layer; the collision is in + the plain program path. + +## Environment + +winbaz, MSYS2 / MINGW64 on Windows 11, gcc 16.1.0, aether v0.515.0 built from +`/c/Users/paul/scm/aether`. Linux comparison: same source, aether 0.512.0, +builds and runs clean. diff --git a/compiler/codegen/codegen.c b/compiler/codegen/codegen.c index bafb79cc3..e78474553 100644 --- a/compiler/codegen/codegen.c +++ b/compiler/codegen/codegen.c @@ -3995,6 +3995,58 @@ static void rename_calls_to(ASTNode* node, const char* from, const char* to) { uses for libc collisions. Dotted stdlib calls (`string.replace_all`) carry a different AST value and are untouched; a bare call resolves to the user's function, which is what gets renamed with it. */ +/* A leading underscore at file scope is RESERVED for the C implementation + (C11 §7.1.3), and MSVCRT/UCRT use that namespace heavily: _write, _read, + _open, _close, _access, _aligned_malloc and dozens more are real declared + functions there. Emitting an Aether `_write(...)` verbatim therefore lands + on top of a CRT prototype and fails to compile: + + error: conflicting types for '_write'; have 'void(const char*, const char*)' + note: previous declaration ... int(int, const void*, unsigned int) + + Windows-only, because glibc declares none of these — so the identical + program builds clean on Linux and dies on MinGW, with the error pointing at + generated C rather than at the function name. Reported from the aeb line, + where one shared `_write(path, content)` fixture broke 10 of 118 tests. + + Renaming is the fix rather than `static` alone: a file-scope static whose + name matches a declared CRT prototype is STILL a conflicting-types error at + compile time. Same `ae_` spelling and same call-rewrite as the #1366 extern + collision below, so the two read as one mechanism. + + Deliberately narrow: only a LEADING underscore, only top-level user + functions. The trailing-underscore file-local convention (#279) is + untouched — `write_` is exactly what a caller should reach for instead, and + it already gets internal linkage. */ +static int name_is_c_reserved(const char* name) { + return name && name[0] == '_'; +} + +static void rename_leading_underscore_functions(ASTNode* program) { + if (!program) return; + for (int i = 0; i < program->child_count; i++) { + ASTNode* fn = program->children[i]; + if (!fn || (fn->type != AST_FUNCTION_DEFINITION && + fn->type != AST_BUILDER_FUNCTION)) continue; + /* @c_callback names must stay externally addressable verbatim, and an + imported function was already renamed by its own module pass. */ + if (fn->is_imported || is_c_callback(fn) || !fn->value) continue; + if (!name_is_c_reserved(fn->value)) continue; + + /* `ae` + the name keeps the underscore, so `_write` becomes + `ae_write` — readable in a backtrace and out of the reserved + namespace, since the leading character is no longer `_`. */ + char safe[280]; + snprintf(safe, sizeof(safe), "ae%s", fn->value); + rename_calls_to(program, fn->value, safe); + char* dup = strdup(safe); + if (dup) { + free(fn->value); + fn->value = dup; + } + } +} + static void rename_extern_colliding_functions(ASTNode* program) { if (!program) return; for (int i = 0; i < program->child_count; i++) { @@ -4158,6 +4210,7 @@ void generate_program(CodeGenerator* gen, ASTNode* program) { // any codegen pass reads their names (must run before escape analysis and // emission, which both key off the identifier names). mangle_keyword_value_idents(program); + rename_leading_underscore_functions(program); rename_extern_colliding_functions(program); // Note: `gen->program` is the source of truth for the // structural-escape-analysis lookup (issue #405). Setting it diff --git a/tests/regression/test_leading_underscore_fn.ae b/tests/regression/test_leading_underscore_fn.ae new file mode 100644 index 000000000..8e64e0084 --- /dev/null +++ b/tests/regression/test_leading_underscore_fn.ae @@ -0,0 +1,96 @@ +// A leading-underscore function name must not land in C's reserved namespace. +// +// C11 §7.1.3 reserves file-scope identifiers beginning with an underscore for +// the implementation, and MSVCRT/UCRT use that namespace heavily — `_write`, +// `_read`, `_open`, `_close`, `_access` and dozens more are real declared +// functions there. Emitting an Aether `_write(...)` verbatim therefore landed +// on top of a CRT prototype: +// +// error: conflicting types for '_write'; have 'void(const char*, const char*)' +// note: previous declaration ... int(int, const void*, unsigned int) +// +// Windows-only, because glibc declares none of them — so the identical program +// built clean on Linux and died on MinGW, with the error pointing at generated +// C rather than at the function name. Reported from the aeb line, where one +// shared `_write(path, content)` fixture broke 10 of its 118 tests. +// +// Codegen now renames such a function to an `ae`-prefixed symbol (`_write` -> +// `ae_write`) and rewrites its call sites, the same shape as the #1366 extern +// collision rename. +// +// THIS TEST RUNS EVERYWHERE, but only Windows could ever have failed to +// compile it. That is deliberate: the point is that the name works on every +// platform, and a Linux-only assertion would not have caught the original bug. +// What every platform CAN check is that the rename is transparent — the +// function is still callable by its Aether name and still behaves. +import std.string + +// The exact name from the report. If codegen ever stops renaming, this file +// stops compiling on Windows — which is the regression being guarded. +_write(prefix: string, content: string) -> string { + return string.concat(prefix, content) +} + +// Siblings from the same CRT namespace, to prove the fix is not special-cased +// to one name. +_read() -> int { + return 42 +} + +_close(n: int) -> int { + return n + 1 +} + +// A leading underscore on a name the CRT does NOT declare must work too — the +// rule is about the namespace, not a blocklist of known names. +_totally_made_up_helper() -> int { + return 7 +} + +check(cond: int, label: string) -> int { + if cond == 1 { + println(" PASS ${label}") + return 0 + } + println(" FAIL ${label}") + return 1 +} + +main() { + println("=== leading-underscore function names ===") + fails = 0 + + // Each must be callable by its Aether-level name; the rename is internal. + r = _write("a", "b") + fails = fails + check(string.equals(r, "ab"), "_write is callable and correct") + string.free(r) + + fails = fails + check(_read() == 42, "_read is callable") + fails = fails + check(_close(1) == 2, "_close is callable and takes args") + fails = fails + check(_totally_made_up_helper() == 7, + "a non-CRT leading-underscore name also works") + + // The trailing-underscore file-local convention (#279) is untouched by + // this change; guard that it still works so the two cannot drift. + fails = fails + check(write_("c", "d") == 1, "trailing-underscore form still works") + + println("") + if fails == 0 { + println("All PASS") + } else { + println("${fails} FAILURE(S)") + exit(1) + } +} + +// #279: a trailing underscore marks a function file-local. Unrelated mechanism, +// asserted here only so a future change to one cannot silently break the other. +write_(a: string, b: string) -> int { + s = string.concat(a, b) + n = 0 + if string.equals(s, "cd") == 1 { + n = 1 + } + string.free(s) + return n +}