builder: interpret linker error messages

This shows nicely formatted error messages for missing symbol names and
for out-of-flash, out-of-RAM conditions (on microcontrollers with
limited flash/RAM).

Unfortunately the missing symbol name errors aren't available on Windows
and WebAssembly because the linker doesn't report source locations yet.
This is something that I could perhaps improve in LLD.

9 files changed, 220 insertions, 44 deletions

diff --git a/builder/build.go b/builder/build.go
index d16bf170b..33c3b2858 100644
--- a/builder/build.go
+++ b/builder/build.go
@@ -779,7 +779,7 @@ func Build(pkgName, outpath, tmpdir string, config *compileopts.Config) (BuildRe
 			}
 			err = link(config.Target.Linker, ldflags...)
 			if err != nil {
-				return &commandError{"failed to link", result.Executable, err}
+				return err
 			}
 
 			var calculatedStacks []string
diff --git a/builder/error.go b/builder/error.go
index 3531007fb..fe1a2d422 100644
--- a/builder/error.go
+++ b/builder/error.go
@@ -15,12 +15,12 @@ func (e *MultiError) Error() string {
 
 // newMultiError returns a *MultiError if there is more than one error, or
 // returns that error directly when there is only one. Passing an empty slice
-// will lead to a panic.
+// will return nil (because there is no error).
 // The importPath may be passed if this error is for a single package.
 func newMultiError(errs []error, importPath string) error {
 	switch len(errs) {
 	case 0:
-		panic("attempted to create empty MultiError")
+		return nil
 	case 1:
 		return errs[0]
 	default:
diff --git a/builder/tools.go b/builder/tools.go
index a23714d60..e1c6443c6 100644
--- a/builder/tools.go
+++ b/builder/tools.go
@@ -1,10 +1,15 @@
 package builder
 
 import (
+	"bytes"
+	"fmt"
+	"go/scanner"
+	"go/token"
 	"os"
 	"os/exec"
-
-	"github.com/tinygo-org/tinygo/goenv"
+	"regexp"
+	"strconv"
+	"strings"
 )
 
 // runCCompiler invokes a C compiler with the given arguments.
@@ -23,22 +28,135 @@ func runCCompiler(flags ...string) error {
 
 // link invokes a linker with the given name and flags.
 func link(linker string, flags ...string) error {
-	if hasBuiltinTools && (linker == "ld.lld" || linker == "wasm-ld") {
-		// Run command with internal linker.
-		cmd := exec.Command(os.Args[0], append([]string{linker}, flags...)...)
-		cmd.Stdout = os.Stdout
-		cmd.Stderr = os.Stderr
-		return cmd.Run()
+	// We only support LLD.
+	if linker != "ld.lld" && linker != "wasm-ld" {
+		return fmt.Errorf("unexpected: linker %s should be ld.lld or wasm-ld", linker)
 	}
 
-	// Fall back to external command.
-	if _, ok := commands[linker]; ok {
-		return execCommand(linker, flags...)
+	var cmd *exec.Cmd
+	if hasBuiltinTools {
+		cmd = exec.Command(os.Args[0], append([]string{linker}, flags...)...)
+	} else {
+		name, err := LookupCommand(linker)
+		if err != nil {
+			return err
+		}
+		cmd = exec.Command(name, flags...)
 	}
-
-	cmd := exec.Command(linker, flags...)
+	var buf bytes.Buffer
 	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
-	cmd.Dir = goenv.Get("TINYGOROOT")
-	return cmd.Run()
+	cmd.Stderr = &buf
+	err := cmd.Run()
+	if err != nil {
+		if buf.Len() == 0 {
+			// The linker failed but ther was no output.
+			// Therefore, show some output anyway.
+			return fmt.Errorf("failed to run linker: %w", err)
+		}
+		return parseLLDErrors(buf.String())
+	}
+	return nil
+}
+
+// Split LLD errors into individual erros (including errors that continue on the
+// next line, using a ">>>" prefix). If possible, replace the raw errors with a
+// more user-friendly version (and one that's more in a Go style).
+func parseLLDErrors(text string) error {
+	// Split linker output in separate error messages.
+	lines := strings.Split(text, "\n")
+	var errorLines []string // one or more line (belonging to a single error) per line
+	for _, line := range lines {
+		line = strings.TrimRight(line, "\r") // needed for Windows
+		if len(errorLines) != 0 && strings.HasPrefix(line, ">>> ") {
+			errorLines[len(errorLines)-1] += "\n" + line
+			continue
+		}
+		if line == "" {
+			continue
+		}
+		errorLines = append(errorLines, line)
+	}
+
+	// Parse error messages.
+	var linkErrors []error
+	var flashOverflow, ramOverflow uint64
+	for _, message := range errorLines {
+		parsedError := false
+
+		// Check for undefined symbols.
+		// This can happen in some cases like with CGo and //go:linkname tricker.
+		if matches := regexp.MustCompile(`^ld.lld: error: undefined symbol: (.*)\n`).FindStringSubmatch(message); matches != nil {
+			symbolName := matches[1]
+			for _, line := range strings.Split(message, "\n") {
+				matches := regexp.MustCompile(`referenced by .* \(((.*):([0-9]+))\)`).FindStringSubmatch(line)
+				if matches != nil {
+					parsedError = true
+					line, _ := strconv.Atoi(matches[3])
+					// TODO: detect common mistakes like -gc=none?
+					linkErrors = append(linkErrors, scanner.Error{
+						Pos: token.Position{
+							Filename: matches[2],
+							Line:     line,
+						},
+						Msg: "linker could not find symbol " + symbolName,
+					})
+				}
+			}
+		}
+
+		// Check for flash/RAM overflow.
+		if matches := regexp.MustCompile(`^ld.lld: error: section '(.*?)' will not fit in region '(.*?)': overflowed by ([0-9]+) bytes$`).FindStringSubmatch(message); matches != nil {
+			region := matches[2]
+			n, err := strconv.ParseUint(matches[3], 10, 64)
+			if err != nil {
+				// Should not happen at all (unless it overflows an uint64 for some reason).
+				continue
+			}
+
+			// Check which area overflowed.
+			// Some chips use differently named memory areas, but these are by
+			// far the most common.
+			switch region {
+			case "FLASH_TEXT":
+				if n > flashOverflow {
+					flashOverflow = n
+				}
+				parsedError = true
+			case "RAM":
+				if n > ramOverflow {
+					ramOverflow = n
+				}
+				parsedError = true
+			}
+		}
+
+		// If we couldn't parse the linker error: show the error as-is to
+		// the user.
+		if !parsedError {
+			linkErrors = append(linkErrors, LinkerError{message})
+		}
+	}
+
+	if flashOverflow > 0 {
+		linkErrors = append(linkErrors, LinkerError{
+			Msg: fmt.Sprintf("program too large for this chip (flash overflowed by %d bytes)\n\toptimization guide: https://tinygo.org/docs/guides/optimizing-binaries/", flashOverflow),
+		})
+	}
+	if ramOverflow > 0 {
+		linkErrors = append(linkErrors, LinkerError{
+			Msg: fmt.Sprintf("program uses too much static RAM on this chip (RAM overflowed by %d bytes)", ramOverflow),
+		})
+	}
+
+	return newMultiError(linkErrors, "")
+}
+
+// LLD linker error that could not be parsed or doesn't refer to a source
+// location.
+type LinkerError struct {
+	Msg string
+}
+
+func (e LinkerError) Error() string {
+	return e.Msg
 }
diff --git a/errors_test.go b/errors_test.go
index 71eaff5ef..62d5af2cb 100644
--- a/errors_test.go
+++ b/errors_test.go
@@ -7,7 +7,6 @@ import (
 	"regexp"
 	"strings"
 	"testing"
-	"time"
 
 	"github.com/tinygo-org/tinygo/compileopts"
 	"github.com/tinygo-org/tinygo/diagnostics"
@@ -15,38 +14,55 @@ import (
 
 // Test the error messages of the TinyGo compiler.
 func TestErrors(t *testing.T) {
-	for _, name := range []string{
-		"cgo",
-		"compiler",
-		"interp",
-		"loader-importcycle",
-		"loader-invaliddep",
-		"loader-invalidpackage",
-		"loader-nopackage",
-		"optimizer",
-		"syntax",
-		"types",
+	// TODO: nicely formatted error messages for:
+	//   - duplicate symbols in ld.lld (currently only prints bitcode file)
+	type errorTest struct {
+		name   string
+		target string
+	}
+	for _, tc := range []errorTest{
+		{name: "cgo"},
+		{name: "compiler"},
+		{name: "interp"},
+		{name: "linker-flashoverflow", target: "cortex-m-qemu"},
+		{name: "linker-ramoverflow", target: "cortex-m-qemu"},
+		{name: "linker-undefined", target: "darwin/arm64"},
+		{name: "linker-undefined", target: "linux/amd64"},
+		//{name: "linker-undefined", target: "windows/amd64"}, // TODO: no source location
+		{name: "linker-undefined", target: "cortex-m-qemu"},
+		//{name: "linker-undefined", target: "wasip1"}, // TODO: no source location
+		{name: "loader-importcycle"},
+		{name: "loader-invaliddep"},
+		{name: "loader-invalidpackage"},
+		{name: "loader-nopackage"},
+		{name: "optimizer"},
+		{name: "syntax"},
+		{name: "types"},
 	} {
+		name := tc.name
+		if tc.target != "" {
+			name += "#" + tc.target
+		}
+		target := tc.target
+		if target == "" {
+			target = "wasip1"
+		}
 		t.Run(name, func(t *testing.T) {
-			testErrorMessages(t, "./testdata/errors/"+name+".go")
+			options := optionsFromTarget(target, sema)
+			testErrorMessages(t, "./testdata/errors/"+tc.name+".go", &options)
 		})
 	}
 }
 
-func testErrorMessages(t *testing.T, filename string) {
+func testErrorMessages(t *testing.T, filename string, options *compileopts.Options) {
+	t.Parallel()
+
 	// Parse expected error messages.
 	expected := readErrorMessages(t, filename)
 
 	// Try to build a binary (this should fail with an error).
 	tmpdir := t.TempDir()
-	err := Build(filename, tmpdir+"/out", &compileopts.Options{
-		Target:        "wasip1",
-		Semaphore:     sema,
-		InterpTimeout: 180 * time.Second,
-		Debug:         true,
-		VerifyIR:      true,
-		Opt:           "z",
-	})
+	err := Build(filename, tmpdir+"/out", options)
 	if err == nil {
 		t.Fatal("expected to get a compiler error")
 	}
diff --git a/main.go b/main.go
index c90bf7e69..32fc22665 100644
--- a/main.go
+++ b/main.go
@@ -1467,7 +1467,8 @@ func main() {
 	case "clang", "ld.lld", "wasm-ld":
 		err := builder.RunTool(command, os.Args[2:]...)
 		if err != nil {
-			fmt.Fprintln(os.Stderr, err)
+			// The tool should have printed an error message already.
+			// Don't print another error message here.
 			os.Exit(1)
 		}
 		os.Exit(0)
diff --git a/main_test.go b/main_test.go
index 057e008e8..ccd490d62 100644
--- a/main_test.go
+++ b/main_test.go
@@ -8,7 +8,6 @@ import (
 	"bytes"
 	"errors"
 	"flag"
-	"fmt"
 	"io"
 	"os"
 	"os/exec"
@@ -696,7 +695,8 @@ func TestMain(m *testing.M) {
 			// Invoke a specific tool.
 			err := builder.RunTool(os.Args[1], os.Args[2:]...)
 			if err != nil {
-				fmt.Fprintln(os.Stderr, err)
+				// The tool should have printed an error message already.
+				// Don't print another error message here.
 				os.Exit(1)
 			}
 			os.Exit(0)
diff --git a/testdata/errors/linker-flashoverflow.go b/testdata/errors/linker-flashoverflow.go
new file mode 100644
index 000000000..46e7d9858
--- /dev/null
+++ b/testdata/errors/linker-flashoverflow.go
@@ -0,0 +1,21 @@
+package main
+
+import "unsafe"
+
+const (
+	a = "0123456789abcdef"            // 16 bytes
+	b = a + a + a + a + a + a + a + a // 128 bytes
+	c = b + b + b + b + b + b + b + b // 1024 bytes
+	d = c + c + c + c + c + c + c + c // 8192 bytes
+	e = d + d + d + d + d + d + d + d // 65536 bytes
+	f = e + e + e + e + e + e + e + e // 524288 bytes
+)
+
+var s = f
+
+func main() {
+	println(unsafe.StringData(s))
+}
+
+// ERROR: program too large for this chip (flash overflowed by {{[0-9]+}} bytes)
+// ERROR: 	optimization guide: https://tinygo.org/docs/guides/optimizing-binaries/
diff --git a/testdata/errors/linker-ramoverflow.go b/testdata/errors/linker-ramoverflow.go
new file mode 100644
index 000000000..866f984ad
--- /dev/null
+++ b/testdata/errors/linker-ramoverflow.go
@@ -0,0 +1,9 @@
+package main
+
+var b [64 << 10]byte // 64kB
+
+func main() {
+	println("ptr:", &b[0])
+}
+
+// ERROR: program uses too much static RAM on this chip (RAM overflowed by {{[0-9]+}} bytes)
diff --git a/testdata/errors/linker-undefined.go b/testdata/errors/linker-undefined.go
new file mode 100644
index 000000000..fda2b623d
--- /dev/null
+++ b/testdata/errors/linker-undefined.go
@@ -0,0 +1,11 @@
+package main
+
+func foo()
+
+func main() {
+	foo()
+	foo()
+}
+
+// ERROR: linker-undefined.go:6: linker could not find symbol {{_?}}main.foo
+// ERROR: linker-undefined.go:7: linker could not find symbol {{_?}}main.foo