Merge pull request #366 from data-pup/that-which-we-call-a-rose

Look at wasm name section

Author: data-pup

lucetc/src/decls.rs

@@ -117,18 +117,30 @@ impl<'a> ModuleDecls<'a> {
         for ix in 0..decls.info.functions.len() {
             let func_index = UniqueFuncIndex::new(ix);
 
+            // Get the name for this function from the module names section, if it exists.
+            // Because names have to be unique, we append the index value (ix) to the name.
+            fn custom_name_for<'a>(
+                ix: usize,
+                func_index: UniqueFuncIndex,
+                decls: &mut ModuleDecls<'a>,
+            ) -> Option<String> {
+                decls
+                    .info
+                    .function_names
+                    .get(func_index)
+                    .map(|s| format!("{}_{}", s, ix))
+            }
+
             fn export_name_for<'a>(
                 func_ix: UniqueFuncIndex,
                 decls: &mut ModuleDecls<'a>,
             ) -> Option<String> {
                 let export = decls.info.functions.get(func_ix).unwrap();
-
                 if !export.export_names.is_empty() {
                     decls.exports.push(ExportFunction {
                         fn_idx: LucetFunctionIndex::from_u32(decls.function_names.len() as u32),
                         names: export.export_names.clone(),
                     });
-
                     Some(format!("guest_func_{}", export.export_names[0]))
                 } else {
                     None
@@ -164,7 +176,6 @@ impl<'a> ModuleDecls<'a> {
                     // if a function is an export and import, it will not have a real function body
                     // in this program, and we must not declare it with Linkage::Export (there will
                     // never be a define to satisfy the symbol!)
-
                     decls.declare_function(clif_module, import_sym, Linkage::Import, func_index)?;
                 }
                 (None, Some(export_sym)) => {
@@ -173,14 +184,12 @@ impl<'a> ModuleDecls<'a> {
                     decls.declare_function(clif_module, export_sym, Linkage::Export, func_index)?;
                 }
                 (None, None) => {
-                    // No import or export for this function. It's local, and we have to make up a
-                    // name.
-                    decls.declare_function(
-                        clif_module,
-                        format!("guest_func_{}", ix),
-                        Linkage::Local,
-                        func_index,
-                    )?;
+                    // No import or export for this function, which means that it is local. We can
+                    // look for a name provided in the custom names section, otherwise we have to
+                    // make up a placeholder name for it using its index.
+                    let local_sym = custom_name_for(ix, func_index, decls)
+                        .unwrap_or_else(|| format!("guest_func_{}", ix));
+                    decls.declare_function(clif_module, local_sym, Linkage::Local, func_index)?;
                 }
             }
         }
@@ -524,9 +533,6 @@ impl<'a> ModuleDecls<'a> {
 
             functions.push(FunctionMetadata {
                 signature: decl.signature_index,
-                // TODO: this is a best-effort attempt to figure out a useful name.
-                // in the future, we should use names from the module names section
-                // and maybe use export names as a fallback.
                 name: Some(name.symbol()),
             });
         }

lucetc/src/module.rs

@@ -1,7 +1,7 @@
 //! Implements ModuleEnvironment for cranelift-wasm. Code derived from cranelift-wasm/environ/dummy.rs
 use crate::error::{LucetcError, LucetcErrorKind};
 use crate::pointer::NATIVE_POINTER;
-use cranelift_codegen::entity::{entity_impl, EntityRef, PrimaryMap};
+use cranelift_codegen::entity::{entity_impl, EntityRef, PrimaryMap, SecondaryMap};
 use cranelift_codegen::ir;
 use cranelift_codegen::isa::TargetFrontendConfig;
 use cranelift_wasm::{
@@ -72,6 +72,8 @@ pub struct ModuleInfo<'a> {
     pub function_mapping: PrimaryMap<FuncIndex, UniqueFuncIndex>,
     /// Function signatures: imported and local
     pub functions: PrimaryMap<UniqueFuncIndex, Exportable<'a, SignatureIndex>>,
+    /// Function names.
+    pub function_names: SecondaryMap<UniqueFuncIndex, &'a str>,
     /// Provided by `declare_table`
     pub tables: PrimaryMap<TableIndex, Exportable<'a, Table>>,
     /// Provided by `declare_memory`
@@ -103,6 +105,7 @@ impl<'a> ModuleInfo<'a> {
             imported_memories: PrimaryMap::new(),
             function_mapping: PrimaryMap::new(),
             functions: PrimaryMap::new(),
+            function_names: SecondaryMap::new(),
             tables: PrimaryMap::new(),
             memories: PrimaryMap::new(),
             globals: PrimaryMap::new(),
@@ -401,4 +404,13 @@ impl<'a> ModuleEnvironment<'a> for ModuleInfo<'a> {
         }
         Ok(())
     }
+
+    fn declare_func_name(&mut self, func_index: FuncIndex, name: &'a str) -> WasmResult<()> {
+        let unique_func_index = *self
+            .function_mapping
+            .get(func_index)
+            .expect("function indices are valid");
+        self.function_names[unique_func_index] = name;
+        Ok(())
+    }
 }

lucetc/tests/wasm.rs

@@ -2,15 +2,20 @@ use lucet_module::bindings::Bindings;
 use std::collections::HashMap;
 use std::path::PathBuf;
 
-pub fn load_wat_module(name: &str) -> Vec<u8> {
+fn load_wat_module(name: &str) -> Vec<u8> {
     use std::fs::File;
     use std::io::Read;
-    use wabt::wat2wasm;
+    use wabt::Wat2Wasm;
     let watfile = PathBuf::from(&format!("tests/wasm/{}.wat", name));
     let mut contents = Vec::new();
     let mut file = File::open(&watfile).expect("open module file");
     file.read_to_end(&mut contents).expect("read module file");
-    wat2wasm(contents).expect("convert module to wasm binary format")
+    Wat2Wasm::new()
+        .write_debug_names(true)
+        .convert(contents)
+        .expect("convert module to wasm binary format")
+        .as_ref()
+        .to_owned()
 }
 
 pub fn test_bindings() -> Bindings {
@@ -508,6 +513,32 @@ mod module_data {
         );
         */
     }
+
+    #[test]
+    fn names_local() {
+        let m = load_wat_module("names_local");
+        let b = super::test_bindings();
+        let h = HeapSettings::default();
+        let c = Compiler::new(
+            &m,
+            OptLevel::default(),
+            CpuFeatures::default(),
+            &b,
+            h,
+            false,
+            &None,
+        )
+        .expect("compile names_local");
+        let mdata = c.module_data().unwrap();
+
+        assert_eq!(mdata.import_functions().len(), 0);
+        assert_eq!(mdata.export_functions().len(), 0);
+        assert_eq!(mdata.function_info().len(), 3);
+        assert_eq!(
+            mdata.function_info().get(0).unwrap().name,
+            Some("func_name_0")
+        )
+    }
 }
 
 mod compile {
@@ -561,7 +592,6 @@ mod compile {
 }
 
 mod validate {
-
     use super::load_wat_module;
     use lucet_validate::Validator;
     use lucetc::{Compiler, CpuFeatures, HeapSettings, OptLevel};

lucetc/tests/wasm/names_local.wat

@@ -0,0 +1,4 @@
+(module
+    (func $func_name
+    )
+)