[Webkit-unassigned] [Bug 228726] New: WebAssembly version of Google Earth fails to run on iPad Pro

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=228726

            Bug ID: 228726
           Summary: WebAssembly version of Google Earth fails to run on
                    iPad Pro
           Product: WebKit
           Version: WebKit Local Build
          Hardware: Unspecified
                OS: Unspecified
            Status: NEW
          Severity: Major
          Priority: P1
         Component: WebAssembly
          Assignee: webkit-unassigned at lists.webkit.org
          Reporter: kbr at google.com
                CC: dino at apple.com

Recently the Emscripten toolchain used to compile Google Earth for the web (WebAssembly version) was upgraded from 2.0.16 to 2.0.23, including a compiler update to LLVM 12. After the upgrade, it was noticed that there was a spike in runtime errors. After some careful analysis, these were determined as all coming from iPad Pro users running Safari, and requesting the desktop version of the site. (Ordinarily, mobile users that visit https://earth.google.com/ are redirected to download the native Google Earth application.)

dino@ told me on Slack that this sort of bug would be considered P1 by WebKit's WebAssembly team, so filing it as such.

There are two failure modes. One is that tiles fail to appear; the other is:

"""
RuntimeError:
Out of bounds memory access (evaluating 'wasmtable.get(func)(arg)')
"""

The Earth and Wasm teams at Google have investigated this a fair amount ​and have narrowed the problem down to two particular pieces of C++ code. This is one of them:

---
int ReadVarInt(const std::string& buffer, int offset, uint64_t* out_value) {
 ​if (offset < 0) {
   ​return 0;
 ​}
 ​uint64_t value = 0;
 ​const int limit = std::min(10, static_cast<int>(buffer.size()) - offset);
 ​uint8_t last_byte = 0x80;
 ​int i = 0;
 ​while (i < limit && (last_byte & 0x80)) {
   ​last_byte = buffer[offset + i];
   ​value += static_cast<uint64_t>(last_byte & 0x7F) << (i * 7);
   ​++i;
 ​}
 ​*out_value = value;
 ​return i && (last_byte & 0x80) ? -1 : i;
}
---

They've compiled the source file containing this function with -fsanitize=undefined and don't see any warnings about C++ undefined behavior in this file. It's producing values <= 0 somewhere that it shouldn't. Adding logging like this makes the bug disappear:

---
int temp = i && (last_byte & 0x80) ? -1 : i;
if (temp <= 0) { ..use logging to print the values.. }
return temp;
---

so we suspect this is a bug in WebKit's Wasm compiler/optimizer for ARM.

Unfortunately it will be difficult to find the compiled code of this function inside the large .wasm binary. We'd like to work with you to track down and fix this bug, as it may crop up in other real-world web applications.

Here are two before-and-after versions of Google Earth for the web:

No crash:
earth.google.com/static/wasm/9.140.0.5/

Crash:
earth.google.com/static/wasm/9.141.0.0/

Please tell us what additional information would be needed to diagnose this. Thanks.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210803/955ef7b4/attachment.htm>