Automatically adding ccs based on suspected regression changelists:

Removes messages.js by szuend@google.com - https://chromium.googlesource.com/v8/v8/+/028d4d8107eaf61ef202675d2b82cf2c3859c2fa

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label.

The reproducer is very fragile. Removing a dead variable can make the difference disappear.

I was able to reproduce locally by comparing these flags:
1) <none>
2) --random-seed -447296549 --stress-scavenge=100

Without the random seed it fails nondeterministically, and also the produced output changes (some more or fewer "undefined" lines).

Local bisect resulted in dd78d603591663dd9abd64d1ed64caab7edfc966 (Ship Array.prototype.{flat,flatMap} 🎉), which is probably also unrelated.

Assigning to memory sheriff for further triage, as it seems to be related to GC timing.

Why is that a memory sheriff issue? The fuzzers check correctness wrt. compiler interpreter, right? 

Since we are already speculating, maybe this is just shifting compilation times? :)

Ok, I looked into this a bit more. I think there is something wrong with our Proxy implementation.

Consider this code:
================================================
this.__proto__ = new Proxy({}, {});
for (var x = 0; x < 5; ++x) {
  try {
    foo;
    console.log("returned");
  } catch (e) {
    console.log("exception:", e);
  }
}
================================================

I think it should print an exception five times (ReferenceError: foo is not defined).
We print this exception once, then we print four times "returned".

This behaviour was introduced in 0410e7e85032c9b49bd84f5a8ba71642264bce3f (Reland ^4 "[builtins] Port getting property from Proxy to CSA"). Before, we printed the ReferenceError five times.
The CL landed last August, so this error is in all channels. Still not a release blocker in my opinion, since it's in there for so long and no one complained yet.

Assigning to Maya for a fix.

This is a d8-only issue. Will post more details soon.

The issue is known, see the below email thread from earlier this year.

To summarize:
- This is not an issue for Chrome.
- This may be an issue for Node, but I wasn't able to reproduce it with 8.10.
- We should not assume frozenness of the global prototype chain in V8.

Maya, could you have a look to see if you find the faulty code? You may need help from Igor who knows all about global property access.


==============


On Mon, Mar 12, 2018 at 1:35 PM, Georg Neis <neis@google.com> wrote:
> Hi all,
> can you remind of the semantics of proxies in the prototype chain of
> the global object? Wasn't there a language change to disallow this? I
> can't find it at the moment.
>
> I'm investigating some weird behavior and as a first step I need to
> understand what the correct behavior is. I think the examples below
> imply at least one bug in V8, possibly more.
>
>
> FIRST EXAMPLE:
>
> $ cat proxy.js
> function bar() { eval("bla"); }
> this.__proto__ = new Proxy({}, {});
> try { gc(); bar(); print('not thrown'); } catch (e) { print('thrown') }
> try { gc(); bar(); print('not thrown'); } catch (e) { print('thrown') }
> try { gc(); bar(); print('not thrown'); } catch (e) { print('thrown') }
> try { gc(); bar(); print('not thrown'); } catch (e) { print('thrown') }
> try { gc(); bar(); print('not thrown'); } catch (e) { print('thrown') }
>
> $ d8 --expose-gc proxy.js
> thrown
> thrown
> thrown
> thrown
> thrown
>
>
> SECOND EXAMPLE:
>
> $ cat proxy2.js
> // like proxy1.js but without gc()
> function bar() { eval("bla"); }
> this.__proto__ = new Proxy({}, {});
> try { bar(); print('not thrown'); } catch (e) { print('thrown') }
> try { bar(); print('not thrown'); } catch (e) { print('thrown') }
> try { bar(); print('not thrown'); } catch (e) { print('thrown') }
> try { bar(); print('not thrown'); } catch (e) { print('thrown') }
> try { bar(); print('not thrown'); } catch (e) { print('thrown') }
>
> $ d8 proxy2.js
> thrown
> thrown
> not thrown
> not thrown
> not thrown
>
>
> THIRD EXAMPLE:
>
> $ cat proxy3.js
> function foo() { try { eval("bla"); } catch(e) { return }; throw 666 }
> function bar() { gc(); foo(); foo(); }
> this.__proto__ = new Proxy({}, {});
> try { bar(); print('not thrown'); } catch (e) { print('thrown') }
> try { bar(); print('not thrown'); } catch (e) { print('thrown') }
> try { bar(); print('not thrown'); } catch (e) { print('thrown') }
> try { bar(); print('not thrown'); } catch (e) { print('thrown') }
> try { bar(); print('not thrown'); } catch (e) { print('thrown') }
>
> $ d8 --expose-gc --noopt proxy3.js
> not thrown
> thrown
> thrown
> thrown
> thrown
>
> $ d8 --expose-gc --always-opt proxy3.js
> not thrown
> thrown
> thrown
> thrown
> not thrown


