47839 2010-10-18 11:24:43 -0700 JSC interpreter regressions after r69940 2010-10-22 03:54:40 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) PC All RESOLVED FIXED P2 Normal --- 1 galpeter webkit-unassigned commit-queue oliver ossy zherczeg oldest_to_newest 295638 0 galpeter 2010-10-18 11:24:43 -0700 After the 69940 revision, the interpreter build was broken. That was fixed, but there are still a "few" regressions, running WebKitTools/Scripts/run-javascriptcore-tests results: 348 regressions found. This is tested with a Qt build, but I assume that there is no Qt specific code inside the parser. So the regressions are on all platforms. 297281 1 zherczeg 2010-10-21 01:13:12 -0700 Bug: JSValue result = JSC::resolveBase(callFrame, ident, callFrame->scopeChain(), isStrictPut); if (!result) { callFrame->r(dst) = result; ASSERT(callFrame->r(dst).jsValue()); } else callFrame->globalData().exception = createErrorForInvalidGlobalAssignment(callFrame, ident.ustring()); "if (!result)" is wrong, because it should test the opposite. Although "if (!!result)" is ok, I refactored the code with "UNLIKELY(!result)". 297283 2 71403 zherczeg 2010-10-21 01:23:25 -0700 Created attachment 71403 patch 297292 3 71403 ossy 2010-10-21 01:50:08 -0700 Comment on attachment 71403 patch View in context: https://bugs.webkit.org/attachment.cgi?id=71403&action=review > JavaScriptCore/interpreter/Interpreter.cpp:292 > - if (!result) { > - callFrame->r(dst) = result; > - ASSERT(callFrame->r(dst).jsValue()); > - } else > + if (UNLIKELY(!result)) { > callFrame->globalData().exception = createErrorForInvalidGlobalAssignment(callFrame, ident.ustring()); > + return; > + } > + callFrame->r(dst) = result; > + ASSERT(callFrame->r(dst).jsValue()); > } I don't think we need to change the order of cases. I prefer only fixing the conditon and add LIKELY hint to the compiler, like this: if (LIKELY(result)) { callFrame->r(dst) = result; ASSERT(callFrame->r(dst).jsValue()); } else callFrame->globalData().exception = createErrorForInvalidGlobalAssignment(callFrame, ident.ustring()); 297332 4 71414 zherczeg 2010-10-21 03:11:44 -0700 Created attachment 71414 Patch for Oszi I don't like it. Anyway it works. 297357 5 ossy 2010-10-21 04:05:07 -0700 (In reply to comment #4) > Created an attachment (id=71414) [details] > Patch for Oszi > > I don't like it. Anyway it works. I do like it. :) 297895 6 71414 commit-queue 2010-10-22 03:54:34 -0700 Comment on attachment 71414 Patch for Oszi Clearing flags on attachment: 71414 Committed r70299: <http://trac.webkit.org/changeset/70299> 297896 7 commit-queue 2010-10-22 03:54:40 -0700 All reviewed patches have been landed. Closing bug. 71403 2010-10-21 01:23:25 -0700 2010-10-22 03:20:24 -0700 patch 0001-interpreter-fix.patch text/plain 2177 zherczeg RnJvbSA0YjJjYTY4NmE2ZDA5YjAxOWIxOGVhZTg2MGNlMmFhMGQ1MjkwMmIzIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBab2x0YW4gSGVyY3plZyA8emhlcmN6ZWdAaW5mLnUtc3plZ2Vk Lmh1PgpEYXRlOiBUaHUsIDIxIE9jdCAyMDEwIDEwOjIxOjE4ICswMjAwClN1YmplY3Q6IFtQQVRD SF0gaW50ZXJwcmV0ZXItZml4CgpTaWduZWQtb2ZmLWJ5OiBab2x0YW4gSGVyY3plZyA8emhlcmN6 ZWdAaW5mLnUtc3plZ2VkLmh1PgotLS0KIEphdmFTY3JpcHRDb3JlL0NoYW5nZUxvZyAgICAgICAg ICAgICAgICAgICB8ICAgMTQgKysrKysrKysrKysrKysKIEphdmFTY3JpcHRDb3JlL2ludGVycHJl dGVyL0ludGVycHJldGVyLmNwcCB8ICAgIDkgKysrKystLS0tCiAyIGZpbGVzIGNoYW5nZWQsIDE5 IGluc2VydGlvbnMoKyksIDQgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nIGIvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IDNhYzU5MTUuLjM5 NmJiM2MgMTAwNjQ0Ci0tLSBhL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZworKysgYi9KYXZhU2Ny aXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNyBAQAorMjAxMC0xMC0yMSAgWm9sdGFuIEhl cmN6ZWcgIDx6aGVyY3plZ0B3ZWJraXQub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIEpTQyBpbnRlcnByZXRlciByZWdyZXNzaW9ucyBhZnRlciBy Njk5NDAKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTQ3 ODM5CisKKyAgICAgICAgV3JvbmcgImlmIjogSXQgc2hvdWxkIHRlc3Qgd2hldGhlciB0aGUgcmVz dWx0IGV4aXN0cywKKyAgICAgICAgYW5kIG5vdCB0aGUgb3Bwb3NpdGUuIEl0IGlzIGFuIGludGVy cHJldGVyIGJ1ZywgaGVuY2UKKyAgICAgICAgdGhlIGJvdHMgZG9lcyBub3QgY2FwdHVyZSBpdC4K KworICAgICAgICAqIGludGVycHJldGVyL0ludGVycHJldGVyLmNwcDoKKyAgICAgICAgKEpTQzo6 SW50ZXJwcmV0ZXI6OnJlc29sdmVCYXNlKToKKwogMjAxMC0xMC0xOSAgS3dhbmcgWXVsIFNlbyAg PHNreXVsQGNvbXBhbnkxMDAubmV0PgogCiAgICAgICAgIFJldmlld2VkIGJ5IERhdmlkIEtpbHpl ci4KZGlmZiAtLWdpdCBhL0phdmFTY3JpcHRDb3JlL2ludGVycHJldGVyL0ludGVycHJldGVyLmNw cCBiL0phdmFTY3JpcHRDb3JlL2ludGVycHJldGVyL0ludGVycHJldGVyLmNwcAppbmRleCAyODc3 NTY1Li4zY2Q1NGE2IDEwMDY0NAotLS0gYS9KYXZhU2NyaXB0Q29yZS9pbnRlcnByZXRlci9JbnRl cnByZXRlci5jcHAKKysrIGIvSmF2YVNjcmlwdENvcmUvaW50ZXJwcmV0ZXIvSW50ZXJwcmV0ZXIu Y3BwCkBAIC0yODMsMTEgKzI4MywxMiBAQCBORVZFUl9JTkxJTkUgdm9pZCBJbnRlcnByZXRlcjo6 cmVzb2x2ZUJhc2UoQ2FsbEZyYW1lKiBjYWxsRnJhbWUsIEluc3RydWN0aW9uKiB2UAogICAgIGJv b2wgaXNTdHJpY3RQdXQgPSB2UENbM10udS5vcGVyYW5kOwogICAgIElkZW50aWZpZXIgaWRlbnQg PSBjYWxsRnJhbWUtPmNvZGVCbG9jaygpLT5pZGVudGlmaWVyKHByb3BlcnR5KTsKICAgICBKU1Zh bHVlIHJlc3VsdCA9IEpTQzo6cmVzb2x2ZUJhc2UoY2FsbEZyYW1lLCBpZGVudCwgY2FsbEZyYW1l LT5zY29wZUNoYWluKCksIGlzU3RyaWN0UHV0KTsKLSAgICBpZiAoIXJlc3VsdCkgewotICAgICAg ICBjYWxsRnJhbWUtPnIoZHN0KSA9IHJlc3VsdDsKLSAgICAgICAgQVNTRVJUKGNhbGxGcmFtZS0+ cihkc3QpLmpzVmFsdWUoKSk7Ci0gICAgfSBlbHNlCisgICAgaWYgKFVOTElLRUxZKCFyZXN1bHQp KSB7CiAgICAgICAgIGNhbGxGcmFtZS0+Z2xvYmFsRGF0YSgpLmV4Y2VwdGlvbiA9IGNyZWF0ZUVy cm9yRm9ySW52YWxpZEdsb2JhbEFzc2lnbm1lbnQoY2FsbEZyYW1lLCBpZGVudC51c3RyaW5nKCkp OworICAgICAgICByZXR1cm47CisgICAgfQorICAgIGNhbGxGcmFtZS0+cihkc3QpID0gcmVzdWx0 OworICAgIEFTU0VSVChjYWxsRnJhbWUtPnIoZHN0KS5qc1ZhbHVlKCkpOwogfQogCiBORVZFUl9J TkxJTkUgYm9vbCBJbnRlcnByZXRlcjo6cmVzb2x2ZUJhc2VBbmRQcm9wZXJ0eShDYWxsRnJhbWUq IGNhbGxGcmFtZSwgSW5zdHJ1Y3Rpb24qIHZQQywgSlNWYWx1ZSYgZXhjZXB0aW9uVmFsdWUpCi0t IAoxLjYuMC40Cgo= 71414 2010-10-21 03:11:44 -0700 2010-10-22 03:54:34 -0700 Patch for Oszi 0001-interpreter-fix.patch text/plain 1821 zherczeg RnJvbSAwMWRkN2Q4MWVlZDkwZmM4YTZlYzNkMDQyZTlmOTgzNzVhNmQ0ZGExIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBab2x0YW4gSGVyY3plZyA8emhlcmN6ZWdAaW5mLnUtc3plZ2Vk Lmh1PgpEYXRlOiBUaHUsIDIxIE9jdCAyMDEwIDEyOjA5OjEzICswMjAwClN1YmplY3Q6IFtQQVRD SF0gaW50ZXJwcmV0ZXItZml4CgpTaWduZWQtb2ZmLWJ5OiBab2x0YW4gSGVyY3plZyA8emhlcmN6 ZWdAaW5mLnUtc3plZ2VkLmh1PgotLS0KIEphdmFTY3JpcHRDb3JlL0NoYW5nZUxvZyAgICAgICAg ICAgICAgICAgICB8ICAgMTQgKysrKysrKysrKysrKysKIEphdmFTY3JpcHRDb3JlL2ludGVycHJl dGVyL0ludGVycHJldGVyLmNwcCB8ICAgIDIgKy0KIDIgZmlsZXMgY2hhbmdlZCwgMTUgaW5zZXJ0 aW9ucygrKSwgMSBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9KYXZhU2NyaXB0Q29yZS9DaGFu Z2VMb2cgYi9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKaW5kZXggM2FjNTkxNS4uMzk2YmIzYyAx MDA2NDQKLS0tIGEvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCisrKyBiL0phdmFTY3JpcHRDb3Jl L0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDEwLTEwLTIxICBab2x0YW4gSGVyY3plZyAg PHpoZXJjemVnQHdlYmtpdC5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BT ISkuCisKKyAgICAgICAgSlNDIGludGVycHJldGVyIHJlZ3Jlc3Npb25zIGFmdGVyIHI2OTk0MAor ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NDc4MzkKKwor ICAgICAgICBXcm9uZyAiaWYiOiBJdCBzaG91bGQgdGVzdCB3aGV0aGVyIHRoZSByZXN1bHQgZXhp c3RzLAorICAgICAgICBhbmQgbm90IHRoZSBvcHBvc2l0ZS4gSXQgaXMgYW4gaW50ZXJwcmV0ZXIg YnVnLCBoZW5jZQorICAgICAgICB0aGUgYm90cyBkb2VzIG5vdCBjYXB0dXJlIGl0LgorCisgICAg ICAgICogaW50ZXJwcmV0ZXIvSW50ZXJwcmV0ZXIuY3BwOgorICAgICAgICAoSlNDOjpJbnRlcnBy ZXRlcjo6cmVzb2x2ZUJhc2UpOgorCiAyMDEwLTEwLTE5ICBLd2FuZyBZdWwgU2VvICA8c2t5dWxA Y29tcGFueTEwMC5uZXQ+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRGF2aWQgS2lsemVyLgpkaWZm IC0tZ2l0IGEvSmF2YVNjcmlwdENvcmUvaW50ZXJwcmV0ZXIvSW50ZXJwcmV0ZXIuY3BwIGIvSmF2 YVNjcmlwdENvcmUvaW50ZXJwcmV0ZXIvSW50ZXJwcmV0ZXIuY3BwCmluZGV4IDI4Nzc1NjUuLjFj YWRlZjggMTAwNjQ0Ci0tLSBhL0phdmFTY3JpcHRDb3JlL2ludGVycHJldGVyL0ludGVycHJldGVy LmNwcAorKysgYi9KYXZhU2NyaXB0Q29yZS9pbnRlcnByZXRlci9JbnRlcnByZXRlci5jcHAKQEAg LTI4Myw3ICsyODMsNyBAQCBORVZFUl9JTkxJTkUgdm9pZCBJbnRlcnByZXRlcjo6cmVzb2x2ZUJh c2UoQ2FsbEZyYW1lKiBjYWxsRnJhbWUsIEluc3RydWN0aW9uKiB2UAogICAgIGJvb2wgaXNTdHJp Y3RQdXQgPSB2UENbM10udS5vcGVyYW5kOwogICAgIElkZW50aWZpZXIgaWRlbnQgPSBjYWxsRnJh bWUtPmNvZGVCbG9jaygpLT5pZGVudGlmaWVyKHByb3BlcnR5KTsKICAgICBKU1ZhbHVlIHJlc3Vs dCA9IEpTQzo6cmVzb2x2ZUJhc2UoY2FsbEZyYW1lLCBpZGVudCwgY2FsbEZyYW1lLT5zY29wZUNo YWluKCksIGlzU3RyaWN0UHV0KTsKLSAgICBpZiAoIXJlc3VsdCkgeworICAgIGlmIChyZXN1bHQp IHsKICAgICAgICAgY2FsbEZyYW1lLT5yKGRzdCkgPSByZXN1bHQ7CiAgICAgICAgIEFTU0VSVChj YWxsRnJhbWUtPnIoZHN0KS5qc1ZhbHVlKCkpOwogICAgIH0gZWxzZQotLSAKMS42LjAuNAoK