Bug 1197097 - Don't use a context-wide cycle-detection mechanism for detecting cycles during JSON.stringify. This prevents nested (yet separate) JSON.stringify, and it causes that algorithm to be affected by specification-unrelated operations like toSource. r=jonco

This commit is contained in:
Jeff Walden 2015-08-21 03:59:28 -07:00
parent 7bba4b4e12
commit 543015292f
3 changed files with 235 additions and 14 deletions

View File

@ -153,6 +153,164 @@ class HandleBase<TraceableHashMap<A,B,C,D,E,F>>
: public TraceableHashMapOperations<JS::Handle<TraceableHashMap<A,B,C,D,E,F>>, A,B,C,D,E,F>
{};
// A TraceableHashSet is a HashSet with an additional trace method that knows
// how to visit all set element. HashSets that contain GC pointers that must
// be traced to be kept alive will generally want to use this TraceableHashSet
// specializeation in lieu of HashSet.
//
// Most types of GC pointers can be traced with no extra infrastructure. For
// structs and non-gc-pointer members, ensure that there is a specialization of
// DefaultTracer<T> with an appropriate trace method available to handle the
// custom type.
//
// Note that although this HashSet's trace will deal correctly with moved
// elements, it does not itself know when to barrier or trace elements. To
// function properly it must either be used with Rooted or barriered and traced
// manually.
template <typename T,
typename HashPolicy = DefaultHasher<T>,
typename AllocPolicy = TempAllocPolicy,
typename ElemTraceFunc = DefaultTracer<T>>
class TraceableHashSet : public HashSet<T, HashPolicy, AllocPolicy>,
public JS::Traceable
{
using Base = HashSet<T, HashPolicy, AllocPolicy>;
public:
explicit TraceableHashSet(AllocPolicy a = AllocPolicy()) : Base(a) {}
static void trace(TraceableHashSet* set, JSTracer* trc) { set->trace(trc); }
void trace(JSTracer* trc) {
if (!this->initialized())
return;
for (typename Base::Enum e(*this); !e.empty(); e.popFront()) {
T elem = e.front();
ElemTraceFunc::trace(trc, &elem, "hashset element");
if (elem != e.front())
e.rekeyFront(elem);
}
}
// TraceableHashSet is movable
TraceableHashSet(TraceableHashSet&& rhs) : Base(mozilla::Forward<TraceableHashSet>(rhs)) {}
void operator=(TraceableHashSet&& rhs) {
MOZ_ASSERT(this != &rhs, "self-move assignment is prohibited");
Base::operator=(mozilla::Forward<TraceableHashSet>(rhs));
}
private:
// TraceableHashSet is not copyable or assignable
TraceableHashSet(const TraceableHashSet& hs) = delete;
TraceableHashSet& operator=(const TraceableHashSet& hs) = delete;
};
template <typename Outer, typename... Args>
class TraceableHashSetOperations
{
using Set = TraceableHashSet<Args...>;
using Lookup = typename Set::Lookup;
using Ptr = typename Set::Ptr;
using AddPtr = typename Set::AddPtr;
using Range = typename Set::Range;
using Enum = typename Set::Enum;
const Set& set() const { return static_cast<const Outer*>(this)->extract(); }
public:
bool initialized() const { return set().initialized(); }
Ptr lookup(const Lookup& l) const { return set().lookup(l); }
AddPtr lookupForAdd(const Lookup& l) const { return set().lookupForAdd(l); }
Range all() const { return set().all(); }
bool empty() const { return set().empty(); }
uint32_t count() const { return set().count(); }
size_t capacity() const { return set().capacity(); }
uint32_t generation() const { return set().generation(); }
bool has(const Lookup& l) const { return set().lookup(l).found(); }
};
template <typename Outer, typename... Args>
class MutableTraceableHashSetOperations
: public TraceableHashSetOperations<Outer, Args...>
{
using Set = TraceableHashSet<Args...>;
using Lookup = typename Set::Lookup;
using Ptr = typename Set::Ptr;
using AddPtr = typename Set::AddPtr;
using Range = typename Set::Range;
using Enum = typename Set::Enum;
Set& set() { return static_cast<Outer*>(this)->extract(); }
public:
bool init(uint32_t len = 16) { return set().init(len); }
void clear() { set().clear(); }
void finish() { set().finish(); }
void remove(const Lookup& l) { set().remove(l); }
template<typename TInput>
bool add(AddPtr& p, TInput&& t) {
return set().add(p, mozilla::Forward<TInput>(t));
}
template<typename TInput>
bool relookupOrAdd(AddPtr& p, const Lookup& l, TInput&& t) {
return set().relookupOrAdd(p, l, mozilla::Forward<TInput>(t));
}
template<typename TInput>
bool put(TInput&& t) {
return set().put(mozilla::Forward<TInput>(t));
}
template<typename TInput>
bool putNew(TInput&& t) {
return set().putNew(mozilla::Forward<TInput>(t));
}
template<typename TInput>
bool putNew(const Lookup& l, TInput&& t) {
return set().putNew(l, mozilla::Forward<TInput>(t));
}
};
template <typename T, typename HP, typename AP, typename TF>
class RootedBase<TraceableHashSet<T, HP, AP, TF>>
: public MutableTraceableHashSetOperations<JS::Rooted<TraceableHashSet<T, HP, AP, TF>>, T, HP, AP, TF>
{
using Set = TraceableHashSet<T, HP, AP, TF>;
friend class TraceableHashSetOperations<JS::Rooted<Set>, T, HP, AP, TF>;
const Set& extract() const { return *static_cast<const JS::Rooted<Set>*>(this)->address(); }
friend class MutableTraceableHashSetOperations<JS::Rooted<Set>, T, HP, AP, TF>;
Set& extract() { return *static_cast<JS::Rooted<Set>*>(this)->address(); }
};
template <typename T, typename HP, typename AP, typename TF>
class MutableHandleBase<TraceableHashSet<T, HP, AP, TF>>
: public MutableTraceableHashSetOperations<JS::MutableHandle<TraceableHashSet<T, HP, AP, TF>>,
T, HP, AP, TF>
{
using Set = TraceableHashSet<T, HP, AP, TF>;
friend class TraceableHashSetOperations<JS::MutableHandle<Set>, T, HP, AP, TF>;
const Set& extract() const {
return *static_cast<const JS::MutableHandle<Set>*>(this)->address();
}
friend class MutableTraceableHashSetOperations<JS::MutableHandle<Set>, T, HP, AP, TF>;
Set& extract() { return *static_cast<JS::MutableHandle<Set>*>(this)->address(); }
};
template <typename T, typename HP, typename AP, typename TF>
class HandleBase<TraceableHashSet<T, HP, AP, TF>>
: public TraceableHashSetOperations<JS::Handle<TraceableHashSet<T, HP, AP, TF>>, T, HP, AP, TF>
{
using Set = TraceableHashSet<T, HP, AP, TF>;
friend class TraceableHashSetOperations<JS::Handle<Set>, T, HP, AP, TF>;
const Set& extract() const { return *static_cast<const JS::Handle<Set>*>(this)->address(); }
};
} /* namespace js */
#endif /* gc_HashTable_h */

