commit dffe56c37dd005738f5890335a4b76cb65d2b1f9
parent 72389f31ee6a23b5fd957e5614ee77e17313efb9
Author: Ryan Sepassi <rsepassi@gmail.com>
Date: Fri, 29 May 2026 11:32:51 -0700
Fix three aa64/-O0 codegen & linkage bugs
Three independent defects, each surfaced once the prior was fixed:
1. weak attribute on a definition was dropped after a non-weak prototype.
cfree_cg_decl only set a symbol's bind when first creating it, so a plain
prototype (e.g. CFREE_API void f(...);) followed by a `weak` definition
emitted a strong global, colliding with any strong override at link time
("duplicate definition of global symbol"). cfree_cg_decl now promotes an
existing strong global to weak when a later declaration carries `weak`
(C allows the attribute on any declaration). decl_apply_redecl_flags
re-emits the symbol when a redeclaration/definition introduces DF_WEAK,
and declare_function calls it on both redeclaration paths.
2. `return <expr>;` did not convert the value to the function's return type.
cfree_cg_ret's api_force_local(v, rty) does not convert types; it loads v
at rty's width. A narrower value (e.g. a _Bool call result returned from
an int function) was therefore reloaded at the wider return width, reading
adjacent bytes. parse_return_stmt now coerces the value to cur_func_ret
before cg_ret, matching the assignment/init paths and the toy frontend's
convert-before-ret contract. coerce_top_to_type is exposed for this.
3. aa64 sret (indirect aggregate return) clobbered its own destination.
aa_plan_ret held the caller-provided sret pointer in AA_TMP1, but
aa_copy_bytes materializes out-of-range source/dest frame offsets into
AA_TMP1, overwriting the destination base mid-copy. Benign for small
frames (offsets fit stur's signed-9 range), corrupting for large ones.
Hold the sret pointer in x8 (the indirect-result register), which the
byte copy never touches.
Diffstat:
8 files changed, 75 insertions(+), 25 deletions(-)
diff --git a/lang/c/decl/decl.c b/lang/c/decl/decl.c
@@ -85,6 +85,34 @@ static CfreeCgInlinePolicy decl_inline_policy(const Decl* d) {
return CFREE_CG_INLINE_DEFAULT;
}
+/* Build the public CG declaration for an already-validated Decl and emit it.
+ * Returns the resulting CfreeCgSym. Emission is idempotent for a name that
+ * already has a symbol: cfree_cg_decl reuses the existing symbol and only
+ * refreshes attributes (e.g. promoting a strong global to weak). */
+static ObjSymId decl_emit_cg_sym(DeclTable* t, const Decl* slot) {
+ CfreeCgDecl decl;
+ memset(&decl, 0, sizeof decl);
+ decl.kind = (slot->type && slot->type->kind == TY_FUNC)
+ ? CFREE_CG_DECL_FUNC
+ : CFREE_CG_DECL_OBJECT;
+ decl.display_name = slot->name;
+ decl.linkage_name = cfree_cg_c_linkage_name(t->c, slot->name);
+ decl.type = type_cg_id_in_pool(t->c, t->pool, slot->type);
+ decl.sym = decl_sym_attrs(slot);
+ if (decl.kind == CFREE_CG_DECL_FUNC) {
+ if (slot->flags & DF_NORETURN) decl.as.func.flags |= CFREE_CG_FUNC_NORETURN;
+ decl.as.func.inline_policy = decl_inline_policy(slot);
+ decl.as.func.section = slot->section_id;
+ decl.as.func.wasm_import_module = slot->wasm_import_module;
+ decl.as.func.wasm_import_name = slot->wasm_import_name;
+ } else {
+ if (slot->flags & DF_THREAD) decl.as.object.flags |= CFREE_CG_OBJ_TLS;
+ decl.as.object.section = slot->section_id;
+ decl.as.object.align = slot->align;
+ }
+ return cfree_cg_decl(t->cg, decl);
+}
+
DeclId decl_declare(DeclTable* t, const Decl* in) {
DeclId id;
Decl* slot;
@@ -101,32 +129,27 @@ DeclId decl_declare(DeclTable* t, const Decl* in) {
compiler_panic(t->c, slot->loc,
"weak attribute requires external linkage");
}
- CfreeCgDecl decl;
- memset(&decl, 0, sizeof decl);
- decl.kind = (slot->type && slot->type->kind == TY_FUNC)
- ? CFREE_CG_DECL_FUNC
- : CFREE_CG_DECL_OBJECT;
- decl.display_name = slot->name;
- decl.linkage_name = cfree_cg_c_linkage_name(t->c, slot->name);
- decl.type = type_cg_id_in_pool(t->c, t->pool, slot->type);
- decl.sym = decl_sym_attrs(slot);
- if (decl.kind == CFREE_CG_DECL_FUNC) {
- if (slot->flags & DF_NORETURN)
- decl.as.func.flags |= CFREE_CG_FUNC_NORETURN;
- decl.as.func.inline_policy = decl_inline_policy(slot);
- decl.as.func.section = slot->section_id;
- decl.as.func.wasm_import_module = slot->wasm_import_module;
- decl.as.func.wasm_import_name = slot->wasm_import_name;
- } else {
- if (slot->flags & DF_THREAD) decl.as.object.flags |= CFREE_CG_OBJ_TLS;
- decl.as.object.section = slot->section_id;
- decl.as.object.align = slot->align;
- }
- slot->obj_sym = cfree_cg_decl(t->cg, decl);
+ slot->obj_sym = decl_emit_cg_sym(t, slot);
}
return id;
}
+void decl_apply_redecl_flags(DeclTable* t, DeclId id, u32 new_flags) {
+ Decl* slot;
+ if (id == DECL_NONE || id >= t->nslots) return;
+ slot = &t->slots[id];
+ /* C lets the `weak` attribute appear on any declaration; if a later
+ * redeclaration or the definition introduces it, the already-emitted symbol
+ * must become weak too. Other redeclaration-merge flags don't affect the
+ * emitted symbol, so weak is all we re-apply here. */
+ if (!(new_flags & DF_WEAK) || (slot->flags & DF_WEAK)) return;
+ if (slot->linkage != DL_EXTERNAL)
+ compiler_panic(t->c, slot->loc,
+ "weak attribute requires external linkage");
+ slot->flags |= DF_WEAK;
+ if (slot->obj_sym != OBJ_SYM_NONE) decl_emit_cg_sym(t, slot);
+}
+
const Decl* decl_get(const DeclTable* t, DeclId id) {
if (!t || id == DECL_NONE || id >= t->nslots) return NULL;
return &t->slots[id];
diff --git a/lang/c/decl/decl.h b/lang/c/decl/decl.h
@@ -98,4 +98,9 @@ DeclId decl_declare(DeclTable*, const Decl*);
const Decl* decl_get(const DeclTable*, DeclId);
ObjSymId decl_obj_sym(const DeclTable*, DeclId);
+/* Re-apply linkage-affecting attributes gathered from a later redeclaration or
+ * definition onto an already-declared symbol. Currently handles `weak`, which
+ * C allows on any declaration of a symbol. */
+void decl_apply_redecl_flags(DeclTable*, DeclId, u32 new_flags);
+
#endif
diff --git a/lang/c/parse/parse.c b/lang/c/parse/parse.c
@@ -1044,6 +1044,7 @@ static SymEntry* declare_function(Parser* p, Sym fname, const Type* fn_ty,
if (out_section_id) *out_section_id = tmp.section_id;
if (out_decl_flags) *out_decl_flags = tmp.flags;
if (out_alias_target) *out_alias_target = tmp.alias_target;
+ decl_apply_redecl_flags(p->decls, existing->decl_id, tmp.flags);
external_func_remember(p, fname, existing);
return existing;
}
@@ -1082,6 +1083,7 @@ static SymEntry* declare_function(Parser* p, Sym fname, const Type* fn_ty,
if (out_section_id) *out_section_id = tmp.section_id;
if (out_decl_flags) *out_decl_flags = tmp.flags;
if (out_alias_target) *out_alias_target = tmp.alias_target;
+ decl_apply_redecl_flags(p->decls, existing->decl_id, tmp.flags);
}
external_func_remember(p, fname, existing);
return existing;
diff --git a/lang/c/parse/parse_expr.c b/lang/c/parse/parse_expr.c
@@ -1105,7 +1105,7 @@ void coerce_top_to_lvalue(Parser* p) {
}
}
-static void coerce_top_to_type(Parser* p, const Type* dst) {
+void coerce_top_to_type(Parser* p, const Type* dst) {
const Type* src = cg_top_type(p->cg);
if (!src || !dst || src == dst) return;
if (type_is_arith(src) && type_is_arith(dst)) {
diff --git a/lang/c/parse/parse_priv.h b/lang/c/parse/parse_priv.h
@@ -523,6 +523,7 @@ int string_literal_initializes_array(Parser* p, const Type* elem, const Tok* t);
u8* decode_string_literal(Parser* p, const Tok* t, size_t* nlen_out);
void to_rvalue(Parser* p);
void coerce_top_to_lvalue(Parser* p);
+void coerce_top_to_type(Parser* p, const Type* dst);
CfreeCgSym emit_string_to_rodata(Parser* p, const u8* bytes, size_t n);
CfreeCgSym emit_string_literal_to_rodata(Parser* p, const u8* bytes,
size_t nbytes, const Type* elem_ty);
diff --git a/lang/c/parse/parse_stmt.c b/lang/c/parse/parse_stmt.c
@@ -180,6 +180,12 @@ static void parse_return_stmt(Parser* p) {
if (!chk.ok)
perr(p, "%.*s", CFREE_SLICE_ARG(cfree_slice_cstr(chk.message)));
}
+ /* Convert the value to the function return type, as `return` performs the
+ * equivalent of assignment to an object of the return type (ยง6.8.6.4).
+ * cg_ret expects the value already in the return type; without this a
+ * narrower value (e.g. a _Bool call result returned from an int function)
+ * would be reloaded at the wider return width and read adjacent bytes. */
+ coerce_top_to_type(p, p->cur_func_ret);
expect_punct(p, ';', "';' after return value");
cg_ret(p->cg, 1);
}
diff --git a/src/arch/aa64/native.c b/src/arch/aa64/native.c
@@ -53,6 +53,8 @@
#endif
enum {
+ AA_X8 = 8u, /* indirect-result (sret) register; usable as a copy base that
+ aa_copy_bytes (which scratches only x16/x17) never clobbers */
AA_TMP0 = 16u,
AA_TMP1 = 17u,
AA_FP = 29u,
@@ -2765,15 +2767,19 @@ static void aa_plan_ret(NativeTarget* t, const CGFuncDesc* fd,
if (nvalues) rets = arena_zarray(t->c->tu, NativeCallPlanRet, 4);
if (nvalues && abi && abi->ret.kind == ABI_ARG_INDIRECT) {
AANativeTarget* a = aa_of(t);
+ /* Hold the sret destination pointer in x8, not AA_TMP1: aa_copy_bytes
+ * materializes out-of-range source/dest frame offsets into AA_TMP1, which
+ * would clobber the destination base mid-copy (only triggered once a frame
+ * is large enough that the source offset escapes stur's signed-9 range). */
NativeLoc dstp =
- aa_reg_loc(builtin_id(CFREE_CG_BUILTIN_I64), NATIVE_REG_INT, AA_TMP1);
+ aa_reg_loc(builtin_id(CFREE_CG_BUILTIN_I64), NATIVE_REG_INT, AA_X8);
NativeLoc saved = aa_stack_loc(dstp.type, a->sret_ptr_slot, 0);
NativeAddr dst_addr, src_addr;
AggregateAccess access;
aa_load_part(t, dstp, saved, 0, 8);
memset(&dst_addr, 0, sizeof dst_addr);
dst_addr.base_kind = NATIVE_ADDR_BASE_REG;
- dst_addr.base.reg = AA_TMP1;
+ dst_addr.base.reg = AA_X8;
dst_addr.base_type = values[0].type;
src_addr = aa_loc_addr(a, values[0], 0);
src_addr.base_type = values[0].type;
diff --git a/src/cg/session.c b/src/cg/session.c
@@ -200,6 +200,13 @@ CfreeCgSym cfree_cg_decl(CfreeCg* g, CfreeCgDecl decl) {
sym = obj_symbol_ex(ob, (Sym)decl.linkage_name, api_map_bind(decl.sym.bind),
api_map_vis(decl.sym.visibility),
api_decl_sym_kind(decl), OBJ_SEC_NONE, 0, 0, 0);
+ } else if (decl.sym.bind == CFREE_SB_WEAK) {
+ /* C permits the `weak` attribute on any declaration of a symbol; a later
+ * weak (re)declaration demotes a previously-strong global to weak. Without
+ * this, a plain prototype followed by a `weak` definition would emit a
+ * strong global and collide with any strong override at link time. */
+ const ObjSym* s = obj_symbol_get(ob, sym);
+ if (s && s->bind == SB_GLOBAL) obj_symbol_set_bind(ob, sym, SB_WEAK);
}
if (decl.sym.flags) {
obj_symbol_set_flags(ob, sym, (u16)decl.sym.flags);
