fix(debugger): use ENABLE_JS_DEBUGGER instead of JS_DEBUGGER (#249)

This patch includes various fixes for how the debugger feature is
enabled at compile time.

- Provide stub implementation of `content_debugger` for builds where
  `ENABLE_JS_DEBUGGER` is not defined
- Move `__wasilibc_initialize_environ` to debugger initialization to
  avoid implicit `wasi/environment` dependency.

There are currently 3 entry points where the content debugger can be
initialized:

1. `http/handler` -- debugger initialization is now explicitly called in
   the StarlingMonkey fetch event
2. `cli/run` -- debugger initialization should be called on engine
   initialization
3. `componentize-js` defined exports -- through componentize-js builtins
   on first call to exported function

Author: andreiltd

CMakeLists.txt

@@ -64,12 +64,9 @@ set(SOURCES
     runtime/event_loop.cpp
     runtime/builtin.cpp
     runtime/script_loader.cpp
+    runtime/debugger.cpp
 )
 
-if (ENABLE_JS_DEBUGGER)
-    list(APPEND SOURCES runtime/debugger.cpp)
-endif()
-
 add_executable(starling-raw.wasm ${SOURCES})
 
 target_link_libraries(starling-raw.wasm PRIVATE host_api extension_api builtins spidermonkey rust-crates)

host-apis/wasi-0.2.0/host_api.cpp

@@ -2,8 +2,6 @@
 #include "bindings/bindings.h"
 #include "handles.h"
 
-#include <wasi/libc-environ.h>
-
 static std::optional<wasi_clocks_monotonic_clock_own_pollable_t> immediately_ready;
 
 size_t poll_handles(vector<WASIHandle<host_api::Pollable>::Borrowed> handles) {
@@ -1032,9 +1030,6 @@ void exports_wasi_http_incoming_handler(exports_wasi_http_incoming_request reque
   // that it properly initializes the runtime and installs a request handler.
   if (!REQUEST_HANDLER) {
     init_from_environment();
-  } else {
-    // Resuming a wizer snapshot, so we have to ensure that the environment is reset.
-    __wasilibc_initialize_environ();
   }
   MOZ_ASSERT(REQUEST_HANDLER);
 

justfile

@@ -59,7 +59,7 @@ format *ARGS:
 
 # Run integration test
 test regex="": (build "integration-test-server")
-    ctest --test-dir {{ builddir }} -j {{ ncpus }} --output-on-failure -R {{ regex }}
+    ctest --test-dir {{ builddir }} -j {{ ncpus }} --output-on-failure {{ if regex == "" { regex } else { "-R " + regex } }}
 
 # Build web platform test suite
 [group('wpt')]

runtime/debugger.cpp

@@ -1,18 +1,27 @@
 #include <debugger.h>
 
-#include <decode.h>
-#include <encode.h>
+#ifdef ENABLE_JS_DEBUGGER
+
+#include "decode.h"
+#include "encode.h"
+#include "sockets.h"
+
 #include <js/CompilationAndEvaluation.h>
 #include <js/SourceText.h>
+
 #include <string_view>
+#include <wasi/libc-environ.h>
+
+namespace {
 
 mozilla::Maybe<std::string> main_path;
+bool debugger_initialized = false;
 
 namespace SocketErrors {
 DEF_ERR(SendFailed, JSEXN_TYPEERR, "Failed to send message via TCP socket", 0)
 } // namespace SocketErrors
 
-static bool dbg_set_content_path(JSContext *cx, unsigned argc, Value *vp) {
+bool dbg_set_content_path(JSContext *cx, unsigned argc, Value *vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
   auto path = core::encode(cx, args.get(0));
   if (!path) {
@@ -24,7 +33,7 @@ static bool dbg_set_content_path(JSContext *cx, unsigned argc, Value *vp) {
   return true;
 }
 
-static bool print_location(JSContext *cx, FILE *fp = stdout) {
+bool print_location(JSContext *cx, FILE *fp = stdout) {
   JS::AutoFilename filename;
   uint32_t lineno;
   JS::ColumnNumberOneOrigin column;
@@ -35,39 +44,17 @@ static bool print_location(JSContext *cx, FILE *fp = stdout) {
   return true;
 }
 
-bool content_debugger::dbg_print(JSContext *cx, unsigned argc, Value *vp) {
-  CallArgs args = CallArgsFromVp(argc, vp);
-
-  if (!print_location(cx)) {
-    return false;
-  }
-
-  for (size_t i = 0; i < args.length(); i++) {
-    auto str = core::encode(cx, args.get(i));
-    if (!str) {
-      return false;
-    }
-    printf("%.*s", static_cast<int>(str.len), str.begin());
-  }
-
-  printf("\n");
-  fflush(stdout);
-  args.rval().setUndefined();
-  return true;
-}
-
-static bool dbg_assert(JSContext *cx, unsigned argc, Value *vp) {
+bool dbg_assert(JSContext *cx, unsigned argc, Value *vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
   if (!ToBoolean(args.get(0))) {
-
     if (!print_location(cx, stderr)) {
       return false;
     }
 
     if (args.length() > 1) {
       auto message = core::encode(cx, args.get(1));
-      fprintf(stderr, "Assert failed in debugger: %.*s",
-        static_cast<int>(message.len), message.begin());
+      fprintf(stderr, "Assert failed in debugger: %.*s", static_cast<int>(message.len),
+              message.begin());
     } else {
       fprintf(stderr, "Assert failed in debugger");
     }
@@ -77,8 +64,6 @@ static bool dbg_assert(JSContext *cx, unsigned argc, Value *vp) {
   return true;
 }
 
-#include "sockets.h"
-
 namespace debugging_socket {
 
 class TCPSocket : public builtins::BuiltinNoConstructor<TCPSocket> {
@@ -127,7 +112,7 @@ bool TCPSocket::receive(JSContext *cx, unsigned argc, JS::Value *vp) {
   return true;
 }
 
-JSObject* TCPSocket::FromSocket(JSContext* cx, host_api::TCPSocket* socket) {
+JSObject *TCPSocket::FromSocket(JSContext *cx, host_api::TCPSocket *socket) {
   RootedObject instance(cx, JS_NewObjectWithGivenProto(cx, &class_, proto_obj));
   if (!instance) {
     return nullptr;
@@ -136,10 +121,8 @@ JSObject* TCPSocket::FromSocket(JSContext* cx, host_api::TCPSocket* socket) {
   return instance;
 }
 
-const JSFunctionSpec TCPSocket::methods[] = {
-  JS_FN("send", TCPSocket::send, 1, 0),
-  JS_FN("receive", TCPSocket::receive, 1, 0), JS_FS_END
-};
+const JSFunctionSpec TCPSocket::methods[] = {JS_FN("send", TCPSocket::send, 1, 0),
+                                             JS_FN("receive", TCPSocket::receive, 1, 0), JS_FS_END};
 
 const JSFunctionSpec TCPSocket::static_methods[] = {JS_FS_END};
 const JSPropertySpec TCPSocket::static_properties[] = {JS_PS_END};
@@ -150,11 +133,13 @@ host_api::HostString read_message(JSContext *cx, host_api::TCPSocket *socket) {
   if (!chunk) {
     return nullptr;
   }
+
   char *end;
   uint16_t message_length = std::strtoul(chunk.begin(), &end, 10);
   if (end == chunk.begin() || *end != '\n') {
     return nullptr;
   }
+
   std::string message = std::string(end + 1, chunk.end());
   while (message.size() < message_length) {
     chunk = socket->receive(message_length - message.size());
@@ -171,24 +156,26 @@ host_api::HostString read_message(JSContext *cx, host_api::TCPSocket *socket) {
 bool initialize_debugger(JSContext *cx, uint16_t port, bool content_already_initialized) {
   auto socket = host_api::TCPSocket::make(host_api::TCPSocket::IPAddressFamily::IPV4);
   MOZ_RELEASE_ASSERT(socket, "Failed to create debugging socket");
+
   if (!socket->connect({127, 0, 0, 1}, port) || !socket->send("get-session-port")) {
     printf("Couldn't connect to debugging socket at port %u, continuing without debugging ...\n",
            port);
     return true;
   }
+
   auto response = socket->receive(128);
   if (!response) {
     printf("Couldn't get debugging session port, continuing without debugging ...\n");
     return true;
   }
-  char *end;
 
   // If StarlingMonkey was loaded with debugging enabled, but no session is active,
   // we can just silently continue execution.
   if (string_view{"no-session"}.compare(0, response.len, response) == 0) {
     return true;
   }
 
+  char *end;
   uint16_t session_port = std::strtoul(response.begin(), &end, 10);
   if (session_port < 1024 || session_port > 65535) {
     printf("Invalid debugging session port '%*s' received, continuing without debugging ...\n",
@@ -199,12 +186,14 @@ bool initialize_debugger(JSContext *cx, uint16_t port, bool content_already_init
 
   socket = host_api::TCPSocket::make(host_api::TCPSocket::IPAddressFamily::IPV4);
   MOZ_RELEASE_ASSERT(socket, "Failed to create debugging session socket");
+
   if (!socket->connect({127, 0, 0, 1}, session_port) || !socket->send("get-debugger")) {
     printf("Couldn't connect to debugging session socket at port %u, "
            "continuing without debugging ...\n",
            session_port);
     return true;
   }
+
   auto debugging_script = debugging_socket::read_message(cx, socket);
   if (!debugging_script) {
     printf("Couldn't get debugger script, continuing without debugging ...\n");
@@ -244,6 +233,7 @@ bool initialize_debugger(JSContext *cx, uint16_t port, bool content_already_init
   if (!socket_obj) {
     return false;
   }
+
   if (!JS_DefineProperty(cx, global, "socket", socket_obj, JSPROP_READONLY)) {
     return false;
   }
@@ -264,6 +254,7 @@ bool initialize_debugger(JSContext *cx, uint16_t port, bool content_already_init
   if (!script) {
     return false;
   }
+
   RootedValue result(cx);
   if (!JS_ExecuteScript(cx, script, &result)) {
     return false;
@@ -272,12 +263,40 @@ bool initialize_debugger(JSContext *cx, uint16_t port, bool content_already_init
   return true;
 }
 
-static bool debugger_initialized = false;
-void content_debugger::maybe_init_debugger(api::Engine * engine, bool content_already_initialized) {
+} // anonymous namespace
+
+namespace content_debugger {
+
+bool dbg_print(JSContext *cx, unsigned argc, Value *vp) {
+  CallArgs args = CallArgsFromVp(argc, vp);
+
+  if (!print_location(cx)) {
+    return false;
+  }
+
+  for (size_t i = 0; i < args.length(); i++) {
+    auto str = core::encode(cx, args.get(i));
+    if (!str) {
+      return false;
+    }
+    printf("%.*s", static_cast<int>(str.len), str.begin());
+  }
+
+  printf("\n");
+  fflush(stdout);
+  args.rval().setUndefined();
+  return true;
+}
+
+void maybe_init_debugger(api::Engine *engine, bool content_already_initialized) {
   if (debugger_initialized || !engine->debugging_enabled()) {
     return;
   }
   debugger_initialized = true;
+
+  // Resuming a wizer snapshot, so we have to ensure that the environment is reset.
+  __wasilibc_initialize_environ();
+
   auto port_str = std::getenv("DEBUGGER_PORT");
   if (port_str) {
     uint32_t port = std::stoi(port_str);
@@ -288,6 +307,31 @@ void content_debugger::maybe_init_debugger(api::Engine * engine, bool content_al
   }
 }
 
-mozilla::Maybe<std::string_view> content_debugger::replacement_script_path() {
-  return main_path;
+mozilla::Maybe<std::string_view> replacement_script_path() {
+  if (main_path.isSome()) {
+    return mozilla::Some(std::string_view{main_path.ref()});
+  }
+  return mozilla::Nothing();
+}
+
+} // namespace content_debugger
+
+#else // !ENABLE_JS_DEBUGGER
+
+// Stub implementations when debugger is disabled
+namespace content_debugger {
+
+void maybe_init_debugger(api::Engine *engine, bool content_already_initialized) {}
+
+bool dbg_print(JSContext *cx, unsigned argc, Value *vp) {
+  MOZ_ASSERT_UNREACHABLE("dbg_print only available with ENABLE_JS_DEBUGGER build option set.");
+  return false;
 }
+
+mozilla::Maybe<std::string_view> replacement_script_path() {
+  return mozilla::Nothing();
+}
+
+} // namespace content_debugger
+
+#endif // ENABLE_JS_DEBUGGER

runtime/debugger.h

@@ -1,5 +1,6 @@
 #ifndef DEBUGGER_H
 #define DEBUGGER_H
+
 #include "extension-api.h"
 
 namespace content_debugger {

runtime/engine.cpp

@@ -1,17 +1,14 @@
 #include "extension-api.h"
-
-#include <cassert>
-#include <chrono>
-#include <cstdlib>
-#include <iostream>
+#include "allocator.h"
+#include "debugger.h"
+#include "encode.h"
+#include "event_loop.h"
+#include "script_loader.h"
 
 // TODO: remove these once the warnings are fixed
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Winvalid-offsetof"
 #pragma clang diagnostic ignored "-Wdeprecated-enum-enum-conversion"
-#include "allocator.h"
-#include "encode.h"
-#include "event_loop.h"
 #include "js/CompilationAndEvaluation.h"
 #include "js/Modules.h"
 #include "js/ForOfIterator.h"
@@ -20,12 +17,10 @@
 #include "jsfriendapi.h"
 #pragma clang diagnostic pop
 
-#ifdef JS_DEBUGGER
-  #include "debugger.h"
-#endif
-#include "script_loader.h"
-
-#include <decode.h>
+#include <cassert>
+#include <chrono>
+#include <cstdlib>
+#include <iostream>
 
 #ifdef MEM_STATS
 #include <string>
@@ -299,15 +294,10 @@ bool create_initializer_global(Engine *engine) {
   JSAutoRealm ar(cx, global);
 
   if (!JS_DefineFunction(cx, global, "defineBuiltinModule", ::define_builtin_module, 2, 0) ||
-      !JS_DefineProperty(cx, global, "contentGlobal", ENGINE->global(), JSPROP_READONLY)) {
-    return false;
-  }
-
-#ifdef JS_DEBUGGER
-  if (!JS_DefineFunction(cx, global, "print", content_debugger::dbg_print, 1, 0)) {
+      !JS_DefineProperty(cx, global, "contentGlobal", ENGINE->global(), JSPROP_READONLY) ||
+      !JS_DefineFunction(cx, global, "print", content_debugger::dbg_print, 1, 0)) {
     return false;
   }
-#endif
 
   INIT_SCRIPT_GLOBAL.init(cx, global);
   return true;
@@ -489,13 +479,11 @@ Engine::Engine(std::unique_ptr<EngineConfig> config) {
     // Debugging isn't supported during wizening, so only try it when doing runtime evaluation.
     // The debugger can be initialized at runtime by whatever export is invoked on the
     // resumed wizer snapshot.
-#ifdef JS_DEBUGGER
-      content_debugger::maybe_init_debugger(this, false);
+    content_debugger::maybe_init_debugger(this, false);
     if (auto replacement_script_path = content_debugger::replacement_script_path()) {
       TRACE("Using replacement script path received from debugger: " << *replacement_script_path);
       content_script_path = replacement_script_path;
     }
-#endif
   }
 
   if (content_script_path) {

tests/integration/fetch/fetch.js

@@ -4,7 +4,12 @@ import { strictEqual, deepStrictEqual, throws } from '../../assert.js';
 export const handler = serveTest(async (t) => {
   await t.test('headers-non-ascii-latin1-field-value', async () => {
     const response = await fetch("https://http-me.glitch.me/meow?header=cat:é");
-    strictEqual(response.headers.get('cat'), "é");
+
+    const val = response.headers.get('cat');
+    const bytes = new Uint8Array([...val].map(c => c.charCodeAt(0)));
+    const decoded = new TextDecoder('utf-8').decode(bytes);
+
+    strictEqual(decoded, "é");
   });
 
   t.test('request-clone-bad-calls', () => {