[wasm] Rename use_trap_handler fields to bounds_checks

This is a three-state field now: kTrapHandler, kExplicitBoundsChecks,
kNoBoundsChecks. It is set once based on the flags
(--wasm-bounds-checks and --wasm-enforce-bounds-checks) and depending on
whether the signal handler for wasm trap handling was installed. All
compilation then only uses the field value, and does not need to check
any flags any more.

R=ahaas@chromium.org

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

src/compiler/wasm-compiler.cc

@@ -461,7 +461,8 @@ WasmGraphBuilder::WasmGraphBuilder(
       sig_(sig),
       source_position_table_(source_position_table),
       isolate_(isolate) {
-  DCHECK_IMPLIES(use_trap_handler(), trap_handler::IsTrapHandlerEnabled());
+  DCHECK_IMPLIES(env && env->bounds_checks == wasm::kTrapHandler,
+                 trap_handler::IsTrapHandlerEnabled());
   DCHECK_NOT_NULL(mcgraph_);
 }
 
@@ -3772,8 +3773,8 @@ WasmGraphBuilder::BoundsCheckMem(uint8_t access_size, Node* index,
   } else if (kSystemPointerSize == kInt32Size) {
     // In memory64 mode on 32-bit systems, the upper 32 bits need to be zero to
     // succeed the bounds check.
-    DCHECK(!use_trap_handler());
-    if (FLAG_wasm_bounds_checks) {
+    DCHECK_NE(wasm::kTrapHandler, env_->bounds_checks);
+    if (env_->bounds_checks == wasm::kExplicitBoundsChecks) {
       Node* high_word = gasm_->TruncateInt64ToInt32(
           gasm_->Word64Shr(index, Int32Constant(32)));
       TrapIfTrue(wasm::kTrapMemOutOfBounds, high_word, position);
@@ -3784,7 +3785,7 @@ WasmGraphBuilder::BoundsCheckMem(uint8_t access_size, Node* index,
 
   // If no bounds checks should be performed (for testing), just return the
   // converted index and assume it to be in-bounds.
-  if (!FLAG_wasm_bounds_checks) return {index, kInBounds};
+  if (env_->bounds_checks == wasm::kNoBoundsChecks) return {index, kInBounds};
 
   // The accessed memory is [index + offset, index + end_offset].
   // Check that the last read byte (at {index + end_offset}) is in bounds.
@@ -3805,7 +3806,8 @@ WasmGraphBuilder::BoundsCheckMem(uint8_t access_size, Node* index,
     return {index, kInBounds};
   }
 
-  if (use_trap_handler() && enforce_check == kCanOmitBoundsCheck) {
+  if (env_->bounds_checks == wasm::kTrapHandler &&
+      enforce_check == kCanOmitBoundsCheck) {
     return {index, kTrapHandler};
   }
 
@@ -7513,7 +7515,7 @@ wasm::WasmCompilationResult CompileWasmMathIntrinsic(
           InstructionSelector::AlignmentRequirements()));
 
   wasm::CompilationEnv env(
-      nullptr, wasm::UseTrapHandler::kNoTrapHandler,
+      nullptr, wasm::kNoBoundsChecks,
       wasm::RuntimeExceptionSupport::kNoRuntimeExceptionSupport,
       wasm::WasmFeatures::All());
 

src/compiler/wasm-compiler.h

@@ -510,8 +510,8 @@ class WasmGraphBuilder {
 
   bool has_simd() const { return has_simd_; }
 
-  wasm::UseTrapHandler use_trap_handler() const {
-    return env_ ? env_->use_trap_handler : wasm::kNoTrapHandler;
+  wasm::BoundsCheckStrategy bounds_checks() const {
+    return env_->bounds_checks;
   }
 
   MachineGraph* mcgraph() { return mcgraph_; }

src/wasm/baseline/liftoff-compiler.cc

@@ -865,7 +865,7 @@ class LiftoffCompiler {
     DCHECK_IMPLIES(ool->stub != WasmCode::kThrowWasmTrapMemOutOfBounds,
                    ool->pc == 0);
 
-    if (env_->use_trap_handler && ool->pc != 0) {
+    if (env_->bounds_checks == kTrapHandler && ool->pc != 0) {
       uint32_t pc = static_cast<uint32_t>(__ pc_offset());
       DCHECK_EQ(pc, __ pc_offset());
       protected_instructions_.emplace_back(
@@ -2666,11 +2666,17 @@ class LiftoffCompiler {
     Register index_ptrsize =
         kNeedI64RegPair && index.is_gp_pair() ? index.low_gp() : index.gp();
 
+    // Without bounds checks (testing only), just return the ptrsize index.
+    if (V8_UNLIKELY(env_->bounds_checks == kNoBoundsChecks)) {
+      return index_ptrsize;
+    }
+
+    // Early return for trap handler.
     if (!force_check && !statically_oob &&
-        (!FLAG_wasm_bounds_checks || env_->use_trap_handler)) {
+        env_->bounds_checks == kTrapHandler) {
       // With trap handlers we should not have a register pair as input (we
       // would only return the lower half).
-      DCHECK_IMPLIES(env_->use_trap_handler, index.is_gp());
+      DCHECK(index.is_gp());
       return index_ptrsize;
     }
 
@@ -2681,7 +2687,7 @@ class LiftoffCompiler {
     Label* trap_label =
         AddOutOfLineTrap(decoder, WasmCode::kThrowWasmTrapMemOutOfBounds, 0);
 
-    if (statically_oob) {
+    if (V8_UNLIKELY(statically_oob)) {
       __ emit_jump(trap_label);
       decoder->SetSucceedingCodeDynamicallyUnreachable();
       return no_reg;
@@ -2810,7 +2816,8 @@ class LiftoffCompiler {
 
   Register AddMemoryMasking(Register index, uintptr_t* offset,
                             LiftoffRegList* pinned) {
-    if (!FLAG_untrusted_code_mitigations || env_->use_trap_handler) {
+    if (!FLAG_untrusted_code_mitigations ||
+        env_->bounds_checks == kTrapHandler) {
       return index;
     }
     CODE_COMMENT("mask memory index");
@@ -2901,7 +2908,7 @@ class LiftoffCompiler {
       uint32_t protected_load_pc = 0;
       __ Load(value, mem, index, offset, type, pinned, &protected_load_pc, true,
               i64_offset);
-      if (env_->use_trap_handler) {
+      if (env_->bounds_checks == kTrapHandler) {
         AddOutOfLineTrap(decoder, WasmCode::kThrowWasmTrapMemOutOfBounds,
                          protected_load_pc);
       }
@@ -2944,7 +2951,7 @@ class LiftoffCompiler {
     __ LoadTransform(value, addr, index, offset, type, transform,
                      &protected_load_pc);
 
-    if (env_->use_trap_handler) {
+    if (env_->bounds_checks == kTrapHandler) {
       AddOutOfLineTrap(decoder, WasmCode::kThrowWasmTrapMemOutOfBounds,
                        protected_load_pc);
     }
@@ -2984,7 +2991,7 @@ class LiftoffCompiler {
 
     __ LoadLane(result, value, addr, index, offset, type, laneidx,
                 &protected_load_pc);
-    if (env_->use_trap_handler) {
+    if (env_->bounds_checks == kTrapHandler) {
       AddOutOfLineTrap(decoder, WasmCode::kThrowWasmTrapMemOutOfBounds,
                        protected_load_pc);
     }
@@ -3032,7 +3039,7 @@ class LiftoffCompiler {
       if (V8_UNLIKELY(FLAG_trace_wasm_memory)) outer_pinned.set(index);
       __ Store(mem, index, offset, value, type, outer_pinned,
                &protected_store_pc, true);
-      if (env_->use_trap_handler) {
+      if (env_->bounds_checks == kTrapHandler) {
         AddOutOfLineTrap(decoder, WasmCode::kThrowWasmTrapMemOutOfBounds,
                          protected_store_pc);
       }
@@ -3062,7 +3069,7 @@ class LiftoffCompiler {
     Register addr = pinned.set(GetMemoryStart(pinned));
     uint32_t protected_store_pc = 0;
     __ StoreLane(addr, index, offset, value, type, lane, &protected_store_pc);
-    if (env_->use_trap_handler) {
+    if (env_->bounds_checks == kTrapHandler) {
       AddOutOfLineTrap(decoder, WasmCode::kThrowWasmTrapMemOutOfBounds,
                        protected_store_pc);
     }

src/wasm/compilation-environment.h

@@ -36,7 +36,14 @@ enum RuntimeExceptionSupport : bool {
   kNoRuntimeExceptionSupport = false
 };
 
-enum UseTrapHandler : bool { kUseTrapHandler = true, kNoTrapHandler = false };
+enum BoundsCheckStrategy : int8_t {
+  // Emit protected instructions, use the trap handler for OOB detection.
+  kTrapHandler,
+  // Emit explicit bounds checks.
+  kExplicitBoundsChecks,
+  // Emit no bounds checks at all (for testing only).
+  kNoBoundsChecks
+};
 
 // The {CompilationEnv} encapsulates the module data that is used during
 // compilation. CompilationEnvs are shareable across multiple compilations.
@@ -44,9 +51,8 @@ struct CompilationEnv {
   // A pointer to the decoded module's static representation.
   const WasmModule* const module;
 
-  // True if trap handling should be used in compiled code, rather than
-  // compiling in bounds checks for each memory access.
-  const UseTrapHandler use_trap_handler;
+  // The bounds checking strategy to use.
+  const BoundsCheckStrategy bounds_checks;
 
   // If the runtime doesn't support exception propagation,
   // we won't generate stack checks, and trap handling will also
@@ -65,11 +71,11 @@ struct CompilationEnv {
   const WasmFeatures enabled_features;
 
   constexpr CompilationEnv(const WasmModule* module,
-                           UseTrapHandler use_trap_handler,
+                           BoundsCheckStrategy bounds_checks,
                            RuntimeExceptionSupport runtime_exception_support,
                            const WasmFeatures& enabled_features)
       : module(module),
-        use_trap_handler(use_trap_handler),
+        bounds_checks(bounds_checks),
         runtime_exception_support(runtime_exception_support),
         // During execution, the memory can never be bigger than what fits in a
         // uintptr_t.

src/wasm/wasm-code-manager.cc

@@ -847,11 +847,12 @@ NativeModule::NativeModule(const WasmFeatures& enabled,
       module_(std::move(module)),
       import_wrapper_cache_(std::unique_ptr<WasmImportWrapperCache>(
           new WasmImportWrapperCache())),
-      // TODO(clemensb): Rename this field.
-      use_trap_handler_(trap_handler::IsTrapHandlerEnabled() &&
-                                !FLAG_wasm_enforce_bounds_checks
-                            ? kUseTrapHandler
-                            : kNoTrapHandler) {
+      bounds_checks_(!FLAG_wasm_bounds_checks
+                         ? kNoBoundsChecks
+                         : FLAG_wasm_enforce_bounds_checks ||
+                                   !trap_handler::IsTrapHandlerEnabled()
+                               ? kExplicitBoundsChecks
+                               : kTrapHandler) {
   DCHECK(engine_scope_);
   // We receive a pointer to an empty {std::shared_ptr}, and install ourselve
   // there.
@@ -928,7 +929,7 @@ void NativeModule::LogWasmCodes(Isolate* isolate, Script script) {
 }
 
 CompilationEnv NativeModule::CreateCompilationEnv() const {
-  return {module(), use_trap_handler_, kRuntimeExceptionSupport,
+  return {module(), bounds_checks_, kRuntimeExceptionSupport,
           enabled_features_};
 }
 

src/wasm/wasm-code-manager.h

@@ -691,7 +691,7 @@ class V8_EXPORT_PRIVATE NativeModule final {
   uint32_t num_imported_functions() const {
     return module_->num_imported_functions;
   }
-  UseTrapHandler use_trap_handler() const { return use_trap_handler_; }
+  BoundsCheckStrategy bounds_checks() const { return bounds_checks_; }
   void set_lazy_compile_frozen(bool frozen) { lazy_compile_frozen_ = frozen; }
   bool lazy_compile_frozen() const { return lazy_compile_frozen_; }
   base::Vector<const uint8_t> wire_bytes() const {
@@ -924,7 +924,7 @@ class V8_EXPORT_PRIVATE NativeModule final {
   // End of fields protected by {allocation_mutex_}.
   //////////////////////////////////////////////////////////////////////////////
 
-  UseTrapHandler use_trap_handler_ = kNoTrapHandler;
+  const BoundsCheckStrategy bounds_checks_;
   bool lazy_compile_frozen_ = false;
   std::atomic<size_t> liftoff_bailout_count_{0};
   std::atomic<size_t> liftoff_code_size_{0};

src/wasm/wasm-objects.cc

@@ -783,7 +783,8 @@ void SetInstanceMemory(Handle<WasmInstanceObject> instance,
                        Handle<JSArrayBuffer> buffer) {
   bool is_wasm_module = instance->module()->origin == wasm::kWasmOrigin;
   bool use_trap_handler =
-      instance->module_object().native_module()->use_trap_handler();
+      instance->module_object().native_module()->bounds_checks() ==
+      wasm::kTrapHandler;
   // Wasm modules compiled to use the trap handler don't have bounds checks,
   // so they must have a memory that has guard regions.
   CHECK_IMPLIES(is_wasm_module && use_trap_handler,

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

@@ -318,7 +318,7 @@ CompilationEnv TestingModuleBuilder::CreateCompilationEnv() {
   const bool use_trap_handler =
       V8_TRAP_HANDLER_SUPPORTED && !i::FLAG_wasm_enforce_bounds_checks;
   return {test_module_.get(),
-          use_trap_handler ? kUseTrapHandler : kNoTrapHandler,
+          use_trap_handler ? kTrapHandler : kExplicitBoundsChecks,
           runtime_exception_support_, enabled_features_};
 }