From d45816805a1f201452aba5cc48d8f93a5ac02d8c Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Thu, 3 Oct 2024 10:36:33 +0900 Subject: [PATCH 1/6] Tests: Improve exclusion and inclusion mechanism --- Tests/WasmKitTests/Spectest/Spectest.swift | 12 ++++++++---- Tests/WasmKitTests/Spectest/TestCase.swift | 6 +++--- Tests/WasmKitTests/SpectestTests.swift | 6 ------ 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/Tests/WasmKitTests/Spectest/Spectest.swift b/Tests/WasmKitTests/Spectest/Spectest.swift index f714bad8..f42cd9ae 100644 --- a/Tests/WasmKitTests/Spectest/Spectest.swift +++ b/Tests/WasmKitTests/Spectest/Spectest.swift @@ -3,11 +3,15 @@ import SystemPackage import WAT import WasmKit +private func loadStringArrayFromEnvironment(_ key: String) -> [String] { + ProcessInfo.processInfo.environment[key]?.split(separator: ",").map(String.init) ?? [] +} + @available(macOS 11, iOS 14.0, watchOS 7.0, tvOS 14.0, *) public func spectest( path: [String], - include: String?, - exclude: String?, + include: [String]? = nil, + exclude: [String]? = nil, verbose: Bool = false, parallel: Bool = true, configuration: EngineConfiguration = .init() @@ -28,8 +32,8 @@ public func spectest( "\(Int(Double(numerator) / Double(denominator) * 100))%" } - let include = include.flatMap { $0.split(separator: ",").map(String.init) } ?? [] - let exclude = exclude.flatMap { $0.split(separator: ",").map(String.init) } ?? [] + let include = include ?? loadStringArrayFromEnvironment("WASMKIT_SPECTEST_INCLUDE") + let exclude = exclude ?? loadStringArrayFromEnvironment("WASMKIT_SPECTEST_EXCLUDE") let testCases: [TestCase] do { diff --git a/Tests/WasmKitTests/Spectest/TestCase.swift b/Tests/WasmKitTests/Spectest/TestCase.swift index 316ac1da..75e3fdd8 100644 --- a/Tests/WasmKitTests/Spectest/TestCase.swift +++ b/Tests/WasmKitTests/Spectest/TestCase.swift @@ -51,10 +51,10 @@ struct TestCase { // FIXME: Skip SIMD proposal tests for now guard !fileName.starts(with: "simd_") else { return false } - let toCheck = fileName.hasSuffix(".wast") ? String(fileName.dropLast(".wast".count)) : fileName - guard !exclude.contains(toCheck) else { return false } + let patternPredicate = { pattern in filePath.path.hasSuffix(pattern) } + guard !exclude.contains(where: patternPredicate) else { return false } guard !include.isEmpty else { return true } - return include.contains(toCheck) + return include.contains(where: patternPredicate) } var testCases: [TestCase] = [] diff --git a/Tests/WasmKitTests/SpectestTests.swift b/Tests/WasmKitTests/SpectestTests.swift index 8d5b2940..5ecab1a5 100644 --- a/Tests/WasmKitTests/SpectestTests.swift +++ b/Tests/WasmKitTests/SpectestTests.swift @@ -18,11 +18,8 @@ final class SpectestTests: XCTestCase { /// Run all the tests in the spectest suite. func testRunAll() async throws { let defaultConfig = EngineConfiguration() - let environment = ProcessInfo.processInfo.environment let ok = try await spectest( path: Self.testPaths, - include: environment["WASMKIT_SPECTEST_INCLUDE"], - exclude: environment["WASMKIT_SPECTEST_EXCLUDE"], parallel: true, configuration: defaultConfig ) @@ -34,12 +31,9 @@ final class SpectestTests: XCTestCase { guard defaultConfig.threadingModel != .token else { return } // Sanity check that non-default threading models work. var config = defaultConfig - let environment = ProcessInfo.processInfo.environment config.threadingModel = .token let ok = try await spectest( path: Self.testPaths, - include: environment["WASMKIT_SPECTEST_INCLUDE"], - exclude: environment["WASMKIT_SPECTEST_EXCLUDE"], parallel: true, configuration: config ) From 264702175a68b101e37a2df9dcfb3f07548fbeb6 Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Thu, 3 Oct 2024 12:24:44 +0900 Subject: [PATCH 2/6] Fix `{table,memory}.{init,copy}` bounds check to pass `assert_trap` tests --- Sources/WasmKit/Execution/Instances.swift | 109 +++++++++++++++--- .../Execution/Instructions/Memory.swift | 56 ++------- .../Execution/Instructions/Table.swift | 29 +---- Sources/WasmKit/Module.swift | 2 +- Tests/WasmKitTests/Spectest/TestCase.swift | 15 ++- Tests/WasmKitTests/SpectestTests.swift | 4 + 6 files changed, 124 insertions(+), 91 deletions(-) diff --git a/Sources/WasmKit/Execution/Instances.swift b/Sources/WasmKit/Execution/Instances.swift index 9f0f6e4f..5dd4fc00 100644 --- a/Sources/WasmKit/Execution/Instances.swift +++ b/Sources/WasmKit/Execution/Instances.swift @@ -300,22 +300,45 @@ struct TableEntity /* : ~Copyable */ { return true } - mutating func initialize(elements source: [Reference], from fromIndex: Int, to toIndex: Int, count: Int) throws { - guard count > 0 else { return } + mutating func initialize(_ segment: InternalElementSegment, from source: Int, to destination: Int, count: Int) throws { + try self.initialize(segment.references, from: source, to: destination, count: count) + } - guard !fromIndex.addingReportingOverflow(count).overflow, - !toIndex.addingReportingOverflow(count).overflow - else { - throw Trap.tableSizeOverflow - } + mutating func initialize(_ references: [Reference], from source: Int, to destination: Int, count: Int) throws { + let (destinationEnd, destinationOverflow) = destination.addingReportingOverflow(count) + let (sourceEnd, sourceOverflow) = source.addingReportingOverflow(count) - guard fromIndex + count <= source.count else { - throw Trap.outOfBoundsTableAccess(fromIndex + count) - } - guard toIndex + count <= self.elements.count else { - throw Trap.outOfBoundsTableAccess(toIndex + count) + guard !destinationOverflow, !sourceOverflow else { throw Trap.tableSizeOverflow } + guard destinationEnd <= elements.count else { throw Trap.outOfBoundsTableAccess(destinationEnd) } + guard sourceEnd <= references.count else { throw Trap.outOfBoundsTableAccess(sourceEnd) } + + elements.withUnsafeMutableBufferPointer { table in + references.withUnsafeBufferPointer { segment in + _ = table[destination.., + _ destinationTable: UnsafeMutableBufferPointer, + from source: Int, to destination: Int, count: Int + ) throws { + let (destinationEnd, destinationOverflow) = destination.addingReportingOverflow(count) + let (sourceEnd, sourceOverflow) = source.addingReportingOverflow(count) + + guard !destinationOverflow, !sourceOverflow else { throw Trap.tableSizeOverflow } + guard destinationEnd <= destinationTable.count else { throw Trap.outOfBoundsTableAccess(Int(destinationEnd)) } + guard sourceEnd <= sourceTable.count else { throw Trap.outOfBoundsTableAccess(Int(sourceEnd)) } + + let source = UnsafeBufferPointer(rebasing: sourceTable[source.. +extension InternalTable { + func copy(_ sourceTable: InternalTable, from source: Int, to destination: Int, count: Int) throws { + // Check if the source and destination tables are the same for dynamic exclusive + // access enforcement + if self == sourceTable { + try withValue { + try $0.elements.withUnsafeMutableBufferPointer { + try TableEntity.copy(UnsafeBufferPointer($0), $0, from: source, to: destination, count: count) + } + } + } else { + try withValue { destinationTable in + try sourceTable.withValue { sourceTable in + try destinationTable.elements.withUnsafeMutableBufferPointer { dest in + try sourceTable.elements.withUnsafeBufferPointer { src in + try TableEntity.copy(src, dest, from: source, to: destination, count: count) + } + } + } + } + } + } +} + /// A WebAssembly `table` instance. /// > Note: /// @@ -395,6 +442,42 @@ struct MemoryEntity /* : ~Copyable */ { return limit.isMemory64 ? .i64(UInt64(result)) : .i32(result) } + mutating func copy(from source: UInt64, to destination: UInt64, count: UInt64) throws { + let (destinationEnd, destinationOverflow) = destination.addingReportingOverflow(count) + let (sourceEnd, sourceOverflow) = source.addingReportingOverflow(count) + + guard !destinationOverflow, destinationEnd <= data.count, + !sourceOverflow, sourceEnd <= data.count else { + throw Trap.outOfBoundsMemoryAccess + } + data.withUnsafeMutableBufferPointer { + guard let base = UnsafeMutableRawPointer($0.baseAddress) else { return } + let dest = base.advanced(by: Int(destination)) + let src = base.advanced(by: Int(source)) + dest.copyMemory(from: src, byteCount: Int(count)) + } + } + + mutating func initialize(_ segment: InternalDataSegment, from source: UInt32, to destination: UInt64, count: UInt32) throws { + let (destinationEnd, destinationOverflow) = destination.addingReportingOverflow(UInt64(count)) + let (sourceEnd, sourceOverflow) = source.addingReportingOverflow(count) + + guard !destinationOverflow, destinationEnd <= data.count, + !sourceOverflow, sourceEnd <= segment.data.count else { + throw Trap.outOfBoundsMemoryAccess + } + data.withUnsafeMutableBufferPointer { memory in + segment.data.withUnsafeBufferPointer { segment in + guard + let memory = UnsafeMutableRawPointer(memory.baseAddress), + let segment = UnsafeRawPointer(segment.baseAddress) else { return } + let dest = memory.advanced(by: Int(destination)) + let src = segment.advanced(by: Int(source)) + dest.copyMemory(from: src, byteCount: Int(count)) + } + } + } + mutating func write(offset: Int, bytes: ArraySlice) throws { let endOffset = offset + bytes.count guard endOffset <= data.count else { diff --git a/Sources/WasmKit/Execution/Instructions/Memory.swift b/Sources/WasmKit/Execution/Instructions/Memory.swift index 72c82cfa..fc86e5e3 100644 --- a/Sources/WasmKit/Execution/Instructions/Memory.swift +++ b/Sources/WasmKit/Execution/Instructions/Memory.swift @@ -58,29 +58,13 @@ extension Execution { mutating func memoryInit(sp: Sp, immediate: Instruction.MemoryInitOperand) throws { let instance = currentInstance(sp: sp) let memory = instance.memories[0] - try memory.withValue { memoryInstance in - let dataInstance = instance.dataSegments[Int(immediate.segmentIndex)] - - let copyCounter = sp[immediate.size].i32 - let sourceIndex = sp[immediate.sourceOffset].i32 - let destinationIndex = sp[immediate.destOffset].asAddressOffset(memoryInstance.limit.isMemory64) - - guard copyCounter > 0 else { return } - - guard - !sourceIndex.addingReportingOverflow(copyCounter).overflow - && !destinationIndex.addingReportingOverflow(UInt64(copyCounter)).overflow - && memoryInstance.data.count >= destinationIndex + UInt64(copyCounter) - && dataInstance.data.count >= sourceIndex + copyCounter - else { - throw Trap.outOfBoundsMemoryAccess - } + try memory.withValue { memory in + let segment = instance.dataSegments[Int(immediate.segmentIndex)] - // FIXME: benchmark if using `replaceSubrange` is faster than this loop - for i in 0.. 0 else { return } - - guard - !sourceIndex.addingReportingOverflow(copyCounter).overflow - && !destinationIndex.addingReportingOverflow(copyCounter).overflow - && memory.data.count >= destinationIndex + copyCounter - && memory.data.count >= sourceIndex + copyCounter - else { - throw Trap.outOfBoundsMemoryAccess - } - - if destinationIndex <= sourceIndex { - for i in 0.. 0 else { - return - } - - guard - !sourceIndex.addingReportingOverflow(copyCounter).overflow && !destinationIndex.addingReportingOverflow(copyCounter).overflow - else { - throw Trap.tableSizeOverflow - } - guard destinationIndex + copyCounter <= sourceTable.elements.count else { - throw Trap.outOfBoundsTableAccess(Int(destinationIndex + copyCounter)) - } - guard destinationIndex + copyCounter <= sourceTable.elements.count && sourceIndex + copyCounter <= destinationTable.elements.count else { - throw Trap.outOfBoundsTableAccess(Int(destinationIndex + copyCounter)) - } - - let valuesToCopy = Array(sourceTable.elements[Int(sourceIndex).. Date: Thu, 3 Oct 2024 12:30:41 +0900 Subject: [PATCH 3/6] Fix bounds check in `table.fill` --- Sources/WasmKit/Execution/Instances.swift | 9 +++++++++ Sources/WasmKit/Execution/Instructions/Table.swift | 12 +----------- Tests/WasmKitTests/SpectestTests.swift | 4 +--- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/Sources/WasmKit/Execution/Instances.swift b/Sources/WasmKit/Execution/Instances.swift index 5dd4fc00..2b078b38 100644 --- a/Sources/WasmKit/Execution/Instances.swift +++ b/Sources/WasmKit/Execution/Instances.swift @@ -319,6 +319,15 @@ struct TableEntity /* : ~Copyable */ { } } + mutating func fill(repeating value: Reference, from index: Int, count: Int) throws { + let (end, overflow) = index.addingReportingOverflow(count) + guard !overflow, end <= elements.count else { throw Trap.tableSizeOverflow } + + elements.withUnsafeMutableBufferPointer { + $0[index.., _ destinationTable: UnsafeMutableBufferPointer, diff --git a/Sources/WasmKit/Execution/Instructions/Table.swift b/Sources/WasmKit/Execution/Instructions/Table.swift index 76362717..3955138a 100644 --- a/Sources/WasmKit/Execution/Instructions/Table.swift +++ b/Sources/WasmKit/Execution/Instructions/Table.swift @@ -42,17 +42,7 @@ extension Execution { let fillValue = sp.getReference(immediate.value, type: table.tableType) let startIndex = sp[immediate.destOffset].asAddressOffset(table.limits.isMemory64) - guard fillCounter > 0 else { - return - } - - guard Int(startIndex + fillCounter) <= table.elements.count else { - throw Trap.outOfBoundsTableAccess(Int(startIndex + fillCounter)) - } - - for i in 0.. Date: Thu, 3 Oct 2024 12:44:00 +0900 Subject: [PATCH 4/6] Tests: Enable assert_return/wat test cases --- Tests/WasmKitTests/Spectest/TestCase.swift | 87 +++++++++------------- 1 file changed, 36 insertions(+), 51 deletions(-) diff --git a/Tests/WasmKitTests/Spectest/TestCase.swift b/Tests/WasmKitTests/Spectest/TestCase.swift index 041f3439..ea3dd609 100644 --- a/Tests/WasmKitTests/Spectest/TestCase.swift +++ b/Tests/WasmKitTests/Spectest/TestCase.swift @@ -168,7 +168,19 @@ extension WastRunContext { } return instance } - func deriveModuleInstance(from execute: WastExecute) throws -> Instance? { + func deriveInstance(by name: String?) throws -> Instance { + let instance: Instance? + if let name { + instance = lookupInstance(name) + } else { + instance = currentInstance + } + guard let instance else { + throw SpectestError("no module to execute") + } + return instance + } + func deriveInstance(from execute: WastExecute) throws -> Instance? { switch execute { case .invoke(let invoke): if let module = invoke.module { @@ -263,54 +275,17 @@ extension WastRunContext { return .passed case .assertReturn(let execute, let results): - let instance: Instance? - do { - instance = try deriveModuleInstance(from: execute) - } catch { - return .failed("failed to derive module instance: \(error)") - } - guard let instance else { - return .failed("no module to execute") - } - let expected = parseValues(args: results) - - switch execute { - case .invoke(let invoke): - let result: [WasmKit.Value] - do { - result = try wastInvoke(call: invoke) - } catch { - return .failed("\(error)") - } - guard result.isTestEquivalent(to: expected) else { - return .failed("invoke result mismatch: expected: \(expected), actual: \(result)") - } - return .passed - - case .get(_, let globalName): - let result: WasmKit.Value - do { - guard case let .global(global) = instance.export(globalName) else { - throw Trap._raw("no global export with name \(globalName) in a module instance \(instance)") - } - result = global.value - } catch { - return .failed("\(error)") - } - guard result.isTestEquivalent(to: expected[0]) else { - return .failed("get result mismatch: expected: \(expected), actual: \(result)") - } - return .passed - case .wat: return .skipped("TBD") + let actual = try wastExecute(execute: execute) + guard actual.isTestEquivalent(to: expected) else { + return .failed("invoke result mismatch: expected: \(expected), actual: \(actual)") } - + return .passed case .assertTrap(let execute, let message): switch execute { case .invoke(let invoke): do { _ = try wastInvoke(call: invoke) - // XXX: This is wrong but just keep it as is return .failed("trap expected: \(message)") } catch let trap as Trap { guard trap.assertionText.contains(message) else { @@ -367,16 +342,26 @@ extension WastRunContext { } } - private func wastInvoke(call: WastInvoke) throws -> [Value] { - let instance: Instance? - do { - instance = try deriveModuleInstance(from: .invoke(call)) - } catch { - throw SpectestError("failed to derive module instance: \(error)") - } - guard let instance else { - throw SpectestError("no module to execute") + private func wastExecute(execute: WastExecute) throws -> [Value] { + switch execute { + case .invoke(let invoke): + return try wastInvoke(call: invoke) + case .get(let module, let globalName): + let instance = try deriveInstance(by: module) + let result: WasmKit.Value + guard case let .global(global) = instance.export(globalName) else { + throw SpectestError("no global export with name \(globalName) in a module instance \(instance)") + } + return [global.value] + case .wat(var wat): + let module = try parseModule(rootPath: rootPath, moduleSource: .binary(wat.encode())) + _ = try instantiate(module: module) + return [] } + } + + private func wastInvoke(call: WastInvoke) throws -> [Value] { + let instance = try deriveInstance(by: call.module) guard let function = instance.exportedFunction(name: call.name) else { throw SpectestError("function \(call.name) not exported") } From d942598e9bba51bd1674937996cff98d9ff668ab Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Thu, 3 Oct 2024 12:44:52 +0900 Subject: [PATCH 5/6] Tests: Enable assert_trap for non-invoke actions --- Tests/WasmKitTests/Spectest/TestCase.swift | 23 +++++++++------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/Tests/WasmKitTests/Spectest/TestCase.swift b/Tests/WasmKitTests/Spectest/TestCase.swift index ea3dd609..83116846 100644 --- a/Tests/WasmKitTests/Spectest/TestCase.swift +++ b/Tests/WasmKitTests/Spectest/TestCase.swift @@ -282,21 +282,16 @@ extension WastRunContext { } return .passed case .assertTrap(let execute, let message): - switch execute { - case .invoke(let invoke): - do { - _ = try wastInvoke(call: invoke) - return .failed("trap expected: \(message)") - } catch let trap as Trap { - guard trap.assertionText.contains(message) else { - return .failed("assertion mismatch: expected: \(message), actual: \(trap.assertionText)") - } - return .passed - } catch { - return .failed("\(error)") + do { + _ = try wastExecute(execute: execute) + return .failed("trap expected: \(message)") + } catch let trap as Trap { + guard trap.assertionText.contains(message) else { + return .failed("assertion mismatch: expected: \(message), actual: \(trap.assertionText)") } - default: - return .failed("assert_trap is not implemented non-invoke actions") + return .passed + } catch { + return .failed("\(error)") } case .assertExhaustion(let call, let message): do { From 33dc3372f18d93122bcb4c1180d77877ed3c0b81 Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Thu, 3 Oct 2024 12:46:53 +0900 Subject: [PATCH 6/6] Tests: Remove unused code in TestCase.swift --- Tests/WasmKitTests/Spectest/TestCase.swift | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/Tests/WasmKitTests/Spectest/TestCase.swift b/Tests/WasmKitTests/Spectest/TestCase.swift index 83116846..5edb5425 100644 --- a/Tests/WasmKitTests/Spectest/TestCase.swift +++ b/Tests/WasmKitTests/Spectest/TestCase.swift @@ -86,7 +86,6 @@ enum Result { case passed case failed(String) case skipped(String) -// case `internal`(Swift.Error) var banner: String { switch self { @@ -96,8 +95,6 @@ enum Result { return "[FAILED]" case .skipped: return "[SKIPPED]" -// case .internal: -// return "[INTERNAL]" } } } @@ -328,11 +325,7 @@ extension WastRunContext { return .skipped("validation is no implemented yet") case .invoke(let invoke): - do { - _ = try wastInvoke(call: invoke) - } catch { - return .failed("\(error)") - } + _ = try wastInvoke(call: invoke) return .passed } } @@ -343,7 +336,6 @@ extension WastRunContext { return try wastInvoke(call: invoke) case .get(let module, let globalName): let instance = try deriveInstance(by: module) - let result: WasmKit.Value guard case let .global(global) = instance.export(globalName) else { throw SpectestError("no global export with name \(globalName) in a module instance \(instance)") }