<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Parse the entire WebAssembly modules"
href="https://bugs.webkit.org/show_bug.cgi?id=147393#c10">Comment # 10</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Parse the entire WebAssembly modules"
href="https://bugs.webkit.org/show_bug.cgi?id=147393">bug 147393</a>
from <span class="vcard"><a class="email" href="mailto:mark.lam@apple.com" title="Mark Lam <mark.lam@apple.com>"> <span class="fn">Mark Lam</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=257910&action=diff" name="attach_257910" title="Patch">attachment 257910</a> <a href="attachment.cgi?id=257910&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=257910&action=review">https://bugs.webkit.org/attachment.cgi?id=257910&action=review</a>
<span class="quote">>>> Source/JavaScriptCore/wasm/WASMReader.cpp:83
>>> + CHECK_READ(1);
>>
>> Why not do everything in the loop?
>
> It's just an optimization for small integers. If this seems like a micro-optimization, let me know and I will fix it.</span >
I'm not sure that optimizing here will yield much difference because we're parsing values from an untrusted file, and we need to do all sorts of error checks anyway. I suggest implementing this all as follows:
1. Use a do-while loop because it is more succinct and easier to read.
2. Use a uint64_t local value instead of storing into the result directly. Using the local allows the compiler to put it in a register, and register operations are cheap. We only incur the memory write to result at the end.
3. When the computation is done,
(1) break out of the loop,
(2) validate that the computed value is within size of a uint32, and
(3) copy the local value into the result.
4. The loop can loop while (value < numeric_limits<uint32_t>::max()). This ensures that we don't overflow our uint64_t local value and that the validation check in 3.2 will work as expected.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>