Skip to content

Commit

Permalink
Merge r230740 - A put is not an ExistingProperty put when we transiti…
Browse files Browse the repository at this point in the history
…on a structure because of an attributes change

https://bugs.webkit.org/show_bug.cgi?id=184706
<rdar://problem/38871451>

Reviewed by Saam Barati.

JSTests:

* stress/put-by-id-direct-strict-transition.js: Added.
(const.foo):
(j.const.obj.set hello):
* stress/put-by-id-direct-transition.js: Added.
(const.foo):
(j.const.obj.set hello):
* stress/put-getter-setter-by-id-strict-transition.js: Added.
(const.foo):
(j.const.obj.set hello):
* stress/put-getter-setter-by-id-transition.js: Added.
(const.foo):
(j.const.obj.set hello):

Source/JavaScriptCore:

When putting a property on a structure and the slot is a different
type, the slot can't be said to have already been existing.

* runtime/JSObjectInlines.h:
(JSC::JSObject::putDirectInternal):
  • Loading branch information
jfbastien authored and carlosgcampos committed Jul 31, 2018
1 parent 44e0d37 commit a74610a
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 4 deletions.
21 changes: 21 additions & 0 deletions JSTests/ChangeLog
@@ -1,3 +1,24 @@
2018-04-17 JF Bastien <jfbastien@apple.com>

A put is not an ExistingProperty put when we transition a structure because of an attributes change
https://bugs.webkit.org/show_bug.cgi?id=184706
<rdar://problem/38871451>

Reviewed by Saam Barati.

* stress/put-by-id-direct-strict-transition.js: Added.
(const.foo):
(j.const.obj.set hello):
* stress/put-by-id-direct-transition.js: Added.
(const.foo):
(j.const.obj.set hello):
* stress/put-getter-setter-by-id-strict-transition.js: Added.
(const.foo):
(j.const.obj.set hello):
* stress/put-getter-setter-by-id-transition.js: Added.
(const.foo):
(j.const.obj.set hello):

2018-04-14 Filip Pizlo <fpizlo@apple.com>

Function.prototype.caller shouldn't return generator bodies
Expand Down
13 changes: 13 additions & 0 deletions JSTests/stress/put-by-id-direct-strict-transition.js
@@ -0,0 +1,13 @@
"use strict"

let theglobal = 0;
for (theglobal = 0; theglobal < 100000; ++theglobal)
;
const foo = (ignored, arg1) => { theglobal = arg1; };
for (let j = 0; j < 10000; ++j) {
const obj = {
set hello(ignored) {},
[theglobal]: 0
};
foo(obj, 'hello');
}
11 changes: 11 additions & 0 deletions JSTests/stress/put-by-id-direct-transition.js
@@ -0,0 +1,11 @@
let theglobal = 0;
for (theglobal = 0; theglobal < 100000; ++theglobal)
;
const foo = (ignored, arg1) => { theglobal = arg1; };
for (let j = 0; j < 10000; ++j) {
const obj = {
set hello(ignored) {},
[theglobal]: 0
};
foo(obj, 'hello');
}
13 changes: 13 additions & 0 deletions JSTests/stress/put-getter-setter-by-id-strict-transition.js
@@ -0,0 +1,13 @@
"use strict"

let theglobal = 0;
for (theglobal = 0; theglobal < 100000; ++theglobal)
;
const foo = (ignored, arg1) => { theglobal = arg1; };
for (let j = 0; j < 10000; ++j) {
const obj = {
[theglobal]: 0,
set hello(ignored) {}
};
foo(obj, 'hello');
}
11 changes: 11 additions & 0 deletions JSTests/stress/put-getter-setter-by-id-transition.js
@@ -0,0 +1,11 @@
let theglobal = 0;
for (theglobal = 0; theglobal < 100000; ++theglobal)
;
const foo = (ignored, arg1) => { theglobal = arg1; };
for (let j = 0; j < 10000; ++j) {
const obj = {
[theglobal]: 0,
set hello(ignored) {}
};
foo(obj, 'hello');
}
14 changes: 14 additions & 0 deletions Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,17 @@
2018-04-17 JF Bastien <jfbastien@apple.com>

A put is not an ExistingProperty put when we transition a structure because of an attributes change
https://bugs.webkit.org/show_bug.cgi?id=184706
<rdar://problem/38871451>

Reviewed by Saam Barati.

When putting a property on a structure and the slot is a different
type, the slot can't be said to have already been existing.

* runtime/JSObjectInlines.h:
(JSC::JSObject::putDirectInternal):

2018-04-30 Keith Miller <keith_miller@apple.com>

Remove unneeded exception check from String.fromCharCode
Expand Down
10 changes: 6 additions & 4 deletions Source/JavaScriptCore/runtime/JSObjectInlines.h
Expand Up @@ -287,12 +287,13 @@ ALWAYS_INLINE bool JSObject::putDirectInternal(VM& vm, PropertyName propertyName

putDirect(vm, offset, value);
structure->didReplaceProperty(offset);
slot.setExistingProperty(this, offset);

if ((attributes & PropertyAttribute::Accessor) != (currentAttributes & PropertyAttribute::Accessor) || (attributes & PropertyAttribute::CustomAccessor) != (currentAttributes & PropertyAttribute::CustomAccessor)) {
ASSERT(!(attributes & PropertyAttribute::ReadOnly));
setStructure(vm, Structure::attributeChangeTransition(vm, structure, propertyName, attributes));
}
} else
slot.setExistingProperty(this, offset);

return true;
}

Expand Down Expand Up @@ -344,13 +345,14 @@ ALWAYS_INLINE bool JSObject::putDirectInternal(VM& vm, PropertyName propertyName
vm, propertyName, value, slot.context() == PutPropertySlot::PutById);
}

slot.setExistingProperty(this, offset);
putDirect(vm, offset, value);

if ((attributes & PropertyAttribute::Accessor) != (currentAttributes & PropertyAttribute::Accessor) || (attributes & PropertyAttribute::CustomAccessor) != (currentAttributes & PropertyAttribute::CustomAccessor)) {
ASSERT(!(attributes & PropertyAttribute::ReadOnly));
setStructure(vm, Structure::attributeChangeTransition(vm, structure, propertyName, attributes));
}
} else
slot.setExistingProperty(this, offset);

return true;
}

Expand Down

0 comments on commit a74610a

Please sign in to comment.