Skip to content
This repository has been archived by the owner on Mar 25, 2018. It is now read-only.

Commit

Permalink
Make private symbols non-enumerable
Browse files Browse the repository at this point in the history
Methods in the runtime that enumerate over properties should never deal with private symbols. Most commonly such methods only loop over enumerable properties. This fix avoids accidentally handling private symbols in methods that only deal with enumerable properties. Methods that need to look at non-enumerable properties as well still have to manually filter private symbols (e.g., the KeyAccumulator).

BUG=chromium:664411

Review-Url: https://codereview.chromium.org/2499593002
Cr-Commit-Position: refs/heads/master@{#40932}
  • Loading branch information
verwaest authored and Commit bot committed Nov 11, 2016
1 parent f18d56d commit 135b9f9
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 2 deletions.
8 changes: 8 additions & 0 deletions src/lookup.cc
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,11 @@ void LookupIterator::PrepareTransitionToDataProperty(
PropertyAttributes attributes, Object::StoreFromKeyed store_mode) {
DCHECK(receiver.is_identical_to(GetStoreTarget()));
if (state_ == TRANSITION) return;

if (!IsElement() && name()->IsPrivate()) {
attributes = static_cast<PropertyAttributes>(attributes | DONT_ENUM);
}

DCHECK(state_ != LookupIterator::ACCESSOR ||
(GetAccessors()->IsAccessorInfo() &&
AccessorInfo::cast(*GetAccessors())->is_special_data_property()));
Expand Down Expand Up @@ -442,6 +447,9 @@ void LookupIterator::TransitionToAccessorProperty(
// handled via a trap. Adding properties to primitive values is not
// observable.
Handle<JSObject> receiver = GetStoreTarget();
if (!IsElement() && name()->IsPrivate()) {
attributes = static_cast<PropertyAttributes>(attributes | DONT_ENUM);
}

if (!IsElement() && !receiver->map()->is_dictionary_map()) {
Handle<Map> old_map(receiver->map(), isolate_);
Expand Down
3 changes: 3 additions & 0 deletions src/property.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class Descriptor BASE_EMBEDDED {

void Init(Handle<Name> key, Handle<Object> value, PropertyDetails details) {
DCHECK(key->IsUniqueName());
DCHECK_IMPLIES(key->IsPrivate(), !details.IsEnumerable());
key_ = key;
value_ = value;
details_ = details;
Expand All @@ -44,6 +45,7 @@ class Descriptor BASE_EMBEDDED {
Descriptor(Handle<Name> key, Handle<Object> value, PropertyDetails details)
: key_(key), value_(value), details_(details) {
DCHECK(key->IsUniqueName());
DCHECK_IMPLIES(key->IsPrivate(), !details_.IsEnumerable());
}

Descriptor(Handle<Name> key, Handle<Object> value,
Expand All @@ -53,6 +55,7 @@ class Descriptor BASE_EMBEDDED {
value_(value),
details_(attributes, type, representation, field_index) {
DCHECK(key->IsUniqueName());
DCHECK_IMPLIES(key->IsPrivate(), !details_.IsEnumerable());
}

friend class DescriptorArray;
Expand Down
4 changes: 2 additions & 2 deletions test/mjsunit/harmony/private.js
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,8 @@ function TestKeyDescriptor(obj) {
assertEquals(i|0, desc.value)
assertTrue(desc.configurable)
assertEquals(i % 2 == 0, desc.writable)
assertEquals(i % 2 == 0, desc.enumerable)
assertEquals(i % 2 == 0,
assertEquals(false, desc.enumerable)
assertEquals(false,
Object.prototype.propertyIsEnumerable.call(obj, symbols[i]))
}
}
Expand Down

0 comments on commit 135b9f9

Please sign in to comment.