164453 2016-11-05 10:09:01 -0700 test262: DataView with explicit undefined byteLength should be the same as it not being present 2016-11-11 20:06:21 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 joepeck joepeck joepeck keith_miller oldest_to_newest 1248499 0 joepeck 2016-11-05 10:09:01 -0700 Summary: DataView with explicit undefined byteLength should be the same as it not being present Currently we coerce the undefined to 0 with normal number conversions. Test: var buffer = new ArrayBuffer(8); assert( new DataView(buffer, 0).byteLength === new DataView(buffer, 0, undefined).byteLength ) Spec: > 24.2.2.1 DataView ( buffer [ , byteOffset [ , byteLength ] ] ) > https://tc39.github.io/ecma262/#sec-dataview-buffer-byteoffset-bytelength > > When the DataView is called with at least one argument buffer, the following steps are taken: > > ... > 8. If byteLength is either not present or undefined, then > Let viewByteLength be bufferByteLength - offset. > 9. Else, > Let viewByteLength be ? ToIndex(byteLength). > If offset+viewByteLength > bufferByteLength, throw a RangeError exception. > ... 1248501 1 293998 joepeck 2016-11-05 10:11:51 -0700 Created attachment 293998 [PATCH] Proposed Fix 1248565 2 293998 darin 2016-11-05 16:46:00 -0700 Comment on attachment 293998 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=293998&action=review > Source/JavaScriptCore/runtime/JSGenericTypedArrayViewConstructorInlines.h:248 > if (argCount > 2) { > - length = exec->uncheckedArgument(2).toIndex(exec, ViewClass::TypedArrayStorageType == TypeDataView ? "byteLength" : "length"); > - RETURN_IF_EXCEPTION(scope, encodedJSValue()); > + if (ViewClass::TypedArrayStorageType == TypeDataView) { > + // If the DataView byteLength is present but undefined, treat it as missing. > + JSValue byteLengthValue = exec->uncheckedArgument(2); > + if (!byteLengthValue.isUndefined()) { > + length = byteLengthValue.toIndex(exec, "byteLength"); > + RETURN_IF_EXCEPTION(scope, encodedJSValue()); > + } > + } else { > + length = exec->uncheckedArgument(2).toIndex(exec, "length"); > + RETURN_IF_EXCEPTION(scope, encodedJSValue()); > + } > } The old school way of doing this is to call exec->argument(2) and check for undefined. It would be pretty easy to do it that way. if (ViewClass::TypedArrayStorageType == TypeDataView) { // If the DataView byteLength is present but undefined, treat it as missing. JSValue byteLengthValue = exec->argument(2); if (!byteLength.isUndefined()) { length = byteLengthValue.toIndex(exec, "byteLength"); RETURN_IF_EXCEPTION(scope, encodedJSValue()); } } else { if (argCount > 2) { length = exec->uncheckedArgument(2).toIndex(exec, "length"); RETURN_IF_EXCEPTION(scope, encodedJSValue()); } } Not sure if it’s better, though. The code it generates is possibly slightly less efficient and I’m not sure it’s easier to read. 1250573 3 293998 joepeck 2016-11-11 19:29:04 -0800 Comment on attachment 293998 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=293998&action=review >> Source/JavaScriptCore/runtime/JSGenericTypedArrayViewConstructorInlines.h:248 >> } > > The old school way of doing this is to call exec->argument(2) and check for undefined. It would be pretty easy to do it that way. > > if (ViewClass::TypedArrayStorageType == TypeDataView) { > // If the DataView byteLength is present but undefined, treat it as missing. > JSValue byteLengthValue = exec->argument(2); > if (!byteLength.isUndefined()) { > length = byteLengthValue.toIndex(exec, "byteLength"); > RETURN_IF_EXCEPTION(scope, encodedJSValue()); > } > } else { > if (argCount > 2) { > length = exec->uncheckedArgument(2).toIndex(exec, "length"); > RETURN_IF_EXCEPTION(scope, encodedJSValue()); > } > } > > Not sure if it’s better, though. The code it generates is possibly slightly less efficient and I’m not sure it’s easier to read. Oh I see, moving the argCount check around. I like the readability advantages of having argCount around both of these (and given the pattern of other code in this function). 1250582 4 joepeck 2016-11-11 20:06:21 -0800 <https://trac.webkit.org/changeset/208640> 293998 2016-11-05 10:11:51 -0700 2016-11-05 16:46:00 -0700 [PATCH] Proposed Fix DataView-construct.patch text/plain 6466 joepeck ZGlmZiAtLWdpdCBhL0pTVGVzdHMvQ2hhbmdlTG9nIGIvSlNUZXN0cy9DaGFuZ2VMb2cKaW5kZXgg MTZjNWMxMy4uODAyZDMzZiAxMDA2NDQKLS0tIGEvSlNUZXN0cy9DaGFuZ2VMb2cKKysrIGIvSlNU ZXN0cy9DaGFuZ2VMb2cKQEAgLTEsNSArMSwxNyBAQAogMjAxNi0xMS0wNSAgSm9zZXBoIFBlY29y YXJvICA8cGVjb3Jhcm9AYXBwbGUuY29tPgogCisgICAgICAgIHRlc3QyNjI6IERhdGFWaWV3IHdp dGggZXhwbGljaXQgdW5kZWZpbmVkIGJ5dGVMZW5ndGggc2hvdWxkIGJlIHRoZSBzYW1lIGFzIGl0 IG5vdCBiZWluZyBwcmVzZW50CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD0xNjQ0NTMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICAqIHN0cmVzcy9kYXRhdmlldy1jb25zdHJ1Y3QuanM6IEFkZGVkLgorICAgICAg ICAoYXNzZXJ0KToKKyAgICAgICAgKHNob3VsZFRocm93KToKKyAgICAgICAgKiB0ZXN0MjYyLnlh bWw6CisKKzIwMTYtMTEtMDUgIEpvc2VwaCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4K KwogICAgICAgICB0ZXN0MjYyOiBEYXRhVmlldyBnZXQgbWV0aG9kcyBzaG91bGQgYWxsb3cgZm9y IG1pc3Npbmcgb2Zmc2V0LCBzZXQgbWV0aG9kcyBzaG91bGQgYWxsb3cgZm9yIG1pc3NpbmcgdmFs dWUKICAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE2NDQ1 MQogCmRpZmYgLS1naXQgYS9KU1Rlc3RzL3N0cmVzcy9kYXRhdmlldy1jb25zdHJ1Y3QuanMgYi9K U1Rlc3RzL3N0cmVzcy9kYXRhdmlldy1jb25zdHJ1Y3QuanMKbmV3IGZpbGUgbW9kZSAxMDA2NDQK aW5kZXggMDAwMDAwMC4uNTRmOWYwNQotLS0gL2Rldi9udWxsCisrKyBiL0pTVGVzdHMvc3RyZXNz L2RhdGF2aWV3LWNvbnN0cnVjdC5qcwpAQCAtMCwwICsxLDc2IEBACitmdW5jdGlvbiBhc3NlcnQo Y29uZGl0aW9uKSB7CisgICAgaWYgKCFjb25kaXRpb24pCisgICAgICAgIHRocm93IG5ldyBFcnJv cigiQmFkIGFzc2VydGlvbiIpOworfQorCitmdW5jdGlvbiBzaG91bGRUaHJvdyhmdW5jLCBtZXNz YWdlKSB7CisgICAgdmFyIGVycm9yVGhyb3duID0gZmFsc2U7CisgICAgdmFyIGVycm9yID0gbnVs bDsKKyAgICB0cnkgeworICAgICAgICBmdW5jKCk7CisgICAgfSBjYXRjaCAoZSkgeworICAgICAg ICBlcnJvclRocm93biA9IHRydWU7CisgICAgICAgIGVycm9yID0gZTsKKyAgICB9CisgICAgaWYg KCFlcnJvclRocm93bikKKyAgICAgICAgdGhyb3cgbmV3IEVycm9yKCdub3QgdGhyb3duJyk7Cisg ICAgaWYgKFN0cmluZyhlcnJvcikgIT09IG1lc3NhZ2UpCisgICAgICAgIHRocm93IG5ldyBFcnJv cigiYmFkIGVycm9yOiAiICsgU3RyaW5nKGVycm9yKSk7Cit9CisKK3ZhciBidWZmZXIgPSBuZXcg QXJyYXlCdWZmZXIoMTI4KTsKK3ZhciBkYXRhVmlldyA9IG51bGw7CisKK2RhdGFWaWV3ID0gbmV3 IERhdGFWaWV3KGJ1ZmZlcik7Cithc3NlcnQoZGF0YVZpZXcuYnl0ZU9mZnNldCA9PT0gMCk7Cith c3NlcnQoZGF0YVZpZXcuYnl0ZUxlbmd0aCA9PT0gMTI4KTsKKworZGF0YVZpZXcgPSBuZXcgRGF0 YVZpZXcoYnVmZmVyLCB1bmRlZmluZWQpOworYXNzZXJ0KGRhdGFWaWV3LmJ5dGVPZmZzZXQgPT09 IDApOworYXNzZXJ0KGRhdGFWaWV3LmJ5dGVMZW5ndGggPT09IDEyOCk7CisKK2RhdGFWaWV3ID0g bmV3IERhdGFWaWV3KGJ1ZmZlciwgMTApOworYXNzZXJ0KGRhdGFWaWV3LmJ5dGVPZmZzZXQgPT09 IDEwKTsKK2Fzc2VydChkYXRhVmlldy5ieXRlTGVuZ3RoID09PSAxMTgpOworCitkYXRhVmlldyA9 IG5ldyBEYXRhVmlldyhidWZmZXIsIDEwLCB1bmRlZmluZWQpOworYXNzZXJ0KGRhdGFWaWV3LmJ5 dGVPZmZzZXQgPT09IDEwKTsKK2Fzc2VydChkYXRhVmlldy5ieXRlTGVuZ3RoID09PSAxMTgpOwor CitkYXRhVmlldyA9IG5ldyBEYXRhVmlldyhidWZmZXIsIDEwLCAyMCk7Cithc3NlcnQoZGF0YVZp ZXcuYnl0ZU9mZnNldCA9PT0gMTApOworYXNzZXJ0KGRhdGFWaWV3LmJ5dGVMZW5ndGggPT09IDIw KTsKKworYXNzZXJ0KG5ldyBEYXRhVmlldyhidWZmZXIsIDEwKS5ieXRlTGVuZ3RoID09PSBuZXcg RGF0YVZpZXcoYnVmZmVyLCAxMCwgdW5kZWZpbmVkKS5ieXRlTGVuZ3RoKTsKKworc2hvdWxkVGhy b3coKCkgPT4geworICAgIG5ldyBEYXRhVmlldzsKK30sICJUeXBlRXJyb3I6IERhdGFWaWV3IGNv bnN0cnVjdG9yIHJlcXVpcmVzIGF0IGxlYXN0IG9uZSBhcmd1bWVudC4iKTsKKworc2hvdWxkVGhy b3coKCkgPT4geworICAgIG5ldyBEYXRhVmlldygxKTsKK30sICJUeXBlRXJyb3I6IEV4cGVjdGVk IEFycmF5QnVmZmVyIGZvciB0aGUgZmlyc3QgYXJndW1lbnQuIik7CisKK3Nob3VsZFRocm93KCgp ID0+IHsKKyAgICBuZXcgRGF0YVZpZXcoYnVmZmVyLCAyNTYpOworfSwgIlJhbmdlRXJyb3I6IExl bmd0aCBvdXQgb2YgcmFuZ2Ugb2YgYnVmZmVyIik7CisKK3Nob3VsZFRocm93KCgpID0+IHsKKyAg ICBuZXcgRGF0YVZpZXcoYnVmZmVyLCAtMSk7Cit9LCAiUmFuZ2VFcnJvcjogYnl0ZU9mZnNldCBj YW5ub3QgYmUgbmVnYXRpdmUiKTsKKworc2hvdWxkVGhyb3coKCkgPT4geworICAgIG5ldyBEYXRh VmlldyhidWZmZXIsIEluZmluaXR5KTsKK30sICJSYW5nZUVycm9yOiBieXRlT2Zmc2V0IHRvbyBs YXJnZSIpOworCitzaG91bGRUaHJvdygoKSA9PiB7CisgICAgbmV3IERhdGFWaWV3KGJ1ZmZlciwg MCwgMjU2KTsKK30sICJSYW5nZUVycm9yOiBMZW5ndGggb3V0IG9mIHJhbmdlIG9mIGJ1ZmZlciIp OworCitzaG91bGRUaHJvdygoKSA9PiB7CisgICAgbmV3IERhdGFWaWV3KGJ1ZmZlciwgMCwgLTEp OworfSwgIlJhbmdlRXJyb3I6IGJ5dGVMZW5ndGggY2Fubm90IGJlIG5lZ2F0aXZlIik7CisKK3No b3VsZFRocm93KCgpID0+IHsKKyAgICBuZXcgRGF0YVZpZXcoYnVmZmVyLCAwLCBJbmZpbml0eSk7 Cit9LCAiUmFuZ2VFcnJvcjogYnl0ZUxlbmd0aCB0b28gbGFyZ2UiKTsKZGlmZiAtLWdpdCBhL0pT VGVzdHMvdGVzdDI2Mi55YW1sIGIvSlNUZXN0cy90ZXN0MjYyLnlhbWwKaW5kZXggNjI0ZTkxZC4u ODBmNzA0NyAxMDA2NDQKLS0tIGEvSlNUZXN0cy90ZXN0MjYyLnlhbWwKKysrIGIvSlNUZXN0cy90 ZXN0MjYyLnlhbWwKQEAgLTEzNTQyLDkgKzEzNTQyLDkgQEAKIC0gcGF0aDogdGVzdDI2Mi90ZXN0 L2J1aWx0LWlucy9EYXRhVmlldy9kZWZpbmVkLWJ5dGVsZW5ndGgtYW5kLWJ5dGVvZmZzZXQuanMK ICAgY21kOiBydW5UZXN0MjYyIDpub3JtYWwsICJOb0V4Y2VwdGlvbiIsIFsiLi4vLi4vLi4vaGFy bmVzcy9hc3NlcnQuanMiLCAiLi4vLi4vLi4vaGFybmVzcy9zdGEuanMiXSwgWzpzdHJpY3RdCiAt IHBhdGg6IHRlc3QyNjIvdGVzdC9idWlsdC1pbnMvRGF0YVZpZXcvZGVmaW5lZC1ieXRlb2Zmc2V0 LXVuZGVmaW5lZC1ieXRlbGVuZ3RoLmpzCi0gIGNtZDogcnVuVGVzdDI2MiA6ZmFpbCwgIk5vRXhj ZXB0aW9uIiwgWyIuLi8uLi8uLi9oYXJuZXNzL2Fzc2VydC5qcyIsICIuLi8uLi8uLi9oYXJuZXNz L3N0YS5qcyJdLCBbXQorICBjbWQ6IHJ1blRlc3QyNjIgOm5vcm1hbCwgIk5vRXhjZXB0aW9uIiwg WyIuLi8uLi8uLi9oYXJuZXNzL2Fzc2VydC5qcyIsICIuLi8uLi8uLi9oYXJuZXNzL3N0YS5qcyJd LCBbXQogLSBwYXRoOiB0ZXN0MjYyL3Rlc3QvYnVpbHQtaW5zL0RhdGFWaWV3L2RlZmluZWQtYnl0 ZW9mZnNldC11bmRlZmluZWQtYnl0ZWxlbmd0aC5qcwotICBjbWQ6IHJ1blRlc3QyNjIgOmZhaWws ICJOb0V4Y2VwdGlvbiIsIFsiLi4vLi4vLi4vaGFybmVzcy9hc3NlcnQuanMiLCAiLi4vLi4vLi4v aGFybmVzcy9zdGEuanMiXSwgWzpzdHJpY3RdCisgIGNtZDogcnVuVGVzdDI2MiA6bm9ybWFsLCAi Tm9FeGNlcHRpb24iLCBbIi4uLy4uLy4uL2hhcm5lc3MvYXNzZXJ0LmpzIiwgIi4uLy4uLy4uL2hh cm5lc3Mvc3RhLmpzIl0sIFs6c3RyaWN0XQogLSBwYXRoOiB0ZXN0MjYyL3Rlc3QvYnVpbHQtaW5z L0RhdGFWaWV3L2RlZmluZWQtYnl0ZW9mZnNldC5qcwogICBjbWQ6IHJ1blRlc3QyNjIgOm5vcm1h bCwgIk5vRXhjZXB0aW9uIiwgWyIuLi8uLi8uLi9oYXJuZXNzL2Fzc2VydC5qcyIsICIuLi8uLi8u Li9oYXJuZXNzL3N0YS5qcyJdLCBbXQogLSBwYXRoOiB0ZXN0MjYyL3Rlc3QvYnVpbHQtaW5zL0Rh dGFWaWV3L2RlZmluZWQtYnl0ZW9mZnNldC5qcwpkaWZmIC0tZ2l0IGEvU291cmNlL0phdmFTY3Jp cHRDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKaW5kZXgg M2Y1NWNhNy4uZWQ5Mjc4OSAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5n ZUxvZworKysgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCkBAIC0xLDUgKzEsMTYg QEAKIDIwMTYtMTEtMDUgIEpvc2VwaCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4KIAor ICAgICAgICB0ZXN0MjYyOiBEYXRhVmlldyB3aXRoIGV4cGxpY2l0IHVuZGVmaW5lZCBieXRlTGVu Z3RoIHNob3VsZCBiZSB0aGUgc2FtZSBhcyBpdCBub3QgYmVpbmcgcHJlc2VudAorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTY0NDUzCisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBydW50aW1lL0pTR2VuZXJp Y1R5cGVkQXJyYXlWaWV3Q29uc3RydWN0b3JJbmxpbmVzLmg6CisgICAgICAgIChKU0M6OmNvbnN0 cnVjdEdlbmVyaWNUeXBlZEFycmF5Vmlldyk6CisgICAgICAgIEhhbmRsZSB0aGUgc3BlY2lhbCBj YXNlIG9mIERhdGFWaWV3IGNvbnN0cnVjdGlvbiB3aXRoIGFuIHVuZGVmaW5lZCBieXRlTGVuZ3Ro IHZhbHVlLgorCisyMDE2LTExLTA1ICBKb3NlcGggUGVjb3Jhcm8gIDxwZWNvcmFyb0BhcHBsZS5j b20+CisKICAgICAgICAgdGVzdDI2MjogRGF0YVZpZXcgZ2V0IG1ldGhvZHMgc2hvdWxkIGFsbG93 IGZvciBtaXNzaW5nIG9mZnNldCwgc2V0IG1ldGhvZHMgc2hvdWxkIGFsbG93IGZvciBtaXNzaW5n IHZhbHVlCiAgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0x NjQ0NTEKIApkaWZmIC0tZ2l0IGEvU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNHZW5l cmljVHlwZWRBcnJheVZpZXdDb25zdHJ1Y3RvcklubGluZXMuaCBiL1NvdXJjZS9KYXZhU2NyaXB0 Q29yZS9ydW50aW1lL0pTR2VuZXJpY1R5cGVkQXJyYXlWaWV3Q29uc3RydWN0b3JJbmxpbmVzLmgK aW5kZXggM2JkZGJlYi4uNTJkYjlhNiAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3Jl L3J1bnRpbWUvSlNHZW5lcmljVHlwZWRBcnJheVZpZXdDb25zdHJ1Y3RvcklubGluZXMuaAorKysg Yi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9KU0dlbmVyaWNUeXBlZEFycmF5Vmlld0Nv bnN0cnVjdG9ySW5saW5lcy5oCkBAIC0yMzQsOCArMjM0LDE3IEBAIEVuY29kZWRKU1ZhbHVlIEpT Q19IT1NUX0NBTEwgY29uc3RydWN0R2VuZXJpY1R5cGVkQXJyYXlWaWV3KEV4ZWNTdGF0ZSogZXhl YykKICAgICAgICAgUkVUVVJOX0lGX0VYQ0VQVElPTihzY29wZSwgZW5jb2RlZEpTVmFsdWUoKSk7 CiAKICAgICAgICAgaWYgKGFyZ0NvdW50ID4gMikgewotICAgICAgICAgICAgbGVuZ3RoID0gZXhl Yy0+dW5jaGVja2VkQXJndW1lbnQoMikudG9JbmRleChleGVjLCBWaWV3Q2xhc3M6OlR5cGVkQXJy YXlTdG9yYWdlVHlwZSA9PSBUeXBlRGF0YVZpZXcgPyAiYnl0ZUxlbmd0aCIgOiAibGVuZ3RoIik7 Ci0gICAgICAgICAgICBSRVRVUk5fSUZfRVhDRVBUSU9OKHNjb3BlLCBlbmNvZGVkSlNWYWx1ZSgp KTsKKyAgICAgICAgICAgIGlmIChWaWV3Q2xhc3M6OlR5cGVkQXJyYXlTdG9yYWdlVHlwZSA9PSBU eXBlRGF0YVZpZXcpIHsKKyAgICAgICAgICAgICAgICAvLyBJZiB0aGUgRGF0YVZpZXcgYnl0ZUxl bmd0aCBpcyBwcmVzZW50IGJ1dCB1bmRlZmluZWQsIHRyZWF0IGl0IGFzIG1pc3NpbmcuCisgICAg ICAgICAgICAgICAgSlNWYWx1ZSBieXRlTGVuZ3RoVmFsdWUgPSBleGVjLT51bmNoZWNrZWRBcmd1 bWVudCgyKTsKKyAgICAgICAgICAgICAgICBpZiAoIWJ5dGVMZW5ndGhWYWx1ZS5pc1VuZGVmaW5l ZCgpKSB7CisgICAgICAgICAgICAgICAgICAgIGxlbmd0aCA9IGJ5dGVMZW5ndGhWYWx1ZS50b0lu ZGV4KGV4ZWMsICJieXRlTGVuZ3RoIik7CisgICAgICAgICAgICAgICAgICAgIFJFVFVSTl9JRl9F WENFUFRJT04oc2NvcGUsIGVuY29kZWRKU1ZhbHVlKCkpOworICAgICAgICAgICAgICAgIH0KKyAg ICAgICAgICAgIH0gZWxzZSB7CisgICAgICAgICAgICAgICAgbGVuZ3RoID0gZXhlYy0+dW5jaGVj a2VkQXJndW1lbnQoMikudG9JbmRleChleGVjLCAibGVuZ3RoIik7CisgICAgICAgICAgICAgICAg UkVUVVJOX0lGX0VYQ0VQVElPTihzY29wZSwgZW5jb2RlZEpTVmFsdWUoKSk7IAorICAgICAgICAg ICAgfQogICAgICAgICB9CiAgICAgfQogCg==