Fix cyclic JSON.stringify / primitive conversion stack overflows

[vgel]

This Pull Request fixes/closes #545

It changes the following:

• Refactor gcobject::RecursionLimiter so it can be used outside of its module for liveness tracking
• Use that struct in Value::to_json to throw a TypeError, per the spec
• Also use it in Context::ordinary_to_primitive, where we return a default value
• Adds tests for above

The biggest change is probably ordinary_to_primitive. Since the spec doesn't mention what to do, I went and looked at what v8 and SpiderMonkey do. They return a default value based on the type hint: either 0. or "". (See example here: https://repl.it/repls/IvoryCircularCertification#index.js) So, I did the same thing. We could alternatively throw a TypeError, but that would both diverge from the spec (which doesn't mention anything) and existing implementations.

[codecov]

Codecov Report

Merging #777 into master will increase coverage by 0.33%.
The diff coverage is 91.80%.

@@            Coverage Diff             @@
##           master     #777      +/-   ##
==========================================
+ Coverage   59.21%   59.54%   +0.33%     
==========================================
  Files         155      155              
  Lines        9908     9943      +35     
==========================================
+ Hits         5867     5921      +54     
+ Misses       4041     4022      -19     

Impacted Files	Coverage Δ
boa/src/context.rs	70.67% <ø> (+0.53%)	⬆️
boa/src/object/mod.rs	45.61% <ø> (ø)
boa/src/object/gcobject.rs	78.57% <91.37%> (+5.46%)	⬆️
boa/src/value/mod.rs	71.57% <100.00%> (-0.72%)	⬇️
boa/src/syntax/ast/node/operator/bin_op/mod.rs	70.37% <0.00%> (+0.92%)	⬆️
boa/src/value/operations.rs	46.73% <0.00%> (+2.06%)	⬆️
boa/src/builtins/console/mod.rs	23.03% <0.00%> (+3.93%)	⬆️
boa/src/value/rcstring.rs	64.28% <0.00%> (+7.14%)	⬆️
... and 1 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6bcfc7a...2cc55b9. Read the comment docs.

[RageKnify]

One thing I think we could also change is the signature of OrdinaryToPrimitive, it is only called in 2 places on the spec, and in both cases it receives values whose types has been asserted to be Object, so we could for example receive GcObject, but I leave that decision to @HalidOdat who knows more about how we represent Objects.

[HalidOdat]

This looks ready to merge, just some small changes :)

[HalidOdat]

One thing I think we could also change is the signature of OrdinaryToPrimitive, it is only called in 2 places on the spec, and in both cases it receives values whose types has been asserted to be Object, so we could for example receive GcObject, but I leave that decision to @HalidOdat who knows more about how we represent Objects.

Yes. I would say its better if we move it to GcObject as pub(crate) and accepting a &self (this will eliminate the debug object check), the Context it is not a good place for it.

[RageKnify]

Looks like you accidentaly broke a lot of tests @vgel. Not sure what you did wrong, but you broke a lot of stuf.

[vgel]

I'm not sure -- the tests seemed to run fine until I merged master. Will look into it.

[HalidOdat]

Looks good to me!