Miscompiled operation in LLVM Wasm backend causes data corruption in dav1d

[bvibber]

While testing the LLVM Wasm backend (with latest-upstream-3441) for ogv.js's codec modules, I found that the dav1d AV1 decoder has some weird image data corruption when compiling at -O1 or higher.

I tracked it down to the padding function in looprestoration_tmpl.c, and poking the disassembly turned up a bit where a bitfield check was being treated like a direct boolean value, causing it to fail when other bitfield values were present. This caused logic errors leading to corrupted image data. I can correct the .wast manually, re-assemble it, and confirm it works.

I've put an isolated test case at https://github.com/brion/emscripten-llvm-funky-bug including the wasm and llvm IR disassembly.

[kripken]

Thanks for the testcase @Brion!

Based on that description it does sound like an LLVM bug, with having (local.get $8) instead of (int32.and (local.get $8) (i32.const 1)). However, I can't reproduce this on latest-upstream-3451, I see this:

  (local.set $0
   (i32.add
    (local.get $0)
    (select
     (i32.const 0)
     (i32.const 3)
     (local.tee $9
      (i32.and
       (local.get $8)
       (i32.const 1)
      )
     )
    )
   )
  )

It has the proper &1, and also has a tee which I don't see in your output (the tee is expected since the value is used again later).

Perhaps this was fixed in that short internal, @aheejin @tlively?

I do seem to recall a similar-sounding bug being fixed in the recent, but not that recent, past. @Brion, is there some chance your path contains an older LLVM, or the LLVM env var pointing it to an older version? (EMCC_DEBUG=1 logging output can confirm)

[bvibber]

Hmm, I still get the error with latest-upstream-3453, without the and... Don't seem to have any stray LLVM. Here's the EMCC_DEBUG=1 output from compilation: https://gist.github.com/brion/aee93a766fd3ebeeefa9b183f3df6056

and my ~/.emscripten:

import os
LLVM_ROOT = '/home/brion/src/emsdk/upstream/3453/bin'
BINARYEN_ROOT = '/home/brion/src/emsdk/upstream/3453'
NODE_JS = '/home/brion/src/emsdk/node/8.9.1_64bit/bin/node'
SPIDERMONKEY_ENGINE = ''
V8_ENGINE = ''
TEMP_DIR = '/tmp'
COMPILER_ENGINE = NODE_JS
JS_ENGINES = [NODE_JS]

Note the system python is 2.7.15 (python 3 is installed separately as python3 on Fedora 29).

[bvibber]

(Using python3 makes no difference, as I expected.) Clearly something's wonky, I just don't know what. :)

[sunfishcode]

From the code snippet above, it looks a lot like https://bugs.llvm.org/show_bug.cgi?id=40805, for which a fix landed on trunk yesterday.

[bvibber]

@sunfishcode yeah I think that's it. :)

@kripken with a fresh emsdk install it builds correctly with upstream-3454, which I think is new enough to include the fix mentioned above. :D

I noticed another problem -- emsdk doesn't download new versions of the upstream wasm-binaries.tbz2 file after the first time, so every update that gets untar'd is the same version you first installed. This was leaving me stuck on 3346 or so even when I was furiously updating...

[kripken]

Oh wow, interesting timing here. Thanks everyone!

@Brion Fix for that emsdk issue is here - emscripten-core/emsdk#223 , pretty silly bug...

[bvibber]

Awesome, I can put this one to bed. :) Thanks everyone! Closing out this issue.