Skip to content

Commit 9699b32

Browse files
PiotrSikoraPiotrSikora
authored
Fix segfault when failing to instantiate Wasm module. (#240)
Signed-off-by: Piotr Sikora <[email protected]>
1 parent 5cb6f88 commit 9699b32

File tree

7 files changed

+229
-25
lines changed

7 files changed

+229
-25
lines changed

‎bazel/external/v8.patch

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
# Disable pointer compression (limits the maximum number of WasmVMs).
2+
3+
diff --git a/BUILD.bazel b/BUILD.bazel
4+
index 1cc0121e60..4947c1dba2 100644
5+
--- a/BUILD.bazel
6+
+++ b/BUILD.bazel
7+
@@ -161,7 +161,7 @@ v8_int(
8+
# If no explicit value for v8_enable_pointer_compression, we set it to 'none'.
9+
v8_string(
10+
name = "v8_enable_pointer_compression",
11+
- default = "none",
12+
+ default = "False",
13+
)
14+
15+
# Default setting for v8_enable_pointer_compression.

‎bazel/repositories.bzl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,8 @@ def proxy_wasm_cpp_host_repositories():
9898
commit = "90f089d97b6e4146ad106eee1829d86ad6392027",
9999
remote = "https://chromium.googlesource.com/v8/v8",
100100
shallow_since = "1643043727 +0000",
101+
patches = ["@proxy_wasm_cpp_host//bazel/external:v8.patch"],
102+
patch_args = ["-p1"],
101103
)
102104

103105
native.bind(

‎src/v8/v8.cc

Lines changed: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -250,25 +250,32 @@ template <typename T, typename U> constexpr T convertValTypesToArgsTuple(const U
250250
bool V8::load(std::string_view bytecode, std::string_view precompiled,
251251
const std::unordered_map<uint32_t, std::string> function_names) {
252252
store_ = wasm::Store::make(engine());
253+
if (store_ == nullptr) {
254+
return false;
255+
}
253256

254257
if (!precompiled.empty()) {
255258
auto vec = wasm::vec<byte_t>::make_uninitialized(precompiled.size());
256259
::memcpy(vec.get(), precompiled.data(), precompiled.size());
257260
module_ = wasm::Module::deserialize(store_.get(), vec);
261+
if (module_ == nullptr) {
262+
return false;
263+
}
258264

259265
} else {
260266
auto vec = wasm::vec<byte_t>::make_uninitialized(bytecode.size());
261267
::memcpy(vec.get(), bytecode.data(), bytecode.size());
262268
module_ = wasm::Module::make(store_.get(), vec);
269+
if (module_ == nullptr) {
270+
return false;
271+
}
263272
}
264273

265-
if (!module_) {
274+
shared_module_ = module_->share();
275+
if (shared_module_ == nullptr) {
266276
return false;
267277
}
268278

269-
shared_module_ = module_->share();
270-
assert(shared_module_ != nullptr);
271-
272279
function_names_index_ = function_names;
273280

274281
return true;
@@ -278,10 +285,26 @@ std::unique_ptr<WasmVm> V8::clone() {
278285
assert(shared_module_ != nullptr);
279286

280287
auto clone = std::make_unique<V8>();
281-
clone->integration().reset(integration()->clone());
288+
if (clone == nullptr) {
289+
return nullptr;
290+
}
291+
282292
clone->store_ = wasm::Store::make(engine());
293+
if (clone->store_ == nullptr) {
294+
return nullptr;
295+
}
283296

284297
clone->module_ = wasm::Module::obtain(clone->store_.get(), shared_module_.get());
298+
if (clone->module_ == nullptr) {
299+
return nullptr;
300+
}
301+
302+
auto integration_clone = integration()->clone();
303+
if (integration_clone == nullptr) {
304+
return nullptr;
305+
}
306+
clone->integration().reset(integration_clone);
307+
285308
clone->function_names_index_ = function_names_index_;
286309

287310
return clone;
@@ -322,7 +345,7 @@ bool V8::link(std::string_view debug_name) {
322345
fail(FailState::UnableToInitializeCode,
323346
std::string("Failed to load Wasm module due to a missing import: ") +
324347
std::string(module) + "." + std::string(name));
325-
break;
348+
return false;
326349
}
327350
auto func = it->second.get()->callback_.get();
328351
if (!equalValTypes(import_type->func()->params(), func->type()->params()) ||
@@ -334,7 +357,7 @@ bool V8::link(std::string_view debug_name) {
334357
printValTypes(import_type->func()->results()) +
335358
", but host exports: " + printValTypes(func->type()->params()) + " -> " +
336359
printValTypes(func->type()->results()));
337-
break;
360+
return false;
338361
}
339362
imports.push_back(func);
340363
} break;
@@ -344,12 +367,19 @@ bool V8::link(std::string_view debug_name) {
344367
fail(FailState::UnableToInitializeCode,
345368
"Failed to load Wasm module due to a missing import: " + std::string(module) + "." +
346369
std::string(name));
370+
return false;
347371
} break;
348372

349373
case wasm::EXTERN_MEMORY: {
350374
assert(memory_ == nullptr);
351375
auto type = wasm::MemoryType::make(import_type->memory()->limits());
376+
if (type == nullptr) {
377+
return false;
378+
}
352379
memory_ = wasm::Memory::make(store_.get(), type.get());
380+
if (memory_ == nullptr) {
381+
return false;
382+
}
353383
imports.push_back(memory_.get());
354384
} break;
355385

@@ -358,7 +388,13 @@ bool V8::link(std::string_view debug_name) {
358388
auto type =
359389
wasm::TableType::make(wasm::ValType::make(import_type->table()->element()->kind()),
360390
import_type->table()->limits());
391+
if (type == nullptr) {
392+
return false;
393+
}
361394
table_ = wasm::Table::make(store_.get(), type.get());
395+
if (table_ == nullptr) {
396+
return false;
397+
}
362398
imports.push_back(table_.get());
363399
} break;
364400
}
@@ -369,6 +405,10 @@ bool V8::link(std::string_view debug_name) {
369405
}
370406

371407
instance_ = wasm::Instance::make(store_.get(), module_.get(), imports.data());
408+
if (instance_ == nullptr) {
409+
fail(FailState::UnableToInitializeCode, "Failed to create new Wasm instance");
410+
return false;
411+
}
372412

373413
const auto export_types = module_.get()->exports();
374414
const auto exports = instance_.get()->exports();
@@ -395,6 +435,9 @@ bool V8::link(std::string_view debug_name) {
395435
assert(export_item->memory() != nullptr);
396436
assert(memory_ == nullptr);
397437
memory_ = exports[i]->memory()->copy();
438+
if (memory_ == nullptr) {
439+
return false;
440+
}
398441
} break;
399442

400443
case wasm::EXTERN_TABLE: {
@@ -403,7 +446,7 @@ bool V8::link(std::string_view debug_name) {
403446
}
404447
}
405448

406-
return !isFailed();
449+
return true;
407450
}
408451

409452
uint64_t V8::getMemorySize() { return memory_->data_size(); }

‎src/wamr/wamr.cc

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,19 @@ class Wamr : public WasmVm {
116116
bool Wamr::load(std::string_view bytecode, std::string_view,
117117
const std::unordered_map<uint32_t, std::string>) {
118118
store_ = wasm_store_new(engine());
119+
if (store_ == nullptr) {
120+
return false;
121+
}
119122

120123
WasmByteVec vec;
121124
wasm_byte_vec_new(vec.get(), bytecode.size(), bytecode.data());
125+
122126
module_ = wasm_module_new(store_.get(), vec.get());
127+
if (module_ == nullptr) {
128+
return false;
129+
}
123130

124-
return module_ != nullptr;
131+
return true;
125132
}
126133

127134
static bool equalValTypes(const wasm_valtype_vec_t *left, const wasm_valtype_vec_t *right) {
@@ -225,7 +232,7 @@ bool Wamr::link(std::string_view debug_name) {
225232
fail(FailState::UnableToInitializeCode,
226233
std::string("Failed to load Wasm module due to a missing import: ") +
227234
std::string(module_name) + "." + std::string(name));
228-
break;
235+
return false;
229236
}
230237

231238
auto func = it->second->callback_.get();
@@ -242,7 +249,7 @@ bool Wamr::link(std::string_view debug_name) {
242249
printValTypes(wasm_functype_results(exp_type)) +
243250
", but host exports: " + printValTypes(wasm_functype_params(actual_type.get())) +
244251
" -> " + printValTypes(wasm_functype_results(actual_type.get())));
245-
break;
252+
return false;
246253
}
247254
imports.push_back(wasm_func_as_extern(func));
248255
} break;
@@ -251,19 +258,32 @@ bool Wamr::link(std::string_view debug_name) {
251258
fail(FailState::UnableToInitializeCode,
252259
"Failed to load Wasm module due to a missing import: " + std::string(module_name) + "." +
253260
std::string(name));
261+
return false;
254262
} break;
255263
case WASM_EXTERN_MEMORY: {
256264
assert(memory_ == nullptr);
257265
const wasm_memorytype_t *memory_type =
258266
wasm_externtype_as_memorytype_const(extern_type); // owned by `extern_type`
267+
if (memory_type == nullptr) {
268+
return false;
269+
}
259270
memory_ = wasm_memory_new(store_.get(), memory_type);
271+
if (memory_ == nullptr) {
272+
return false;
273+
}
260274
imports.push_back(wasm_memory_as_extern(memory_.get()));
261275
} break;
262276
case WASM_EXTERN_TABLE: {
263277
assert(table_ == nullptr);
264278
const wasm_tabletype_t *table_type =
265279
wasm_externtype_as_tabletype_const(extern_type); // owned by `extern_type`
280+
if (table_type == nullptr) {
281+
return false;
282+
}
266283
table_ = wasm_table_new(store_.get(), table_type, nullptr);
284+
if (table_ == nullptr) {
285+
return false;
286+
}
267287
imports.push_back(wasm_table_as_extern(table_.get()));
268288
} break;
269289
}
@@ -275,7 +295,10 @@ bool Wamr::link(std::string_view debug_name) {
275295

276296
wasm_extern_vec_t imports_vec = {imports.size(), imports.data()};
277297
instance_ = wasm_instance_new(store_.get(), module_.get(), &imports_vec, nullptr);
278-
assert(instance_ != nullptr);
298+
if (instance_ == nullptr) {
299+
fail(FailState::UnableToInitializeCode, "Failed to create new Wasm instance");
300+
return false;
301+
}
279302

280303
WasmExportTypeVec export_types;
281304
wasm_module_exports(module_.get(), export_types.get());
@@ -302,7 +325,9 @@ bool Wamr::link(std::string_view debug_name) {
302325
case WASM_EXTERN_MEMORY: {
303326
assert(memory_ == nullptr);
304327
memory_ = wasm_memory_copy(wasm_extern_as_memory(actual_extern));
305-
assert(memory_ != nullptr);
328+
if (memory_ == nullptr) {
329+
return false;
330+
}
306331
} break;
307332
case WASM_EXTERN_TABLE: {
308333
// TODO(mathetake): add support when/if needed.

‎src/wasmtime/wasmtime.cc

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -112,28 +112,49 @@ class Wasmtime : public WasmVm {
112112
bool Wasmtime::load(std::string_view bytecode, std::string_view,
113113
const std::unordered_map<uint32_t, std::string>) {
114114
store_ = wasm_store_new(engine());
115+
if (store_ == nullptr) {
116+
return false;
117+
}
115118

116119
WasmByteVec vec;
117120
wasm_byte_vec_new(vec.get(), bytecode.size(), bytecode.data());
118121

119122
module_ = wasm_module_new(store_.get(), vec.get());
120-
if (!module_) {
123+
if (module_ == nullptr) {
121124
return false;
122125
}
123126

124127
shared_module_ = wasm_module_share(module_.get());
125-
assert(shared_module_ != nullptr);
128+
if (shared_module_ == nullptr) {
129+
return false;
130+
}
126131

127132
return true;
128133
}
129134

130135
std::unique_ptr<WasmVm> Wasmtime::clone() {
131136
assert(shared_module_ != nullptr);
137+
132138
auto clone = std::make_unique<Wasmtime>();
139+
if (clone == nullptr) {
140+
return nullptr;
141+
}
133142

134-
clone->integration().reset(integration()->clone());
135143
clone->store_ = wasm_store_new(engine());
144+
if (clone->store_ == nullptr) {
145+
return nullptr;
146+
}
147+
136148
clone->module_ = wasm_module_obtain(clone->store_.get(), shared_module_.get());
149+
if (clone->module_ == nullptr) {
150+
return nullptr;
151+
}
152+
153+
auto integration_clone = integration()->clone();
154+
if (integration_clone == nullptr) {
155+
return nullptr;
156+
}
157+
clone->integration().reset(integration_clone);
137158

138159
return clone;
139160
}
@@ -239,7 +260,7 @@ bool Wasmtime::link(std::string_view debug_name) {
239260
fail(FailState::UnableToInitializeCode,
240261
std::string("Failed to load Wasm module due to a missing import: ") +
241262
std::string(module_name) + "." + std::string(name));
242-
break;
263+
return false;
243264
}
244265

245266
auto func = it->second->callback_.get();
@@ -256,7 +277,7 @@ bool Wasmtime::link(std::string_view debug_name) {
256277
printValTypes(wasm_functype_results(exp_type)) +
257278
", but host exports: " + printValTypes(wasm_functype_params(actual_type.get())) +
258279
" -> " + printValTypes(wasm_functype_results(actual_type.get())));
259-
break;
280+
return false;
260281
}
261282
imports.push_back(wasm_func_as_extern(func));
262283
} break;
@@ -265,19 +286,32 @@ bool Wasmtime::link(std::string_view debug_name) {
265286
fail(FailState::UnableToInitializeCode,
266287
"Failed to load Wasm module due to a missing import: " + std::string(module_name) + "." +
267288
std::string(name));
289+
return false;
268290
} break;
269291
case WASM_EXTERN_MEMORY: {
270292
assert(memory_ == nullptr);
271293
const wasm_memorytype_t *memory_type =
272294
wasm_externtype_as_memorytype_const(extern_type); // owned by `extern_type`
295+
if (memory_type == nullptr) {
296+
return false;
297+
}
273298
memory_ = wasm_memory_new(store_.get(), memory_type);
299+
if (memory_ == nullptr) {
300+
return false;
301+
}
274302
imports.push_back(wasm_memory_as_extern(memory_.get()));
275303
} break;
276304
case WASM_EXTERN_TABLE: {
277305
assert(table_ == nullptr);
278306
const wasm_tabletype_t *table_type =
279307
wasm_externtype_as_tabletype_const(extern_type); // owned by `extern_type`
308+
if (table_type == nullptr) {
309+
return false;
310+
}
280311
table_ = wasm_table_new(store_.get(), table_type, nullptr);
312+
if (table_ == nullptr) {
313+
return false;
314+
}
281315
imports.push_back(wasm_table_as_extern(table_.get()));
282316
} break;
283317
}
@@ -289,7 +323,10 @@ bool Wasmtime::link(std::string_view debug_name) {
289323

290324
wasm_extern_vec_t imports_vec = {imports.size(), imports.data()};
291325
instance_ = wasm_instance_new(store_.get(), module_.get(), &imports_vec, nullptr);
292-
assert(instance_ != nullptr);
326+
if (instance_ == nullptr) {
327+
fail(FailState::UnableToInitializeCode, "Failed to create new Wasm instance");
328+
return false;
329+
}
293330

294331
WasmExportTypeVec export_types;
295332
wasm_module_exports(module_.get(), export_types.get());
@@ -316,7 +353,9 @@ bool Wasmtime::link(std::string_view debug_name) {
316353
case WASM_EXTERN_MEMORY: {
317354
assert(memory_ == nullptr);
318355
memory_ = wasm_memory_copy(wasm_extern_as_memory(actual_extern));
319-
assert(memory_ != nullptr);
356+
if (memory_ == nullptr) {
357+
return false;
358+
}
320359
} break;
321360
case WASM_EXTERN_TABLE: {
322361
// TODO(mathetake): add support when/if needed.

0 commit comments

Comments
 (0)