From ad4d722f54b1521e163ce5d0687f51817fccf617 Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Sat, 16 Mar 2024 16:31:36 +0100 Subject: [PATCH] all: move -panic=trap support to the compiler/runtime Support for `-panic=trap` was previously a pass in the optimization pipeline. This change moves it to the compiler and runtime, which in my opinion is a much better place. As a side effect, it also fixes https://github.com/tinygo-org/tinygo/issues/4161 by trapping inside runtime.runtimePanicAt and not just runtime.runtimePanic. This change also adds a test for the list of imported functions. This is a more generic test where it's easy to add more tests for WebAssembly file properties, such as exported functions. --- builder/build.go | 1 + compiler/compiler.go | 8 +++++ main_test.go | 60 +++++++++++++++++++++++++++++++++ src/runtime/panic.go | 16 +++++++++ transform/optimizer.go | 4 --- transform/panic.go | 33 ------------------ transform/panic_test.go | 12 ------- transform/testdata/panic.ll | 22 ------------ transform/testdata/panic.out.ll | 25 -------------- transform/transform_test.go | 1 + 10 files changed, 86 insertions(+), 96 deletions(-) delete mode 100644 transform/panic.go delete mode 100644 transform/panic_test.go delete mode 100644 transform/testdata/panic.ll delete mode 100644 transform/testdata/panic.out.ll diff --git a/builder/build.go b/builder/build.go index b9965c1e85..7b7cc4728c 100644 --- a/builder/build.go +++ b/builder/build.go @@ -200,6 +200,7 @@ func Build(pkgName, outpath, tmpdir string, config *compileopts.Config) (BuildRe MaxStackAlloc: config.MaxStackAlloc(), NeedsStackObjects: config.NeedsStackObjects(), Debug: !config.Options.SkipDWARF, // emit DWARF except when -internal-nodwarf is passed + PanicStrategy: config.PanicStrategy(), } // Load the target machine, which is the LLVM object that contains all diff --git a/compiler/compiler.go b/compiler/compiler.go index 2ebd5a9f22..63fc214c99 100644 --- a/compiler/compiler.go +++ b/compiler/compiler.go @@ -56,6 +56,7 @@ type Config struct { MaxStackAlloc uint64 NeedsStackObjects bool Debug bool // Whether to emit debug information in the LLVM module. + PanicStrategy string } // compilerContext contains function-independent data that should still be @@ -1855,6 +1856,13 @@ func (b *builder) createFunctionCall(instr *ssa.CallCommon) (llvm.Value, error) supportsRecover = 1 } return llvm.ConstInt(b.ctx.Int1Type(), supportsRecover, false), nil + case name == "runtime.panicStrategy": + // These constants are defined in src/runtime/panic.go. + panicStrategy := map[string]uint64{ + "print": 1, // panicStrategyPrint + "trap": 2, // panicStrategyTrap + }[b.Config.PanicStrategy] + return llvm.ConstInt(b.ctx.Int8Type(), panicStrategy, false), nil case name == "runtime/interrupt.New": return b.createInterruptGlobal(instr) } diff --git a/main_test.go b/main_test.go index b3e86420f4..9c28eb86d6 100644 --- a/main_test.go +++ b/main_test.go @@ -15,11 +15,13 @@ import ( "reflect" "regexp" "runtime" + "slices" "strings" "sync" "testing" "time" + "github.com/aykevl/go-wasm" "github.com/tinygo-org/tinygo/builder" "github.com/tinygo-org/tinygo/compileopts" "github.com/tinygo-org/tinygo/goenv" @@ -404,6 +406,64 @@ func runTestWithConfig(name string, t *testing.T, options compileopts.Options, c } } +// Test WebAssembly files for certain properties. +func TestWebAssembly(t *testing.T) { + t.Parallel() + type testCase struct { + name string + panicStrategy string + imports []string + } + for _, tc := range []testCase{ + // Test whether there really are no imports when using -panic=trap. This + // tests the bugfix for https://github.com/tinygo-org/tinygo/issues/4161. + {name: "panic-default", imports: []string{"wasi_snapshot_preview1.fd_write"}}, + {name: "panic-trap", panicStrategy: "trap", imports: []string{}}, + } { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + tmpdir := t.TempDir() + options := optionsFromTarget("wasi", sema) + options.PanicStrategy = tc.panicStrategy + config, err := builder.NewConfig(&options) + if err != nil { + t.Fatal(err) + } + + result, err := builder.Build("testdata/trivialpanic.go", ".wasm", tmpdir, config) + if err != nil { + t.Fatal("failed to build binary:", err) + } + f, err := os.Open(result.Binary) + if err != nil { + t.Fatal("could not open output binary:", err) + } + defer f.Close() + module, err := wasm.Parse(f) + if err != nil { + t.Fatal("could not parse output binary:", err) + } + + // Test the list of imports. + if tc.imports != nil { + var imports []string + for _, section := range module.Sections { + switch section := section.(type) { + case *wasm.SectionImport: + for _, symbol := range section.Entries { + imports = append(imports, symbol.Module+"."+symbol.Field) + } + } + } + if !slices.Equal(imports, tc.imports) { + t.Errorf("import list not as expected!\nexpected: %v\nactual: %v", tc.imports, imports) + } + } + }) + } +} + func TestTest(t *testing.T) { t.Parallel() diff --git a/src/runtime/panic.go b/src/runtime/panic.go index e8a67e4b64..062305f15c 100644 --- a/src/runtime/panic.go +++ b/src/runtime/panic.go @@ -21,6 +21,16 @@ func tinygo_longjmp(frame *deferFrame) // Returns whether recover is supported on the current architecture. func supportsRecover() bool +const ( + panicStrategyPrint = 1 + panicStrategyTrap = 2 +) + +// Compile intrinsic. +// Returns which strategy is used. This is usually "print" but can be changed +// using the -panic= compiler flag. +func panicStrategy() uint8 + // DeferFrame is a stack allocated object that stores information for the // current "defer frame", which is used in functions that use the `defer` // keyword. @@ -37,6 +47,9 @@ type deferFrame struct { // Builtin function panic(msg), used as a compiler intrinsic. func _panic(message interface{}) { + if panicStrategy() == panicStrategyTrap { + trap() + } if supportsRecover() { frame := (*deferFrame)(task.Current().DeferFrame) if frame != nil { @@ -60,6 +73,9 @@ func runtimePanic(msg string) { } func runtimePanicAt(addr unsafe.Pointer, msg string) { + if panicStrategy() == panicStrategyTrap { + trap() + } if hasReturnAddr { printstring("panic: runtime error at ") printptr(uintptr(addr) - callInstSize) diff --git a/transform/optimizer.go b/transform/optimizer.go index e73e976006..cd4d1ee883 100644 --- a/transform/optimizer.go +++ b/transform/optimizer.go @@ -40,10 +40,6 @@ func Optimize(mod llvm.Module, config *compileopts.Config) []error { fn.SetLinkage(llvm.ExternalLinkage) } - if config.PanicStrategy() == "trap" { - ReplacePanicsWithTrap(mod) // -panic=trap - } - // run a check of all of our code if config.VerifyIR() { errs := ircheck.Module(mod) diff --git a/transform/panic.go b/transform/panic.go deleted file mode 100644 index dee3bae06d..0000000000 --- a/transform/panic.go +++ /dev/null @@ -1,33 +0,0 @@ -package transform - -import ( - "tinygo.org/x/go-llvm" -) - -// ReplacePanicsWithTrap replaces each call to panic (or similar functions) with -// calls to llvm.trap, to reduce code size. This is the -panic=trap command-line -// option. -func ReplacePanicsWithTrap(mod llvm.Module) { - ctx := mod.Context() - builder := ctx.NewBuilder() - defer builder.Dispose() - - trap := mod.NamedFunction("llvm.trap") - if trap.IsNil() { - trapType := llvm.FunctionType(ctx.VoidType(), nil, false) - trap = llvm.AddFunction(mod, "llvm.trap", trapType) - } - for _, name := range []string{"runtime._panic", "runtime.runtimePanic"} { - fn := mod.NamedFunction(name) - if fn.IsNil() { - continue - } - for _, use := range getUses(fn) { - if use.IsACallInst().IsNil() || use.CalledValue() != fn { - panic("expected use of a panic function to be a call") - } - builder.SetInsertPointBefore(use) - builder.CreateCall(trap.GlobalValueType(), trap, nil, "") - } - } -} diff --git a/transform/panic_test.go b/transform/panic_test.go deleted file mode 100644 index ea4efd0e7d..0000000000 --- a/transform/panic_test.go +++ /dev/null @@ -1,12 +0,0 @@ -package transform_test - -import ( - "testing" - - "github.com/tinygo-org/tinygo/transform" -) - -func TestReplacePanicsWithTrap(t *testing.T) { - t.Parallel() - testTransform(t, "testdata/panic", transform.ReplacePanicsWithTrap) -} diff --git a/transform/testdata/panic.ll b/transform/testdata/panic.ll deleted file mode 100644 index 660e30f2f5..0000000000 --- a/transform/testdata/panic.ll +++ /dev/null @@ -1,22 +0,0 @@ -target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64" -target triple = "armv7m-none-eabi" - -@"runtime.lookupPanic$string" = constant [18 x i8] c"index out of range" - -declare void @runtime.runtimePanic(ptr, i32) - -declare void @runtime._panic(i32, ptr) - -define void @runtime.lookupPanic() { - call void @runtime.runtimePanic(ptr @"runtime.lookupPanic$string", i32 18) - ret void -} - -; This is equivalent to the following code: -; func someFunc(x interface{}) { -; panic(x) -; } -define void @someFunc(i32 %typecode, ptr %value) { - call void @runtime._panic(i32 %typecode, ptr %value) - unreachable -} diff --git a/transform/testdata/panic.out.ll b/transform/testdata/panic.out.ll deleted file mode 100644 index 458e4c2477..0000000000 --- a/transform/testdata/panic.out.ll +++ /dev/null @@ -1,25 +0,0 @@ -target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64" -target triple = "armv7m-none-eabi" - -@"runtime.lookupPanic$string" = constant [18 x i8] c"index out of range" - -declare void @runtime.runtimePanic(ptr, i32) - -declare void @runtime._panic(i32, ptr) - -define void @runtime.lookupPanic() { - call void @llvm.trap() - call void @runtime.runtimePanic(ptr @"runtime.lookupPanic$string", i32 18) - ret void -} - -define void @someFunc(i32 %typecode, ptr %value) { - call void @llvm.trap() - call void @runtime._panic(i32 %typecode, ptr %value) - unreachable -} - -; Function Attrs: cold noreturn nounwind -declare void @llvm.trap() #0 - -attributes #0 = { cold noreturn nounwind } diff --git a/transform/transform_test.go b/transform/transform_test.go index f23a480a90..40769dbe86 100644 --- a/transform/transform_test.go +++ b/transform/transform_test.go @@ -137,6 +137,7 @@ func compileGoFileForTesting(t *testing.T, filename string) llvm.Module { Scheduler: config.Scheduler(), AutomaticStackSize: config.AutomaticStackSize(), Debug: true, + PanicStrategy: config.PanicStrategy(), } machine, err := compiler.NewTargetMachine(compilerConfig) if err != nil {