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..635987d303 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 { @@ -225,6 +230,7 @@ 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", } @@ -267,6 +273,60 @@ 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 TestFilterEmulatorOutput(t *testing.T) { tests := []struct { name string 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) {