Debug: create private code memories per store when debugging is enabled. (#12051)

* Debug: create private code memories per store when debugging is enabled.

This will allow patching code to implement e.g. breakpoints. (That is,
for now the copies are redundant, but soon they will not be.)

This change follows the discussion [here] and offline to define a few
types that better encapsulate the distinction we want to enforce.
Basically, there is almost never a bare `CodeMemory`; they are always
wrapped in an `EngineCode` or `StoreCode`, the latter being a per-store
instance of the former. Accessors are moved to the relevant place so
that, for example, one cannot get a pointer to a Wasm function's body
without being in the context of a `Store` where the containing module
has been registered. The registry then returns a `ModuleWithCode` that
boxes up a `Module` reference and `StoreCode` together for cases where
we need both the metadata from the module and the raw code to derive
something.

The only case where we return raw code pointers to the `EngineCode`
directly have to do with Wasm-to-array trampolines: in some cases, e.g.
`InstancePre` pre-creating data structures with references to host
functions, it breaks our expected performance characteristics to make
the function pointers store-specific. This is fine as long as the
Wasm-to-array trampolines never bake in direct calls to Wasm functions;
the strong invariant is that Wasm functions never execute from
`EngineCode` directly. Some parts of the component runtime would also
have to be substantially refactored if we wanted to do away with this
exception.

The per-`Store` module registry is substantially refactored in this PR.
I got rid of the modules-without-code distinction (the case where a
module only has trampolines and no defined functions still works fine),
and organized the BTreeMaps to key on start address rather than end
address, which I find a little more intuitive (one then queries with the
dual to the range -- 0-up-to-PC and last entry found).

[here]: #12051 (review)

* Review feedback: do not assume a reasonable code alignment; error when it cannot be known

* Review feedback: fail properly in profiler when we are cloning code

* Fix guest-profiler C API.

* Review feedback: make private-code representation impossible in non-debugging-support builds.

* Add TODO comment referencing issue for cloning only .text.

* clang-format

* Review feedback: add back Component::image_range.

* Review feedback: error on registering profiling metadata when debug is enabled.

* rustfmt

* Remove early bail on profiling-data registration when debugging is enabled: this always happens so we cannot error out.

Author: cfallin

crates/c-api/include/wasmtime/profiling.h

@@ -56,6 +56,7 @@ typedef struct wasmtime_guestprofiler_modules {
 /**
  * \brief Begin profiling a new guest.
  *
+ * \param engine         engine in which to perform the profiling
  * \param module_name    name recorded in the profile
  * \param interval_nanos intended sampling interval in nanoseconds recorded in
  *                       the profile
@@ -71,8 +72,9 @@ typedef struct wasmtime_guestprofiler_modules {
  * https://docs.wasmtime.dev/api/wasmtime/struct.GuestProfiler.html#method.new
  */
 WASM_API_EXTERN /* own */ wasmtime_guestprofiler_t *wasmtime_guestprofiler_new(
-    const wasm_name_t *module_name, uint64_t interval_nanos,
-    const wasmtime_guestprofiler_modules_t *modules, size_t modules_len);
+    const wasm_engine_t *engine, const wasm_name_t *module_name,
+    uint64_t interval_nanos, const wasmtime_guestprofiler_modules_t *modules,
+    size_t modules_len);
 
 /**
  * \brief Add a sample to the profile.

crates/c-api/src/profiling.rs

@@ -1,4 +1,7 @@
-use crate::{wasm_byte_vec_t, wasm_name_t, wasmtime_error_t, wasmtime_module_t, wasmtime_store_t};
+use crate::{
+    wasm_byte_vec_t, wasm_engine_t, wasm_name_t, wasmtime_error_t, wasmtime_module_t,
+    wasmtime_store_t,
+};
 use std::slice;
 use std::str::from_utf8;
 use std::time::Duration;
@@ -18,6 +21,7 @@ pub struct wasmtime_guestprofiler_modules_t<'a> {
 
 #[unsafe(no_mangle)]
 pub unsafe extern "C" fn wasmtime_guestprofiler_new(
+    engine: &wasm_engine_t,
     module_name: &wasm_name_t,
     interval_nanos: u64,
     modules: *const wasmtime_guestprofiler_modules_t,
@@ -36,7 +40,13 @@ pub unsafe extern "C" fn wasmtime_guestprofiler_new(
         })
         .collect::<Vec<_>>();
     Box::new(wasmtime_guestprofiler_t {
-        guest_profiler: GuestProfiler::new(module_name, Duration::from_nanos(interval_nanos), list),
+        guest_profiler: GuestProfiler::new(
+            &engine.engine,
+            module_name,
+            Duration::from_nanos(interval_nanos),
+            list,
+        )
+        .unwrap(),
     })
 }
 

crates/environ/src/key.rs

@@ -515,4 +515,29 @@ impl FuncKey {
             _ => panic!("`FuncKey::unwrap_resource_drop_trampoline` called on {self:?}"),
         }
     }
+
+    /// Is this "Store-invariant"? This allows us to execute
+    /// EngineCode directly rather than StoreCode.
+    ///
+    /// Any function that is either directly from Wasm code, or calls
+    /// it directly (not indirected through a runtime-provided
+    /// function pointer), is "store-variant": we need to use a
+    /// StoreCode-specific version of the code to hit any patching
+    /// that our specific instantiations may have (due to debugging
+    /// breakpoints, etc). Trampolines into the runtime cannot be
+    /// patched and so can use EngineCode instead. This allows for
+    /// less complex plumbing in some places where we can avoid
+    /// looking up the StoreCode (or having access to the Store).
+    pub fn is_store_invariant(&self) -> bool {
+        match self {
+            Self::DefinedWasmFunction(..) | Self::ArrayToWasmTrampoline(..) => false,
+            Self::WasmToArrayTrampoline(..)
+            | Self::WasmToBuiltinTrampoline(..)
+            | Self::PulleyHostCall(..) => true,
+            #[cfg(feature = "component-model")]
+            Self::ComponentTrampoline(..)
+            | Self::ResourceDropTrampoline
+            | Self::UnsafeIntrinsic(..) => true,
+        }
+    }
 }