From 51e495a4e25a35a5d4f37a2c53635830f820f238 Mon Sep 17 00:00:00 2001
From: Sebastian Markbage <sema@fb.com>
Date: Mon, 16 Jul 2018 12:25:26 -0700
Subject: [PATCH] Impure abstract getters on prototype chain (#2257)

Summary:
This fixes an issue where get on abstract top val didn't consider Receiver.

This also illustrates two common patterns of getters that are not pure. One is reading a mutable property known by Prepack and one is lazily initializing a shared mutable property.

I believe we'll want to continue supporting these patterns since they almost always work anyway. Note that supporting these don't add much negative consequence. We can still eliminate unused getters. Most of the time these patterns doesn't happen where the prototype is unknown or havoced.

Usually the getter is abstract because the Receiver itself is unknown or havoced (not its prototype). So havocing even more doesn't have any further negative downside.
Pull Request resolved: https://github.com/facebook/prepack/pull/2257

Differential Revision: D8862827

Pulled By: sebmarkbage

fbshipit-source-id: 7e6e98f8b85b6fceaa5131136cc6163d94f5d289
---
 src/values/AbstractObjectValue.js             |  4 +--
 .../GetterOnAbstractPrototype.js              | 33 +++++++++++++++++
 .../GetterOnAbstractPrototype2.js             | 36 +++++++++++++++++++
 3 files changed, 71 insertions(+), 2 deletions(-)
 create mode 100644 test/serializer/pure-functions/GetterOnAbstractPrototype.js
 create mode 100644 test/serializer/pure-functions/GetterOnAbstractPrototype2.js

diff --git a/src/values/AbstractObjectValue.js b/src/values/AbstractObjectValue.js
index 72752e777..5b42da336 100644
--- a/src/values/AbstractObjectValue.js
+++ b/src/values/AbstractObjectValue.js
@@ -498,7 +498,7 @@ export default class AbstractObjectValue extends AbstractValue {
 
     if (this.values.isTop()) {
       let generateAbstractGet = () => {
-        let ob = this;
+        let ob = Receiver;
         if (this.kind === "explicit conversion to object") ob = this.args[0];
         let type = Value;
         if (P === "length" && Value.isTypeCompatibleWith(this.getType(), ArrayValue)) type = NumberValue;
@@ -520,7 +520,7 @@ export default class AbstractObjectValue extends AbstractValue {
         return generateAbstractGet();
       } else if (this.$Realm.isInPureScope()) {
         // This object might have leaked to a getter.
-        Havoc.value(this.$Realm, this);
+        Havoc.value(this.$Realm, Receiver);
         // The getter might throw anything.
         return this.$Realm.evaluateWithPossibleThrowCompletion(
           generateAbstractGet,
diff --git a/test/serializer/pure-functions/GetterOnAbstractPrototype.js b/test/serializer/pure-functions/GetterOnAbstractPrototype.js
new file mode 100644
index 000000000..d0ea88e27
--- /dev/null
+++ b/test/serializer/pure-functions/GetterOnAbstractPrototype.js
@@ -0,0 +1,33 @@
+var FooPrototype = global.__abstract
+  ? __abstract("object", "({ get legacyCache() { return this._cache || (this._cache = {}); } })")
+  : {
+      get legacyCache() {
+        return this._cache || (this._cache = {});
+      },
+    };
+function Foo() {}
+Foo.prototype = FooPrototype;
+
+function fn() {
+  var Bar = new Foo();
+  Object.defineProperty(Bar, "_cache", {
+    writable: true,
+    value: undefined,
+  });
+  Object.defineProperty(Bar, "cache", {
+    get() {
+      return this._cache || (this._cache = {});
+    },
+  });
+  return [Bar.legacyCache, Bar.cache];
+}
+
+global.fn = fn;
+if (global.__optimize) {
+  __optimize(fn);
+}
+
+inspect = function() {
+  let [cache1, cache2] = fn();
+  return cache1 === cache2;
+};
diff --git a/test/serializer/pure-functions/GetterOnAbstractPrototype2.js b/test/serializer/pure-functions/GetterOnAbstractPrototype2.js
new file mode 100644
index 000000000..a3e6d2731
--- /dev/null
+++ b/test/serializer/pure-functions/GetterOnAbstractPrototype2.js
@@ -0,0 +1,36 @@
+var FooPrototype = global.__abstract
+  ? __abstract("object", "({ get name() { return this._name; } })")
+  : {
+      get name() {
+        return this._name;
+      },
+    };
+function Foo(name) {
+  // Field initializer
+  Object.defineProperty(this, "_name", {
+    writable: true,
+    value: name,
+  });
+}
+Foo.prototype = FooPrototype;
+
+function fn() {
+  var Bar = new Foo("Sebastian");
+  Object.defineProperty(Bar, "setName", {
+    value: function(name) {
+      return (this._name = name);
+    },
+  });
+  var name = Bar.name;
+  Bar.setName("Nikolai");
+  return name;
+}
+
+global.fn = fn;
+if (global.__optimize) {
+  __optimize(fn);
+}
+
+inspect = function() {
+  return fn();
+};