From 8cbfbcae5a4a2249d0e38973345d04b79a23aa1c Mon Sep 17 00:00:00 2001
From: Ayke van Laethem <aykevanlaethem@gmail.com>
Date: Thu, 28 Sep 2023 15:46:35 +0200
Subject: [PATCH] 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).
---
 builder/build.go | 50 +++++++++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/builder/build.go b/builder/build.go
index a7c83a5204..d189ed2177 100644
--- a/builder/build.go
+++ b/builder/build.go
@@ -114,6 +114,30 @@ func Build(pkgName, outpath, tmpdir string, config *compileopts.Config) (BuildRe
 		cacheDir = tmpdir
 	}
 
+	// Create default global values.
+	globalValues := map[string]map[string]string{
+		"runtime": {
+			"buildVersion": goenv.Version(),
+		},
+		"testing": {},
+	}
+	if config.TestConfig.CompileTestBinary {
+		// The testing.testBinary is set to "1" when in a test.
+		// This is needed for testing.Testing() to work correctly.
+		globalValues["testing"]["testBinary"] = "1"
+	}
+
+	// Copy over explicitly set global values, like
+	// -ldflags="-X main.Version="1.0"
+	for pkgPath, vals := range config.Options.GlobalValues {
+		if _, ok := globalValues[pkgPath]; !ok {
+			globalValues[pkgPath] = map[string]string{}
+		}
+		for k, v := range vals {
+			globalValues[pkgPath][k] = v
+		}
+	}
+
 	// Check for a libc dependency.
 	// As a side effect, this also creates the headers for the given libc, if
 	// the libc needs them.
@@ -216,30 +240,12 @@ func Build(pkgName, outpath, tmpdir string, config *compileopts.Config) (BuildRe
 	var packageJobs []*compileJob
 	packageActionIDJobs := make(map[string]*compileJob)
 
-	if config.Options.GlobalValues == nil {
-		config.Options.GlobalValues = make(map[string]map[string]string)
-	}
-	if config.Options.GlobalValues["runtime"]["buildVersion"] == "" {
-		if config.Options.GlobalValues["runtime"] == nil {
-			config.Options.GlobalValues["runtime"] = make(map[string]string)
-		}
-		config.Options.GlobalValues["runtime"]["buildVersion"] = goenv.Version()
-	}
-	if config.TestConfig.CompileTestBinary {
-		// The testing.testBinary is set to "1" when in a test.
-		// This is needed for testing.Testing() to work correctly.
-		if config.Options.GlobalValues["testing"] == nil {
-			config.Options.GlobalValues["testing"] = make(map[string]string)
-		}
-		config.Options.GlobalValues["testing"]["testBinary"] = "1"
-	}
-
 	var embedFileObjects []*compileJob
 	for _, pkg := range lprogram.Sorted() {
 		pkg := pkg // necessary to avoid a race condition
 
 		var undefinedGlobals []string
-		for name := range config.Options.GlobalValues[pkg.Pkg.Path()] {
+		for name := range globalValues[pkg.Pkg.Path()] {
 			undefinedGlobals = append(undefinedGlobals, name)
 		}
 		sort.Strings(undefinedGlobals)
@@ -567,7 +573,7 @@ func Build(pkgName, outpath, tmpdir string, config *compileopts.Config) (BuildRe
 
 			// Run all optimization passes, which are much more effective now
 			// that the optimizer can see the whole program at once.
-			err := optimizeProgram(mod, config)
+			err := optimizeProgram(mod, config, globalValues)
 			if err != nil {
 				return err
 			}
@@ -1036,7 +1042,7 @@ func createEmbedObjectFile(data, hexSum, sourceFile, sourceDir, tmpdir string, c
 // optimizeProgram runs a series of optimizations and transformations that are
 // needed to convert a program to its final form. Some transformations are not
 // optional and must be run as the compiler expects them to run.
-func optimizeProgram(mod llvm.Module, config *compileopts.Config) error {
+func optimizeProgram(mod llvm.Module, config *compileopts.Config, globalValues map[string]map[string]string) error {
 	err := interp.Run(mod, config.Options.InterpTimeout, config.DumpSSA())
 	if err != nil {
 		return err
@@ -1055,7 +1061,7 @@ func optimizeProgram(mod llvm.Module, config *compileopts.Config) error {
 	}
 
 	// Insert values from -ldflags="-X ..." into the IR.
-	err = setGlobalValues(mod, config.Options.GlobalValues)
+	err = setGlobalValues(mod, globalValues)
 	if err != nil {
 		return err
 	}