38538 2010-05-04 12:10:50 -0700 JSC TimeoutChecker::didTimeOut overflows on ARM 2011-04-19 05:15:29 -0700 1 1 1 Unclassified WebKit WebKit Qt 528+ (Nightly build) Other Linux RESOLVED FIXED Qt, QtTriaged P2 Normal --- 38654 44675 1 diegohcg webkit-qt-unassigned abarth ademar barraclough benjamin commit-queue dominik.holland eric ggaren kenneth kent.hansen oliver thomas tonikitoo webkit.review.bot zherczeg oldest_to_newest 220704 0 diegohcg 2010-05-04 12:10:50 -0700 The infinite loop is never stopped. It seems happen because the double to unsigned conversion overflows, returning always the maximum unsigned size and keeping time constant not getting the time out. JavaScriptCore/runtime/TimeoutChecker.cpp: 119 bool TimeoutChecker::didTimeOut(ExecState* exec) 120 { 121 unsigned currentTime = getCPUTime(); 223147 1 tonikitoo 2010-05-09 20:42:03 -0700 Diego, pls attach the test program you made, showing that the problem is maemo specific 223422 2 55578 diegohcg 2010-05-10 11:30:29 -0700 Created attachment 55578 Simple test case see the double to unsigned conversion problem on ARM 223426 3 kenneth 2010-05-10 11:43:24 -0700 This problem makes the Qt autotest tst_QWebPage::infiniteLoopJS() does not terminate on Maemo 5 (ARM based platform). 223477 4 zherczeg 2010-05-10 13:05:29 -0700 how getCPUTime() is implemented on maemo? Is it a time from the begining of the program, or some absolute time? What about uint64_t ? 243656 5 59892 kent.hansen 2010-06-28 04:55:22 -0700 Created attachment 59892 Patch to skip the infiniteLoopJS test on maemo5 Skipping the test at least allows the qwebpage test to finish. Feel free to commit unless someone a proper fix is imminent. 248819 6 kent.hansen 2010-07-09 01:10:54 -0700 Committed r62913: <http://trac.webkit.org/changeset/62913> 271637 7 65957 benjamin 2010-08-30 14:33:23 -0700 Created attachment 65957 Patch Make getCPUTime() return values relative to the first call. The previous implementation relied on simply on currentTime(), which return a time since epoch and not a time since the thread started. This made the return value of getCPUTime() overflow on 32 bits. 271650 8 65960 benjamin 2010-08-30 14:46:43 -0700 Created attachment 65960 Patch for Qt Enable the test previously failing on Maemo. 271654 9 benjamin 2010-08-30 14:49:49 -0700 Thanks Kenneth for those insanely quick reviews! :) 271655 10 tonikitoo 2010-08-30 14:50:28 -0700 (In reply to comment #9) > Thanks Kenneth for those insanely quick reviews! :) he is a bot :-) 271659 11 kenneth 2010-08-30 14:55:02 -0700 (In reply to comment #10) > (In reply to comment #9) > > Thanks Kenneth for those insanely quick reviews! :) > > he is a bot :-) Not last that I checked :-) 271695 12 oliver 2010-08-30 15:22:53 -0700 Have you test performance of this patch? 272025 13 benjamin 2010-08-31 04:10:07 -0700 (In reply to comment #12) > Have you test performance of this patch? I haven't, but the function is called so rarely I doubt I'll be able to see any time difference out of the noise. Do you want me to test with callgrind to get an idea of the difference in number of instructions? 272043 14 65957 commit-queue 2010-08-31 05:07:12 -0700 Comment on attachment 65957 Patch Clearing flags on attachment: 65957 Committed r66475: <http://trac.webkit.org/changeset/66475> 272057 15 benjamin 2010-08-31 05:44:30 -0700 Add blocking 39313, it would be nice to have this fixed in the branch QtWebKit 2.0. 272060 16 65960 commit-queue 2010-08-31 05:58:08 -0700 Comment on attachment 65960 Patch for Qt Clearing flags on attachment: 65960 Committed r66478: <http://trac.webkit.org/changeset/66478> 272061 17 commit-queue 2010-08-31 05:58:14 -0700 All reviewed patches have been landed. Closing bug. 272197 18 oliver 2010-08-31 10:38:04 -0700 I want perf numbers for this patch -- you never change JSC without perf testing, it's that simple. Something being uncommon has not stopped it from impacting performance in the past, so while i agree with your assessment that this is unlikely to regress perf there is a difference between "unlikely to regress" and "does not regress". 272298 19 kenneth 2010-08-31 13:05:21 -0700 (In reply to comment #18) > I want perf numbers for this patch -- you never change JSC without perf testing, it's that simple. Something being uncommon has not stopped it from impacting performance in the past, so while i agree with your assessment that this is unlikely to regress perf there is a difference between "unlikely to regress" and "does not regress". Benjamin, are you working on the perf testing? 272307 20 benjamin 2010-08-31 13:16:32 -0700 (In reply to comment #19) > Benjamin, are you working on the perf testing? Will do....... 272497 21 webkit.review.bot 2010-08-31 16:43:52 -0700 http://trac.webkit.org/changeset/66478 might have broken Leopard Intel Debug (Tests) 272959 22 benjamin 2010-09-01 13:07:35 -0700 Before patch: ============================================ RESULTS (means and 95% confidence intervals) -------------------------------------------- Total: 4164.3ms +/- 0.3% -------------------------------------------- 3d: 528.2ms +/- 0.2% cube: 176.3ms +/- 0.3% morph: 148.1ms +/- 0.6% raytrace: 203.8ms +/- 0.3% access: 513.1ms +/- 0.3% binary-trees: 119.4ms +/- 0.6% fannkuch: 149.4ms +/- 0.2% nbody: 168.2ms +/- 0.6% nsieve: 76.1ms +/- 0.8% bitops: 205.8ms +/- 0.4% 3bit-bits-in-byte: 29.6ms +/- 1.2% bits-in-byte: 45.8ms +/- 1.0% bitwise-and: 53.3ms +/- 0.6% nsieve-bits: 77.1ms +/- 0.5% controlflow: 27.2ms +/- 1.1% recursive: 27.2ms +/- 1.1% crypto: 265.7ms +/- 1.9% aes: 165.3ms +/- 3.0% md5: 59.2ms +/- 1.4% sha1: 41.2ms +/- 1.1% date: 558.3ms +/- 0.3% format-tofte: 269.8ms +/- 0.7% format-xparb: 288.5ms +/- 0.4% math: 496.0ms +/- 0.1% cordic: 166.0ms +/- 0.2% partial-sums: 231.1ms +/- 0.2% spectral-norm: 98.9ms +/- 0.2% regexp: 120.4ms +/- 0.6% dna: 120.4ms +/- 0.6% string: 1449.6ms +/- 0.7% base64: 214.7ms +/- 0.5% fasta: 202.7ms +/- 5.5% tagcloud: 265.0ms +/- 0.7% unpack-code: 543.1ms +/- 0.6% validate-input: 224.1ms +/- 0.3% After patch: ============================================ RESULTS (means and 95% confidence intervals) -------------------------------------------- Total: 4165.4ms +/- 0.3% -------------------------------------------- 3d: 529.8ms +/- 0.3% cube: 177.4ms +/- 0.6% morph: 148.2ms +/- 0.7% raytrace: 204.2ms +/- 0.2% access: 513.0ms +/- 0.6% binary-trees: 120.2ms +/- 2.2% fannkuch: 149.4ms +/- 0.2% nbody: 168.1ms +/- 0.5% nsieve: 75.3ms +/- 0.9% bitops: 205.9ms +/- 0.8% 3bit-bits-in-byte: 29.2ms +/- 1.0% bits-in-byte: 45.5ms +/- 0.8% bitwise-and: 53.3ms +/- 0.6% nsieve-bits: 77.9ms +/- 2.2% controlflow: 27.9ms +/- 3.3% recursive: 27.9ms +/- 3.3% crypto: 256.7ms +/- 0.4% aes: 162.7ms +/- 0.4% md5: 54.9ms +/- 0.7% sha1: 39.1ms +/- 2.0% date: 560.0ms +/- 0.5% format-tofte: 269.5ms +/- 0.7% format-xparb: 290.5ms +/- 0.7% math: 496.3ms +/- 0.2% cordic: 166.2ms +/- 0.4% partial-sums: 231.1ms +/- 0.3% spectral-norm: 99.0ms +/- 0.0% regexp: 121.0ms +/- 0.7% dna: 121.0ms +/- 0.7% string: 1454.8ms +/- 0.8% base64: 215.8ms +/- 0.5% fasta: 200.6ms +/- 2.6% tagcloud: 265.2ms +/- 0.5% unpack-code: 547.1ms +/- 0.4% validate-input: 226.1ms +/- 4.1% No real difference. I ran the test 3 times each, and the results seem to vary +-2ms overall. All of those numbers are for the N900. 272980 23 oliver 2010-09-01 13:40:59 -0700 thanks 273039 24 ademar 2010-09-01 14:44:43 -0700 Revision r62913 cherry-picked into qtwebkit-2.1 with commit 61b154f6c494990ea79983b1667d8551524c1d41 Revision r66475 cherry-picked into qtwebkit-2.1 with commit 4f9cc597fab2122f3c7f987de1323cd4cc01f0e1 Revision r66478 cherry-picked into qtwebkit-2.1 with commit 836b33d3b0f2690dd5e1edc174ef7ca67d2eee07 55578 2010-05-10 11:30:29 -0700 2010-05-10 11:30:29 -0700 Simple test case see the double to unsigned conversion problem on ARM timetst.cpp text/x-c++src 462 diegohcg I2luY2x1ZGUgPFFBcHBsaWNhdGlvbj4KI2luY2x1ZGUgPFFEZWJ1Zz4KI2luY2x1ZGUgPFFUZXN0 PgoKI2luY2x1ZGUgPG1hdGguaD4KI2luY2x1ZGUgPHN5cy90aW1lLmg+CgppbnQgbWFpbihpbnQg YXJnYywgY2hhciAqYXJndltdKQp7CiAgICBRQXBwbGljYXRpb24gYXBwKGFyZ2MsIGFyZ3YpOwoK ICAgIHN0cnVjdCB0aW1ldmFsIG5vdzsKCiAgICB3aGlsZSgxKSB7CiAgICAgICAgZ2V0dGltZW9m ZGF5KCZub3csIDApOwogICAgICAgIGRvdWJsZSB0aW1lID0gKG5vdy50dl9zZWMgKyBub3cudHZf dXNlYyAvIDEwMDAwMDAuMCkgKiAxMDAwOwoJICAgIHVuc2lnbmVkIGN1cnJlbnRUaW1lID0gdGlt ZTsKICAgICAgICBwcmludGYoImN1cnJlblRpbWU6ICV1XG4iLCBjdXJyZW50VGltZSk7CiAgICAg ICAgUVRlc3Q6OnFXYWl0KDUwMCk7CiAgICB9CgogICAgYXBwLmV4ZWMoKTsKCiAgICByZXR1cm4g MDsKfQoK 59892 2010-06-28 04:55:22 -0700 2010-07-09 01:11:59 -0700 Patch to skip the infiniteLoopJS test on maemo5 infiniteLoopJSMaemo5.diff text/plain 1335 kent.hansen ZGlmZiAtLWdpdCBhL1dlYktpdC9xdC9DaGFuZ2VMb2cgYi9XZWJLaXQvcXQvQ2hhbmdlTG9nCmlu ZGV4IDczYmY1ZDUuLjQ5Zjk0OTEgMTAwNjQ0Ci0tLSBhL1dlYktpdC9xdC9DaGFuZ2VMb2cKKysr IGIvV2ViS2l0L3F0L0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDEwLTA2LTI4ICBLZW50 IEhhbnNlbiAgPGtlbnQuaGFuc2VuQG5va2lhLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICBbUXRdIFNraXAgdGVzdCB0aGF0IG5ldmVyIHRlcm1p bmF0ZXMgb24gbWFlbW81CisKKyAgICAgICAgRHVlIHRvIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD0zODUzOAorICAgICAgICB0aGUgdHN0X1FXZWJQYWdlOjppbmZpbml0 ZUxvb3BKUygpIGF1dG90ZXN0IG5ldmVyIHRlcm1pbmF0ZXMuCisgICAgICAgIFNraXAgdGhlIHRl c3Qgc28gdGhhdCB0aGUgdGVzdCBjYXNlIG1heSBydW4gdG8gY29tcGxldGlvbi4KKworICAgICAg ICBQYXRjaCBieSBEb21pbmlrIEhvbGxhbmQgPGRvbWluaWsuaG9sbGFuZEBub2tpYS5jb20+CisK KyAgICAgICAgKiB0ZXN0cy9xd2VicGFnZS90c3RfcXdlYnBhZ2UuY3BwOgorICAgICAgICAodHN0 X1FXZWJQYWdlOjppbmZpbml0ZUxvb3BKUyk6CisKIDIwMTAtMDYtMjggIEFuZHJlYXMgS2xpbmcg IDxhbmRyZWFzLmtsaW5nQG5va2lhLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBTaW1vbiBI YXVzbWFubi4KZGlmZiAtLWdpdCBhL1dlYktpdC9xdC90ZXN0cy9xd2VicGFnZS90c3RfcXdlYnBh Z2UuY3BwIGIvV2ViS2l0L3F0L3Rlc3RzL3F3ZWJwYWdlL3RzdF9xd2VicGFnZS5jcHAKaW5kZXgg NWU2ZjRlNi4uNzA0NjZjNyAxMDA2NDQKLS0tIGEvV2ViS2l0L3F0L3Rlc3RzL3F3ZWJwYWdlL3Rz dF9xd2VicGFnZS5jcHAKKysrIGIvV2ViS2l0L3F0L3Rlc3RzL3F3ZWJwYWdlL3RzdF9xd2VicGFn ZS5jcHAKQEAgLTIzNCw2ICsyMzQsOSBAQCBwcml2YXRlOgogCiB2b2lkIHRzdF9RV2ViUGFnZTo6 aW5maW5pdGVMb29wSlMoKQogeworI2lmZGVmIFFfV1NfTUFFTU9fNQorICAgIFFTS0lQKCJUZXN0 IG5ldmVyIHRlcm1pbmF0ZXMgb24gTWFlbW8gNSA6IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD0zODUzOCIsIFNraXBBbGwpOworI2VuZGlmCiAgICAgSlNUZXN0UGFnZSog bmV3UGFnZSA9IG5ldyBKU1Rlc3RQYWdlKG1fdmlldyk7CiAgICAgbV92aWV3LT5zZXRQYWdlKG5l d1BhZ2UpOwogICAgIG1fdmlldy0+c2V0SHRtbChRU3RyaW5nKCI8aHRtbD48Ym9keT50ZXN0PC9i b2R5PjwvaHRtbD4iKSwgUVVybCgpKTsK 65957 2010-08-30 14:33:23 -0700 2010-08-31 05:07:12 -0700 Patch 38538.patch text/plain 1448 benjamin ZGlmZiAtLWdpdCBhL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZyBiL0phdmFTY3JpcHRDb3JlL0No YW5nZUxvZwppbmRleCBkYTEwODk1Li44NzM2OTI1IDEwMDY0NAotLS0gYS9KYXZhU2NyaXB0Q29y ZS9DaGFuZ2VMb2cKKysrIGIvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTgg QEAKKzIwMTAtMDgtMzAgIEJlbmphbWluIFBvdWxhaW4gIDxiZW5qYW1pbi5wb3VsYWluQG5va2lh LmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBK U0MgVGltZW91dENoZWNrZXI6OmRpZFRpbWVPdXQgb3ZlcmZsb3dzIG9uIEFSTQorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9Mzg1MzgKKworICAgICAgICBN YWtlIGdldENQVVRpbWUoKSByZXR1cm4gdmFsdWVzIHJlbGF0aXZlIHRvIHRoZSBmaXJzdCBjYWxs LgorICAgICAgICBUaGUgcHJldmlvdXMgaW1wbGVtZW50YXRpb24gcmVsaWVkIG9uIHNpbXBseSBv biBjdXJyZW50VGltZSgpLCB3aGljaAorICAgICAgICByZXR1cm4gYSB0aW1lIHNpbmNlIGVwb2No IGFuZCBub3QgYSB0aW1lIHNpbmNlIHRoZSB0aHJlYWQgc3RhcnRlZC4gVGhpcworICAgICAgICBt YWRlIHRoZSByZXR1cm4gdmFsdWUgb2YgZ2V0Q1BVVGltZSgpIG92ZXJmbG93IG9uIDMyIGJpdHMu CisKKyAgICAgICAgKiBydW50aW1lL1RpbWVvdXRDaGVja2VyLmNwcDoKKyAgICAgICAgKEpTQzo6 Z2V0Q1BVVGltZSk6CisKIDIwMTAtMDgtMjUgIFhhbiBMb3BleiAgPHhsb3BlekBpZ2FsaWEuY29t PgogCiAgICAgICAgIFJldmlld2VkIGJ5IEtlbnQgVGFtdXJhLgpkaWZmIC0tZ2l0IGEvSmF2YVNj cmlwdENvcmUvcnVudGltZS9UaW1lb3V0Q2hlY2tlci5jcHAgYi9KYXZhU2NyaXB0Q29yZS9ydW50 aW1lL1RpbWVvdXRDaGVja2VyLmNwcAppbmRleCAyZGMxMDI4Li4wNGQ5MDRkIDEwMDY0NAotLS0g YS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL1RpbWVvdXRDaGVja2VyLmNwcAorKysgYi9KYXZhU2Ny aXB0Q29yZS9ydW50aW1lL1RpbWVvdXRDaGVja2VyLmNwcApAQCAtOTgsNyArOTgsMTAgQEAgc3Rh dGljIGlubGluZSB1bnNpZ25lZCBnZXRDUFVUaW1lKCkKICAgICByZXR1cm4gR0VUVVBUSU1FTVMo KTsKICNlbHNlCiAgICAgLy8gRklYTUU6IFdlIHNob3VsZCByZXR1cm4gdGhlIHRpbWUgdGhlIGN1 cnJlbnQgdGhyZWFkIGhhcyBzcGVudCBleGVjdXRpbmcuCi0gICAgcmV0dXJuIGN1cnJlbnRUaW1l KCkgKiAxMDAwOworCisgICAgLy8gdXNlIGEgcmVsYXRpdmUgdGltZSBmcm9tIGZpcnN0IGNhbGwg aW4gb3JkZXIgdG8gYXZvaWQgYW4gb3ZlcmZsb3cKKyAgICBzdGF0aWMgZG91YmxlIGZpcnN0VGlt ZSA9IGN1cnJlbnRUaW1lKCk7CisgICAgcmV0dXJuIChjdXJyZW50VGltZSgpIC0gZmlyc3RUaW1l KSAqIDEwMDA7CiAjZW5kaWYKIH0KIAo= 65960 2010-08-30 14:46:43 -0700 2010-08-31 05:58:08 -0700 Patch for Qt 38538.patch text/plain 1194 benjamin ZGlmZiAtLWdpdCBhL1dlYktpdC9xdC9DaGFuZ2VMb2cgYi9XZWJLaXQvcXQvQ2hhbmdlTG9nCmlu ZGV4IDk5NjllYmQuLjc4NThjYzQgMTAwNjQ0Ci0tLSBhL1dlYktpdC9xdC9DaGFuZ2VMb2cKKysr IGIvV2ViS2l0L3F0L0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDEwLTA4LTMwICBCZW5q YW1pbiBQb3VsYWluICA8YmVuamFtaW4ucG91bGFpbkBub2tpYS5jb20+CisKKyAgICAgICAgUmV2 aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSlNDIFRpbWVvdXRDaGVja2VyOjpk aWRUaW1lT3V0IG92ZXJmbG93cyBvbiBBUk0KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTM4NTM4CisKKyAgICAgICAgUmUtZW5hYmxlZCB0aGUgdGVzdCB0 aGF0IHdhcyBza2lwcGVkIGZvciBNYWVtbyA1LgorCisgICAgICAgICogdGVzdHMvcXdlYnBhZ2Uv dHN0X3F3ZWJwYWdlLmNwcDoKKyAgICAgICAgKHRzdF9RV2ViUGFnZTo6aW5maW5pdGVMb29wSlMp OgorCiAyMDEwLTA4LTI1ICBMZW8gWWFuZyAgPGxlby55YW5nQHRvcmNobW9iaWxlLmNvbS5jbj4K IAogICAgICAgICBSZXZpZXdlZCBieSBTaW1vbiBIYXVzbWFubi4KZGlmZiAtLWdpdCBhL1dlYktp dC9xdC90ZXN0cy9xd2VicGFnZS90c3RfcXdlYnBhZ2UuY3BwIGIvV2ViS2l0L3F0L3Rlc3RzL3F3 ZWJwYWdlL3RzdF9xd2VicGFnZS5jcHAKaW5kZXggNjdlM2VjZC4uOWI0YjZkOCAxMDA2NDQKLS0t IGEvV2ViS2l0L3F0L3Rlc3RzL3F3ZWJwYWdlL3RzdF9xd2VicGFnZS5jcHAKKysrIGIvV2ViS2l0 L3F0L3Rlc3RzL3F3ZWJwYWdlL3RzdF9xd2VicGFnZS5jcHAKQEAgLTIzNCw5ICsyMzQsNiBAQCBw cml2YXRlOgogCiB2b2lkIHRzdF9RV2ViUGFnZTo6aW5maW5pdGVMb29wSlMoKQogewotI2lmZGVm IFFfV1NfTUFFTU9fNQotICAgIFFTS0lQKCJUZXN0IG5ldmVyIHRlcm1pbmF0ZXMgb24gTWFlbW8g NSA6IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zODUzOCIsIFNraXBB bGwpOwotI2VuZGlmCiAgICAgSlNUZXN0UGFnZSogbmV3UGFnZSA9IG5ldyBKU1Rlc3RQYWdlKG1f dmlldyk7CiAgICAgbV92aWV3LT5zZXRQYWdlKG5ld1BhZ2UpOwogICAgIG1fdmlldy0+c2V0SHRt bChRU3RyaW5nKCI8aHRtbD48Ym9keT50ZXN0PC9ib2R5PjwvaHRtbD4iKSwgUVVybCgpKTsK