206231 2020-01-14 07:39:53 -0800 [ARMv7] Assembler is generating wrong instruction for ldr r2, [r3, #7] 2020-01-22 16:00:28 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED DoNotImportToRadar P2 Normal --- 203563 205581 206602 1 ticaiolima ticaiolima commit-queue ews-watchlist jsc32 keith_miller mark.lam msaboff saam tzagallo webkit-bug-importer oldest_to_newest 1606382 0 ticaiolima 2020-01-14 07:39:53 -0800 ... 1609532 1 388419 ticaiolima 2020-01-22 07:38:41 -0800 Created attachment 388419 Patch 1609537 2 388420 ticaiolima 2020-01-22 07:43:59 -0800 Created attachment 388420 Patch 1609563 3 388420 mark.lam 2020-01-22 08:56:50 -0800 Comment on attachment 388420 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=388420&action=review r=me > Source/JavaScriptCore/assembler/ARMv7Assembler.h:1034 > + // We can use Encoding T1 when imm is a multiple of 4. > + // (see A8.8.63 on ARM Architecture Reference Manual ARMv7-A and ARMv7-R edition) I think you mean A8.8.62 LDR (immediate, Thumb) on page A8-406. See https://static.docs.arm.com/ddi0406/cc/DDI0406C_C_arm_architecture_reference_manual.pdf. Please update the comment to say "We can only use Encoding T1 when imm is a multiple of 4." Please also update comment with the reference to the section of the spec. Alternatively, if you have a reference to a newer version of the spec where the relevant details are in section A8.8.63, please provide a url to that spec so that the reader / reviewer can confirm the correctness of this implementation. Thanks. 1609627 4 388420 ticaiolima 2020-01-22 11:09:19 -0800 Comment on attachment 388420 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=388420&action=review Thank you very much for the review. >> Source/JavaScriptCore/assembler/ARMv7Assembler.h:1034 >> + // (see A8.8.63 on ARM Architecture Reference Manual ARMv7-A and ARMv7-R edition) > > I think you mean A8.8.62 LDR (immediate, Thumb) on page A8-406. See https://static.docs.arm.com/ddi0406/cc/DDI0406C_C_arm_architecture_reference_manual.pdf. > > Please update the comment to say "We can only use Encoding T1 when imm is a multiple of 4." > Please also update comment with the reference to the section of the spec. Alternatively, if you have a reference to a newer version of the spec where the relevant details are in section A8.8.63, please provide a url to that spec so that the reader / reviewer can confirm the correctness of this implementation. Thanks. Yes, I meant the section you pointed out LDR (immediate, Thumb). Since I'm using a 2018 version of the doc, I updated the comment with a link to it and kept the section A8.8.63. 1609629 5 388444 ticaiolima 2020-01-22 11:14:35 -0800 Created attachment 388444 Patch 1609725 6 388444 commit-queue 2020-01-22 13:43:03 -0800 Comment on attachment 388444 Patch Clearing flags on attachment: 388444 Committed r254943: <https://trac.webkit.org/changeset/254943> 1609726 7 commit-queue 2020-01-22 13:43:05 -0800 All reviewed patches have been landed. Closing bug. 1609824 8 webkit-bug-importer 2020-01-22 15:58:40 -0800 <rdar://problem/58814740> 388419 2020-01-22 07:38:41 -0800 2020-01-22 07:43:54 -0800 Patch bug-206231-20200122123839.patch text/plain 1900 ticaiolima U3VidmVyc2lvbiBSZXZpc2lvbjogMjU0OTIwCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBk ZmE0N2M4M2FkMWY4YjE1ZmZjNTdjNzdmNzQ4YTEyYWIyZWFhMDkxLi4wOGI4YTc4ODU4MjkxYjIx M2NmZTBkZmRkZTcwYTFlN2ZiYTliM2M1IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxMyBAQAorMjAyMC0wMS0yMiAgQ2FpbyBMaW1hICA8dGljYWlvbGltYUBnbWFpbC5jb20+ CisKKyAgICAgICAgW0FSTXY3XSBBc3NlbWJsZXIgaXMgZ2VuZXJhdGluZyB3cm9uZyBpbnN0cnVj dGlvbiBmb3IgbGRyIHIyLCBbcjMsICM3XQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9y Zy9zaG93X2J1Zy5jZ2k/aWQ9MjA2MjMxCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChP T1BTISkuCisKKyAgICAgICAgKiBhc3NlbWJsZXIvQVJNdjdBc3NlbWJsZXIuaDoKKyAgICAgICAg KEpTQzo6QVJNdjdBc3NlbWJsZXI6Omxkcik6CisKIDIwMjAtMDEtMjEgIFJvc3MgS2lyc2xpbmcg IDxyb3NzLmtpcnNsaW5nQHNvbnkuY29tPgogCiAgICAgICAgIFtKU0NdIERhdGUgcGFyc2UgbG9n aWMgc2hvdWxkIGJlIGxlc3MgcmVkdW5kYW50CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvYXNzZW1ibGVyL0FSTXY3QXNzZW1ibGVyLmggYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUv YXNzZW1ibGVyL0FSTXY3QXNzZW1ibGVyLmgKaW5kZXggNjE1MWMwYmEyOTMwMzk5ZDM0YTRjNGE5 YWRmZGEzODc0NjQ2Y2M3NS4uZWQ2Y2VjYWIxYzkxZTgzZjIyNmZjOGI1Nzc4MzFmZDU4OGQ2YmE4 ZCAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL2Fzc2VtYmxlci9BUk12N0Fzc2Vt Ymxlci5oCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9hc3NlbWJsZXIvQVJNdjdBc3NlbWJs ZXIuaApAQCAtMTAyOSw5ICsxMDI5LDExIEBAIHB1YmxpYzoKICAgICAgICAgQVNTRVJUKHJuICE9 IEFSTVJlZ2lzdGVyczo6cGMpOyAvLyBMRFIgKGxpdGVyYWwpCiAgICAgICAgIEFTU0VSVChpbW0u aXNVSW50MTIoKSk7CiAKLSAgICAgICAgaWYgKCEoKHJ0IHwgcm4pICYgOCkgJiYgaW1tLmlzVUlu dDcoKSkKKyAgICAgICAgaWYgKCEoKHJ0IHwgcm4pICYgOCkgJiYgaW1tLmlzVUludDcoKSAmJiAh KGltbS5nZXRVSW50NygpICUgNCkpIHsKKyAgICAgICAgICAgIC8vIFdlIGNhbiB1c2UgRW5jb2Rp bmcgVDEgd2hlbiBpbW0gaXMgYSBtdWx0aXBsZSBvZiBUNC4KKyAgICAgICAgICAgIC8vIChzZWUg QTguOC42MyBvbiBBUk0gQXJjaGl0ZWN0dXJlIFJlZmVyZW5jZSBNYW51YWwgQVJNdjctQSBhbmQg QVJNdjctUiBlZGl0aW9uKQogICAgICAgICAgICAgbV9mb3JtYXR0ZXIub25lV29yZE9wNUltbTVS ZWczUmVnMyhPUF9MRFJfaW1tX1QxLCBpbW0uZ2V0VUludDcoKSA+PiAyLCBybiwgcnQpOwotICAg ICAgICBlbHNlIGlmICgocm4gPT0gQVJNUmVnaXN0ZXJzOjpzcCkgJiYgIShydCAmIDgpICYmIGlt bS5pc1VJbnQxMCgpKQorICAgICAgICB9IGVsc2UgaWYgKChybiA9PSBBUk1SZWdpc3RlcnM6OnNw KSAmJiAhKHJ0ICYgOCkgJiYgaW1tLmlzVUludDEwKCkpCiAgICAgICAgICAgICBtX2Zvcm1hdHRl ci5vbmVXb3JkT3A1UmVnM0ltbTgoT1BfTERSX2ltbV9UMiwgcnQsIHN0YXRpY19jYXN0PHVpbnQ4 X3Q+KGltbS5nZXRVSW50MTAoKSA+PiAyKSk7CiAgICAgICAgIGVsc2UKICAgICAgICAgICAgIG1f Zm9ybWF0dGVyLnR3b1dvcmRPcDEyUmVnNFJlZzRJbW0xMihPUF9MRFJfaW1tX1QzLCBybiwgcnQs IGltbS5nZXRVSW50MTIoKSk7Cg== 388420 2020-01-22 07:43:59 -0800 2020-01-22 11:14:32 -0800 Patch bug-206231-20200122124357.patch text/plain 2199 ticaiolima U3VidmVyc2lvbiBSZXZpc2lvbjogMjU0OTIwCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBk ZmE0N2M4M2FkMWY4YjE1ZmZjNTdjNzdmNzQ4YTEyYWIyZWFhMDkxLi43OWNlNzVkMWQxOTJlZDRi MWI2YjJhNjczOGQ2OWVhYTc5MjA4NmM3IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxOCBAQAorMjAyMC0wMS0yMiAgQ2FpbyBMaW1hICA8dGljYWlvbGltYUBnbWFpbC5jb20+ CisKKyAgICAgICAgW0FSTXY3XSBBc3NlbWJsZXIgaXMgZ2VuZXJhdGluZyB3cm9uZyBpbnN0cnVj dGlvbiBmb3IgbGRyIHIyLCBbcjMsICM3XQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9y Zy9zaG93X2J1Zy5jZ2k/aWQ9MjA2MjMxCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChP T1BTISkuCisKKyAgICAgICAgV2hlbiBnZW5lcmF0aW5nIGNvZGUgZm9yIGBsZHJgIG9uIEFSTXY3 LCB3ZSB3ZXJlIGVuY29kaW5nIGltbWVkaWF0ZXMgbm90CisgICAgICAgIG11bHRpcGxlIG9mIDQg dXNpbmcgRW5jb2RpbmcgVDEsIHdoaWNoIG9ubHkgYWNjZXB0cyBtdWx0aXBsZSBvZiA0IGFzCisg ICAgICAgIGltbWVkaWF0ZXMuIFRoaXMgcGF0Y2ggZml4ZXMgc3VjaCBpc3N1ZSBhbmQgYWxsb3cg dXMgZ2VuZXJhdGUgcHJvcGVyCisgICAgICAgIGNvZGUgd2hlbiBvZmZzZXQgaW1lbWRpYXRlcyBh cmUgbm90IG11bHRpcGxlIG9mIDQuCisKKyAgICAgICAgKiBhc3NlbWJsZXIvQVJNdjdBc3NlbWJs ZXIuaDoKKyAgICAgICAgKEpTQzo6QVJNdjdBc3NlbWJsZXI6Omxkcik6CisKIDIwMjAtMDEtMjEg IFJvc3MgS2lyc2xpbmcgIDxyb3NzLmtpcnNsaW5nQHNvbnkuY29tPgogCiAgICAgICAgIFtKU0Nd IERhdGUgcGFyc2UgbG9naWMgc2hvdWxkIGJlIGxlc3MgcmVkdW5kYW50CmRpZmYgLS1naXQgYS9T b3VyY2UvSmF2YVNjcmlwdENvcmUvYXNzZW1ibGVyL0FSTXY3QXNzZW1ibGVyLmggYi9Tb3VyY2Uv SmF2YVNjcmlwdENvcmUvYXNzZW1ibGVyL0FSTXY3QXNzZW1ibGVyLmgKaW5kZXggNjE1MWMwYmEy OTMwMzk5ZDM0YTRjNGE5YWRmZGEzODc0NjQ2Y2M3NS4uZDIxYmI3NjMwOGMzNGY2NmQ4MjgwMjRl Nzc4M2RjM2QwYzYyZTUzMiAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL2Fzc2Vt Ymxlci9BUk12N0Fzc2VtYmxlci5oCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9hc3NlbWJs ZXIvQVJNdjdBc3NlbWJsZXIuaApAQCAtMTAyOSw5ICsxMDI5LDExIEBAIHB1YmxpYzoKICAgICAg ICAgQVNTRVJUKHJuICE9IEFSTVJlZ2lzdGVyczo6cGMpOyAvLyBMRFIgKGxpdGVyYWwpCiAgICAg ICAgIEFTU0VSVChpbW0uaXNVSW50MTIoKSk7CiAKLSAgICAgICAgaWYgKCEoKHJ0IHwgcm4pICYg OCkgJiYgaW1tLmlzVUludDcoKSkKKyAgICAgICAgaWYgKCEoKHJ0IHwgcm4pICYgOCkgJiYgaW1t LmlzVUludDcoKSAmJiAhKGltbS5nZXRVSW50NygpICUgNCkpIHsKKyAgICAgICAgICAgIC8vIFdl IGNhbiB1c2UgRW5jb2RpbmcgVDEgd2hlbiBpbW0gaXMgYSBtdWx0aXBsZSBvZiA0LgorICAgICAg ICAgICAgLy8gKHNlZSBBOC44LjYzIG9uIEFSTSBBcmNoaXRlY3R1cmUgUmVmZXJlbmNlIE1hbnVh bCBBUk12Ny1BIGFuZCBBUk12Ny1SIGVkaXRpb24pCiAgICAgICAgICAgICBtX2Zvcm1hdHRlci5v bmVXb3JkT3A1SW1tNVJlZzNSZWczKE9QX0xEUl9pbW1fVDEsIGltbS5nZXRVSW50NygpID4+IDIs IHJuLCBydCk7Ci0gICAgICAgIGVsc2UgaWYgKChybiA9PSBBUk1SZWdpc3RlcnM6OnNwKSAmJiAh KHJ0ICYgOCkgJiYgaW1tLmlzVUludDEwKCkpCisgICAgICAgIH0gZWxzZSBpZiAoKHJuID09IEFS TVJlZ2lzdGVyczo6c3ApICYmICEocnQgJiA4KSAmJiBpbW0uaXNVSW50MTAoKSkKICAgICAgICAg ICAgIG1fZm9ybWF0dGVyLm9uZVdvcmRPcDVSZWczSW1tOChPUF9MRFJfaW1tX1QyLCBydCwgc3Rh dGljX2Nhc3Q8dWludDhfdD4oaW1tLmdldFVJbnQxMCgpID4+IDIpKTsKICAgICAgICAgZWxzZQog ICAgICAgICAgICAgbV9mb3JtYXR0ZXIudHdvV29yZE9wMTJSZWc0UmVnNEltbTEyKE9QX0xEUl9p bW1fVDMsIHJuLCBydCwgaW1tLmdldFVJbnQxMigpKTsK 388444 2020-01-22 11:14:35 -0800 2020-01-22 13:43:03 -0800 Patch bug-206231-20200122161434.patch text/plain 2319 ticaiolima U3VidmVyc2lvbiBSZXZpc2lvbjogMjU0OTIwCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBk ZmE0N2M4M2FkMWY4YjE1ZmZjNTdjNzdmNzQ4YTEyYWIyZWFhMDkxLi42MmJjZDgxOTk0NTcxMTVm ODc3MmI0ZjhmOTY2OGM3NmRlYTkyNjUyIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxOCBAQAorMjAyMC0wMS0yMiAgQ2FpbyBMaW1hICA8dGljYWlvbGltYUBnbWFpbC5jb20+ CisKKyAgICAgICAgW0FSTXY3XSBBc3NlbWJsZXIgaXMgZ2VuZXJhdGluZyB3cm9uZyBpbnN0cnVj dGlvbiBmb3IgbGRyIHIyLCBbcjMsICM3XQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9y Zy9zaG93X2J1Zy5jZ2k/aWQ9MjA2MjMxCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTWFyayBMYW0u CisKKyAgICAgICAgV2hlbiBnZW5lcmF0aW5nIGNvZGUgZm9yIGBsZHJgIG9uIEFSTXY3LCB3ZSB3 ZXJlIGVuY29kaW5nIGltbWVkaWF0ZXMgbm90CisgICAgICAgIG11bHRpcGxlIG9mIDQgdXNpbmcg RW5jb2RpbmcgVDEsIHdoaWNoIG9ubHkgYWNjZXB0cyBtdWx0aXBsZSBvZiA0IGFzCisgICAgICAg IGltbWVkaWF0ZXMuIFRoaXMgcGF0Y2ggZml4ZXMgc3VjaCBpc3N1ZSBhbmQgYWxsb3cgdXMgZ2Vu ZXJhdGUgcHJvcGVyCisgICAgICAgIGNvZGUgd2hlbiBvZmZzZXQgaW1lbWRpYXRlcyBhcmUgbm90 IG11bHRpcGxlIG9mIDQuCisKKyAgICAgICAgKiBhc3NlbWJsZXIvQVJNdjdBc3NlbWJsZXIuaDoK KyAgICAgICAgKEpTQzo6QVJNdjdBc3NlbWJsZXI6Omxkcik6CisKIDIwMjAtMDEtMjEgIFJvc3Mg S2lyc2xpbmcgIDxyb3NzLmtpcnNsaW5nQHNvbnkuY29tPgogCiAgICAgICAgIFtKU0NdIERhdGUg cGFyc2UgbG9naWMgc2hvdWxkIGJlIGxlc3MgcmVkdW5kYW50CmRpZmYgLS1naXQgYS9Tb3VyY2Uv SmF2YVNjcmlwdENvcmUvYXNzZW1ibGVyL0FSTXY3QXNzZW1ibGVyLmggYi9Tb3VyY2UvSmF2YVNj cmlwdENvcmUvYXNzZW1ibGVyL0FSTXY3QXNzZW1ibGVyLmgKaW5kZXggNjE1MWMwYmEyOTMwMzk5 ZDM0YTRjNGE5YWRmZGEzODc0NjQ2Y2M3NS4uNDJjNjhhNDRmYThjNjAwODFjYmFiODlkNmY5M2Mw ZTVlNDNhOWNkYyAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL2Fzc2VtYmxlci9B Uk12N0Fzc2VtYmxlci5oCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9hc3NlbWJsZXIvQVJN djdBc3NlbWJsZXIuaApAQCAtMTAyOSw5ICsxMDI5LDEzIEBAIHB1YmxpYzoKICAgICAgICAgQVNT RVJUKHJuICE9IEFSTVJlZ2lzdGVyczo6cGMpOyAvLyBMRFIgKGxpdGVyYWwpCiAgICAgICAgIEFT U0VSVChpbW0uaXNVSW50MTIoKSk7CiAKLSAgICAgICAgaWYgKCEoKHJ0IHwgcm4pICYgOCkgJiYg aW1tLmlzVUludDcoKSkKKyAgICAgICAgaWYgKCEoKHJ0IHwgcm4pICYgOCkgJiYgaW1tLmlzVUlu dDcoKSAmJiAhKGltbS5nZXRVSW50NygpICUgNCkpIHsKKyAgICAgICAgICAgIC8vIFdlIGNhbiBv bmx5IHVzZSBFbmNvZGluZyBUMSB3aGVuIGltbSBpcyBhIG11bHRpcGxlIG9mIDQuCisgICAgICAg ICAgICAvLyBGb3IgZGV0YWlscyBzZWUgQTguOC42MyBvbiBBUk0gQXJjaGl0ZWN0dXJlIFJlZmVy ZW5jZQorICAgICAgICAgICAgLy8gTWFudWFsIEFSTXY3LUEgYW5kIEFSTXY3LVIgZWRpdGlvbiBh dmFpbGFibGUgb24KKyAgICAgICAgICAgIC8vIGh0dHBzOi8vc3RhdGljLmRvY3MuYXJtLmNvbS9k ZGkwNDA2L2NkL0RESTA0MDZDX2RfYXJtdjdhcl9hcm0ucGRmCiAgICAgICAgICAgICBtX2Zvcm1h dHRlci5vbmVXb3JkT3A1SW1tNVJlZzNSZWczKE9QX0xEUl9pbW1fVDEsIGltbS5nZXRVSW50Nygp ID4+IDIsIHJuLCBydCk7Ci0gICAgICAgIGVsc2UgaWYgKChybiA9PSBBUk1SZWdpc3RlcnM6OnNw KSAmJiAhKHJ0ICYgOCkgJiYgaW1tLmlzVUludDEwKCkpCisgICAgICAgIH0gZWxzZSBpZiAoKHJu ID09IEFSTVJlZ2lzdGVyczo6c3ApICYmICEocnQgJiA4KSAmJiBpbW0uaXNVSW50MTAoKSkKICAg ICAgICAgICAgIG1fZm9ybWF0dGVyLm9uZVdvcmRPcDVSZWczSW1tOChPUF9MRFJfaW1tX1QyLCBy dCwgc3RhdGljX2Nhc3Q8dWludDhfdD4oaW1tLmdldFVJbnQxMCgpID4+IDIpKTsKICAgICAgICAg ZWxzZQogICAgICAgICAgICAgbV9mb3JtYXR0ZXIudHdvV29yZE9wMTJSZWc0UmVnNEltbTEyKE9Q X0xEUl9pbW1fVDMsIHJuLCBydCwgaW1tLmdldFVJbnQxMigpKTsK