On Mon, Mar 12, 2018 at 3:43 PM Daniel Ehrenberg <littledan@chromium.org> wrote:
> The prototype chain of the global object is supposed to be immutable.
> This is specified as the following:
> - Object.prototype is an immutable prototype exotic object [1]
> - In WebIDL, [Global] interfaces have a [[SetPrototype]] method which
> is the same as immutable prototype exotic objects [2]
> - In WebIDL, named properties objects are the same way [3]
> 
> In V8 and Chromium, I implemented this through a bit on Maps for an
> immutable prototype which bootstrapper.cc sets on Object.prototype [4]
> the Chrome WebIDL binding calls out to for all global objects [5], and
> separately for the named properties object [6].
> 
> This was all to fix the bug that Toon was getting at with a
> cross-origin leak, described in [7] in Chrome and in [8] in TC39.
> 
> I don't think I did anything to freeze d8's (or Node's) global object.
> Should it be frozen?
> 
> [1] https://tc39.github.io/ecma262/#sec-properties-of-the-object-prototype-object
> [2] https://heycam.github.io/webidl/#platform-object-setprototypeof
> [3] https://heycam.github.io/webidl/#named-properties-object-setprototypeof
> [4] https://cs.chromium.org/chromium/src/v8/src/bootstrapper.cc?q=bootstra&sq=package:chromium&l=782
> [5] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl?type=cs&q=SetImmutableProto&sq=package:chromium&l=479
> [6] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/templates/interface.cpp.tmpl?type=cs&q=SetImmutableProto&sq=package:chromium&l=982
> [7] https://bugs.chromium.org/p/chromium/issues/detail?id=399951
> [8] https://github.com/tc39/ecma262/issues/272


On Mon, Mar 12, 2018 at 5:45 PM, Toon Verwaest <verwaest@google.com> wrote:
> I don't think it needs to be frozen in d8. Node.js actually relies on being
> able to set up different prototype chains afaik. It might just have made us
> think somewhere that it *was* always frozen and that a JSProxy couldn't occur
> there, where it still can in d8. That would leak to bugs observable from d8
> (but not from Chrome).


On Mon, Mar 12, 2018 at 6:43 PM Daniel Ehrenberg <littledan@chromium.org> wrote:
> OK, sounds like a plan. Anyway, any throwing should happen when __proto__ is
> set to, not on later property accesses on the global object. So, sounds like
> the whole freeze-the-global-object thing doesn't really have to do with the
> weird behavior Georg is observing.

Assigning to Maja, so it has an owner. If we don't fix this, the fuzzers will find it again and again.

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/d8f0237af211e69a86dc5f78affa56c2d9f4e48e

commit d8f0237af211e69a86dc5f78affa56c2d9f4e48e
Author: Maya Lekova <mslekova@chromium.org>
Date: Thu Jul 05 09:52:48 2018

[builtins] Add reference error for global object property access

Fixes V8 correctness failure when there's a proxy in the global object
prototype chain and unsuccessful attempt is made to access a property.

Bug:  chromium:849024 
Change-Id: I829e1a6c038982b7c7a77f8bdefb61facb4614f0
Reviewed-on: https://chromium-review.googlesource.com/1124446
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54237}
[modify] https://crrev.com/d8f0237af211e69a86dc5f78affa56c2d9f4e48e/src/builtins/builtins-definitions.h
[modify] https://crrev.com/d8f0237af211e69a86dc5f78affa56c2d9f4e48e/src/builtins/builtins-proxy-gen.cc
[modify] https://crrev.com/d8f0237af211e69a86dc5f78affa56c2d9f4e48e/src/ic/accessor-assembler.cc
[modify] https://crrev.com/d8f0237af211e69a86dc5f78affa56c2d9f4e48e/src/ic/accessor-assembler.h
[modify] https://crrev.com/d8f0237af211e69a86dc5f78affa56c2d9f4e48e/src/objects.cc
[modify] https://crrev.com/d8f0237af211e69a86dc5f78affa56c2d9f4e48e/src/objects.h
[modify] https://crrev.com/d8f0237af211e69a86dc5f78affa56c2d9f4e48e/src/runtime/runtime-proxy.cc
[modify] https://crrev.com/d8f0237af211e69a86dc5f78affa56c2d9f4e48e/src/runtime/runtime.h
[add] https://crrev.com/d8f0237af211e69a86dc5f78affa56c2d9f4e48e/test/mjsunit/regress/regress-crbug-849024.js

ClusterFuzz has detected this issue as fixed in range 54236:54237.

Detailed report: https://clusterfuzz.com/testcase?key=4721154926051328

Fuzzer: foozzie_js_mutation
Job Type: v8_foozzie
Platform Id: linux

Crash Type: V8 correctness failure
Crash Address: 
Crash State:
  configs: x64,ignition:x64,slow_path
  sources: 3d4
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=v8_foozzie&range=52426:52427
Fixed: https://clusterfuzz.com/revisions?job=v8_foozzie&range=54236:54237

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=4721154926051328

See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

ClusterFuzz testcase 4721154926051328 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.