diff --git a/cl/_testlto/_globaldce_reflect_method_by_name_ltoplugin_string_abi_unknown/in.go b/cl/_testlto/_globaldce_reflect_method_by_name_ltoplugin_string_abi_unknown/in.go new file mode 100644 index 0000000000..55c305178c --- /dev/null +++ b/cl/_testlto/_globaldce_reflect_method_by_name_ltoplugin_string_abi_unknown/in.go @@ -0,0 +1,25 @@ +package main + +import ( + "os" + "reflect" +) + +type Unknown struct{} + +//go:noinline +func (Unknown) UnknownA() {} + +//go:noinline +func (Unknown) UnknownB() {} + +func unknownName() string { + if len(os.Args) == 0 { + return "UnknownA" + } + return os.Args[0] +} + +func main() { + _, _ = reflect.TypeOf(Unknown{}).MethodByName(unknownName()) +} diff --git a/cl/_testlto/globaldce_reflect_method_by_name_ltoplugin_string_abi/expect.txt b/cl/_testlto/globaldce_reflect_method_by_name_ltoplugin_string_abi/expect.txt new file mode 100644 index 0000000000..53c3919d14 --- /dev/null +++ b/cl/_testlto/globaldce_reflect_method_by_name_ltoplugin_string_abi/expect.txt @@ -0,0 +1,4 @@ +direct +concat +slice +forward diff --git a/cl/_testlto/globaldce_reflect_method_by_name_ltoplugin_string_abi/in.go b/cl/_testlto/globaldce_reflect_method_by_name_ltoplugin_string_abi/in.go new file mode 100644 index 0000000000..a0121e93b7 --- /dev/null +++ b/cl/_testlto/globaldce_reflect_method_by_name_ltoplugin_string_abi/in.go @@ -0,0 +1,53 @@ +// LITTEST +package main + +import ( + "reflect" +) + +// SYMBOL-NOT: globaldce_reflect_method_by_name_ltoplugin_string_abi{{.*}}Known{{.*}}Drop +// SYMBOL-DAG: globaldce_reflect_method_by_name_ltoplugin_string_abi{{.*}}Known{{.*}}Direct +// SYMBOL-DAG: globaldce_reflect_method_by_name_ltoplugin_string_abi{{.*}}Known{{.*}}Concat +// SYMBOL-DAG: globaldce_reflect_method_by_name_ltoplugin_string_abi{{.*}}Known{{.*}}Slice +// SYMBOL-DAG: globaldce_reflect_method_by_name_ltoplugin_string_abi{{.*}}Known{{.*}}Forward +// SYMBOL-NOT: globaldce_reflect_method_by_name_ltoplugin_string_abi{{.*}}Known{{.*}}Drop + +type Known struct{} + +//go:noinline +func (Known) Direct() string { return "direct" } + +//go:noinline +func (Known) Concat() string { return "concat" } + +//go:noinline +func (Known) Slice() string { return "slice" } + +//go:noinline +func (Known) Forward() string { return "forward" } + +//go:noinline +func (Known) Drop() string { panic("unreachable") } + +func callForward(name string) string { + out := reflect.ValueOf(Known{}).MethodByName(name).Call(nil) + return out[0].String() +} + +func callConcat(prefix, suffix string) string { + out := reflect.ValueOf(Known{}).MethodByName(prefix + suffix).Call(nil) + return out[0].String() +} + +func callSlice(source string) string { + out := reflect.ValueOf(Known{}).MethodByName(source[2:7]).Call(nil) + return out[0].String() +} + +func main() { + v := reflect.ValueOf(Known{}) + println(v.MethodByName("Direct").Call(nil)[0].String()) + println(callConcat("Con", "cat")) + println(callSlice("__Slice__")) + println(callForward("Forward")) +} diff --git a/cl/compile_test.go b/cl/compile_test.go index 108cbb7353..1b9b43b00f 100644 --- a/cl/compile_test.go +++ b/cl/compile_test.go @@ -21,6 +21,8 @@ package cl_test import ( "os" + "os/exec" + "path/filepath" "runtime" "strings" "testing" @@ -28,8 +30,10 @@ import ( "github.com/goplus/llgo/cl/cltest" "github.com/goplus/llgo/internal/build" "github.com/goplus/llgo/internal/buildenv" + "github.com/goplus/llgo/internal/cabi" "github.com/goplus/llgo/internal/llgen" "github.com/goplus/llgo/internal/lto" + llvmenv "github.com/goplus/llgo/xtool/env/llvm" ) func testCompile(t *testing.T, src, expected string) { @@ -187,6 +191,7 @@ func TestRunAndTestFromTestlto(t *testing.T) { "./_testlto/globaldce_reflect_method_by_name_ltoplugin_param", "./_testlto/globaldce_reflect_method_by_name_ltoplugin_range_literal", "./_testlto/globaldce_reflect_method_by_name_ltoplugin_slice", + "./_testlto/globaldce_reflect_method_by_name_ltoplugin_string_abi", "./_testlto/globaldce_reflect_method_by_name_ltoplugin_switch", } if !buildenv.Dev { @@ -206,6 +211,15 @@ func TestRunAndTestFromTestlto(t *testing.T) { cltest.RunAndTestFromDir(t, "", "./_testlto", ignore, cltest.WithRunConfig(conf)) } +func TestRunAndTestFromTestltoDWARF(t *testing.T) { + t.Setenv("LLGO_BUILD_CACHE", "off") + conf := build.NewDefaultConf(build.ModeRun) + conf.LTO = lto.Full + conf.LinkOptions.DWARF = build.DWARFPreserve + cltest.RunAndTestFromDir(t, "reflectmk_runtime", "./_testlto", nil, + cltest.WithRunConfig(conf), cltest.WithIRCheck(false)) +} + var testltoSymbolChecks = []string{ "globaldce_interface_matrix", "globaldce_interface_slots", @@ -225,9 +239,17 @@ var testltoLTOPluginTests = []string{ "globaldce_reflect_method_by_name_ltoplugin_param", "globaldce_reflect_method_by_name_ltoplugin_range_literal", "globaldce_reflect_method_by_name_ltoplugin_slice", + "globaldce_reflect_method_by_name_ltoplugin_string_abi", "globaldce_reflect_method_by_name_ltoplugin_switch", } +var testltoLTOPluginDWARFTests = []string{ + "globaldce_reflect_method_by_name_ltoplugin_concat", + "globaldce_reflect_method_by_name_ltoplugin_global_slice", + "globaldce_reflect_method_by_name_ltoplugin_param", + "globaldce_reflect_method_by_name_ltoplugin_slice", +} + func TestBuildAndCheckSymbolsFromTestlto(t *testing.T) { if !buildenv.Dev { t.Skip("globaldce symbol checks require dev build") @@ -260,6 +282,14 @@ func TestRunAndTestFromTestltoLTOPlugin(t *testing.T) { ) } +func TestRunAndTestFromTestltoLTOPluginDWARF(t *testing.T) { + t.Setenv("LLGO_BUILD_CACHE", "off") + conf := testltoLTOPluginConf(t, build.ModeRun) + conf.LinkOptions.DWARF = build.DWARFPreserve + cltest.RunAndTestFromDir(t, "ltoplugin_switch", "./_testlto", nil, + cltest.WithRunConfig(conf), cltest.WithIRCheck(false)) +} + func TestBuildAndCheckSymbolsFromTestltoLTOPlugin(t *testing.T) { buildConf := testltoLTOPluginConf(t, build.ModeBuild) cltest.BuildAndCheckSymbolsFromDir(t, "", "./_testlto", testltoLTOPluginTests, @@ -267,6 +297,69 @@ func TestBuildAndCheckSymbolsFromTestltoLTOPlugin(t *testing.T) { ) } +func runTestltoLTOPluginAggregateABI(t *testing.T, fixture string) string { + t.Helper() + conf := testltoLTOPluginConf(t, build.ModeGen) + // Apply a fixed LP64 ABI below so the aggregate form is tested on every host. + conf.AbiMode = cabi.ModeNone + plugin := conf.LTOPlugin.Path + pkgs, err := build.Do([]string{fixture}, conf) + if err != nil { + t.Fatalf("generate aggregate string module: %v", err) + } + if len(pkgs) != 1 { + t.Fatalf("generate aggregate string module: got %d packages", len(pkgs)) + } + cabi.NewTransformer(pkgs[0].LPkg.Prog, "arm64-unknown-linux", "", cabi.ModeAllFunc, true). + TransformModule(pkgs[0].PkgPath, pkgs[0].LPkg.Module()) + aggregateIR := pkgs[0].LPkg.String() + pkgs[0].LPkg.Prog.Dispose() + if !strings.Contains(aggregateIR, `runtime.String" "llgo.reflect.methodbyname.name"`) { + t.Fatalf("MethodByName string argument was not captured in aggregate form:\n%s", aggregateIR) + } + + opt := filepath.Join(llvmenv.New("").BinDir(), "opt") + cmd := exec.Command(opt, "-load-pass-plugin="+plugin, + "-passes=llgo-lto-pre-globaldce", "-S", "-o", "-") + cmd.Stdin = strings.NewReader(aggregateIR) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("run LTO plugin for aggregate ABI: %v\n%s", err, out) + } + return string(out) +} + +func TestBuildAndCheckSymbolsFromTestltoLTOPluginAggregateABI(t *testing.T) { + result := runTestltoLTOPluginAggregateABI(t, + "./_testlto/globaldce_reflect_method_by_name_ltoplugin_string_abi") + for _, name := range []string{"Direct", "Concat", "Slice", "Forward"} { + marker := `metadata !"go.method.value.reflect.` + name + `"` + if !strings.Contains(result, marker) { + t.Fatalf("aggregate ABI output missing %s\n%s", marker, result) + } + } + if strings.Contains(result, `metadata !"go.method.value.reflect"`) { + t.Fatalf("aggregate ABI output retained the generic value marker\n%s", result) + } + + // A generic marker conservatively retains every matching method, so test + // unknown-name fallback in a separate module from the Drop symbol check. + unknownResult := runTestltoLTOPluginAggregateABI(t, + "./_testlto/_globaldce_reflect_method_by_name_ltoplugin_string_abi_unknown") + if !strings.Contains(unknownResult, `metadata !"go.method.type.reflect"`) { + t.Fatalf("aggregate ABI output lost the unknown-name type marker\n%s", unknownResult) + } +} + +func TestBuildAndCheckSymbolsFromTestltoLTOPluginDWARF(t *testing.T) { + t.Setenv("LLGO_BUILD_CACHE", "off") + buildConf := testltoLTOPluginConf(t, build.ModeBuild) + buildConf.LinkOptions.DWARF = build.DWARFPreserve + cltest.BuildAndCheckSymbolsFromDir(t, "", "./_testlto", testltoLTOPluginDWARFTests, + cltest.WithRunConfig(buildConf), + ) +} + func TestFilterEmulatorOutput(t *testing.T) { tests := []struct { name string diff --git a/cl/rewrite_internal_test.go b/cl/rewrite_internal_test.go index 325f062078..ae09d45065 100644 --- a/cl/rewrite_internal_test.go +++ b/cl/rewrite_internal_test.go @@ -26,6 +26,10 @@ func init() { } func compileWithRewrites(t *testing.T, src string, rewrites map[string]string) string { + return compileWithRewritesMode(t, src, rewrites, ssa.SanityCheckFunctions|ssa.InstantiateGenerics) +} + +func compileWithRewritesMode(t *testing.T, src string, rewrites map[string]string, mode ssa.BuilderMode) string { t.Helper() fset := token.NewFileSet() file, err := parser.ParseFile(fset, "rewrite.go", src, parser.ParseComments) @@ -33,7 +37,6 @@ func compileWithRewrites(t *testing.T, src string, rewrites map[string]string) s t.Fatalf("parse failed: %v", err) } importer := gpackages.NewImporter(fset) - mode := ssa.SanityCheckFunctions | ssa.InstantiateGenerics pkg, _, err := ssautil.BuildPackage(&types.Config{Importer: importer}, fset, types.NewPackage(file.Name.Name, file.Name.Name), []*ast.File{file}, mode) if err != nil { @@ -154,6 +157,75 @@ func Use() callbackType { return CallbackTypes[1] } } } +func TestStaticGlobalSliceLiteralInitWithDebugRefs(t *testing.T) { + const src = `package staticinit + +var CallbackTypes = []string{"BeforeCreate", "AfterCreate"} + +func Use() string { return CallbackTypes[1] } +` + ir := compileWithRewritesMode(t, src, nil, + ssa.SanityCheckFunctions|ssa.InstantiateGenerics|ssa.GlobalDebug) + for _, want := range []string{ + `@"staticinit.CallbackTypes$data" = global [2 x %"github.com/goplus/llgo/runtime/internal/runtime.String"]`, + `@staticinit.CallbackTypes = global %"github.com/goplus/llgo/runtime/internal/runtime.Slice" { ptr @"staticinit.CallbackTypes$data", i64 2, i64 2 }`, + `c"BeforeCreate"`, + `c"AfterCreate"`, + } { + if !strings.Contains(ir, want) { + t.Fatalf("missing static slice initializer %q with debug refs:\n%s", want, ir) + } + } + assertNoStoreToGlobal(t, ir, "@staticinit.CallbackTypes") + if strings.Contains(ir, "runtime.AllocZ") { + t.Fatalf("static slice initializer allocates at runtime with debug refs:\n%s", ir) + } +} + +func TestStaticSliceInitRejectsExecutableReferrers(t *testing.T) { + const src = `package foo + +var Values []int + +func useSlice([]int) {} +func usePointer(*int) {} + +func sliceUser() { + backing := [2]int{1, 2} + values := backing[:] + Values = values + useSlice(values) +} + +func elementUser() { + var backing [2]int + elem := &backing[0] + *elem = 1 + usePointer(elem) + Values = backing[:] +} +` + ssapkg := buildSSAPackage(t, src) + global := ssapkg.Members["Values"].(*ssa.Global) + for _, name := range []string{"sliceUser", "elementUser"} { + fn := ssapkg.Func(name) + var globalStore *ssa.Store + for _, block := range fn.Blocks { + for _, instr := range block.Instrs { + if store, ok := instr.(*ssa.Store); ok && store.Addr == global { + globalStore = store + } + } + } + if globalStore == nil { + t.Fatalf("%s: store to Values not found", name) + } + if _, ok := staticSliceInitOf(globalStore); ok { + t.Fatalf("%s: static slice init accepted an executable referrer", name) + } + } +} + func TestStaticGlobalZeroSizedSliceLiteralFallsBack(t *testing.T) { const src = `package staticinit diff --git a/cl/static_init.go b/cl/static_init.go index bbbe94cfb4..7877f591e6 100644 --- a/cl/static_init.go +++ b/cl/static_init.go @@ -204,16 +204,16 @@ func staticSliceInitOf(store *ssa.Store) (*staticSliceInit, bool) { values: make(map[int]*ssa.Const), instrs: []ssa.Instruction{alloc, slice, store}, } - sliceRefs := slice.Referrers() - if sliceRefs == nil || len(*sliceRefs) != 1 || (*sliceRefs)[0] != store { + sliceRefs, ok := nonDebugReferrers(slice) + if !ok || len(sliceRefs) != 1 || sliceRefs[0] != store { return nil, false } - refs := alloc.Referrers() - if refs == nil { + refs, ok := nonDebugReferrers(alloc) + if !ok { return nil, false } seenSlice := false - for _, ref := range *refs { + for _, ref := range refs { switch ref := ref.(type) { case *ssa.Slice: if ref != slice || seenSlice { @@ -228,11 +228,11 @@ func staticSliceInitOf(store *ssa.Store) (*staticSliceInit, bool) { if !ok || index >= int(array.Len()) { return nil, false } - indexRefs := ref.Referrers() - if indexRefs == nil || len(*indexRefs) != 1 { + indexRefs, ok := nonDebugReferrers(ref) + if !ok || len(indexRefs) != 1 { return nil, false } - elemStore, ok := (*indexRefs)[0].(*ssa.Store) + elemStore, ok := indexRefs[0].(*ssa.Store) if !ok || elemStore.Addr != ref { return nil, false } diff --git a/internal/build/build.go b/internal/build/build.go index 2de1f96536..b19ae77e98 100644 --- a/internal/build/build.go +++ b/internal/build/build.go @@ -466,10 +466,7 @@ func Do(args []string, conf *Config) ([]Package, error) { buildMode := ssaBuildMode cabiOptimize := true - passOpt := true - if emitDebugInfo || mode == ModeGen { - passOpt = false - } + passOpt := shouldRunLLVMPasses(mode) if emitDebugInfo { buildMode |= ssa.GlobalDebug cabiOptimize = false @@ -477,6 +474,7 @@ func Do(args []string, conf *Config) ([]Package, error) { if !IsOptimizeEnabled() { buildMode |= ssa.NaiveForm } + prog.SetDebugInfoOptimized(passOpt && conf.OptLevel != optlevel.O0) progSSA := ssa.NewProgram(initial[0].Fset, buildMode) patches := make(cl.Patches, len(altPkgPaths)) altSSAPkgs(progSSA, patches, altPkgs[1:], conf, verbose) @@ -2240,6 +2238,10 @@ func llvmPassPipeline(level optlevel.Level, ltoMode lto.Mode) string { } } +func shouldRunLLVMPasses(mode Mode) bool { + return mode != ModeGen +} + func IsWasiThreadsEnabled() bool { return isEnvOn(llgoWasiThreads, true) } diff --git a/internal/build/optlevel_test.go b/internal/build/optlevel_test.go index ce6d1e7791..857f0a92c5 100644 --- a/internal/build/optlevel_test.go +++ b/internal/build/optlevel_test.go @@ -71,3 +71,14 @@ func TestLLVMPassPipeline(t *testing.T) { } } } + +func TestShouldRunLLVMPasses(t *testing.T) { + for _, mode := range []Mode{ModeBuild, ModeInstall, ModeRun, ModeTest, ModeCmpTest} { + if !shouldRunLLVMPasses(mode) { + t.Errorf("shouldRunLLVMPasses(%v) = false, want true", mode) + } + } + if shouldRunLLVMPasses(ModeGen) { + t.Fatal("shouldRunLLVMPasses(ModeGen) = true, want false") + } +} diff --git a/internal/build/ssa_order_fix.go b/internal/build/ssa_order_fix.go index 57235740c4..1efd9e46b4 100644 --- a/internal/build/ssa_order_fix.go +++ b/internal/build/ssa_order_fix.go @@ -186,6 +186,9 @@ func moveAssignDepsAfterRecv(b *ssa.BasicBlock, roots []ssa.Value, recv ssa.Valu if len(move) == 0 { return false } + // Metadata uses must follow the definitions they describe rather than + // blocking an otherwise safe source-order repair. + includeDebugRefsForMovedValues(b.Instrs, move, recvIdx) if moveWouldBreakSSA(b.Instrs, move, recvIdx) { return false } @@ -205,6 +208,30 @@ func moveAssignDepsAfterRecv(b *ssa.BasicBlock, roots []ssa.Value, recv ssa.Valu return true } +func includeDebugRefsForMovedValues(instrs []ssa.Instruction, move map[int]struct{}, through int) { + moved := make(map[ssa.Value]struct{}, len(move)) + for i := range move { + if v, ok := instrs[i].(ssa.Value); ok && v != nil { + moved[v] = struct{}{} + } + } + for i := 0; i <= through && i < len(instrs); i++ { + if _, moving := move[i]; moving { + continue + } + ref, ok := instrs[i].(*ssa.DebugRef) + if !ok { + continue + } + for v := range moved { + if instrUsesValue(ref, v) { + move[i] = struct{}{} + break + } + } + } +} + func moveWouldBreakSSA(instrs []ssa.Instruction, move map[int]struct{}, recvIdx int) bool { moved := make(map[ssa.Value]struct{}, len(move)) for i := range move { @@ -302,11 +329,15 @@ func fixSSAOrderBlock(b *ssa.BasicBlock) { continue } - // If the loaded value is used by any instruction between its current - // position and the return (excluding return itself), moving it may place - // its definition after one of those uses and break SSA form. + // DebugRefs are metadata-only and move with the value they describe. Any + // executable use before Return still makes reordering unsafe. + moving := map[ssa.Instruction]struct{}{u: {}} usedBeforeReturn := false for i := loadIdx + 1; i < retIdx; i++ { + if ref, ok := b.Instrs[i].(*ssa.DebugRef); ok && instrUsesValue(ref, u) { + moving[ref] = struct{}{} + continue + } if instrUsesValue(b.Instrs[i], u) { usedBeforeReturn = true break @@ -316,9 +347,7 @@ func fixSSAOrderBlock(b *ssa.BasicBlock) { continue } - // Move the load right after the last call (but before Return). - b.Instrs = moveInstr(b.Instrs, loadIdx, lastCallIdx+1) - // Adjust retIdx for subsequent moves in this block. + b.Instrs = moveInstrsAfter(b.Instrs, moving, b.Instrs[lastCallIdx]) retIdx = indexOfInstr(b.Instrs, ret) } } @@ -391,41 +420,29 @@ func valueDependsOn(v, target ssa.Value, seen map[ssa.Value]struct{}) bool { return false } -// moveInstr moves instrs[from] to position to (like inserting before to), -// preserving relative order of other elements. -func moveInstr(instrs []ssa.Instruction, from, to int) []ssa.Instruction { - if from < 0 || from >= len(instrs) { - return instrs - } - if to < 0 { - to = 0 - } - if to > len(instrs) { - to = len(instrs) - } - if from == to || from+1 == to { +// moveInstrsAfter moves selected instructions as a stable group immediately +// after anchor. +func moveInstrsAfter(instrs []ssa.Instruction, moving map[ssa.Instruction]struct{}, anchor ssa.Instruction) []ssa.Instruction { + if len(moving) == 0 || anchor == nil { return instrs } - - ins := instrs[from] - // Remove. - copy(instrs[from:], instrs[from+1:]) - instrs = instrs[:len(instrs)-1] - - // Recompute insertion index after removal. - if to > from { - to-- - } - if to < 0 { - to = 0 - } - if to > len(instrs) { - to = len(instrs) + moved := make([]ssa.Instruction, 0, len(moving)) + remaining := make([]ssa.Instruction, 0, len(instrs)) + for _, instr := range instrs { + if _, ok := moving[instr]; ok { + moved = append(moved, instr) + continue + } + remaining = append(remaining, instr) + } + for i, instr := range remaining { + if instr == anchor { + ret := make([]ssa.Instruction, 0, len(instrs)) + ret = append(ret, remaining[:i+1]...) + ret = append(ret, moved...) + ret = append(ret, remaining[i+1:]...) + return ret + } } - - // Insert. - instrs = append(instrs, nil) - copy(instrs[to+1:], instrs[to:]) - instrs[to] = ins return instrs } diff --git a/internal/build/ssa_order_fix_test.go b/internal/build/ssa_order_fix_test.go index 7408ae261c..33ff26041f 100644 --- a/internal/build/ssa_order_fix_test.go +++ b/internal/build/ssa_order_fix_test.go @@ -11,6 +11,7 @@ import ( "strings" "testing" + "golang.org/x/tools/go/packages" "golang.org/x/tools/go/ssa" "golang.org/x/tools/go/ssa/ssautil" ) @@ -28,11 +29,12 @@ func f() { case *fp(&x, 100) = <-fc(c, 1): } }` - fn := buildSSAOrderTestPackage(t, src) - got := instrOrder(fn, "fc(", "<-", "fp(", "*t") - if !inOrder(got, "fc(", "<-", "fp(") { - t.Fatalf("single-case select receive assignment order = %v, want fc/receive before fp", got) - } + testSSAOrderModes(t, src, func(t *testing.T, fn *ssa.Function) { + got := instrOrder(fn, "fc(", "<-", "fp(", "*t") + if !inOrder(got, "fc(", "<-", "fp(") { + t.Fatalf("single-case select receive assignment order = %v, want fc/receive before fp", got) + } + }) } func TestFixSSAOrderPlainRecvAssignKeepsLeftToRight(t *testing.T) { @@ -66,11 +68,12 @@ func f() { case m[fn(13, 100)] = <-fc(c, 1): } }` - fn := buildSSAOrderTestPackage(t, src) - got := instrOrder(fn, "fc(", "<-", "fn(") - if !inOrder(got, "fc(", "<-", "fn(") { - t.Fatalf("single-case select map receive assignment order = %v, want fc/receive before fn", got) - } + testSSAOrderModes(t, src, func(t *testing.T, fn *ssa.Function) { + got := instrOrder(fn, "fc(", "<-", "fn(") + if !inOrder(got, "fc(", "<-", "fn(") { + t.Fatalf("single-case select map receive assignment order = %v, want fc/receive before fn", got) + } + }) } func TestFixSSAOrderSingleCaseSelectTwoValueRecv(t *testing.T) { @@ -87,11 +90,12 @@ func f() { case *fp(&x, 100), ok = <-fc(c, 1): } }` - fn := buildSSAOrderTestPackage(t, src) - got := instrOrder(fn, "fc(", "<-", "fp(", "*t") - if !inOrder(got, "fc(", "<-", "fp(") { - t.Fatalf("single-case select two-value receive assignment order = %v, want fc/receive before fp", got) - } + testSSAOrderModes(t, src, func(t *testing.T, fn *ssa.Function) { + got := instrOrder(fn, "fc(", "<-", "fp(", "*t") + if !inOrder(got, "fc(", "<-", "fp(") { + t.Fatalf("single-case select two-value receive assignment order = %v, want fc/receive before fp", got) + } + }) } func TestFixSSAOrderMultiCaseSelectKeepsLeftToRight(t *testing.T) { @@ -115,7 +119,161 @@ func f() { } } +func TestFixSSAOrderReturnLoadWithDebugRefs(t *testing.T) { + const src = `package p +type value struct { n int } +func (v *value) mutate() bool { v.n = 1; return true } +func f() (value, bool) { + var v value + return v, v.mutate() +}` + base := ssa.SanityCheckFunctions | ssa.InstantiateGenerics + for _, test := range []struct { + name string + mode ssa.BuilderMode + }{ + {name: "default", mode: base}, + {name: "global-debug", mode: base | ssa.GlobalDebug}, + } { + t.Run(test.name, func(t *testing.T) { + fn := buildSSAOrderTestPackageMode(t, src, test.mode) + checkReturnLoadAfterMutation(t, fn, "mutate") + }) + } +} + +func TestFixSSAOrderCryptoX509ParseOID(t *testing.T) { + fset := token.NewFileSet() + loaded, err := packages.Load(&packages.Config{ + Mode: packages.LoadSyntax, + Fset: fset, + }, "crypto/x509") + if err != nil { + t.Fatal(err) + } + if packages.PrintErrors(loaded) != 0 || len(loaded) != 1 { + t.Fatal("failed to load crypto/x509") + } + mode := ssa.SanityCheckFunctions | ssa.InstantiateGenerics | ssa.GlobalDebug + prog, ssaPackages := ssautil.Packages(loaded, mode) + prog.Build() + pkg := ssaPackages[0] + fixSSAOrder(pkg, loaded[0].Syntax) + checkReturnLoadAfterMutation(t, pkg.Func("ParseOID"), "unmarshalOIDText") +} + +func TestMoveInstrsAfter(t *testing.T) { + const src = `package p +func f() { + println(1) + println(2) + println(3) +}` + fn := buildSSAOrderTestPackage(t, src) + instrs := fn.Blocks[0].Instrs + if len(instrs) < 4 { + t.Fatalf("instructions = %d, want at least 4", len(instrs)) + } + + assertOrder := func(t *testing.T, got, want []ssa.Instruction) { + t.Helper() + if len(got) != len(want) { + t.Fatalf("instruction count = %d, want %d", len(got), len(want)) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("instruction %d = %v, want %v", i, got[i], want[i]) + } + } + } + + t.Run("empty", func(t *testing.T) { + assertOrder(t, moveInstrsAfter(instrs, nil, instrs[1]), instrs) + }) + t.Run("nil-anchor", func(t *testing.T) { + moving := map[ssa.Instruction]struct{}{instrs[0]: {}} + assertOrder(t, moveInstrsAfter(instrs, moving, nil), instrs) + }) + t.Run("missing-anchor", func(t *testing.T) { + moving := map[ssa.Instruction]struct{}{instrs[0]: {}} + assertOrder(t, moveInstrsAfter(instrs, moving, &ssa.Return{}), instrs) + }) + t.Run("stable", func(t *testing.T) { + moving := map[ssa.Instruction]struct{}{ + instrs[0]: {}, + instrs[2]: {}, + } + want := []ssa.Instruction{instrs[1], instrs[0], instrs[2]} + want = append(want, instrs[3:]...) + assertOrder(t, moveInstrsAfter(instrs, moving, instrs[1]), want) + }) +} + +func checkReturnLoadAfterMutation(t *testing.T, fn *ssa.Function, mutation string) { + t.Helper() + var ret *ssa.Return + var mutationCall ssa.CallInstruction + for _, block := range fn.Blocks { + for _, instr := range block.Instrs { + if candidate, ok := instr.(*ssa.Return); ok { + ret = candidate + } + if call, ok := instr.(ssa.CallInstruction); ok { + callee := call.Common().StaticCallee() + if callee != nil && callee.Name() == mutation { + mutationCall = call + } + } + } + } + if ret == nil || len(ret.Results) != 2 || mutationCall == nil { + t.Fatalf("unexpected return instruction: %v", ret) + } + callInstr := mutationCall.(ssa.Instruction) + callIdx := indexOfInstr(callInstr.Block().Instrs, callInstr) + loadIdx := -1 + debugRefIdx := -1 + var resultLoad *ssa.UnOp + for i, instr := range callInstr.Block().Instrs { + if load, ok := instr.(*ssa.UnOp); ok && load.Op == token.MUL { + if alloc, ok := load.X.(*ssa.Alloc); ok && callUsesValue(mutationCall, alloc) { + loadIdx = i + resultLoad = load + } + } + if ref, ok := instr.(*ssa.DebugRef); ok && resultLoad != nil && instrUsesValue(ref, resultLoad) { + debugRefIdx = i + } + } + if callIdx < 0 || loadIdx <= callIdx { + t.Fatalf("return load index = %d, mutation call index = %d; want load after call\n%s", loadIdx, callIdx, fn) + } + if debugRefIdx >= 0 && debugRefIdx <= loadIdx { + t.Fatalf("DebugRef index = %d, load index = %d; want DebugRef after its load\n%s", debugRefIdx, loadIdx, fn) + } +} + func buildSSAOrderTestPackage(t *testing.T, src string) *ssa.Function { + return buildSSAOrderTestPackageMode(t, src, ssa.SanityCheckFunctions|ssa.InstantiateGenerics) +} + +func testSSAOrderModes(t *testing.T, src string, check func(*testing.T, *ssa.Function)) { + t.Helper() + base := ssa.SanityCheckFunctions | ssa.InstantiateGenerics + for _, test := range []struct { + name string + mode ssa.BuilderMode + }{ + {name: "default", mode: base}, + {name: "global-debug", mode: base | ssa.GlobalDebug}, + } { + t.Run(test.name, func(t *testing.T) { + check(t, buildSSAOrderTestPackageMode(t, src, test.mode)) + }) + } +} + +func buildSSAOrderTestPackageMode(t *testing.T, src string, mode ssa.BuilderMode) *ssa.Function { t.Helper() fset := token.NewFileSet() file, err := parser.ParseFile(fset, "p.go", src, 0) @@ -129,7 +287,7 @@ func buildSSAOrderTestPackage(t *testing.T, src string) *ssa.Function { fset, pkg, files, - ssa.SanityCheckFunctions|ssa.InstantiateGenerics, + mode, ) if err != nil { t.Fatalf("BuildPackage: %v", err) diff --git a/ltoplugin/LLGOReflectMethodByNamePass.cpp b/ltoplugin/LLGOReflectMethodByNamePass.cpp index 68c95283f5..57440ba617 100644 --- a/ltoplugin/LLGOReflectMethodByNamePass.cpp +++ b/ltoplugin/LLGOReflectMethodByNamePass.cpp @@ -173,20 +173,40 @@ bool isRuntimeStringSlice2(CallBase *CB) { return Callee && Callee->getName().ends_with(RuntimeStringSlice2Suffix); } +bool consumeStringCallArgument(CallBase *CB, unsigned &NextArgIndex, + const DataLayout &DL, + SmallVectorImpl &Names, + unsigned Depth) { + if (NextArgIndex >= CB->arg_size()) + return false; + + Value *Arg = CB->getArgOperand(NextArgIndex); + if (Arg->getType()->isStructTy()) { + ++NextArgIndex; + return collectStringSetFromStringValue(Arg, DL, Names, Depth + 1); + } + + if (NextArgIndex + 1 >= CB->arg_size()) + return false; + Value *Len = CB->getArgOperand(NextArgIndex + 1); + if (!Arg->getType()->isPointerTy() || !Len->getType()->isIntegerTy()) + return false; + NextArgIndex += 2; + return collectStringSet(Arg, Len, DL, Names, Depth + 1); +} + bool collectStringSetFromStringCat(CallBase *CB, const DataLayout &DL, SmallVectorImpl &Names, unsigned Depth) { if (!isRuntimeStringCat(CB)) return false; - if (CB->arg_size() != 4) - return false; SmallVector Prefixes; SmallVector Suffixes; - if (!collectStringSet(CB->getArgOperand(0), CB->getArgOperand(1), DL, - Prefixes, Depth + 1) || - !collectStringSet(CB->getArgOperand(2), CB->getArgOperand(3), DL, - Suffixes, Depth + 1)) + unsigned NextArgIndex = 0; + if (!consumeStringCallArgument(CB, NextArgIndex, DL, Prefixes, Depth) || + !consumeStringCallArgument(CB, NextArgIndex, DL, Suffixes, Depth) || + NextArgIndex != CB->arg_size()) return false; return addConcatenatedNames(Names, Prefixes, Suffixes); } @@ -196,18 +216,19 @@ bool collectStringSetFromStringSlice2(CallBase *CB, const DataLayout &DL, unsigned Depth) { if (!isRuntimeStringSlice2(CB)) return false; - if (CB->arg_size() != 6) - return false; SmallVector Bases; - if (!collectStringSet(CB->getArgOperand(0), CB->getArgOperand(1), DL, Bases, - Depth + 1)) + unsigned NextArgIndex = 0; + if (!consumeStringCallArgument(CB, NextArgIndex, DL, Bases, Depth) || + CB->arg_size() != NextArgIndex + 4) return false; SmallVector Lows; SmallVector Highs; - if (!collectIntegerChoices(CB->getArgOperand(2), Lows, Depth + 1) || - !collectIntegerChoices(CB->getArgOperand(3), Highs, Depth + 1)) + if (!collectIntegerChoices(CB->getArgOperand(NextArgIndex), Lows, + Depth + 1) || + !collectIntegerChoices(CB->getArgOperand(NextArgIndex + 1), Highs, + Depth + 1)) return false; for (const std::string &Base : Bases) { @@ -886,6 +907,37 @@ bool collectStringSetFromFunctionStringArg(Value *Ptr, Value *Len, return true; } +bool collectStringSetFromFunctionStringValueArg( + Value *StringValue, const DataLayout &DL, + SmallVectorImpl &Names, unsigned Depth) { + auto *Arg = dyn_cast(StringValue); + if (!Arg || !Arg->getType()->isStructTy()) + return false; + + Function *F = Arg->getParent(); + if (!F) + return false; + + unsigned ArgNo = Arg->getArgNo(); + SmallVector Callers; + for (User *U : F->users()) { + auto *CB = dyn_cast(U); + if (!CB || CB->getCalledFunction() != F || ArgNo >= CB->arg_size()) + return false; + Callers.push_back(CB); + } + if (Callers.empty()) + return false; + + for (CallBase *CB : Callers) { + Value *CallValue = CB->getArgOperand(ArgNo); + if (!CallValue->getType()->isStructTy() || + !collectStringSetFromStringValue(CallValue, DL, Names, Depth + 1)) + return false; + } + return true; +} + bool collectStringSetFromStringValue(Value *StringValue, const DataLayout &DL, SmallVectorImpl &Names, unsigned Depth) { @@ -907,6 +959,9 @@ bool collectStringSetFromStringValue(Value *StringValue, const DataLayout &DL, return collectStringSetFromStringConstant(Folded, DL, Names, Depth + 1); } + if (collectStringSetFromFunctionStringValueArg(StringValue, DL, Names, Depth)) + return true; + if (auto *Insert = dyn_cast(StringValue)) { Value *Ptr = findInsertedValue(Insert, 0); Value *Len = findInsertedValue(Insert, 1); @@ -1039,23 +1094,18 @@ std::optional findReflectMethodNameArg(CallBase *ReflectCall) { return std::nullopt; } -// The plugin runs during LTO, after LLGo has applied C ABI lowering to every -// module that participates in the link. At this point a Go string argument is -// always split into (ptr, len). LLGo marks the string pointer parameter, and -// the length is the following parameter produced by the same lowering step. +// Optimized C ABI lowering splits a Go string into (ptr, len), with the name +// attribute on the pointer. Debug-preserving builds can retain the original +// aggregate string argument instead. Accept both representations so enabling +// DWARF does not change MethodByName reachability. bool collectStringSetFromCallArgs(CallBase *ReflectCall, const DataLayout &DL, SmallVectorImpl &Names) { std::optional NameArg = findReflectMethodNameArg(ReflectCall); if (!NameArg) return false; - if (*NameArg + 1 >= ReflectCall->arg_size()) - return false; - Value *Ptr = ReflectCall->getArgOperand(*NameArg); - Value *Len = ReflectCall->getArgOperand(*NameArg + 1); - if (!Ptr->getType()->isPointerTy() || !Len->getType()->isIntegerTy()) - return false; - return collectStringSet(Ptr, Len, DL, Names); + unsigned NextArgIndex = *NameArg; + return consumeStringCallArgument(ReflectCall, NextArgIndex, DL, Names, 0); } bool isTypeCheckedLoad(CallBase *CB) { diff --git a/ssa/di_debug_test.go b/ssa/di_debug_test.go index aebb9d016f..8b81848c01 100644 --- a/ssa/di_debug_test.go +++ b/ssa/di_debug_test.go @@ -166,6 +166,48 @@ type Shape struct { } } +func TestDeferInitBuilderInheritsDebugLocation(t *testing.T) { + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "defer.go", `package p +func f() {} +`, 0) + if err != nil { + t.Fatal(err) + } + typesPkg, err := (&types.Config{}).Check("example.com/p", fset, []*ast.File{file}, nil) + if err != nil { + t.Fatal(err) + } + + prog := NewProgram(&Target{OptLevel: optlevel.O0}) + defer prog.Dispose() + prog.TypeSizes(types.SizesFor("gc", runtime.GOARCH)) + pkg := prog.NewPackage("p", "example.com/p") + pkg.InitDebug("p", "example.com/p", fset) + decl := file.Decls[0].(*ast.FuncDecl) + object := typesPkg.Scope().Lookup("f").(*types.Func) + fn := pkg.NewFunc("example.com/p.f", object.Type().(*types.Signature), InGo) + builder := fn.MakeBody(1) + defer builder.Dispose() + bodyPos := fset.Position(decl.Body.Lbrace) + builder.DebugFunction(fn, object.Scope(), fset.Position(object.Pos()), bodyPos) + builder.DISetCurrentDebugLocation(fn, bodyPos) + builder.Return() + + deferBuilder, next := fn.deferInitBuilder(builder) + defer deferBuilder.Dispose() + loc := deferBuilder.impl.GetCurrentDebugLocation() + if loc.Line != uint(bodyPos.Line) || loc.Col != uint(bodyPos.Column) || loc.Scope != fn.diFunc.ll { + t.Fatalf("defer debug location = %+v, want %s:%d:%d", loc, bodyPos.Filename, bodyPos.Line, bodyPos.Column) + } + deferBuilder.Jump(next) + + pkg.FinalizeDebug() + if err := llvm.VerifyModule(pkg.Module(), llvm.ReturnStatusAction); err != nil { + t.Fatalf("defer debug metadata is invalid: %v\n%s", err, pkg.Module().String()) + } +} + func newDebugRuntimePackage() *types.Package { pkg := types.NewPackage(PkgRuntime, "runtime") unsafePointer := types.Typ[types.UnsafePointer] diff --git a/ssa/eh.go b/ssa/eh.go index b8ead4eb64..e18149ebc3 100644 --- a/ssa/eh.go +++ b/ssa/eh.go @@ -136,8 +136,14 @@ func (b Builder) Longjmp(jb, retval Expr) { // ----------------------------------------------------------------------------- -func (p Function) deferInitBuilder() (b Builder, next BasicBlock) { +func (p Function) deferInitBuilder(from Builder) (b Builder, next BasicBlock) { b = p.NewBuilder() + if p.diFunc != nil { + loc := from.impl.GetCurrentDebugLocation() + if !loc.Scope.IsNil() { + b.impl.SetCurrentDebugLocation(loc.Line, loc.Col, loc.Scope, loc.InlinedAt) + } + } next = b.setBlockMoveLast(p.blks[0]) p.blks[0].last = next.last return @@ -197,7 +203,7 @@ func (b Builder) getDefer(kind DoAction) *aDefer { // TODO(xsw): check if in pkg.init var next, panicBlk BasicBlock if kind != DeferAlways { - b, next = self.deferInitBuilder() + b, next = self.deferInitBuilder(b) } blks := self.MakeBlocks(2) diff --git a/ssa/package.go b/ssa/package.go index 8282a676d6..4aa785080b 100644 --- a/ssa/package.go +++ b/ssa/package.go @@ -363,6 +363,12 @@ func (p Program) EnableLTOPluginMarkers(enable bool) { p.enableLTOPluginMarker = enable } +// SetDebugInfoOptimized records whether the LLVM IR optimization pipeline runs. +// It only controls DWARF's optimized marker and never selects compiler passes. +func (p Program) SetDebugInfoOptimized(enable bool) { + p.debugInfoOptimized = enable +} + func (p Program) SetNoInterfaceMethod(fullName string) { p.noInterface[fullName] = none{} } diff --git a/test/go/dwarf_semantics_acceptance_test.go b/test/go/dwarf_semantics_acceptance_test.go new file mode 100644 index 0000000000..29a8c34fc8 --- /dev/null +++ b/test/go/dwarf_semantics_acceptance_test.go @@ -0,0 +1,62 @@ +//go:build !llgo + +package gotest + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +const dwarfReturnOrderProbe = `package main + +type value struct { + n int +} + +func (v *value) mutate() bool { + v.n = 1 + return true +} + +func result() (value, bool) { + var v value + return v, v.mutate() +} + +func main() { + v, ok := result() + if !ok || v.n != 1 { + panic("return value was loaded before mutation") + } + println("RETURN_ORDER_OK") +} +` + +func TestDWARFReturnOrderSemantics(t *testing.T) { + repoRoot := findStringConversionRepoRoot(t) + source := filepath.Join(t.TempDir(), "main.go") + if err := os.WriteFile(source, []byte(dwarfReturnOrderProbe), 0o600); err != nil { + t.Fatal(err) + } + cmd := exec.Command( + "go", "run", "./cmd/llgo", "run", "-ldflags=-w=false", source, + ) + cmd.Dir = repoRoot + cmd.Env = append(os.Environ(), + "LLGO_ROOT="+repoRoot, + "LLGO_BUILD_CACHE=off", + "GOMAXPROCS=2", + "GOMEMLIMIT=6GiB", + "GOFLAGS=-p=1", + ) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("LLGo DWARF return-order acceptance failed: %v\n%s", err, out) + } + if !strings.Contains(string(out), "RETURN_ORDER_OK") { + t.Fatalf("LLGo DWARF return-order acceptance did not report success:\n%s", out) + } +}