Bug 1532689: Use a ModuleEnvironment pointer (instead of a reference) to work around a bindgen bug; r=sunfish
authorBenjamin Bouvier <benj@benj.me>
Tue, 05 Mar 2019 18:31:26 +0100
changeset 521002 bf190c7c3de34db32f46a258c396e5d885e491cf
parent 521001 15d3e8135d577b3ef62ca25050b4bdd6c0af8142
child 521003 2aa82e63f173faee492b493150dacbe8b76fc89f
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssunfish
bugs1532689
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1532689: Use a ModuleEnvironment pointer (instead of a reference) to work around a bindgen bug; r=sunfish The structure layout was incorrectly computed because of the reference, meaning that the data located after the reference was incorrect. In particular, it means the minimal memory size wasn't correctly read. This works around it by using a pointer, and rename a few function parameters to make their role clearer. Differential Revision: https://phabricator.services.mozilla.com/D22139
js/src/wasm/WasmCraneliftCompile.cpp
js/src/wasm/cranelift/baldrapi.h
--- a/js/src/wasm/WasmCraneliftCompile.cpp
+++ b/js/src/wasm/WasmCraneliftCompile.cpp
@@ -242,50 +242,52 @@ CraneliftStaticEnvironment::CraneliftSta
 // This function translates between the two.
 
 static size_t globalToTlsOffset(size_t globalOffset) {
   return offsetof(wasm::TlsData, globalArea) + globalOffset;
 }
 
 CraneliftModuleEnvironment::CraneliftModuleEnvironment(
     const ModuleEnvironment& env)
-    : env(env), min_memory_length(env.minMemoryLength) {}
+    : env(&env),
+      min_memory_length(env.minMemoryLength)
+{}
 
 TypeCode env_unpack(BD_ValType valType) {
   return TypeCode(UnpackTypeCodeType(PackedTypeCode(valType.packed)));
 }
 
 const FuncTypeWithId* env_function_signature(
-    const CraneliftModuleEnvironment* env, size_t funcIndex) {
-  return env->env.funcTypes[funcIndex];
+    const CraneliftModuleEnvironment* wrapper, size_t funcIndex) {
+  return wrapper->env->funcTypes[funcIndex];
 }
 
-size_t env_func_import_tls_offset(const CraneliftModuleEnvironment* env,
+size_t env_func_import_tls_offset(const CraneliftModuleEnvironment* wrapper,
                                   size_t funcIndex) {
-  return globalToTlsOffset(env->env.funcImportGlobalDataOffsets[funcIndex]);
+  return globalToTlsOffset(wrapper->env->funcImportGlobalDataOffsets[funcIndex]);
 }
 
-bool env_func_is_import(const CraneliftModuleEnvironment* env,
+bool env_func_is_import(const CraneliftModuleEnvironment* wrapper,
                         size_t funcIndex) {
-  return env->env.funcIsImport(funcIndex);
+  return wrapper->env->funcIsImport(funcIndex);
 }
 
-const FuncTypeWithId* env_signature(const CraneliftModuleEnvironment* env,
+const FuncTypeWithId* env_signature(const CraneliftModuleEnvironment* wrapper,
                                     size_t funcTypeIndex) {
-  return &env->env.types[funcTypeIndex].funcType();
+  return &wrapper->env->types[funcTypeIndex].funcType();
 }
 
-const TableDesc* env_table(const CraneliftModuleEnvironment* env,
+const TableDesc* env_table(const CraneliftModuleEnvironment* wrapper,
                            size_t tableIndex) {
-  return &env->env.tables[tableIndex];
+  return &wrapper->env->tables[tableIndex];
 }
 
-const GlobalDesc* env_global(const CraneliftModuleEnvironment* env,
+const GlobalDesc* env_global(const CraneliftModuleEnvironment* wrapper,
                              size_t globalIndex) {
-  return &env->env.globals[globalIndex];
+  return &wrapper->env->globals[globalIndex];
 }
 
 bool wasm::CraneliftCompileFunctions(const ModuleEnvironment& env,
                                      LifoAlloc& lifo,
                                      const FuncCompileInputVector& inputs,
                                      CompiledCode* code, UniqueChars* error) {
   MOZ_RELEASE_ASSERT(CraneliftCanCompile());
 
@@ -327,18 +329,17 @@ bool wasm::CraneliftCompileFunctions(con
     const FuncTypeIdDesc& funcTypeId = env.funcTypes[clifInput.index]->id;
 
     FuncOffsets offsets;
     if (!GenerateCraneliftCode(masm, clifFunc, funcTypeId, lineOrBytecode,
                                &offsets)) {
       return false;
     }
 
-    if (!code->codeRanges.emplaceBack(func.index, func.lineOrBytecode,
-                                      offsets)) {
+    if (!code->codeRanges.emplaceBack(func.index, lineOrBytecode, offsets)) {
       return false;
     }
   }
 
   masm.finish();
   if (masm.oom()) {
     return false;
   }
--- a/js/src/wasm/cranelift/baldrapi.h
+++ b/js/src/wasm/cranelift/baldrapi.h
@@ -78,17 +78,18 @@ struct CraneliftStaticEnvironment {
   // Not bindgen'd because it's inlined.
   inline CraneliftStaticEnvironment();
 };
 
 // This structure proxies the C++ ModuleEnvironment and the information it
 // contains.
 
 struct CraneliftModuleEnvironment {
-  const js::wasm::ModuleEnvironment& env;
+  // This is a pointer and not a reference to work-around a bug in bindgen.
+  const js::wasm::ModuleEnvironment* env;
   uint32_t min_memory_length;
 
   // Not bindgen'd because it's inlined.
   explicit inline CraneliftModuleEnvironment(
       const js::wasm::ModuleEnvironment& env);
 };
 
 // Data for a single wasm function to be compiled by Cranelift.
@@ -114,18 +115,18 @@ struct CraneliftFuncCompileInput {
 struct CraneliftMetadataEntry {
   enum Which {
     DirectCall,
     IndirectCall,
     Trap,
     MemoryAccess,
     SymbolicAccess
   } which;
-  uint32_t offset;
-  uint32_t srcLoc;
+  uint32_t offset; // relative to the beginning of the function generated code
+  uint32_t srcLoc; // relative to the beginning of the module bytecode
   size_t extra;
 };
 
 // The result of a single function compilation, containing the machine code
 // generated by Cranelift, as well as some useful metadata to generate the
 // prologue/epilogue etc.
 
 struct CraneliftCompiledFunc {