diff options
| -rw-r--r-- | builder/build.go | 2 | ||||
| -rw-r--r-- | builder/error.go | 4 | ||||
| -rw-r--r-- | builder/tools.go | 150 | ||||
| -rw-r--r-- | errors_test.go | 60 | ||||
| -rw-r--r-- | main.go | 3 | ||||
| -rw-r--r-- | main_test.go | 4 | ||||
| -rw-r--r-- | testdata/errors/linker-flashoverflow.go | 21 | ||||
| -rw-r--r-- | testdata/errors/linker-ramoverflow.go | 9 | ||||
| -rw-r--r-- | testdata/errors/linker-undefined.go | 11 |
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") } @@ -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 |