View File

@ -135,13 +135,19 @@ class StringifyContext
: sb(sb),
gap(gap),
replacer(cx, replacer),
stack(cx, TraceableHashSet<JSObject*>(cx)),
propertyList(propertyList),
depth(0)
{}
bool init() {
return stack.init(8);
}
StringBuffer& sb;
const StringBuffer& gap;
RootedObject replacer;
Rooted<TraceableHashSet<JSObject*>> stack;
const AutoIdVector& propertyList;
uint32_t depth;
};
@ -290,6 +296,32 @@ IsFilteredValue(const Value& v)
return v.isUndefined() || v.isSymbol() || IsCallable(v);
}
class CycleDetector
{
public:
CycleDetector(StringifyContext* scx, HandleObject obj)
: stack(&scx->stack), obj_(obj) {
}
bool foundCycle(JSContext* cx) {
auto addPtr = stack.lookupForAdd(obj_);
if (addPtr) {
JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_JSON_CYCLIC_VALUE,
js_object_str);
return false;
}
return stack.add(addPtr, obj_);
}
~CycleDetector() {
stack.remove(obj_);
}
private:
MutableHandle<TraceableHashSet<JSObject*>> stack;
HandleObject obj_;
};
/* ES5 15.12.3 JO. */
static bool
JO(JSContext* cx, HandleObject obj, StringifyContext* scx)
@ -305,14 +337,9 @@ JO(JSContext* cx, HandleObject obj, StringifyContext* scx)
*/
/* Steps 1-2, 11. */
AutoCycleDetector detect(cx, obj);
if (!detect.init())
CycleDetector detect(scx, obj);
if (!detect.foundCycle(cx))
return false;
if (detect.foundCycle()) {
JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_JSON_CYCLIC_VALUE,
js_object_str);
return false;
}
if (!scx->sb.append('{'))
return false;
@ -396,14 +423,9 @@ JA(JSContext* cx, HandleObject obj, StringifyContext* scx)
*/
/* Steps 1-2, 11. */
AutoCycleDetector detect(cx, obj);
if (!detect.init())
CycleDetector detect(scx, obj);
if (!detect.foundCycle(cx))
return false;
if (detect.foundCycle()) {
JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_JSON_CYCLIC_VALUE,
js_object_str);
return false;
}
if (!scx->sb.append('['))
return false;
@ -670,6 +692,8 @@ js::Stringify(JSContext* cx, MutableHandleValue vp, JSObject* replacer_, Value s
/* Step 11. */
StringifyContext scx(cx, sb, gap, replacer, propertyList);
if (!scx.init())
return false;
if (!PreprocessValue(cx, wrapper, HandleId(emptyId), vp, &scx))
return false;
if (IsFilteredValue(vp))

View File

@ -0,0 +1,39 @@
// Any copyright is dedicated to the Public Domain.
// http://creativecommons.org/licenses/publicdomain/
//-----------------------------------------------------------------------------
var BUGNUMBER = 1197097;
var summary = "JSON.stringify shouldn't use context-wide cycle detection";
print(BUGNUMBER + ": " + summary);
/**************
* BEGIN TEST *
**************/
var arr;
// Nested yet separate JSON.stringify is okay.
arr = [{}];
assertEq(JSON.stringify(arr, function(k, v) {
assertEq(JSON.stringify(arr), "[{}]");
return v;
}), "[{}]");
// SpiderMonkey censors cycles in array-joining. This mechanism must not
// interfere with the cycle detection in JSON.stringify.
arr = [{
toString: function() {
var s = JSON.stringify(arr);
assertEq(s, "[{}]");
return s;
}
}];
assertEq(arr.join(), "[{}]");
/******************************************************************************/
if (typeof reportCompare === "function")
reportCompare(true, true);
print("Tests complete");