diff --git a/README.md b/README.md index 6507d96819..9685bd2a8b 100644 --- a/README.md +++ b/README.md @@ -125,6 +125,24 @@ func main() { Because calls into C compile to native calls against the C ABI, there is no Go-to-C stack or scheduler transition, so frequent C calls stay cheap. +#### Promoted C variadic methods + +Forwarding an incoming C ellipsis through a promoted-method wrapper, including +an interface call, is currently supported only on hosted native targets without +an explicit `-target` profile. The underlying C method must have a pointer +receiver. The wrapper and callee must have matching fixed LLVM prototypes and +calling conventions, with no result conversion. Fixed parameters must be +scalars, with zero or one scalar result: booleans, integers, floating-point +values, and pointers qualify. + +Value receivers, aggregate fixed parameters or results, multiple results, +Wasm, and explicit `-target` profiles (including custom native profiles) remain +outside this support. Their existing wrapper lowering can silently drop the +incoming ellipsis; for example, a promoted value-receiver method called through +an interface still loses arguments at O0. Valid LLVM IR for an excluded wrapper +does not establish correct argument forwarding. Support and clear diagnostics +for these forms are tracked in [#2855](https://github.com/xgo-dev/llgo/issues/2855). + #### Windows API On Windows, bind APIs declared with `WINAPI` or `__stdcall` through the `stdcall.` namespace. The convention is distinct on 386; Windows amd64 and arm64 use their unified native C ABI. An explicitly decorated 386 name such as `_MessageBoxW@16` is also accepted and is normalized to `MessageBoxW` on 64-bit targets. diff --git a/cl/_testrt/vamethod/in.go b/cl/_testrt/vamethod/in.go index 10a6db6f30..03c6833ee8 100644 --- a/cl/_testrt/vamethod/in.go +++ b/cl/_testrt/vamethod/in.go @@ -67,7 +67,7 @@ func main() { // CHECK: br i1 %[[PTRNIL]], label %{{[^,]+}}, label %{{[^ ]+}} // CHECK: call void @"{{.*}}/runtime/internal/runtime.AssertNilDeref"(i1 true) // CHECK: %[[PTRFMT:[0-9]+]] = load ptr, ptr %[[PTRFIELD]], align 8 -// CHECK: %[[PTRRET:[0-9]+]] = call i32 (ptr, ...) @printf(ptr %[[PTRFMT]]) +// CHECK: %[[PTRRET:[0-9]+]] = {{(musttail )?}}call i32 (ptr, ...) @printf(ptr %[[PTRFMT]]{{(, \.\.\.)?}}) // CHECK: ret i32 %[[PTRRET]] // CHECK-LABEL: define void @"main.(*CFmt).SetFormat"( diff --git a/cl/instr.go b/cl/instr.go index dbc98dcb34..d5d778e69e 100644 --- a/cl/instr.go +++ b/cl/instr.go @@ -2665,7 +2665,13 @@ func (p *context) callEx(b llssa.Builder, act llssa.DoAction, call *ssa.CallComm p.inCFunc = true args := p.compileValues(b, args, kind) p.inCFunc = false - ret = p.emitDo(b, act, ds, mayRecover, aFn.Expr, llssa.Builder.Call, args...) + if act == llssa.Call && p.forwardCVariadicWrapper(call) && b.CanForwardVarargs(aFn.Expr) { + // A tail transfer removes the synthetic frame itself, so no + // recover-frame alias may surround it (or run after the call). + ret = b.CallForwardVarargs(aFn.Expr, args...) + } else { + ret = p.emitDo(b, act, ds, mayRecover, aFn.Expr, llssa.Builder.Call, args...) + } case goFunc: args := p.compileValues(b, args, kind) ret = p.emitDo(b, act, ds, mayRecover, aFn.Expr, llssa.Builder.Call, args...) @@ -2820,6 +2826,61 @@ func (p *context) callEx(b llssa.Builder, act llssa.DoAction, call *ssa.CallComm return } +func (p *context) forwardCVariadicWrapper(call *ssa.CallCommon) bool { + fn := p.goFn + callee := call.Signature() + // Promoted pointer methods adjust the receiver and return the C call's + // result. compileValues omits the incoming ellipsis marker: the wrapper + // must forward the unknown register/stack arguments with a mandatory tail + // call. Wasm does not support variadic tail calls. Explicit -target + // profiles (including custom native profiles) remain excluded because + // their backend and storage ABI may not support this forwarding path. + // Excluded variadic wrappers keep their existing lowering, which can + // silently drop the incoming ellipsis. Correct forwarding or diagnostics + // for these forms is tracked in #2855. + target := p.prog.Target() + if target.GOARCH == "wasm" || target.Target != "" { + return false + } + if fn == nil || fn.Synthetic == "" { + return false + } + // Value receivers do not have LLVM's required matching fixed prototype. + if fn.Signature.Recv() == nil || !isPointerGoType(fn.Signature.Recv().Type()) || + callee.Recv() == nil || !isPointerGoType(callee.Recv().Type()) { + return false + } + if !llssa.HasNameValist(fn.Signature) || !llssa.HasNameValist(callee) || + len(call.Args) == 0 || len(fn.Params) == 0 { + return false + } + // Tuple returns need extract/insert instructions after the call, while + // aggregate ABI lowering can replace the call and lose its tail marker. + // Neither path can preserve the immediate return required by musttail. + results := callee.Results() + if results.Len() > 1 || results.Len() == 1 && !isCVariadicForwardScalar(results.At(0).Type()) { + return false + } + params := callee.Params() + for i := 0; i < params.Len()-1; i++ { + if !isCVariadicForwardScalar(params.At(i).Type()) { + return false + } + } + param, ok := call.Args[len(call.Args)-1].(*ssa.Parameter) + return ok && param == fn.Params[len(fn.Params)-1] +} + +func isCVariadicForwardScalar(t types.Type) bool { + switch t := t.Underlying().(type) { + case *types.Pointer: + return true + case *types.Basic: + return t.Info()&(types.IsBoolean|types.IsInteger|types.IsFloat) != 0 || t.Kind() == types.UnsafePointer + } + return false +} + func (p *context) compileDynamicCallValues(b llssa.Builder, call *ssa.CallCommon, hasVArg int) []llssa.Expr { args := p.compileValues(b, call.Args, hasVArg) params := call.Signature().Params() diff --git a/cl/instr_variadic_method_test.go b/cl/instr_variadic_method_test.go new file mode 100644 index 0000000000..f0c9ad5e89 --- /dev/null +++ b/cl/instr_variadic_method_test.go @@ -0,0 +1,172 @@ +package cl + +import ( + "os" + "strings" + "testing" + + "github.com/xgo-dev/llgo/internal/cabi" + "github.com/xgo-dev/llgo/internal/optlevel" + llssa "github.com/xgo-dev/llgo/ssa" + "github.com/xgo-dev/llvm" +) + +func TestCVariadicMethodWrapperForwarding(t *testing.T) { + source, err := os.ReadFile("_testrt/vamethod/in.go") + if err != nil { + t.Fatal(err) + } + ssaPkg, _, files := buildGoSSAPkg(t, string(source)) + for _, target := range []llssa.Target{ + {GOOS: "linux", GOARCH: "amd64"}, + {GOOS: "linux", GOARCH: "arm64"}, + {GOOS: "linux", GOARCH: "386"}, + {GOOS: "linux", GOARCH: "arm"}, + {GOOS: "linux", GOARCH: "riscv64"}, + {GOOS: "darwin", GOARCH: "amd64"}, + {GOOS: "darwin", GOARCH: "arm64"}, + {GOOS: "windows", GOARCH: "amd64", LLVMTarget: "x86_64-pc-windows-msvc"}, + {GOOS: "windows", GOARCH: "amd64", LLVMTarget: "x86_64-w64-windows-gnu"}, + {GOOS: "windows", GOARCH: "arm64", LLVMTarget: "aarch64-pc-windows-msvc"}, + {GOOS: "windows", GOARCH: "arm64", LLVMTarget: "aarch64-w64-windows-gnu"}, + {GOOS: "windows", GOARCH: "386", LLVMTarget: "i686-pc-windows-msvc"}, + {GOOS: "windows", GOARCH: "386", LLVMTarget: "i686-w64-windows-gnu"}, + } { + t.Run(target.GOOS+"/"+target.GOARCH+"/"+target.LLVMTarget, func(t *testing.T) { + target.OptLevel = optlevel.O0 + prog := newLLSSAProgForTarget(t, &target) + defer prog.Dispose() + pkg, err := NewPackage(prog, ssaPkg, files) + if err != nil { + t.Fatal(err) + } + mod := pkg.Module() + cabi.NewTransformer(prog, target.LLVMTarget, "", false).TransformModule(pkg.Path(), mod) + fn := mustNamedFunction(t, mod, "main.(*CFmt).Printf") + if text := fn.String(); !strings.Contains(text, "musttail call i32 (ptr, ...) @printf(ptr ") || + !strings.Contains(text, ", ...)") { + t.Fatalf("wrapper does not forward its incoming ellipsis:\n%s", text) + } + if fn.GetStringAttributeAtIndex(-1, "thunk").IsNil() { + t.Fatal("variadic forwarding requires the LLVM thunk attribute") + } + if err := llvm.VerifyModule(mod, llvm.ReturnStatusAction); err != nil { + t.Fatal(err) + } + object, err := prog.TargetMachine().EmitToMemoryBuffer(mod, llvm.ObjectFile) + if err != nil { + t.Fatal(err) + } + object.Dispose() + }) + } +} + +func TestCVariadicMethodExternalDeclaration(t *testing.T) { + // An external declaration has no Go body to prove that it cannot recover. + // The forwarding thunk must still tail-call it without a post-call alias + // cleanup, and preserve its extra fixed arguments and void result. + const source = `package foo +type T int8 +//llgo:link (*T).Check C.check +func (*T) Check(tag int32, number float64, data *int8, __llgo_va_list ...any) +type Wrapper struct { *T } +func use(w *Wrapper) { w.Check(7, 1.5, new(int8), int32(42)) } +` + ssaPkg, _, files := buildGoSSAPkg(t, source) + prog := newLLSSAProgForTarget(t, &llssa.Target{GOOS: "linux", GOARCH: "amd64", OptLevel: optlevel.O0}) + defer prog.Dispose() + pkg, err := NewPackage(prog, ssaPkg, files) + if err != nil { + t.Fatal(err) + } + fn := mustNamedFunction(t, pkg.Module(), "foo.(*Wrapper).Check") + if text := fn.String(); !strings.Contains(text, "musttail call void (ptr, i32, double, ptr, ...) @check(") || + strings.Contains(text, "RecoverFrameAlias") { + t.Fatalf("external variadic declaration is not tail-forwarded:\n%s", text) + } + if err := llvm.VerifyModule(pkg.Module(), llvm.ReturnStatusAction); err != nil { + t.Fatal(err) + } +} + +func TestCVariadicValueReceiverWrapperIR(t *testing.T) { + const source = `package foo +type T int32 +//llgo:link T.Check C.check +func (T) Check(__llgo_va_list ...any) int32 { return 0 } +type Wrapper struct { T } +func use(w *Wrapper) int32 { return w.Check(int32(42)) } +` + ssaPkg, _, files := buildGoSSAPkg(t, source) + prog := newLLSSAProgForTarget(t, &llssa.Target{GOOS: "linux", GOARCH: "amd64"}) + defer prog.Dispose() + pkg, err := NewPackage(prog, ssaPkg, files) + if err != nil { + t.Fatal(err) + } + // Loading a scalar receiver changes the fixed ABI from ptr to i32. Such + // wrappers cannot use LLVM's variadic musttail forwarding mechanism. + // This checks only valid IR and exclusion from the forwarding path; the + // ordinary call can still lose incoming arguments through an interface. + // Correct forwarding or diagnostics is tracked in #2855. + fn := mustNamedFunction(t, pkg.Module(), "foo.(*Wrapper).Check") + if text := fn.String(); !strings.Contains(text, "call i32 (i32, ...) @check(") || strings.Contains(text, "musttail") { + t.Fatalf("excluded receiver ABI unexpectedly changed lowering:\n%s", text) + } + if err := llvm.VerifyModule(pkg.Module(), llvm.ReturnStatusAction); err != nil { + t.Fatal(err) + } +} + +func TestCVariadicIncompatibleSignatureIR(t *testing.T) { + // Ordinary methods have no ellipsis to forward. Aggregate signatures need + // instructions or ABI rewrites that cannot retain a musttail call followed + // immediately by its return. These checks establish valid IR with the + // existing lowering, not correct ellipsis forwarding for excluded variadic + // signatures. Execution coverage and diagnostics are tracked in #2855. + for _, tc := range []struct { + name, declaration, use string + }{ + {"non_variadic", + "func (*T) Check(value int32) int32 { return 0 }", + "func use(w *Wrapper) int32 { return w.Check(42) }"}, + {"multiple_results", + "func (*T) Check(__llgo_va_list ...any) (int32, int32) { return 0, 0 }", + "func use(w *Wrapper) (int32, int32) { return w.Check(int32(42)) }"}, + {"aggregate_result", + "func (*T) Check(__llgo_va_list ...any) R { return R{} }", + "func use(w *Wrapper) R { return w.Check(int32(42)) }"}, + {"aggregate_parameter", + "func (*T) Check(tag R, __llgo_va_list ...any) int32 { return 0 }", + "func use(w *Wrapper) int32 { return w.Check(R{}, int32(42)) }"}, + } { + t.Run(tc.name, func(t *testing.T) { + source := "package foo\ntype T int8\ntype R struct { a, b int64 }\n" + + "//llgo:link (*T).Check C.check\n" + tc.declaration + + "\ntype Wrapper struct { *T }\n" + tc.use + ssaPkg, _, files := buildGoSSAPkg(t, source) + prog := newLLSSAProgForTarget(t, &llssa.Target{GOOS: "linux", GOARCH: "amd64"}) + defer prog.Dispose() + pkg, err := NewPackage(prog, ssaPkg, files) + if err != nil { + t.Fatal(err) + } + mod := pkg.Module() + fn := mustNamedFunction(t, mod, "foo.(*Wrapper).Check") + if text := fn.String(); !strings.Contains(text, "@check(") || strings.Contains(text, "musttail") { + t.Fatalf("excluded wrapper unexpectedly changed lowering:\n%s", text) + } + if !fn.GetStringAttributeAtIndex(-1, "thunk").IsNil() { + t.Fatal("incompatible wrapper must not be marked as a forwarding thunk") + } + if err := llvm.VerifyModule(mod, llvm.ReturnStatusAction); err != nil { + t.Fatal(err) + } + cabi.NewTransformer(prog, "", "", false).TransformModule(pkg.Path(), mod) + if err := llvm.VerifyModule(mod, llvm.ReturnStatusAction); err != nil { + t.Fatal(err) + } + }) + } +} diff --git a/internal/build/testdata/variadicmethod/_wrap/check.c b/internal/build/testdata/variadicmethod/_wrap/check.c new file mode 100644 index 0000000000..6845e0d2bf --- /dev/null +++ b/internal/build/testdata/variadicmethod/_wrap/check.c @@ -0,0 +1,18 @@ +#include +#include + +int32_t variadic_method_check(const int32_t *marker, ...) { + if (*marker == 0) return 42; + if (*marker != 7) return -1; + va_list args; + va_start(args, marker); + int ok = 1; + for (int i = 0; i < 10; i++) { + if (va_arg(args, int) != 100 + i) ok = 0; + } + for (int i = 0; i < 10; i++) { + if (va_arg(args, double) != i + 0.25) ok = 0; + } + va_end(args); + return ok ? 42 : -1; +} diff --git a/internal/build/testdata/variadicmethod/main.go b/internal/build/testdata/variadicmethod/main.go new file mode 100644 index 0000000000..b5b0a3f4d0 --- /dev/null +++ b/internal/build/testdata/variadicmethod/main.go @@ -0,0 +1,69 @@ +package main + +import _ "unsafe" + +const LLGoFiles = "_wrap/check.c" + +type Receiver int32 + +//llgo:link (*Receiver).Check C.variadic_method_check +func (*Receiver) Check(__llgo_va_list ...any) int32 { return 0 } + +type Embedded struct { + padding [3]int64 + *Receiver +} + +type Nested struct { + padding int64 + *Embedded +} + +type Checker interface { + Check(__llgo_va_list ...any) int32 +} + +//go:noinline +func check(value Checker) { + // More arguments than the native GP/FP register sets can hold: both + // register and stack ellipsis arguments must survive receiver adjustment. + if value.Check( + int32(100), int32(101), int32(102), int32(103), int32(104), + int32(105), int32(106), int32(107), int32(108), int32(109), + 0.25, 1.25, 2.25, 3.25, 4.25, 5.25, 6.25, 7.25, 8.25, 9.25, + ) != 42 { + panic("variadic method arguments were corrupted") + } +} + +//go:noinline +func empty(value Checker) { + if value.Check() != 42 { + panic("empty variadic method call failed") + } +} + +func checkNil(value Checker) { + defer func() { + if recover() == nil { + panic("nil promoted receiver did not panic") + } + }() + check(value) +} + +func main() { + marker := Receiver(7) + embedded := Embedded{Receiver: &marker} + nested := Nested{Embedded: &embedded} + check(&embedded) + check(embedded) + check(&nested) + check(nested) + marker = 0 + empty(&embedded) + empty(&nested) + checkNil((*Embedded)(nil)) + checkNil(&Nested{}) + println("variadic-method-ok") +} diff --git a/internal/build/variadic_method_test.go b/internal/build/variadic_method_test.go new file mode 100644 index 0000000000..b0b5058864 --- /dev/null +++ b/internal/build/variadic_method_test.go @@ -0,0 +1,36 @@ +package build + +import ( + "path/filepath" + "strings" + "testing" + + "github.com/xgo-dev/llgo/internal/optlevel" +) + +func TestCVariadicPromotedMethod(t *testing.T) { + for _, level := range []optlevel.Level{optlevel.O0, optlevel.O2} { + for _, fixture := range []struct { + path string + want string + }{ + {"../../cl/_testrt/vamethod", "hello (100)\n(200) world\nifmt (100,200)\n"}, + {"./testdata/variadicmethod", "variadic-method-ok\n"}, + } { + t.Run(level.String()+"/"+filepath.Base(fixture.path), func(t *testing.T) { + conf := NewDefaultConf(ModeBuild) + conf.OptLevel = level + // The independent DWARF parameter-index bug is tracked by #2837. + conf.LinkOptions.DWARF = DWARFOmit + bin := filepath.Join(t.TempDir(), "variadic-method"+defaultAppExt(conf)) + conf.OutFile = bin + if _, err := Do([]string{fixture.path}, conf); err != nil { + t.Fatal(err) + } + if got := strings.ReplaceAll(runBinary(t, bin), "\r\n", "\n"); got != fixture.want { + t.Fatalf("output = %q, want %q", got, fixture.want) + } + }) + } + } +} diff --git a/ssa/expr.go b/ssa/expr.go index 3d80a9dfd3..06ecafc62b 100644 --- a/ssa/expr.go +++ b/ssa/expr.go @@ -1485,6 +1485,35 @@ func (b Builder) InlineCall(fn Expr, args ...Expr) (ret Expr) { return b.Call(fn, args...) } +// CanForwardVarargs checks the unlowered ABI of a direct variadic forwarding +// call. The physical result must equal the logical Go result so Call cannot +// insert a result conversion between the tail call and its return. +func (b Builder) CanForwardVarargs(fn Expr) bool { + if fn.kind != vkFuncDecl || !b.Func.hasVArg || fn.ll != b.Func.ll || + fn.impl.FunctionCallConv() != b.Func.impl.FunctionCallConv() { + return false + } + sig := fn.raw.Type.(*types.Signature) + return fn.ll.ReturnType() == b.Prog.retType(sig).ll +} + +// CallForwardVarargs forwards a variadic thunk's incoming ellipsis unchanged. +// CanForwardVarargs must hold, the caller must return the result immediately, +// and later ABI lowering must preserve the call and return without rewrites. +func (b Builder) CallForwardVarargs(fn Expr, args ...Expr) Expr { + if !b.CanForwardVarargs(fn) { + panic(fmt.Sprintf("cannot forward C variadic arguments from %s to %s: incompatible ABI", + b.Func.Name(), fn.impl.Name())) + } + ret := b.Call(fn, args...) + // The preflight result check guarantees Call returns its raw instruction. + // LLVM's thunk attribute makes musttail forward the incoming ellipsis, + // including arguments that have no explicit operands in the call IR. + b.Func.impl.AddFunctionAttr(b.Prog.ctx.CreateStringAttribute("thunk", "")) + ret.impl.SetTailCallKind(llvm.TailCallKindMustTail) + return ret +} + // The Call instruction represents a function call. // // The Call instruction yields the function result if there is exactly diff --git a/ssa/variadic_forward_test.go b/ssa/variadic_forward_test.go new file mode 100644 index 0000000000..63a1b84fad --- /dev/null +++ b/ssa/variadic_forward_test.go @@ -0,0 +1,66 @@ +package ssa + +import ( + "fmt" + "go/token" + "go/types" + "strings" + "testing" + + "github.com/xgo-dev/llvm" +) + +func TestCVariadicForwardingRejectsResultConversion(t *testing.T) { + // Hosted wasm32 stores Go uintptr in 64 bits but returns C uintptr in + // 32 bits. Even matching native prototypes cannot tail-forward through + // Call's result widening. Check this independently of the cl target gate. + prog := NewProgram(&Target{GOOS: "wasip1", GOARCH: "wasm", WasmProfile: "w32"}) + defer prog.Dispose() + pkg := prog.NewPackage("forward", "forward") + sig := types.NewSignatureType(nil, nil, nil, types.NewTuple(VArg()), + types.NewTuple(types.NewVar(token.NoPos, nil, "", types.Typ[types.Uintptr])), true) + callee := pkg.NewFunc("native", sig, CDecl) + caller := pkg.NewFunc("wrapper", sig, CDecl) + if callee.ll != caller.ll || callee.ll.ReturnType() == prog.retType(sig).ll { + t.Fatal("fixture must have matching prototypes but require result conversion") + } + b := caller.MakeBody(1) + if b.CanForwardVarargs(callee.Expr) { + t.Fatal("result conversion must prevent variadic tail forwarding") + } + defer func() { + message := fmt.Sprint(recover()) + if !strings.Contains(message, "from wrapper to native: incompatible ABI") { + t.Fatalf("unexpected diagnostic: %s", message) + } + if !caller.impl.EntryBasicBlock().FirstInstruction().IsNil() { + t.Fatal("ABI rejection must happen before emitting the call or conversion") + } + b.Unreachable() + if err := llvm.VerifyModule(pkg.Module(), llvm.ReturnStatusAction); err != nil { + t.Fatal(err) + } + }() + b.CallForwardVarargs(callee.Expr) +} + +func TestCVariadicForwardingRejectsDifferentPrototype(t *testing.T) { + prog := NewProgram(&Target{GOOS: "linux", GOARCH: "amd64"}) + defer prog.Dispose() + pkg := prog.NewPackage("forward", "forward") + signature := func(fixed types.Type) *types.Signature { + return types.NewSignatureType(nil, nil, nil, types.NewTuple( + types.NewVar(token.NoPos, nil, "fixed", fixed), VArg(), + ), nil, true) + } + callee := pkg.NewFunc("native", signature(types.Typ[types.Int32]), CDecl) + caller := pkg.NewFunc("wrapper", signature(types.Typ[types.Float64]), CDecl) + b := caller.MakeBody(1) + if b.CanForwardVarargs(callee.Expr) { + t.Fatal("different fixed-argument ABIs must prevent variadic forwarding") + } + b.Return() + if err := llvm.VerifyModule(pkg.Module(), llvm.ReturnStatusAction); err != nil { + t.Fatal(err) + } +}