From cdb59304c354d1357b9732fe6fd1189318437574 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Thu, 6 Jun 2024 08:26:15 -0500
Subject: [PATCH] Handle defined shared memories in dwarf processing (#8750)

This commit resolves an assert in the dwarf generating of core wasm
modules when the module has a defined linear memory which is flagged
`shared`. This is represented slightly differently in the `VMContext`
than owned memories that aren't `shared`, and looks more like an
imported memory. With support in #8740 it's now much easier to support
this.

Closes #8652
---
 crates/cranelift/src/debug.rs                 | 28 ++++++++-----------
 crates/test-programs/artifacts/build.rs       |  2 +-
 crates/test-programs/build.rs                 |  2 ++
 .../src/bin/dwarf_shared_memory.rs            |  1 +
 tests/all/debug/lldb.rs                       |  6 ++++
 5 files changed, 22 insertions(+), 17 deletions(-)
 create mode 100644 crates/test-programs/src/bin/dwarf_shared_memory.rs

diff --git a/crates/cranelift/src/debug.rs b/crates/cranelift/src/debug.rs
index ed9c473105..542a070922 100644
--- a/crates/cranelift/src/debug.rs
+++ b/crates/cranelift/src/debug.rs
@@ -5,8 +5,8 @@ use cranelift_codegen::isa::TargetIsa;
 use object::write::SymbolId;
 use std::collections::HashMap;
 use wasmtime_environ::{
-    DefinedFuncIndex, EntityRef, MemoryIndex, ModuleTranslation, OwnedMemoryIndex, PrimaryMap,
-    PtrSize, StaticModuleIndex, Tunables, VMOffsets,
+    DefinedFuncIndex, DefinedMemoryIndex, EntityRef, MemoryIndex, ModuleTranslation,
+    OwnedMemoryIndex, PrimaryMap, PtrSize, StaticModuleIndex, Tunables, VMOffsets,
 };
 
 /// Memory definition offset in the VMContext structure.
@@ -88,25 +88,21 @@ impl<'a> Compilation<'a> {
             );
 
             let memory_offset = if ofs.num_imported_memories > 0 {
+                let index = MemoryIndex::new(0);
                 ModuleMemoryOffset::Imported {
-                    offset_to_vm_memory_definition: ofs.vmctx_vmmemory_import(MemoryIndex::new(0))
+                    offset_to_vm_memory_definition: ofs.vmctx_vmmemory_import(index)
                         + u32::from(ofs.vmmemory_import_from()),
                     offset_to_memory_base: ofs.ptr.vmmemory_definition_base().into(),
                 }
+            } else if ofs.num_owned_memories > 0 {
+                let index = OwnedMemoryIndex::new(0);
+                ModuleMemoryOffset::Defined(ofs.vmctx_vmmemory_definition_base(index))
             } else if ofs.num_defined_memories > 0 {
-                // The addition of shared memory makes the following assumption,
-                // "owned memory index = 0", possibly false. If the first memory
-                // is a shared memory, the base pointer will not be stored in
-                // the `owned_memories` array. The following code should
-                // eventually be fixed to not only handle shared memories but
-                // also multiple memories.
-                assert_eq!(
-                    ofs.num_defined_memories, ofs.num_owned_memories,
-                    "the memory base pointer may be incorrect due to sharing memory"
-                );
-                ModuleMemoryOffset::Defined(
-                    ofs.vmctx_vmmemory_definition_base(OwnedMemoryIndex::new(0)),
-                )
+                let index = DefinedMemoryIndex::new(0);
+                ModuleMemoryOffset::Imported {
+                    offset_to_vm_memory_definition: ofs.vmctx_vmmemory_pointer(index),
+                    offset_to_memory_base: ofs.ptr.vmmemory_definition_base().into(),
+                }
             } else {
                 ModuleMemoryOffset::None
             };
diff --git a/crates/test-programs/artifacts/build.rs b/crates/test-programs/artifacts/build.rs
index 0fa76093ee..20209eaf6d 100644
--- a/crates/test-programs/artifacts/build.rs
+++ b/crates/test-programs/artifacts/build.rs
@@ -90,7 +90,7 @@ fn build_and_generate_tests() {
         }
 
         // Generate a component from each test.
-        if kind == "nn" || target == "dwarf_imported_memory" {
+        if kind == "nn" || target == "dwarf_imported_memory" || target == "dwarf_shared_memory" {
             continue;
         }
         let adapter = match target.as_str() {
diff --git a/crates/test-programs/build.rs b/crates/test-programs/build.rs
index 734cbddbb4..8f055264c6 100644
--- a/crates/test-programs/build.rs
+++ b/crates/test-programs/build.rs
@@ -1,4 +1,6 @@
 fn main() {
     println!("cargo:rustc-link-arg-bin=dwarf_imported_memory=--import-memory");
     println!("cargo:rustc-link-arg-bin=dwarf_imported_memory=--export-memory");
+    println!("cargo:rustc-link-arg-bin=dwarf_shared_memory=--no-check-features");
+    println!("cargo:rustc-link-arg-bin=dwarf_shared_memory=--shared-memory");
 }
diff --git a/crates/test-programs/src/bin/dwarf_shared_memory.rs b/crates/test-programs/src/bin/dwarf_shared_memory.rs
new file mode 100644
index 0000000000..fe4ca3784c
--- /dev/null
+++ b/crates/test-programs/src/bin/dwarf_shared_memory.rs
@@ -0,0 +1 @@
+include!("./dwarf_simple.rs");
diff --git a/tests/all/debug/lldb.rs b/tests/all/debug/lldb.rs
index dadf8a154a..ee347a162c 100644
--- a/tests/all/debug/lldb.rs
+++ b/tests/all/debug/lldb.rs
@@ -370,4 +370,10 @@ check: exited with status = 0
             &["--preload=env=./tests/all/debug/satisfy_memory_import.wat"],
         )
     }
+
+    #[test]
+    #[ignore]
+    fn dwarf_shared_memory() -> Result<()> {
+        test_dwarf_simple(DWARF_SHARED_MEMORY, &[])
+    }
 }