From 1e345ecec31097e4f7a0d1985ce1afb1be414600 Mon Sep 17 00:00:00 2001 From: Abhishek Goyal Date: Thu, 23 Apr 2026 21:17:03 +0530 Subject: [PATCH] builder,interp: fix -ldflags -X not overriding variables with default values Currently, -ldflags "-X pkg.Var=value" fails to override string variables that have a default value in source code: var Version = "dev" // -X override is ignored, stays "dev" var Version string // -X override works correctly In standard Go, -X works in both cases. This patch brings TinyGo's behavior in line with Go. The root cause is the timing of when -X values are applied relative to the interp pass. The previous approach: 1. Erase global, make it undefined 2. Run interp (executes `Version = "dev"` from init code) 3. Link in -X value via makeGlobalsModule By step 2, interp had already stored the default value, and the linking step couldn't reliably override it. This patch changes the approach: 1. Set -X value as initializer before interp runs 2. Run interp, but skip stores to -X globals 3. Global retains the -X value This also means dependent compile-time computations work correctly: var Version = "dev" var VersionLen = len(Version) // now evaluates with -X value With this fix, makeGlobalsModule becomes redundant - we no longer need a separate module to work around type renaming issues since we set the value directly on the global using its existing type during per-package compilation. I've removed makeGlobalsModule as part of this change. If there are edge cases I haven't considered where keeping it would be safer, happy to restore it as a fallback. --- builder/build.go | 111 ++++++++++-------------------------------- interp/interp.go | 37 +++++++------- interp/interpreter.go | 14 ++++++ 3 files changed, 61 insertions(+), 101 deletions(-) diff --git a/builder/build.go b/builder/build.go index 714a331a39..afded69f48 100644 --- a/builder/build.go +++ b/builder/build.go @@ -88,7 +88,7 @@ type packageAction struct { EmbeddedFiles map[string]string // hash of all the //go:embed files in the package Imports map[string]string // map from imported package to action ID hash OptLevel string // LLVM optimization level (O0, O1, O2, Os, Oz) - UndefinedGlobals []string // globals that are left as external globals (no initializer) + UndefinedGlobals map[string]string // globals set via -ldflags -X (name -> value), for cache key } // Build performs a single package to executable Go build. It takes in a package @@ -268,11 +268,11 @@ func Build(pkgName, outpath, tmpdir string, config *compileopts.Config) (BuildRe for _, pkg := range lprogram.Sorted() { pkg := pkg // necessary to avoid a race condition - var undefinedGlobals []string - for name := range globalValues[pkg.Pkg.Path()] { - undefinedGlobals = append(undefinedGlobals, name) + // Collect -X globals for this package (name -> value) + undefinedGlobals := globalValues[pkg.Pkg.Path()] + if undefinedGlobals == nil { + undefinedGlobals = map[string]string{} } - sort.Strings(undefinedGlobals) // Make compile jobs to load files to be embedded in the output binary. var actionIDDependencies []*compileJob @@ -444,30 +444,36 @@ func Build(pkgName, outpath, tmpdir string, config *compileopts.Config) (BuildRe } } - // Erase all globals that are part of the undefinedGlobals list. - // This list comes from the -ldflags="-X pkg.foo=val" option. - // Instead of setting the value directly in the AST (which would - // mean the value, which may be a secret, is stored in the build - // cache), the global itself is left external (undefined) and is - // only set at the end of the compilation. - for _, name := range undefinedGlobals { + // Set -X global values before interp runs. + // This ensures that dependent init code (e.g., var VersionLen = len(Version)) + // sees the correct -X value during interpretation. + // The -X global names are passed to interp so it skips stores to them. + undefinedGlobalNames := make(map[string]struct{}, len(undefinedGlobals)) + for name, value := range undefinedGlobals { globalName := pkg.Pkg.Path() + "." + name + undefinedGlobalNames[globalName] = struct{}{} global := mod.NamedGlobal(globalName) if global.IsNil() { return errors.New("global not found: " + globalName) } globalType := global.GlobalValueType() if globalType.TypeKind() != llvm.StructTypeKind || globalType.StructName() != "runtime._string" { - // Verify this is indeed a string. This is needed so - // that makeGlobalsModule can just create the right - // globals of string type without checking. + // Verify this is indeed a string. return fmt.Errorf("%s: not a string", globalName) } - name := global.Name() - newGlobal := llvm.AddGlobal(mod, globalType, name+".tmp") - global.ReplaceAllUsesWith(newGlobal) - global.EraseFromParentAsGlobal() - newGlobal.SetName(name) + // Set the -X value as the initializer. + // interp will skip stores to this global, preserving this value. + stringType := globalType + uintptrType := mod.Context().IntType(int(config.TargetBits())) + bufInitializer := mod.Context().ConstString(value, false) + buf := llvm.AddGlobal(mod, bufInitializer.Type(), globalName+".str") + buf.SetInitializer(bufInitializer) + buf.SetAlignment(1) + buf.SetUnnamedAddr(true) + buf.SetLinkage(llvm.PrivateLinkage) + length := llvm.ConstInt(uintptrType, uint64(len(value)), false) + initializer := llvm.ConstNamedStruct(stringType, []llvm.Value{buf, length}) + global.SetInitializer(initializer) } // Try to interpret package initializers at compile time. @@ -477,7 +483,7 @@ func Build(pkgName, outpath, tmpdir string, config *compileopts.Config) (BuildRe if pkgInit.IsNil() { panic("init not found for " + pkg.Pkg.Path()) } - err := interp.RunFunc(pkgInit, config.Options.InterpTimeout, config.DumpSSA()) + err := interp.RunFunc(pkgInit, undefinedGlobalNames, config.Options.InterpTimeout, config.DumpSSA()) if err != nil { return err } @@ -569,15 +575,6 @@ func Build(pkgName, outpath, tmpdir string, config *compileopts.Config) (BuildRe } } - // Insert values from -ldflags="-X ..." into the IR. - // This is a separate module, so that the "runtime._string" type - // doesn't need to match precisely. LLVM tends to rename that type - // sometimes, leading to errors. But linking in a separate module - // works fine. See: - // https://github.com/tinygo-org/tinygo/issues/4810 - globalsMod := makeGlobalsModule(ctx, globalValues, machine) - llvm.LinkModules(mod, globalsMod) - // Create runtime.initAll function that calls the runtime // initializer of each package. llvmInitFn := mod.NamedFunction("runtime.initAll") @@ -1226,60 +1223,6 @@ func optimizeProgram(mod llvm.Module, config *compileopts.Config) error { return nil } -func makeGlobalsModule(ctx llvm.Context, globals map[string]map[string]string, machine llvm.TargetMachine) llvm.Module { - mod := ctx.NewModule("cmdline-globals") - targetData := machine.CreateTargetData() - defer targetData.Dispose() - mod.SetDataLayout(targetData.String()) - - stringType := ctx.StructCreateNamed("runtime._string") - uintptrType := ctx.IntType(targetData.PointerSize() * 8) - stringType.StructSetBody([]llvm.Type{ - llvm.PointerType(ctx.Int8Type(), 0), - uintptrType, - }, false) - - var pkgPaths []string - for pkgPath := range globals { - pkgPaths = append(pkgPaths, pkgPath) - } - sort.Strings(pkgPaths) - for _, pkgPath := range pkgPaths { - pkg := globals[pkgPath] - var names []string - for name := range pkg { - names = append(names, name) - } - sort.Strings(names) - for _, name := range names { - value := pkg[name] - globalName := pkgPath + "." + name - - // Create a buffer for the string contents. - bufInitializer := mod.Context().ConstString(value, false) - buf := llvm.AddGlobal(mod, bufInitializer.Type(), ".string") - buf.SetInitializer(bufInitializer) - buf.SetAlignment(1) - buf.SetUnnamedAddr(true) - buf.SetLinkage(llvm.PrivateLinkage) - - // Create the string value, which is a {ptr, len} pair. - length := llvm.ConstInt(uintptrType, uint64(len(value)), false) - initializer := llvm.ConstNamedStruct(stringType, []llvm.Value{ - buf, - length, - }) - - // Create the string global. - global := llvm.AddGlobal(mod, stringType, globalName) - global.SetInitializer(initializer) - global.SetAlignment(targetData.PrefTypeAlignment(stringType)) - } - } - - return mod -} - // functionStackSizes keeps stack size information about a single function // (usually a goroutine). type functionStackSize struct { diff --git a/interp/interp.go b/interp/interp.go index 30b0872485..58f77e6cf3 100644 --- a/interp/interp.go +++ b/interp/interp.go @@ -19,22 +19,23 @@ const checks = true // runner contains all state related to one interp run. type runner struct { - mod llvm.Module - targetData llvm.TargetData - builder llvm.Builder - pointerSize uint32 // cached pointer size from the TargetData - dataPtrType llvm.Type // often used type so created in advance - uintptrType llvm.Type // equivalent to uintptr in Go - maxAlign int // maximum alignment of an object, alignment of runtime.alloc() result - byteOrder binary.ByteOrder // big-endian or little-endian - debug bool // log debug messages - pkgName string // package name of the currently executing package - functionCache map[llvm.Value]*function // cache of compiled functions - objects []object // slice of objects in memory - globals map[llvm.Value]int // map from global to index in objects slice - start time.Time - timeout time.Duration - callsExecuted uint64 + mod llvm.Module + targetData llvm.TargetData + builder llvm.Builder + pointerSize uint32 // cached pointer size from the TargetData + dataPtrType llvm.Type // often used type so created in advance + uintptrType llvm.Type // equivalent to uintptr in Go + maxAlign int // maximum alignment of an object, alignment of runtime.alloc() result + byteOrder binary.ByteOrder // big-endian or little-endian + debug bool // log debug messages + pkgName string // package name of the currently executing package + functionCache map[llvm.Value]*function // cache of compiled functions + objects []object // slice of objects in memory + globals map[llvm.Value]int // map from global to index in objects slice + start time.Time + timeout time.Duration + undefinedGlobals map[string]struct{} // globals set via -X, skip stores to these + callsExecuted uint64 } func newRunner(mod llvm.Module, timeout time.Duration, debug bool) *runner { @@ -204,11 +205,13 @@ func Run(mod llvm.Module, timeout time.Duration, debug bool) error { // RunFunc evaluates a single package initializer at compile time. // Set debug to true if it should print output while running. -func RunFunc(fn llvm.Value, timeout time.Duration, debug bool) error { +// undefinedGlobals contains globals set via -X ldflags; stores to these are skipped. +func RunFunc(fn llvm.Value, undefinedGlobals map[string]struct{}, timeout time.Duration, debug bool) error { // Create and initialize *runner object. mod := fn.GlobalParent() r := newRunner(mod, timeout, debug) defer r.dispose() + r.undefinedGlobals = undefinedGlobals initName := fn.Name() if !strings.HasSuffix(initName, ".init") { return errorAt(fn, "interp: unexpected function name (expected *.init)") diff --git a/interp/interpreter.go b/interp/interpreter.go index e8f5545d5d..fc18bb5e02 100644 --- a/interp/interpreter.go +++ b/interp/interpreter.go @@ -547,6 +547,20 @@ func (r *runner) run(fn *function, params []value, parentMem *memoryView, indent if err != nil { return nil, mem, r.errorAt(inst, err) } + // Skip stores to -X globals. These globals already have the correct + // value set via -ldflags, and we don't want the init code to + // overwrite it with the source code's default value. + if r.undefinedGlobals != nil { + obj := mem.get(ptr.index()) + if !obj.llvmGlobal.IsNil() { + if _, isUndefined := r.undefinedGlobals[obj.llvmGlobal.Name()]; isUndefined { + if r.debug { + fmt.Fprintln(os.Stderr, indent+"skip store to -X global:", obj.llvmGlobal.Name()) + } + continue + } + } + } if inst.llvmInst.IsVolatile() || inst.llvmInst.Ordering() != llvm.AtomicOrderingNotAtomic || mem.hasExternalLoadOrStore(ptr) { err := r.runAtRuntime(fn, inst, locals, &mem, indent) if err != nil {