Commit d6e2554d authored by Clemens Backes's avatar Clemens Backes Committed by V8 LUCI CQ

[wasm] Fix memory growth near the maximum

If we grow memory (out-of-place, so only without trap handling and only
if the maximum is >1GB) and the previous size is close to the maximum,
then the minimum growth we calculate can be bigger than the allowed
maximum. In this situation, the {std::clamp} has undefined behaviour,
since the provided lower limit is bigger then the upper limit.

Thus apply {std::min} and {std::max} in an order such that {max_pages}
has precedence over {min_growth}.

R=thibaudm@chromium.org

Bug: chromium:1348335
Change-Id: I4f9e9ce10a0685892248eaf0e06ffd2e84b9a069
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3793396
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: 's avatarThibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82081}
parent c30e800c
......@@ -1015,7 +1015,11 @@ int32_t WasmMemoryObject::Grow(Isolate* isolate,
// These numbers are kept small because we must be careful about address
// space consumption on 32-bit platforms.
size_t min_growth = old_pages + 8 + (old_pages >> 3);
size_t new_capacity = std::clamp(new_pages, min_growth, max_pages);
// First apply {min_growth}, then {max_pages}. The order is important, because
// {min_growth} can be bigger than {max_pages}, and in that case we want to
// cap to {max_pages}.
size_t new_capacity = std::min(max_pages, std::max(new_pages, min_growth));
DCHECK_LT(old_pages, new_capacity);
std::unique_ptr<BackingStore> new_backing_store =
backing_store->CopyWasmMemory(isolate, new_pages, new_capacity,
memory_object->is_memory64()
......
......@@ -439,3 +439,40 @@ function testMemoryGrowPreservesDataMemOpBase(size, load_fn, store_fn) {
assertEquals(-1, result);
}
})();
(function testGrowFromNearlyMaximum() {
print(arguments.callee.name);
// Regression test for https://crbug.com/1347668.
const builder = genMemoryGrowBuilder();
// The maximum needs to be >1GB, so we do not reserve everything upfront.
const GB = 1024 * 1024 * 1024;
const max_pages = 1 * GB / kPageSize + 10;
builder.addMemory(0, max_pages);
let module;
const is_oom = e =>
(e instanceof RangeError) && e.message.includes('Out of memory');
// Allow instantiation to fail with OOM.
try {
module = builder.instantiate();
} catch (e) {
if (is_oom(e)) return;
// Everything else is a bug.
throw e;
}
const grow = module.exports.grow_memory;
// First, grow close to the limit.
// Growing can always fail if the system runs out of resources.
let grow_result = grow(max_pages - 1);
if (grow_result == -1) return;
assertEquals(0, grow_result);
// Then, grow by another page (this triggered the error in
// https://crbug.com/1347668).
grow_result = grow(1);
if (grow_result == -1) return;
assertEquals(max_pages - 1, grow_result);
assertEquals(max_pages, grow(0));
assertEquals(-1, grow(1)); // Fails.
})();
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment