Revert "Reland "[codegen] Trap on abort in release mode""

This reverts commit fd3b14d5fb.

Reason for revert: fails on https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux%20-%20arm%20-%20sim/48544/overview

Original change's description:
> Reland "[codegen] Trap on abort in release mode"
>
> This is a reland of commit a889f04e2d.
> PS2 fixes all architectures.
>
> Original change's description:
> > [codegen] Trap on abort in release mode
> >
> > Remove the --trap-on-abort flag and make this the default if no debug
> > code is requested.
> >
> > This saves 3-5% of generated code size, and potentially increases
> > performance as well because the code is more compact.
> >
> > Drive-by: Avoid redundant source location output in the three code
> > comments on MacroAssembler::Abort. This makes the comments more
> > readable.
> >
> > R=jkummerow@chromium.org
> >
> > Bug: 389879224
> > Change-Id: I3119a61c28b8e289db4a6b6ee06f11e27c37cb50
> > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6172287
> > Commit-Queue: Clemens Backes <clemensb@chromium.org>
> > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#98147}
>
> Bug: 389879224
> Change-Id: Iada8add57f174a4c4bc8a7be1c6c36e829a5522a
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6172464
> Commit-Queue: Clemens Backes <clemensb@chromium.org>
> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#98154}

Bug: 389879224
Change-Id: I1a9b9f495af8f37f13f7b59beaf90d21f5742785
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6179681
Commit-Queue: Eva Herencsárová <evih@chromium.org>
Owners-Override: Eva Herencsárová <evih@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#98156}

src/codegen/arm/macro-assembler-arm.cc

@@ -2447,13 +2447,16 @@ void MacroAssembler::Check(Condition cond, AbortReason reason) {
 
 void MacroAssembler::Abort(AbortReason reason) {
   ASM_CODE_COMMENT(this);
+  Label abort_start;
+  bind(&abort_start);
   if (v8_flags.code_comments) {
-    RecordComment("Abort message:", SourceLocation{});
-    RecordComment(GetAbortReason(reason), SourceLocation{});
+    const char* msg = GetAbortReason(reason);
+    RecordComment("Abort message: ");
+    RecordComment(msg);
   }
 
-  // Without debug code, save the code size and just trap.
-  if (!v8_flags.debug_code) {
+  // Avoid emitting call to builtin if requested.
+  if (trap_on_abort()) {
     stop();
     return;
   }

src/codegen/arm64/macro-assembler-arm64.cc

@@ -4368,12 +4368,12 @@ void MacroAssembler::DebugBreak() { Debug("DebugBreak", 0, BREAK); }
 void MacroAssembler::Abort(AbortReason reason) {
   ASM_CODE_COMMENT(this);
   if (v8_flags.code_comments) {
-    RecordComment("Abort message:", SourceLocation{});
-    RecordComment(GetAbortReason(reason), SourceLocation{});
+    RecordComment("Abort message: ");
+    RecordComment(GetAbortReason(reason));
   }
 
-  // Without debug code, save the code size and just trap.
-  if (!v8_flags.debug_code) {
+  // Avoid emitting call to builtin if requested.
+  if (trap_on_abort()) {
     Brk(0);
     return;
   }

src/codegen/ia32/macro-assembler-ia32.cc

@@ -1894,14 +1894,14 @@ void MacroAssembler::AlignStackPointer() {
 }
 
 void MacroAssembler::Abort(AbortReason reason) {
-  ASM_CODE_COMMENT(this);
   if (v8_flags.code_comments) {
-    RecordComment("Abort message:", SourceLocation{});
-    RecordComment(GetAbortReason(reason), SourceLocation{});
+    const char* msg = GetAbortReason(reason);
+    RecordComment("Abort message: ");
+    RecordComment(msg);
   }
 
-  // Without debug code, save the code size and just trap.
-  if (!v8_flags.debug_code) {
+  // Avoid emitting call to builtin if requested.
+  if (trap_on_abort()) {
     int3();
     return;
   }

src/codegen/loong64/macro-assembler-loong64.cc

@@ -3943,14 +3943,16 @@ void MacroAssembler::SbxCheck(Condition cc, AbortReason reason, Register rj,
 }
 
 void MacroAssembler::Abort(AbortReason reason) {
-  ASM_CODE_COMMENT(this);
+  Label abort_start;
+  bind(&abort_start);
   if (v8_flags.code_comments) {
-    RecordComment("Abort message:", SourceLocation{});
-    RecordComment(GetAbortReason(reason), SourceLocation{});
+    const char* msg = GetAbortReason(reason);
+    RecordComment("Abort message: ");
+    RecordComment(msg);
   }
 
-  // Without debug code, save the code size and just trap.
-  if (!v8_flags.debug_code) {
+  // Avoid emitting call to builtin if requested.
+  if (trap_on_abort()) {
     stop();
     return;
   }
@@ -3967,9 +3969,6 @@ void MacroAssembler::Abort(AbortReason reason) {
     return;
   }
 
-  Label abort_start;
-  bind(&abort_start);
-
   Move(a0, Smi::FromInt(static_cast<int>(reason)));
 
   {

src/codegen/macro-assembler-base.h

@@ -57,6 +57,8 @@ class V8_EXPORT_PRIVATE MacroAssemblerBase : public Assembler {
   bool root_array_available() const { return root_array_available_; }
   void set_root_array_available(bool v) { root_array_available_ = v; }
 
+  bool trap_on_abort() const { return trap_on_abort_; }
+
   bool should_abort_hard() const { return hard_abort_; }
   void set_abort_hard(bool v) { hard_abort_ = v; }
 
@@ -135,6 +137,9 @@ class V8_EXPORT_PRIVATE MacroAssemblerBase : public Assembler {
   // Whether kRootRegister has been initialized.
   bool root_array_available_ = true;
 
+  // Immediately trap instead of calling {Abort} when debug code fails.
+  bool trap_on_abort_ = v8_flags.trap_on_abort;
+
   // Emit a C call to abort instead of a runtime call.
   bool hard_abort_ = false;
 

src/codegen/mips64/macro-assembler-mips64.cc

@@ -5359,14 +5359,16 @@ void MacroAssembler::Check(Condition cc, AbortReason reason, Register rs,
 }
 
 void MacroAssembler::Abort(AbortReason reason) {
-  ASM_CODE_COMMENT(this);
+  Label abort_start;
+  bind(&abort_start);
   if (v8_flags.code_comments) {
-    RecordComment("Abort message:", SourceLocation{});
-    RecordComment(GetAbortReason(reason), SourceLocation{});
+    const char* msg = GetAbortReason(reason);
+    RecordComment("Abort message: ");
+    RecordComment(msg);
   }
 
-  // Without debug code, save the code size and just trap.
-  if (!v8_flags.debug_code) {
+  // Avoid emitting call to builtin if requested.
+  if (trap_on_abort()) {
     stop();
     return;
   }
@@ -5383,9 +5385,6 @@ void MacroAssembler::Abort(AbortReason reason) {
     return;
   }
 
-  Label abort_start;
-  bind(&abort_start);
-
   Move(a0, Smi::FromInt(static_cast<int>(reason)));
 
   {

src/codegen/ppc/macro-assembler-ppc.cc

@@ -2440,14 +2440,16 @@ void MacroAssembler::Check(Condition cond, AbortReason reason, CRegister cr) {
 }
 
 void MacroAssembler::Abort(AbortReason reason) {
-  ASM_CODE_COMMENT(this);
+  Label abort_start;
+  bind(&abort_start);
   if (v8_flags.code_comments) {
-    RecordComment("Abort message:", SourceLocation{});
-    RecordComment(GetAbortReason(reason), SourceLocation{});
+    const char* msg = GetAbortReason(reason);
+    RecordComment("Abort message: ");
+    RecordComment(msg);
   }
 
-  // Without debug code, save the code size and just trap.
-  if (!v8_flags.debug_code) {
+  // Avoid emitting call to builtin if requested.
+  if (trap_on_abort()) {
     stop();
     return;
   }

src/codegen/riscv/macro-assembler-riscv.cc

@@ -6351,14 +6351,16 @@ void MacroAssembler::SbxCheck(Condition cc, AbortReason reason, Register rs,
 }
 
 void MacroAssembler::Abort(AbortReason reason) {
-  ASM_CODE_COMMENT(this);
+  Label abort_start;
+  bind(&abort_start);
   if (v8_flags.code_comments) {
-    RecordComment("Abort message:", SourceLocation{});
-    RecordComment(GetAbortReason(reason), SourceLocation{});
+    const char* msg = GetAbortReason(reason);
+    RecordComment("Abort message: ");
+    RecordComment(msg);
   }
 
-  // Without debug code, save the code size and just trap.
-  if (!v8_flags.debug_code) {
+  // Avoid emitting call to builtin if requested.
+  if (trap_on_abort()) {
     ebreak();
     return;
   }
@@ -6375,9 +6377,6 @@ void MacroAssembler::Abort(AbortReason reason) {
     return;
   }
 
-  Label abort_start;
-  bind(&abort_start);
-
   Move(a0, Smi::FromInt(static_cast<int>(reason)));
 
   {

src/codegen/s390/macro-assembler-s390.cc

@@ -2278,14 +2278,16 @@ void MacroAssembler::Check(Condition cond, AbortReason reason, CRegister cr) {
 }
 
 void MacroAssembler::Abort(AbortReason reason) {
-  ASM_CODE_COMMENT(this);
+  Label abort_start;
+  bind(&abort_start);
   if (v8_flags.code_comments) {
-    RecordComment("Abort message:", SourceLocation{});
-    RecordComment(GetAbortReason(reason), SourceLocation{});
+    const char* msg = GetAbortReason(reason);
+    RecordComment("Abort message: ");
+    RecordComment(msg);
   }
 
-  // Without debug code, save the code size and just trap.
-  if (!v8_flags.debug_code) {
+  // Avoid emitting call to builtin if requested.
+  if (trap_on_abort()) {
     stop();
     return;
   }

src/codegen/x64/macro-assembler-x64.cc

@@ -1056,12 +1056,13 @@ void MacroAssembler::AlignStackPointer() {
 void MacroAssembler::Abort(AbortReason reason) {
   ASM_CODE_COMMENT(this);
   if (v8_flags.code_comments) {
-    RecordComment("Abort message:", SourceLocation{});
-    RecordComment(GetAbortReason(reason), SourceLocation{});
+    const char* msg = GetAbortReason(reason);
+    RecordComment("Abort message: ");
+    RecordComment(msg);
   }
 
-  // Without debug code, save the code size and just trap.
-  if (!v8_flags.debug_code) {
+  // Avoid emitting call to builtin if requested.
+  if (trap_on_abort()) {
     int3();
     return;
   }

src/flags/flag-definitions.h

@@ -3063,6 +3063,7 @@ DEFINE_BOOL_READONLY(enable_slow_asserts, false,
 
 // codegen-ia32.cc / codegen-arm.cc / macro-assembler-*.cc
 DEFINE_BOOL(print_ast, false, "print source AST")
+DEFINE_BOOL(trap_on_abort, false, "replace aborts by breakpoints")
 
 // compiler.cc
 DEFINE_BOOL(print_scopes, false, "print scopes")

test/unittests/assembler/macro-assembler-arm-unittest.cc

@@ -15,18 +15,14 @@ namespace internal {
 
 #define __ masm.
 
-const char* ErrorMessage(const char* msg) {
 // If we are running on android and the output is not redirected (i.e. ends up
 // in the android log) then we cannot find the error message in the output. This
 // macro just returns the empty string in that case.
 #if defined(ANDROID) && !defined(V8_ANDROID_LOG_STDOUT)
-  return "";
+#define ERROR_MESSAGE(msg) ""
 #else
-  // We only print the abort reason if debug code is enabled. Otherwise we just
-  // stop execution. Return an empty string in that case.
-  return v8_flags.debug_code ? msg : "";
+#define ERROR_MESSAGE(msg) msg
 #endif
-}
 
 // Test the x64 assembler by compiling some simple functions into
 // a buffer and executing them.  These tests do not initialize the
@@ -49,7 +45,7 @@ TEST_F(MacroAssemblerTest, TestHardAbort) {
   // We need an isolate here to execute in the simulator.
   auto f = GeneratedCode<void>::FromBuffer(isolate(), buffer->start());
 
-  ASSERT_DEATH_IF_SUPPORTED({ f.Call(); }, ErrorMessage("abort: no reason"));
+  ASSERT_DEATH_IF_SUPPORTED({ f.Call(); }, ERROR_MESSAGE("abort: no reason"));
 }
 
 TEST_F(MacroAssemblerTest, TestCheck) {
@@ -73,7 +69,7 @@ TEST_F(MacroAssemblerTest, TestCheck) {
 
   f.Call(0);
   f.Call(18);
-  ASSERT_DEATH_IF_SUPPORTED({ f.Call(17); }, ErrorMessage("abort: no reason"));
+  ASSERT_DEATH_IF_SUPPORTED({ f.Call(17); }, ERROR_MESSAGE("abort: no reason"));
 }
 
 struct MoveObjectAndSlotTestCase {
@@ -188,6 +184,7 @@ INSTANTIATE_TEST_SUITE_P(MacroAssemblerTest,
                          ::testing::ValuesIn(kMoveObjectAndSlotTestCases));
 
 #undef __
+#undef ERROR_MESSAGE
 
 }  // namespace internal
 }  // namespace v8

test/unittests/assembler/macro-assembler-arm64-unittest.cc

@@ -15,18 +15,14 @@ namespace internal {
 
 #define __ masm.
 
-const char* ErrorMessage(const char* msg) {
 // If we are running on android and the output is not redirected (i.e. ends up
 // in the android log) then we cannot find the error message in the output. This
 // macro just returns the empty string in that case.
 #if defined(ANDROID) && !defined(V8_ANDROID_LOG_STDOUT)
-  return "";
+#define ERROR_MESSAGE(msg) ""
 #else
-  // We only print the abort reason if debug code is enabled. Otherwise we just
-  // stop execution. Return an empty string in that case.
-  return v8_flags.debug_code ? msg : "";
+#define ERROR_MESSAGE(msg) msg
 #endif
-}
 
 // Test the x64 assembler by compiling some simple functions into
 // a buffer and executing them.  These tests do not initialize the
@@ -54,7 +50,7 @@ TEST_F(MacroAssemblerTest, TestHardAbort) {
   // We need an isolate here to execute in the simulator.
   auto f = GeneratedCode<void>::FromBuffer(isolate(), buffer->start());
 
-  ASSERT_DEATH_IF_SUPPORTED({ f.Call(); }, ErrorMessage("abort: no reason"));
+  ASSERT_DEATH_IF_SUPPORTED({ f.Call(); }, ERROR_MESSAGE("abort: no reason"));
 }
 
 TEST_F(MacroAssemblerTest, TestCheck) {
@@ -83,7 +79,7 @@ TEST_F(MacroAssemblerTest, TestCheck) {
 
   f.Call(0);
   f.Call(18);
-  ASSERT_DEATH_IF_SUPPORTED({ f.Call(17); }, ErrorMessage("abort: no reason"));
+  ASSERT_DEATH_IF_SUPPORTED({ f.Call(17); }, ERROR_MESSAGE("abort: no reason"));
 }
 
 TEST_F(MacroAssemblerTest, CompareAndBranch) {
@@ -253,6 +249,7 @@ INSTANTIATE_TEST_SUITE_P(MacroAssemblerTest,
                          ::testing::ValuesIn(kMoveObjectAndSlotTestCases));
 
 #undef __
+#undef ERROR_MESSAGE
 
 }  // namespace internal
 }  // namespace v8

test/unittests/assembler/macro-assembler-ia32-unittest.cc

@@ -38,8 +38,7 @@ TEST_F(MacroAssemblerTest, TestHardAbort) {
   buffer->MakeExecutable();
   auto f = GeneratedCode<void>::FromBuffer(isolate(), buffer->start());
 
-  ASSERT_DEATH_IF_SUPPORTED(
-      { f.Call(); }, v8_flags.debug_code ? "abort: no reason" : "");
+  ASSERT_DEATH_IF_SUPPORTED({ f.Call(); }, "abort: no reason");
 }
 
 TEST_F(MacroAssemblerTest, TestCheck) {
@@ -70,8 +69,7 @@ TEST_F(MacroAssemblerTest, TestCheck) {
 
   f.Call(0);
   f.Call(18);
-  ASSERT_DEATH_IF_SUPPORTED(
-      { f.Call(17); }, v8_flags.debug_code ? "abort: no reason" : "");
+  ASSERT_DEATH_IF_SUPPORTED({ f.Call(17); }, "abort: no reason");
 }
 
 TEST_F(MacroAssemblerTest, TestPCRelLea) {

test/unittests/assembler/macro-assembler-loong64-unittest.cc

@@ -33,8 +33,7 @@ TEST_F(MacroAssemblerTest, TestHardAbort) {
   buffer->MakeExecutable();
   // We need an isolate here to execute in the simulator.
   auto f = GeneratedCode<void>::FromBuffer(isolate(), buffer->start());
-  ASSERT_DEATH_IF_SUPPORTED(
-      { f.Call(); }, v8_flags.debug_code ? "abort: no reason" : "");
+  ASSERT_DEATH_IF_SUPPORTED({ f.Call(); }, "abort: no reason");
 }
 
 TEST_F(MacroAssemblerTest, TestCheck) {
@@ -56,8 +55,7 @@ TEST_F(MacroAssemblerTest, TestCheck) {
 
   f.Call(0);
   f.Call(18);
-  ASSERT_DEATH_IF_SUPPORTED(
-      { f.Call(17); }, v8_flags.debug_code ? "abort: no reason" : "");
+  ASSERT_DEATH_IF_SUPPORTED({ f.Call(17); }, "abort: no reason");
 }
 
 #undef __

test/unittests/assembler/macro-assembler-mips64-unittest.cc

@@ -35,8 +35,7 @@ TEST_F(MacroAssemblerTest, TestHardAbort) {
   // We need an isolate here to execute in the simulator.
   auto f = GeneratedCode<void>::FromBuffer(isolate(), buffer->start());
 
-  ASSERT_DEATH_IF_SUPPORTED(
-      { f.Call(); }, v8_flags.debug_code ? "abort: no reason" : "");
+  ASSERT_DEATH_IF_SUPPORTED({ f.Call(); }, "abort: no reason");
 }
 
 TEST_F(MacroAssemblerTest, TestCheck) {
@@ -58,8 +57,7 @@ TEST_F(MacroAssemblerTest, TestCheck) {
 
   f.Call(0);
   f.Call(18);
-  ASSERT_DEATH_IF_SUPPORTED(
-      { f.Call(17); }, v8_flags.debug_code ? "abort: no reason" : "");
+  ASSERT_DEATH_IF_SUPPORTED({ f.Call(17); }, "abort: no reason");
 }
 
 #undef __

test/unittests/assembler/macro-assembler-ppc-unittest.cc

@@ -35,8 +35,7 @@ TEST_F(MacroAssemblerTest, TestHardAbort) {
   // We need an isolate here to execute in the simulator.
   auto f = GeneratedCode<void>::FromBuffer(isolate(), buffer->start());
 
-  ASSERT_DEATH_IF_SUPPORTED(
-      { f.Call(); }, v8_flags.debug_code ? "abort: no reason" : "");
+  ASSERT_DEATH_IF_SUPPORTED({ f.Call(); }, "abort: no reason");
 }
 
 TEST_F(MacroAssemblerTest, TestCheck) {
@@ -60,8 +59,7 @@ TEST_F(MacroAssemblerTest, TestCheck) {
 
   f.Call(0);
   f.Call(18);
-  ASSERT_DEATH_IF_SUPPORTED(
-      { f.Call(17); }, v8_flags.debug_code ? "abort: no reason" : "");
+  ASSERT_DEATH_IF_SUPPORTED({ f.Call(17); }, "abort: no reason");
 }
 
 TEST_F(MacroAssemblerTest, ReverseBitsU64) {

test/unittests/assembler/macro-assembler-riscv-unittest.cc

@@ -33,8 +33,7 @@ TEST_F(MacroAssemblerTest, TestHardAbort) {
   buffer->MakeExecutable();
   // We need an isolate here to execute in the simulator.
   auto f = GeneratedCode<void>::FromBuffer(isolate(), buffer->start());
-  ASSERT_DEATH_IF_SUPPORTED(
-      { f.Call(); }, v8_flags.debug_code ? "abort: no reason" : "");
+  ASSERT_DEATH_IF_SUPPORTED({ f.Call(); }, "abort: no reason");
 }
 
 TEST_F(MacroAssemblerTest, TestCheck) {
@@ -56,8 +55,7 @@ TEST_F(MacroAssemblerTest, TestCheck) {
 
   f.Call(0);
   f.Call(18);
-  ASSERT_DEATH_IF_SUPPORTED(
-      { f.Call(17); }, v8_flags.debug_code ? "abort: no reason" : "");
+  ASSERT_DEATH_IF_SUPPORTED({ f.Call(17); }, "abort: no reason");
 }
 
 #undef __

test/unittests/assembler/macro-assembler-s390-unittest.cc

@@ -35,8 +35,7 @@ TEST_F(MacroAssemblerTest, TestHardAbort) {
   // We need an isolate here to execute in the simulator.
   auto f = GeneratedCode<void>::FromBuffer(isolate(), buffer->start());
 
-  ASSERT_DEATH_IF_SUPPORTED(
-      { f.Call(); }, v8_flags.debug_code ? "abort: no reason" : "");
+  ASSERT_DEATH_IF_SUPPORTED({ f.Call(); }, "abort: no reason");
 }
 
 TEST_F(MacroAssemblerTest, TestCheck) {
@@ -60,8 +59,7 @@ TEST_F(MacroAssemblerTest, TestCheck) {
 
   f.Call(0);
   f.Call(18);
-  ASSERT_DEATH_IF_SUPPORTED(
-      { f.Call(17); }, v8_flags.debug_code ? "abort: no reason" : "");
+  ASSERT_DEATH_IF_SUPPORTED({ f.Call(17); }, "abort: no reason");
 }
 
 #undef __

test/unittests/assembler/macro-assembler-x64-unittest.cc

@@ -81,8 +81,7 @@ TEST_F(MacroAssemblerX64Test, TestHardAbort) {
   buffer->MakeExecutable();
   auto f = GeneratedCode<void>::FromBuffer(isolate(), buffer->start());
 
-  ASSERT_DEATH_IF_SUPPORTED(
-      { f.Call(); }, v8_flags.debug_code ? "abort: no reason" : "");
+  ASSERT_DEATH_IF_SUPPORTED({ f.Call(); }, "abort: no reason");
 }
 
 TEST_F(MacroAssemblerX64Test, TestCheck) {
@@ -105,8 +104,7 @@ TEST_F(MacroAssemblerX64Test, TestCheck) {
 
   f.Call(0);
   f.Call(18);
-  ASSERT_DEATH_IF_SUPPORTED(
-      { f.Call(17); }, v8_flags.debug_code ? "abort: no reason" : "");
+  ASSERT_DEATH_IF_SUPPORTED({ f.Call(17); }, "abort: no reason");
 }
 
 #undef __

tools/testrunner/local/variants.py

@@ -242,6 +242,7 @@ INCOMPATIBLE_FLAGS_PER_BUILD_VARIABLE = {
         "--trace_wasm_serialization",
         "--trace_wasm_stack_switching",
         "--trace_wasm_streaming",
+        "--trap_on_abort",
     ],
     "!verify_heap": ["--verify-heap"],
     "!debug_code": ["--debug-code"],