From 40fd892cf5da51b7b07d124e74a30e5886cf5729 Mon Sep 17 00:00:00 2001 From: kalsprite Date: Mon, 24 Aug 2026 00:27:26 -0700 Subject: [PATCH] Reject a named asm template used as a value instead of aborting the compiler --- src/check_decl.cpp | 3 ++ src/check_expr.cpp | 16 +++++++ src/checker.hpp | 1 + tests/issues/run.bat | 1 + tests/issues/run.sh | 10 ++++ .../test_issue_asm_template_as_value.odin | 48 +++++++++++++++++++ 6 files changed, 79 insertions(+) create mode 100644 tests/issues/test_issue_asm_template_as_value.odin diff --git a/src/check_decl.cpp b/src/check_decl.cpp index 7bfc22d5e..35f0da9ba 100644 --- a/src/check_decl.cpp +++ b/src/check_decl.cpp @@ -2024,11 +2024,14 @@ gb_internal void check_asm_group_decl(CheckerContext *ctx, Entity *asm_entity, D arg = arg->BinaryExpr.left; } + Ast *prev_hint = ctx->asm_template_hint; + ctx->asm_template_hint = arg; if (arg->kind == Ast_Ident) { e = check_ident(ctx, &o, arg, nullptr, nullptr, true); } else if (arg->kind == Ast_SelectorExpr) { e = check_selector(ctx, &o, arg, nullptr); } + ctx->asm_template_hint = prev_hint; if (e == nullptr) { error(arg, "Expected a valid entity name in asm template group, got %.*s", LIT(ast_strings[arg->kind])); continue; diff --git a/src/check_expr.cpp b/src/check_expr.cpp index 3562a850c..091ebdac2 100644 --- a/src/check_expr.cpp +++ b/src/check_expr.cpp @@ -2097,6 +2097,12 @@ gb_internal Entity *check_ident(CheckerContext *c, Operand *o, Ast *n, Type *nam break; case Entity_AsmTemplate: + if (c->asm_template_hint != n) { + error(n, "'asm' templates must either be defined as a declaration or within a procedure call directly"); + o->mode = Addressing_Invalid; + o->type = t_invalid; + return e; + } o->mode = Addressing_Value; break; @@ -6373,6 +6379,12 @@ gb_internal Entity *check_selector(CheckerContext *c, Operand *operand, Ast *nod break; case Entity_AsmTemplate: + if (c->asm_template_hint != node) { + error(node, "'asm' templates must either be defined as a declaration or within a procedure call directly"); + operand->mode = Addressing_Invalid; + operand->type = t_invalid; + return entity; + } operand->mode = Addressing_Value; break; } @@ -8931,7 +8943,11 @@ gb_internal ExprKind check_call_expr(CheckerContext *c, Operand *operand, Ast *c operand->expr = proc; add_type_and_value(c, proc, operand->mode, operand->type, operand->value); } else { + // the callee is the one position where an asm template is allowed to produce a value + Ast *prev_hint = c->asm_template_hint; + c->asm_template_hint = unnested_proc; check_expr_or_type(c, operand, proc); + c->asm_template_hint = prev_hint; } } else { GB_ASSERT(operand->expr != nullptr); diff --git a/src/checker.hpp b/src/checker.hpp index bdfd2c9c3..89b6318d7 100644 --- a/src/checker.hpp +++ b/src/checker.hpp @@ -839,6 +839,7 @@ struct CheckerContext { Scope * polymorphic_scope; Ast *assignment_lhs_hint; + Ast *asm_template_hint; }; gb_internal u64 check_vet_flags(CheckerContext *c); diff --git a/tests/issues/run.bat b/tests/issues/run.bat index 4cbb424b4..c049f4133 100644 --- a/tests/issues/run.bat +++ b/tests/issues/run.bat @@ -46,6 +46,7 @@ set COMMON=-define:ODIN_TEST_FANCY=false -file -vet -strict-style -ignore-unused ..\..\..\odin check ..\test_issue_ellipsis_type_call.odin -no-entry-point %COMMON% 2>&1 | find /c "Error:" | findstr /x "10" || exit /b ..\..\..\odin check ..\test_issue_foreign_redeclaration.odin -no-entry-point %COMMON% || exit /b ..\..\..\odin check ..\test_issue_foreign_redeclaration_mismatch.odin -no-entry-point %COMMON% 2>&1 | find /c "Error:" | findstr /x "1" || exit /b +..\..\..\odin check ..\test_issue_asm_template_as_value.odin -no-entry-point %COMMON% 2>&1 | find /c "Error:" | findstr /x "10" || exit /b ..\..\..\odin build ..\test_issue_7037.odin %COMMON% -o:none || exit /b ..\..\..\odin build ..\test_issue_7188.odin %COMMON% || exit /b clang -c ..\test_issue_sysv_abi.c -o test_issue_sysv_abi_c.o || exit /b diff --git a/tests/issues/run.sh b/tests/issues/run.sh index 98c144d9c..a02921c67 100755 --- a/tests/issues/run.sh +++ b/tests/issues/run.sh @@ -110,6 +110,16 @@ else exit 1 fi +# `asm` templates are amd64-only, so this file is empty on every other architecture +if [[ "$(uname -m)" == "x86_64" || "$(uname -m)" == "amd64" ]]; then + if [[ $($ODIN check ../test_issue_asm_template_as_value.odin -no-entry-point $COMMON_CHECK 2>&1 >/dev/null | grep -c "Error:") -eq 10 ]]; then + echo "SUCCESSFUL 1/1" + else + echo "SUCCESSFUL 0/1" + exit 1 + fi +fi + if [[ $($ODIN build ../test_issue_7108.odin $COMMON 2>&1 >/dev/null | grep -c "Error:") -eq 2 ]]; then echo "SUCCESSFUL 1/1" else diff --git a/tests/issues/test_issue_asm_template_as_value.odin b/tests/issues/test_issue_asm_template_as_value.odin new file mode 100644 index 000000000..2062fa3c7 --- /dev/null +++ b/tests/issues/test_issue_asm_template_as_value.odin @@ -0,0 +1,48 @@ +#+build amd64 +// A named asm template got a plain `Addressing_Value`, so every value gate let it through: +// a cast, a transmute, an `auto_cast`, a blank assignment, a polymorphic parameter and a +// comparison against `nil` all passed the checker and then aborted the compiler in the +// backend, which has no value to lower for a template. Only a direct call and a listing in +// an `asm` group are legal. +package test_issues + +t :: asm(a: i32) -> (v: i32) { mov v, a; } + +a32 :: asm(a: i32) -> (v: i32) { mov v, a; } +a64 :: asm(a: i64) -> (v: i64) { mov v, a; } +g :: asm { a32, a64 } + +G := cast(rawptr)(t) + +take_rawptr :: proc(p: rawptr) { + _ = p +} + +poly :: proc(x: $T) { + _ = size_of(T) +} + +bad :: proc() { + _ = cast(proc "c" (i32) -> i32)(t) + _ = transmute(proc "c" (i32) -> i32)(t) + _ = cast(rawptr)(t) + _ = transmute(uintptr)(t) + take_rawptr(auto_cast t) + take_rawptr(cast(rawptr)(t)) + _ = t + poly(t) + if t == nil { + take_rawptr(nil) + } +} + +// these forms must remain valid +good :: proc() -> i32 { + x := t(1) + y := (t)(2) + z := g(i32(3)) + w := g(i64(4)) + v := asm(a: i32) -> (v: i32) { mov v, a; }(5) + take_rawptr(G) + return x + y + z + i32(w) + v +}