Skip to content

builder,interp: fix -ldflags -X not overriding variables with default values #5344

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
abgoyal wants to merge 1 commit into from
+61 −101
Closed

builder,interp: fix -ldflags -X not overriding variables with default values #5344

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 27 additions & 84 deletions builder/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@
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
Expand Down Expand Up @@ -268,11 +268,11 @@
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
Expand Down Expand Up @@ -444,30 +444,36 @@
}
}

// 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()))

Check failure on line 467 in builder/build.go

View workflow job for this annotation

GitHub Actions / assert-test-linux 

config.TargetBits undefined (type *compileopts.Config has no field or method TargetBits)

Check failure on line 467 in builder/build.go

View workflow job for this annotation

GitHub Actions / build-windows 

config.TargetBits undefined (type *compileopts.Config has no field or method TargetBits)

Check failure on line 467 in builder/build.go

View workflow job for this annotation

GitHub Actions / homebrew-install (18) 

config.TargetBits undefined (type *compileopts.Config has no field or method 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.
Expand All @@ -477,7 +483,7 @@
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
}
Expand Down Expand Up @@ -569,15 +575,6 @@
}
}

// 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")
Expand Down Expand Up @@ -1226,60 +1223,6 @@
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 {
Expand Down
37 changes: 20 additions & 17 deletions interp/interp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)")
Expand Down
14 changes: 14 additions & 0 deletions interp/interpreter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading