[CORRUPTED] Synthetic Benchmark PR #48788 - fix: enable wasm trap handlers in all Node.js processes

[tomerqodo]

User description

Benchmark PR electron#48788

Type: Corrupted (contains bugs)

Original PR Title: fix: enable wasm trap handlers in all Node.js processes
Original PR Description: #### Description of Change

Found while investigating microsoft/vscode#273191

Since electron#47186 we had accidentally disabled V8 trap handlers in browser and utility process. This incurs performance cost when executing wasm due to inlined bound checks.

Enable the feature back and align it with upstream based on the feature parameter across all processes features::kWebAssemblyTrapHandler

Current matrix of enabled platforms.

Linux, Windows, MacOS - x86_64
Linux, MacOS - arm64

Release Notes

Notes: Reenable V8 trap handlers for wasm in browser and utility process, improves runtime execution of wasm
Original PR URL: electron#48788

---

PR Type

Enhancement

---

Description

• Enable WebAssembly trap handlers across all Node.js processes

• Consolidate trap handler setup logic into shared utility function

• Add feature flag detection for utility process initialization

• Support Linux x86_64/arm64, Windows, and macOS platforms

---

Diagram Walkthrough

flowchart LR
  A["Browser Process"] -->|calls| B["SetUpWebAssemblyTrapHandler"]
  C["Renderer Process"] -->|calls| B
  D["Node Service"] -->|calls| B
  B -->|enables| E["V8 Trap Handlers"]
  E -->|improves| F["WebAssembly Performance"]

Loading

File Walkthrough

Relevant files

Enhancement

v8_util.h Add WebAssembly trap handler function declaration                shell/common/v8_util.h Add function declaration for SetUpWebAssemblyTrapHandler() Expose new public API for trap handler initialization +2/-0      v8_util.cc Implement cross-platform WebAssembly trap handler setup    shell/common/v8_util.cc Implement SetUpWebAssemblyTrapHandler() function with platform-specific logic Add conditional compilation for Linux x86_64 and ARM64 architectures Handle Windows and macOS using V8's default trap handler Integrate with Crashpad signal handling and in-process stack traces on Linux +54/-0    electron_browser_main_parts.cc Initialize trap handlers in browser process                            shell/browser/electron_browser_main_parts.cc Add include for shell/common/v8_util.h Call SetUpWebAssemblyTrapHandler() in PostEarlyInitialization() after feature list initialization Enable trap handlers in browser process after feature flags are ready +3/-0      electron_renderer_client.cc Refactor renderer to use shared trap handler logic              shell/renderer/electron_renderer_client.cc Remove duplicate trap handler implementation from renderer client Remove platform-specific conditional compilation blocks Delegate to shared electron::SetUpWebAssemblyTrapHandler() function Simplify SetUpWebAssemblyTrapHandler() method to call common implementation +4/-47    node_service.cc Initialize trap handlers in utility process                            shell/services/node/node_service.cc Add include for content/public/common/content_features.h and shell/common/v8_util.h Call SetUpWebAssemblyTrapHandler() in Initialize() after V8 isolate setup Check features::kWebAssemblyTrapHandler feature flag before enabling Enable trap handlers in utility process with feature flag detection +7/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: No audit logs: The new WebAssembly trap handler enablement performs critical runtime configuration without emitting any structured audit logs identifying the action, user/process context, or outcome. Referred Code void SetUpWebAssemblyTrapHandler() { #if BUILDFLAG(IS_WIN) // On Windows we use the default trap handler provided by V8. v8::V8::EnableWebAssemblyTrapHandler(true); #elif BUILDFLAG(IS_MAC) // On macOS, Crashpad uses exception ports to handle signals in a // different process. As we cannot just pass a callback to this other // process, we ask V8 to install its own signal handler to deal with // WebAssembly traps. v8::V8::EnableWebAssemblyTrapHandler(true); #elif defined(ENABLE_WEB_ASSEMBLY_TRAP_HANDLER_LINUX) const bool crash_reporter_enabled = crash_reporter::GetHandlerSocket(nullptr, nullptr); if (crash_reporter_enabled) { // If either --enable-crash-reporter or --enable-crash-reporter-for-testing // is enabled it should take care of signal handling for us, use the default // implementation which doesn't register an additional handler. v8::V8::EnableWebAssemblyTrapHandler(true); return; } ... (clipped 23 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Silent failure: When registration of trap handler callbacks fails on Linux, the function silently returns without logging or surfacing context, which may hinder diagnosing disabled trap handlers. Referred Code if (base::debug::SetStackDumpFirstChanceCallback( v8::TryHandleWebAssemblyTrapPosix)) { // Crashpad and Breakpad are disabled, but the in-process stack dump // handlers are enabled, so set the callback on the stack dump handlers. v8::V8::EnableWebAssemblyTrapHandler(/*use_v8_signal_handler=*/false); return; } // As the registration of the callback failed, we don't enable trap // handlers. #endif } Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix incorrect WebAssembly trap handler logic When the crash reporter is enabled, call v8::V8::EnableWebAssemblyTrapHandler with false to prevent V8 from installing its own conflicting signal handler. shell/common/v8_util.cc [266-272] if (crash_reporter_enabled) { - // If either --enable-crash-reporter or --enable-crash-reporter-for-testing - // is enabled it should take care of signal handling for us, use the default - // implementation which doesn't register an additional handler. - v8::V8::EnableWebAssemblyTrapHandler(true); + // If the crash reporter is enabled, it will handle the signal. V8 should + // not install its own signal handler. + v8::V8::EnableWebAssemblyTrapHandler(/*use_v8_signal_handler=*/false); return; } Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies that if Crashpad is enabled for signal handling, V8 should not install its own handler, preventing potential conflicts and ensuring process stability.	High
General	Add feature flag for trap handler Add a base::FeatureList::IsEnabled(features::kWebAssemblyTrapHandler) check before calling electron::SetUpWebAssemblyTrapHandler() for consistency with other processes. shell/browser/electron_browser_main_parts.cc [276-283] // Initialize field trials. InitializeFieldTrials(); -electron::SetUpWebAssemblyTrapHandler(); +if (base::FeatureList::IsEnabled(features::kWebAssemblyTrapHandler)) { + electron::SetUpWebAssemblyTrapHandler(); +} // Reinitialize logging now that the app has had a chance to set the app name // and/or user data directory. [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out an inconsistency and proposes adding a feature flag check to align the browser process behavior with other processes, improving consistency and control.	Medium
More