User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/90.0.4430.212 Safari/537.36 Edg/90.0.818.66

Steps to reproduce:

this bug effects the stable version

./js bug.js
===================bug.js==============
function opt() {
function v3() {
function v6(v7) {
new Uint8ClampedArray(v7);
return v3++;
}
function v13(v15) {
try {
const v16 = { "length": v13, "get": eval, "ownKeys": v15, "set": 1, "exx": 2 };
const v18 = new Proxy(v16, v16);
v15(v18);
} catch (v20) {
}
}
v13(v6);
}
v3();
const v24 = { "123": v3 };
return v24;
}

for (i = 0; i < 10000; i++) {
opt();
}
c1 = opt();
print(c1["123"]);

Actual results:

NaN

Expected results:

function opt() {
function v3() {
function v6(v7) {
new Uint8ClampedArray(v7);
return v3++;
}
function v13(v15) {
try {
const v16 = { "length": v13, "get": eval, "ownKeys": v15, "set": 1, "exx": 2 };
const v18 = new Proxy(v16, v16);
v15(v18);
} catch (v20) {
}
}
v13(v6);
}
v3();
const v24 = { "123": v3 };
return v24;
}

The expected result is supposed to be the v3 function. I was able to reproduce it in a page and not in a debugger console, which suggest that this is indeed the JIT.

function opt() {
  function v3() {
    function v6(v7) {
      new Uint8ClampedArray(v7);
      return v3++;
    }
    function v13(v15) {
      try {
        const v16 = { "length": v13, "get": eval, "ownKeys": v15, "set": 1, "exx": 2 };
        const v18 = new Proxy(v16, v16);
        v15(v18);
      } catch (v20) {}
    }
    v13(v6);
  }
  v3();
  const v24 = { "123": v3 };
  return v24;
}

for (i = 0; i < 10000; i++) {
  opt();
}
c1 = opt();
print(c1["123"]);

The only way to get v3 as NaN is if v15(v18) line and new Uint8ClampedArray(v7); are executed, with v15 = v6 and v18 = new Proxy(v16, v16).
The Uint8ClampedArray allocation is supposed to fail because of bad arguments. (the length argument being a function v13)

However, in the IonMonkey / WarpBuilder, we do ignore overflow allocation, but we should not ignore bad arguments.
This might be the source of the problem.

Iain, can you investigate what is wrong? We probably miss some input validation before calling the constructor. Or we should consider it fallible when we are not able to validate its input at compilation time.

This is a tricky one, but I think it's actually working as intended.

Here's a cleaned-up version of the testcase:

var okay = true;

function foo(arg) {
  new Uint8ClampedArray(arg);

  // Unreachable?
  okay = false;
}

const handler = {
  get: () => "fake",
  ownKeys: foo,
};
const proxy = new Proxy({}, handler);

function bar() {
  try {
    foo(proxy);
  } catch {}
}

for (i = 0; i < 100; i++) {
  bar();
}
assertEq(okay, true);

The assertion succeeds in a normal run, but fails with --fast-warmup --no-threads. However, it's not because the Uint8ClampedArray allocation succeeded in Ion. Instead, what happens is:

1. We call new Uint8ClampedArray from Ion, which calls into TypedArrayObjectTemplate<js::uint8_clamped>::new.
2. The proxy handler's implementation of get is non-functional, so we can't retrieve an iterator from it. We throw an error here.
3. To create the error, we call DecompileValueGenerator, which in turn calls DecompileExpressionFromStack. DecompileExpressionFromStack will bail out early if we are in Ion (or doing differential testing!).
4. The fallback in DecompileValueGenerator is to call ValueToSource on the proxy. As a side effect of ValueToSource, we trigger the ownKeys trap, passing in the target object.
5. This calls foo (v6 in the original testcase) with a normal, non-proxy argument. We make a nested call to new Uint8ClampedArray, and this one succeeds. We therefore reach the "unreachable" code.

So what this testcase really shows is that DecompileValueGenerator behaves differently when decompiling values in Ion frames. But we already knew that; that's why we added the js::SupportDifferentialTesting() early exit in DecompileExpressionFromStack. Sure enough, running the original testcase with --differential-testing will always print NaN, even if Ion is disabled.

vulbugs: Thanks for the report! When doing differential fuzzing, you can reduce the rate of false positives like this one using the --differential-testing flag. In addition to making behaviour more consistent between different optimization tiers, the flag will also make SM print a message to stderr for OOM, stack overflow, and other similar cases where resource exhaustion may happen at different points depending on how we optimize. Testcases that emit such messages are generally not interesting from a correctness / security perspective.

iireland ,

Thanks for your suggestion. I will try it .

(In reply to Iain Ireland [:iain] from comment #2)

[…] running the original testcase with --differential-testing will always print NaN, even if Ion is disabled.

The fact that Firefox devtools, and chromium webpage + devtools are showing the v3 function and not NaN is ok?
Is this because error handling is underspecified?

Shouldn't we have a consistent evaluation across all compilation tiers? Otherwise, the compilation tier becomes observable?

Yeah, this happens because error messages aren't specified, and proxies make accesses to an object while creating error messages observable.

I don't think that guaranteeing the exact same error message in every compilation tier would be worth the cost. This testcase uses a proxy instead of examining the error message, but it's the same general idea. We're never going to plug every hole that makes the compilation tier observable.