Skip to content

Commit 8cbfbca

Browse files
aykevldeadprogram
authored andcommitted
build: avoid sharing GlobalValues between build instances
This happens with `tinygo test` for example when testing multiple packages at the same time. I found this because the compiler crashed in `make tinygo-test-fast`: fatal error: concurrent map writes fatal error: concurrent map read and map write goroutine 15 [running]: github.com/tinygo-org/tinygo/builder.Build({0x40002d0be0, 0xa}, {0x0, 0x0}, {0x4000398048, 0x14}, 0x40003b0000) /home/ayke/src/tinygo/tinygo/builder/build.go:131 +0x388 main.buildAndRun({0x40002d0be0, 0xa}, 0x40003b0000, {0x8bc178, 0x40003a4090}, {0x0, 0x0, 0x0}, {0x0, 0x0, ...}, ...) /home/ayke/src/tinygo/tinygo/main.go:856 +0x45c main.Test({0x40002d0be0, 0xa}, {0x8bc118, 0x40000b3a08}, {0x0?, 0x0?}, 0x40001e6000, {0x0, 0x0}) /home/ayke/src/tinygo/tinygo/main.go:270 +0x758 main.main.func3() /home/ayke/src/tinygo/tinygo/main.go:1718 +0xe4 created by main.main in goroutine 1 /home/ayke/src/tinygo/tinygo/main.go:1712 +0x34ec My solution is essentially to copy the map over instead of modifying it directly. I've also moved the code up a little, because I think that's a more sensible place (out of the way of the whole package compile logic).
1 parent 5cd8ba2 commit 8cbfbca

File tree

1 file changed

+28
-22
lines changed

1 file changed

+28
-22
lines changed

builder/build.go

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,30 @@ func Build(pkgName, outpath, tmpdir string, config *compileopts.Config) (BuildRe
114114
cacheDir = tmpdir
115115
}
116116

117+
// Create default global values.
118+
globalValues := map[string]map[string]string{
119+
"runtime": {
120+
"buildVersion": goenv.Version(),
121+
},
122+
"testing": {},
123+
}
124+
if config.TestConfig.CompileTestBinary {
125+
// The testing.testBinary is set to "1" when in a test.
126+
// This is needed for testing.Testing() to work correctly.
127+
globalValues["testing"]["testBinary"] = "1"
128+
}
129+
130+
// Copy over explicitly set global values, like
131+
// -ldflags="-X main.Version="1.0"
132+
for pkgPath, vals := range config.Options.GlobalValues {
133+
if _, ok := globalValues[pkgPath]; !ok {
134+
globalValues[pkgPath] = map[string]string{}
135+
}
136+
for k, v := range vals {
137+
globalValues[pkgPath][k] = v
138+
}
139+
}
140+
117141
// Check for a libc dependency.
118142
// As a side effect, this also creates the headers for the given libc, if
119143
// the libc needs them.
@@ -216,30 +240,12 @@ func Build(pkgName, outpath, tmpdir string, config *compileopts.Config) (BuildRe
216240
var packageJobs []*compileJob
217241
packageActionIDJobs := make(map[string]*compileJob)
218242

219-
if config.Options.GlobalValues == nil {
220-
config.Options.GlobalValues = make(map[string]map[string]string)
221-
}
222-
if config.Options.GlobalValues["runtime"]["buildVersion"] == "" {
223-
if config.Options.GlobalValues["runtime"] == nil {
224-
config.Options.GlobalValues["runtime"] = make(map[string]string)
225-
}
226-
config.Options.GlobalValues["runtime"]["buildVersion"] = goenv.Version()
227-
}
228-
if config.TestConfig.CompileTestBinary {
229-
// The testing.testBinary is set to "1" when in a test.
230-
// This is needed for testing.Testing() to work correctly.
231-
if config.Options.GlobalValues["testing"] == nil {
232-
config.Options.GlobalValues["testing"] = make(map[string]string)
233-
}
234-
config.Options.GlobalValues["testing"]["testBinary"] = "1"
235-
}
236-
237243
var embedFileObjects []*compileJob
238244
for _, pkg := range lprogram.Sorted() {
239245
pkg := pkg // necessary to avoid a race condition
240246

241247
var undefinedGlobals []string
242-
for name := range config.Options.GlobalValues[pkg.Pkg.Path()] {
248+
for name := range globalValues[pkg.Pkg.Path()] {
243249
undefinedGlobals = append(undefinedGlobals, name)
244250
}
245251
sort.Strings(undefinedGlobals)
@@ -567,7 +573,7 @@ func Build(pkgName, outpath, tmpdir string, config *compileopts.Config) (BuildRe
567573

568574
// Run all optimization passes, which are much more effective now
569575
// that the optimizer can see the whole program at once.
570-
err := optimizeProgram(mod, config)
576+
err := optimizeProgram(mod, config, globalValues)
571577
if err != nil {
572578
return err
573579
}
@@ -1036,7 +1042,7 @@ func createEmbedObjectFile(data, hexSum, sourceFile, sourceDir, tmpdir string, c
10361042
// optimizeProgram runs a series of optimizations and transformations that are
10371043
// needed to convert a program to its final form. Some transformations are not
10381044
// optional and must be run as the compiler expects them to run.
1039-
func optimizeProgram(mod llvm.Module, config *compileopts.Config) error {
1045+
func optimizeProgram(mod llvm.Module, config *compileopts.Config, globalValues map[string]map[string]string) error {
10401046
err := interp.Run(mod, config.Options.InterpTimeout, config.DumpSSA())
10411047
if err != nil {
10421048
return err
@@ -1055,7 +1061,7 @@ func optimizeProgram(mod llvm.Module, config *compileopts.Config) error {
10551061
}
10561062

10571063
// Insert values from -ldflags="-X ..." into the IR.
1058-
err = setGlobalValues(mod, config.Options.GlobalValues)
1064+
err = setGlobalValues(mod, globalValues)
10591065
if err != nil {
10601066
return err
10611067
}

0 commit comments

Comments
 (0)