src: add more checks and clarify docs for external references

To help catch unregistered bindings.

PR-URL: #61719
Refs: #61718
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Author: joyeecheung

src/README.md

@@ -506,12 +506,17 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
 }  // namespace util
 }  // namespace node
 
-// The first argument passed to `NODE_BINDING_EXTERNAL_REFERENCE`,
-// which is `util` here, needs to be added to the
-// `EXTERNAL_REFERENCE_BINDING_LIST_BASE` list in node_external_reference.h
 NODE_BINDING_EXTERNAL_REFERENCE(util, node::util::RegisterExternalReferences)
 ```
 
+And add the first argument passed to `NODE_BINDING_EXTERNAL_REFERENCE` to
+the list of external references in `src/node_external_reference.h`:
+
+```cpp
+#define EXTERNAL_REFERENCE_LIST_BASE(V) \
+  V(util) \
+```
+
 Otherwise, you might see an error message like this when building the
 executables:
 

src/node_snapshotable.cc

@@ -968,6 +968,61 @@ std::optional<SnapshotConfig> ReadSnapshotConfig(const char* config_path) {
   return result;
 }
 
+// Find bindings that have been loaded by internalBinding() but the external
+// reference method have not been called. This requires that the caller
+// match the id passed into their NODE_BINDING_CONTEXT_AWARE_INTERNAL() and
+// NODE_BINDING_EXTERNAL_REFERENCE() calls. Note that this only serves as a
+// preemptive check. Binding methods create the actual external references
+// (usually through function templates) and there's currently no easy way
+// to verify at that level of granularity. See "Registering binding functions
+// used in bootstrap" in src/README.md.
+bool ValidateBindings(Environment* env) {
+  std::set<std::string> registered;
+#define V(modname) registered.insert(#modname);
+  EXTERNAL_REFERENCE_BINDING_LIST(V)
+#undef V
+
+  std::set<std::string> bindings_without_external_references = {
+      "async_context_frame",
+      "constants",
+      "symbols",
+  };
+
+  std::set<std::string> unregistered;
+  for (auto* mod : env->principal_realm()->internal_bindings) {
+    if (registered.count(mod->nm_modname) == 0 &&
+        bindings_without_external_references.count(mod->nm_modname) == 0) {
+      unregistered.insert(mod->nm_modname);
+    }
+  }
+
+  if (unregistered.size() == 0) {
+    return true;
+  }
+
+  FPrintF(
+      stderr,
+      "\n---- snapshot building check failed ---\n\n"
+      "The following bindings are loaded during the snapshot building process,"
+      " but their external reference registration methods have not been "
+      "called:\n\n");
+  for (auto& binding : unregistered) {
+    FPrintF(stderr, " - %s\n", binding);
+  }
+  FPrintF(stderr,
+          "\nIf the binding does not have any external references, "
+          "add it to the list of bindings_without_external_references "
+          "in src/node_snapshotable.cc.\n"
+          "Otherwise, make sure to call NODE_BINDING_EXTERNAL_REFERENCE() "
+          "with an appropriate register method for the binding, "
+          "and add it to EXTERNAL_REFERENCE_BINDING_LIST in "
+          "src/node_external_reference.h"
+          "\n\nSee \"Registering binding functions used in bootstrap\" "
+          "in src/README.md for more details."
+          "\n----\n\n");
+  return false;
+}
+
 ExitCode BuildSnapshotWithoutCodeCache(
     SnapshotData* out,
     const std::vector<std::string>& args,
@@ -1033,6 +1088,11 @@ ExitCode BuildSnapshotWithoutCodeCache(
     if (exit_code != ExitCode::kNoFailure) {
       return exit_code;
     }
+
+    if (snapshot_type == SnapshotMetadata::Type::kDefault &&
+        !ValidateBindings(env)) {
+      return ExitCode::kStartupSnapshotFailure;
+    }
   }
 
   return SnapshotBuilder::CreateSnapshot(out, setup.get());