WebAssembly version of Google Earth fails to run on iPad Pro

• Component: WebAssembly
• Status: NEW ---

[Kenneth Russell]

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.

[Radar WebKit Bug Importer]

<rdar://problem/81445608>

[Alexey Proskuryakov]

I can reproduce missing tiles on https://earth.google.com/static/wasm/9.141.0.0/ on an iPad Pro 3 running iOS 15 beta.

[Keith Miller]

Thanks for the report Kenneth!

Is it possible/easy to create a copy of the site where the `ReadVarInt` function (or just a copy of it) is exported out of the wasm module to JS?

That should help us figure out which function it is in the module by tracking it backwards from the export name. It would also be interesting to know if LLVM is inlining that function anywhere, you can disable that by adding `__attribute__((__noinline__))` to the declaration of ReadVarInt.

[Derek Schuff]

It might be possible to spin a new binary, I think we're looking into that.

But also I took a look at the symbols from the production binary (9.141.0.0), and I believe that ReadVarInt function is inlined into the function at index 7067 (indirectly via several layers, it appears; it's unfortunately quite a large function). A tool such as Wabt's wasm-objdump will give you the disassembly, and I assume JSC also tracks the function index for error reporting, etc. So if you have the ability to restrict just that function to a lower tier or something, it might be one way to check whether there are any differences in behavior.

[Alon Zakai]

Ok, updated builds have been made:

Before the crash:       https://ipad-dot-earthfoody.appspot.com/

With the crash in iPad: https://ipad2-dot-earthfoody.appspot.com/

Those builds call a JS import "Z" which does a console.log saying "mymarker". There are two calls to that import, the first right before the relevant code and the second after. So those should indicate the relevant region.

[Alon Zakai]

Sorry, the links I posted are not available externally, which I was not aware of. I'll attach the wasm files.

[Alon Zakai]

Even gzipped they are too large to be attached here, sorry. Here is a drive link:

https://drive.google.com/drive/u/1/folders/1SjC7biOWbR6WKeLQdUeL3kXEEHnk4IWz

First is the working one, second is the crashing one.

[Alon Zakai]

The earth3.wasm.gz file there is with noinline as requested earlier.

[Keith Miller]

Great, thanks for the links Alon! Much appreciated.

[Alon Zakai]

(Btw, the no inlining build resolves the issue with ReadVarInt (missing tiles))

[Kenneth Russell]

Hi WebKit folks, is any more information needed in order to diagnose this compilation bug? We're happy to gather and provide it if so.