[wasm] Introduce --wasm-enforce-bounds-checks flag

There currently is no way to enforce explicit bounds checks if the
embedder installed the signal handler for wasm trap handling (queried
via {trap_handler::IsTrapHandlerEnabled()}).
This CL adds a respective flag and makes all compilation emit explicit
bounds checks if it is disabled.

R=ahaas@chromium.org

Bug: v8:11926
Change-Id: Ie19faab1766d3105f3c22cb4470c0f15398f1d09
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2989129Reviewed-by: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75453}

src/compiler/backend/arm64/code-generator-arm64.cc

@@ -437,7 +437,7 @@ class WasmProtectedInstructionTrap final : public WasmOutOfLineTrap {
       : WasmOutOfLineTrap(gen, instr), pc_(pc) {}
 
   void Generate() override {
-    DCHECK(FLAG_wasm_bounds_checks && FLAG_wasm_trap_handler);
+    DCHECK(FLAG_wasm_bounds_checks && !FLAG_wasm_enforce_bounds_checks);
     gen_->AddProtectedInstructionLanding(pc_, __ pc_offset());
     GenerateWithTrapId(TrapId::kTrapMemOutOfBounds);
   }

src/compiler/backend/x64/code-generator-x64.cc

@@ -543,7 +543,7 @@ class WasmProtectedInstructionTrap final : public WasmOutOfLineTrap {
       : WasmOutOfLineTrap(gen, instr), pc_(pc) {}
 
   void Generate() final {
-    DCHECK(FLAG_wasm_bounds_checks && FLAG_wasm_trap_handler);
+    DCHECK(FLAG_wasm_bounds_checks && !FLAG_wasm_enforce_bounds_checks);
     gen_->AddProtectedInstructionLanding(pc_, __ pc_offset());
     GenerateWithTrapId(TrapId::kTrapMemOutOfBounds);
   }

src/flags/flag-definitions.h

@@ -1021,6 +1021,11 @@ DEFINE_BOOL(
     "enable bounds checks (disable for performance testing only)")
 DEFINE_BOOL(wasm_stack_checks, true,
             "enable stack checks (disable for performance testing only)")
+DEFINE_BOOL(
+    wasm_enforce_bounds_checks, false,
+    "enforce explicit bounds check even if the trap handler is available")
+// "no bounds checks" implies "no enforced bounds checks".
+DEFINE_NEG_NEG_IMPLICATION(wasm_bounds_checks, wasm_enforce_bounds_checks)
 DEFINE_BOOL(wasm_math_intrinsics, true,
             "intrinsify some Math imports into wasm")
 
@@ -1029,8 +1034,6 @@ DEFINE_BOOL(wasm_loop_unrolling, true,
 DEFINE_BOOL(wasm_trap_handler, true,
             "use signal handlers to catch out of bounds memory access in wasm"
             " (currently Linux x86_64 only)")
-// "no bounds checks" implies "no trap handlers".
-DEFINE_NEG_NEG_IMPLICATION(wasm_bounds_checks, wasm_trap_handler)
 DEFINE_BOOL(wasm_fuzzer_gen_test, false,
             "generate a test case when running a wasm fuzzer")
 DEFINE_IMPLICATION(wasm_fuzzer_gen_test, single_threaded)

src/wasm/wasm-code-manager.cc

@@ -847,7 +847,10 @@ NativeModule::NativeModule(const WasmFeatures& enabled,
       module_(std::move(module)),
       import_wrapper_cache_(std::unique_ptr<WasmImportWrapperCache>(
           new WasmImportWrapperCache())),
-      use_trap_handler_(trap_handler::IsTrapHandlerEnabled() ? kUseTrapHandler
+      // TODO(clemensb): Rename this field.
+      use_trap_handler_(trap_handler::IsTrapHandlerEnabled() &&
+                                !FLAG_wasm_enforce_bounds_checks
+                            ? kUseTrapHandler
                             : kNoTrapHandler) {
   DCHECK(engine_scope_);
   // We receive a pointer to an empty {std::shared_ptr}, and install ourselve

test/cctest/cctest.cc

@@ -344,11 +344,9 @@ int main(int argc, char* argv[]) {
   v8::V8::InitializeExternalStartupData(argv[0]);
 
 #if V8_ENABLE_WEBASSEMBLY && V8_TRAP_HANDLER_SUPPORTED
-  if (i::FLAG_wasm_trap_handler) {
-    constexpr bool use_default_signal_handler = true;
-    CHECK(v8::V8::EnableWebAssemblyTrapHandler(use_default_signal_handler));
-  }
-#endif  // V8_TRAP_HANDLER_SUPPORTED && V8_ENABLE_WEBASSEMBLY
+  constexpr bool kUseDefaultTrapHandler = true;
+  CHECK(v8::V8::EnableWebAssemblyTrapHandler(kUseDefaultTrapHandler));
+#endif  // V8_ENABLE_WEBASSEMBLY && V8_TRAP_HANDLER_SUPPORTED
 
   CcTest::set_array_buffer_allocator(
       v8::ArrayBuffer::Allocator::NewDefaultAllocator());

test/cctest/wasm/test-run-wasm-atomics.cc

@@ -319,7 +319,8 @@ WASM_EXEC_TEST(AtomicFence) {
 
 WASM_EXEC_TEST(AtomicStoreNoConsideredEffectful) {
   EXPERIMENTAL_FLAG_SCOPE(threads);
-  FLAG_wasm_trap_handler = false;  // To use {Load} instead of {ProtectedLoad}.
+  // Use {Load} instead of {ProtectedLoad}.
+  FLAG_SCOPE(wasm_enforce_bounds_checks);
   WasmRunner<uint32_t> r(execution_tier);
   r.builder().AddMemoryElems<int32_t>(kWasmPageSize / sizeof(int32_t));
   r.builder().SetHasSharedMemory();
@@ -332,7 +333,8 @@ WASM_EXEC_TEST(AtomicStoreNoConsideredEffectful) {
 
 void RunNoEffectTest(TestExecutionTier execution_tier, WasmOpcode wasm_op) {
   EXPERIMENTAL_FLAG_SCOPE(threads);
-  FLAG_wasm_trap_handler = false;  // To use {Load} instead of {ProtectedLoad}.
+  // Use {Load} instead of {ProtectedLoad}.
+  FLAG_SCOPE(wasm_enforce_bounds_checks);
   WasmRunner<uint32_t> r(execution_tier);
   r.builder().AddMemoryElems<int32_t>(kWasmPageSize / sizeof(int32_t));
   r.builder().SetHasSharedMemory();
@@ -353,7 +355,8 @@ WASM_EXEC_TEST(AtomicExchangeNoConsideredEffectful) {
 
 WASM_EXEC_TEST(AtomicCompareExchangeNoConsideredEffectful) {
   EXPERIMENTAL_FLAG_SCOPE(threads);
-  FLAG_wasm_trap_handler = false;  // To use {Load} instead of {ProtectedLoad}.
+  // Use {Load} instead of {ProtectedLoad}.
+  FLAG_SCOPE(wasm_enforce_bounds_checks);
   WasmRunner<uint32_t> r(execution_tier);
   r.builder().AddMemoryElems<int32_t>(kWasmPageSize / sizeof(int32_t));
   r.builder().SetHasSharedMemory();

test/cctest/wasm/test-run-wasm-atomics64.cc

@@ -648,7 +648,8 @@ WASM_EXEC_TEST(I64AtomicCompareExchange32UFail) {
 
 WASM_EXEC_TEST(AtomicStoreNoConsideredEffectful) {
   EXPERIMENTAL_FLAG_SCOPE(threads);
-  FLAG_wasm_trap_handler = false;  // To use {Load} instead of {ProtectedLoad}.
+  // Use {Load} instead of {ProtectedLoad}.
+  FLAG_SCOPE(wasm_enforce_bounds_checks);
   WasmRunner<uint32_t> r(execution_tier);
   r.builder().AddMemoryElems<int64_t>(kWasmPageSize / sizeof(int64_t));
   r.builder().SetHasSharedMemory();
@@ -661,7 +662,8 @@ WASM_EXEC_TEST(AtomicStoreNoConsideredEffectful) {
 
 void RunNoEffectTest(TestExecutionTier execution_tier, WasmOpcode wasm_op) {
   EXPERIMENTAL_FLAG_SCOPE(threads);
-  FLAG_wasm_trap_handler = false;  // To use {Load} instead of {ProtectedLoad}.
+  // Use {Load} instead of {ProtectedLoad}.
+  FLAG_SCOPE(wasm_enforce_bounds_checks);
   WasmRunner<uint32_t> r(execution_tier);
   r.builder().AddMemoryElems<int64_t>(kWasmPageSize / sizeof(int64_t));
   r.builder().SetHasSharedMemory();
@@ -682,7 +684,8 @@ WASM_EXEC_TEST(AtomicExchangeNoConsideredEffectful) {
 
 WASM_EXEC_TEST(AtomicCompareExchangeNoConsideredEffectful) {
   EXPERIMENTAL_FLAG_SCOPE(threads);
-  FLAG_wasm_trap_handler = false;  // To use {Load} instead of {ProtectedLoad}.
+  // Use {Load} instead of {ProtectedLoad}.
+  FLAG_SCOPE(wasm_enforce_bounds_checks);
   WasmRunner<uint32_t> r(execution_tier);
   r.builder().AddMemoryElems<uint64_t>(kWasmPageSize / sizeof(uint64_t));
   r.builder().SetHasSharedMemory();

test/cctest/wasm/wasm-run-utils.cc

@@ -314,10 +314,10 @@ uint32_t TestingModuleBuilder::AddPassiveElementSegment(
 CompilationEnv TestingModuleBuilder::CreateCompilationEnv() {
   // This is a hack so we don't need to call
   // trap_handler::IsTrapHandlerEnabled().
-  const bool is_trap_handler_enabled =
-      V8_TRAP_HANDLER_SUPPORTED && i::FLAG_wasm_trap_handler;
+  const bool use_trap_handler =
+      V8_TRAP_HANDLER_SUPPORTED && !i::FLAG_wasm_enforce_bounds_checks;
   return {test_module_.get(),
-          is_trap_handler_enabled ? kUseTrapHandler : kNoTrapHandler,
+          use_trap_handler ? kUseTrapHandler : kNoTrapHandler,
           runtime_exception_support_, enabled_features_};
 }