giving up on NaN, with one patch
On Tue, Mar 26, 2013 at 10:15 PM, David Herman <dherman at mozilla.com> wrote:
Fix 2: Patch the semantics of writing non-writable properties
I agree with Sam that writing to a non-writable property is silly. We can fix the semantics so that it doesn't actually modify the value if SameValue holds. The only observable difference should be this NaN issue, which is what we wanted to fix in the first place.
I'm fine with either fix, but note that if we go with Fix 2, there's a finer distinction in the language than SameValue.
Jeff Walden pointed out that the semantics don't actually have this bug -- see the ValidateAndApplyPropertyDescriptor algorithm, which doesn't update in the SameValue case.
On 3/26/2013 7:19 PM, Sam Tobin-Hochstadt wrote:
On Tue, Mar 26, 2013 at 10:15 PM, David Herman <dherman at mozilla.com> wrote:
Fix 2: Patch the semantics of writing non-writable properties
I agree with Sam that writing to a non-writable property is silly. We can fix the semantics so that it doesn't actually modify the value if SameValue holds. The only observable difference should be this NaN issue, which is what we wanted to fix in the first place.
I'm fine with either fix, but note that if we go with Fix 2, there's a finer distinction in the language than SameValue. Jeff Walden pointed out that the semantics don't actually have this bug -- see the ValidateAndApplyPropertyDescriptor algorithm, which doesn't update in the SameValue case.
Indeed, and my earlier test showed that V8, in fact, does not change the value of an immutable property given two different NaNs (silently ignores the attempt to change). As I said, in V8 it IS possible to hide values in NaNs, but this doesn't violate any invariants.
On Mar 26, 2013, at 7:19 PM, Sam Tobin-Hochstadt <samth at ccs.neu.edu> wrote:
On Tue, Mar 26, 2013 at 10:15 PM, David Herman <dherman at mozilla.com> wrote:
Fix 2: Patch the semantics of writing non-writable properties
I agree with Sam that writing to a non-writable property is silly. We can fix the semantics so that it doesn't actually modify the value if SameValue holds. The only observable difference should be this NaN issue, which is what we wanted to fix in the first place.
I'm fine with either fix, but note that if we go with Fix 2, there's a finer distinction in the language than SameValue.
Jeff Walden pointed out that the semantics don't actually have this bug -- see the ValidateAndApplyPropertyDescriptor algorithm, which doesn't update in the SameValue case.
Ah, okay, even better. We still might want to modify the SameValue semantics in ES6 so that it truly is the finest distinction possible. But if it's not observable it doesn't matter.
On Tue, Mar 26, 2013 at 10:58 PM, David Herman <dherman at mozilla.com> wrote:
On Mar 26, 2013, at 7:19 PM, Sam Tobin-Hochstadt <samth at ccs.neu.edu> wrote:
On Tue, Mar 26, 2013 at 10:15 PM, David Herman <dherman at mozilla.com> wrote:
Fix 2: Patch the semantics of writing non-writable properties
I agree with Sam that writing to a non-writable property is silly. We can fix the semantics so that it doesn't actually modify the value if SameValue holds. The only observable difference should be this NaN issue, which is what we wanted to fix in the first place.
I'm fine with either fix, but note that if we go with Fix 2, there's a finer distinction in the language than SameValue.
Jeff Walden pointed out that the semantics don't actually have this bug -- see the ValidateAndApplyPropertyDescriptor algorithm, which doesn't update in the SameValue case.
Ah, okay, even better. We still might want to modify the SameValue semantics in ES6 so that it truly is the finest distinction possible. But if it's not observable it doesn't matter.
I think that would be a mistake. As Brandon showed, modern engines silently treat writes of different NaN values to immutable fields as no-ops, but changing SameValue would make that an error. Similarly, we plan to use SameValue in the definition of Map, and we really don't want to distinguish different NaN values there.
I'm ready to surrender on the NaN issue, so long as we fix the loophole Mark described.
While SpiderMonkey has demonstrated that it's possible to do read-canonicalization efficiently, the lack of write-canonicalization is still detectable. Apparently different Math operations use internal operations that can produce different NaN bit patterns, and canonicalizing those results hurt SunSpider benchmark scores so wasn't an option.
(...pause to allow everyone to curse SunSpider for the umpteenth time...)
Quick demonstration in SpiderMonkey:
If it were 2008, my argument about "breaking JS invariants" would hold more water, but the fact is they're woven into the fabric of the web and multiple high-performance engines (V8, SpiderMonkey) are unwilling to take the performance regression that would be required to restore an invariant that's been broken for many years.
But this still leaves open the security issue. I propose either of two possible fixes:
Fix 1: Patch the definition of SameValue
The SameValue test currently treats all NaNs as equivalent. Instead, we redefine it to essentially do the same observation that my above code does: two NaNs are considered the SameValue iff their bit patterns as observable via typed arrays are identical.
Fix 2: Patch the semantics of writing non-writable properties
I agree with Sam that writing to a non-writable property is silly. We can fix the semantics so that it doesn't actually modify the value if SameValue holds. The only observable difference should be this NaN issue, which is what we wanted to fix in the first place.
I'm fine with either fix, but note that if we go with Fix 2, there's a finer distinction in the language than SameValue.
I'm ready to surrender on the NaN issue, so long as we fix the loophole Mark described. While SpiderMonkey has demonstrated that it's possible to do read-canonicalization efficiently, the lack of write-canonicalization is still detectable. Apparently different Math operations use internal operations that can produce different NaN bit patterns, and canonicalizing those results hurt SunSpider benchmark scores so wasn't an option. (...pause to allow everyone to curse SunSpider for the umpteenth time...) Quick demonstration in SpiderMonkey: function bytes(a) { return [].map.call(new Uint8Array(a.buffer), function(x) { return x }); } bytes(new Float64Array([ NaN ])); // [ 0, 0, 0, 0, 0, 0, 248, 127 ] bytes(new Float64Array([ Math.sqrt(-1) ]); // [ 0, 0, 0, 0, 0, 0, 248, 255 ] If it were 2008, my argument about "breaking JS invariants" would hold more water, but the fact is they're woven into the fabric of the web and multiple high-performance engines (V8, SpiderMonkey) are unwilling to take the performance regression that would be required to restore an invariant that's been broken for many years. But this still leaves open the security issue. I propose either of two possible fixes: ## Fix 1: Patch the definition of SameValue The SameValue test currently treats all NaNs as equivalent. Instead, we redefine it to essentially do the same observation that my above code does: two NaNs are considered the SameValue iff their bit patterns as observable via typed arrays are identical. ## Fix 2: Patch the semantics of writing non-writable properties I agree with Sam that writing to a non-writable property is silly. We can fix the semantics so that it doesn't actually modify the value if SameValue holds. The only observable difference should be this NaN issue, which is what we wanted to fix in the first place. I'm fine with either fix, but note that if we go with Fix 2, there's a finer distinction in the language than SameValue. Dave