144908 2015-05-12 06:14:25 -0700 [GTK] Avoid std::chrono::microseconds overflows in setSharedTimerFireInterval 2015-12-07 10:01:35 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 zan zan andersca cgarcia darin gustavo mrobinson svillar oldest_to_newest 1094139 0 zan 2015-05-12 06:14:25 -0700 [GTK] Avoid std::chrono::microseconds overflows in setSharedTimerFireInterval 1094144 1 252960 zan 2015-05-12 06:31:25 -0700 Created attachment 252960 Patch 1094148 2 252960 cgarcia 2015-05-12 06:44:50 -0700 Comment on attachment 252960 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252960&action=review > Source/WebCore/platform/gtk/SharedTimerGtk.cpp:53 > + // Passed-in value is in seconds, which might overflow when casting to std::chrono::microseconds. > + auto interval = std::chrono::duration<double>(intervalValue); > + auto delay = std::chrono::microseconds::max(); > + if (interval < delay) > + delay = std::chrono::duration_cast<std::chrono::microseconds>(interval); Could we do something like this in GMainLoopSource instead? To fix any other cases where we are passing a double casted to microseconds. 1094152 3 252960 zan 2015-05-12 06:59:22 -0700 Comment on attachment 252960 Patch Needs a bit more thought. 1095485 4 252960 darin 2015-05-17 10:26:16 -0700 Comment on attachment 252960 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252960&action=review > Source/WebCore/platform/gtk/SharedTimerGtk.cpp:49 > + // Passed-in value is in seconds, which might overflow when casting to std::chrono::microseconds. This seems like an issue that might happen cross-platform as we move from the traditional “doubles in seconds” to std::chrono across the board. I wonder what the good idiom to avoid this is. 1146788 5 zan 2015-12-07 02:18:44 -0800 A version of this patch landed in r192058. https://trac.webkit.org/changeset/192058 1146791 6 zan 2015-12-07 02:20:47 -0800 (In reply to comment #4) > Comment on attachment 252960 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=252960&action=review > > > Source/WebCore/platform/gtk/SharedTimerGtk.cpp:49 > > + // Passed-in value is in seconds, which might overflow when casting to std::chrono::microseconds. > > This seems like an issue that might happen cross-platform as we move from > the traditional “doubles in seconds” to std::chrono across the board. I > wonder what the good idiom to avoid this is. If possible, we could try specializing the std::chrono::duration<> template for the WTF::Checked class. 1146886 7 darin 2015-12-07 10:01:35 -0800 For most uses we might need clamping casts that turn out-of-range values into min/max. 252960 2015-05-12 06:31:25 -0700 2015-05-12 06:59:22 -0700 Patch bug-144908-20150512153002.patch text/plain 2736 zan U3VidmVyc2lvbiBSZXZpc2lvbjogMTg0MTkzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZjA5ZTk2ZGM5NTBiN2U3 NTlhNThmNDdmNjRjMzRkYmJjZDNjMWRjMy4uZjg4YTg3MDkyN2QwMWQ1NTJiNWI5NGM5MGNlN2Fj MDQ1ZjdlZjcwNyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDIzIEBACiAyMDE1LTA1LTEyICBaYW4g RG9iZXJzZWsgIDx6ZG9iZXJzZWtAaWdhbGlhLmNvbT4KIAorICAgICAgICBbR1RLXSBBdm9pZCBz dGQ6OmNocm9ubzo6bWljcm9zZWNvbmRzIG92ZXJmbG93cyBpbiBzZXRTaGFyZWRUaW1lckZpcmVJ bnRlcnZhbAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MTQ0OTA4CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg VGhlIHBhc3NlZC1pbiB2YWx1ZSB0byBzZXRTaGFyZWRUaW1lckZpcmVJbnRlcnZhbCBpcyBwYXNz ZWQgaW4gYXMgYSBkb3VibGUKKyAgICAgICAgcmVwcmVzZW50aW5nIHNlY29uZHMuIEl0J3MgcG9z c2libGUgdGhhdCB0aGUgdmFsdWUgaXMgbGFyZ2UgZW5vdWdoIHRvIG92ZXJmbG93CisgICAgICAg IHdoZW4gY2FzdGluZyB0byBzdGQ6OmNocm9ubzo6bWljcm9zZWNvbmRzLCByZXN1bHRpbmcgaW4g YSBuZWdhdGl2ZSB2YWx1ZSBiZWluZworICAgICAgICBzZXQgYXMgdGhlIG5leHQgZmlyaW5nIHRp bWUgb2YgdGhlIEdNYWluTG9vcFNvdXJjZS4KKworICAgICAgICBUaGlzIGNhbiByZXN1bHQgaW4g YW4gZWFybHkgY2FsbGJhY2sgZXhlY3V0aW9uIGFuZCB3b3JzZW5zIHRoaW5ncyBpZiB0aGF0Cisg ICAgICAgIGNhbGxiYWNrIGNvbnRpbnVlcyB0byBiZSBzY2hlZHVsZWQgb24gdGhlIG1haW4gbG9v cCBpbiBhbiBleGNlc3NpdmUgYW1vdW50LgorCisgICAgICAgICogcGxhdGZvcm0vZ3RrL1NoYXJl ZFRpbWVyR3RrLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OnNldFNoYXJlZFRpbWVyRmlyZUludGVy dmFsKToKKworMjAxNS0wNS0xMiAgWmFuIERvYmVyc2VrICA8emRvYmVyc2VrQGlnYWxpYS5jb20+ CisKICAgICAgICAgTW92ZSBUcmFuc2Zvcm1PcGVyYXRpb24tYmFzZWQgY2xhc3NlcyBvZmYgb2Yg UGFzc1JlZlB0cgogICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9MTQ0ODc2CiAKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2d0ay9TaGFy ZWRUaW1lckd0ay5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ndGsvU2hhcmVkVGltZXJH dGsuY3BwCmluZGV4IDEzM2U5MDZmYTBmMDU4MDliNmE1MGEwMGU5OTE4YmRjMWQ2ODAzOTguLjQx ZmI5NGQ4MWY0ZjdkZDUyZDA3NjRkNWY2ZmFhZTkzZjRjMzczZmEgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL3BsYXRmb3JtL2d0ay9TaGFyZWRUaW1lckd0ay5jcHAKKysrIGIvU291cmNlL1dl YkNvcmUvcGxhdGZvcm0vZ3RrL1NoYXJlZFRpbWVyR3RrLmNwcApAQCAtNDIsMTQgKzQyLDE5IEBA IHZvaWQgc2V0U2hhcmVkVGltZXJGaXJlZEZ1bmN0aW9uKHZvaWQgKCpmKSgpKQogICAgICAgICBn U2hhcmVkVGltZXIuY2FuY2VsKCk7CiB9CiAKLXZvaWQgc2V0U2hhcmVkVGltZXJGaXJlSW50ZXJ2 YWwoZG91YmxlIGludGVydmFsKQordm9pZCBzZXRTaGFyZWRUaW1lckZpcmVJbnRlcnZhbChkb3Vi bGUgaW50ZXJ2YWxWYWx1ZSkKIHsKICAgICBBU1NFUlQoc2hhcmVkVGltZXJGaXJlZEZ1bmN0aW9u KTsKIAorICAgIC8vIFBhc3NlZC1pbiB2YWx1ZSBpcyBpbiBzZWNvbmRzLCB3aGljaCBtaWdodCBv dmVyZmxvdyB3aGVuIGNhc3RpbmcgdG8gc3RkOjpjaHJvbm86Om1pY3Jvc2Vjb25kcy4KKyAgICBh dXRvIGludGVydmFsID0gc3RkOjpjaHJvbm86OmR1cmF0aW9uPGRvdWJsZT4oaW50ZXJ2YWxWYWx1 ZSk7CisgICAgYXV0byBkZWxheSA9IHN0ZDo6Y2hyb25vOjptaWNyb3NlY29uZHM6Om1heCgpOwor ICAgIGlmIChpbnRlcnZhbCA8IGRlbGF5KQorICAgICAgICBkZWxheSA9IHN0ZDo6Y2hyb25vOjpk dXJhdGlvbl9jYXN0PHN0ZDo6Y2hyb25vOjptaWNyb3NlY29uZHM+KGludGVydmFsKTsKKwogICAg IC8vIFRoaXMgaXMgR0RLX1BSSU9SSVRZX1JFRFJBVywgYnV0IHdlIGRvbid0IHdhbnQgdG8gZGVw ZW5kIG9uIEdESyBoZXJlIGp1c3QgdG8gdXNlIGEgY29uc3RhbnQuCiAgICAgc3RhdGljIGNvbnN0 IGludCBwcmlvcml0eSA9IEdfUFJJT1JJVFlfSElHSF9JRExFICsgMjA7Ci0gICAgZ1NoYXJlZFRp bWVyLnNjaGVkdWxlQWZ0ZXJEZWxheSgiW1dlYktpdF0gc2hhcmVkVGltZXJUaW1lb3V0Q2FsbGJh Y2siLCBzdGQ6OmZ1bmN0aW9uPHZvaWQoKT4oc2hhcmVkVGltZXJGaXJlZEZ1bmN0aW9uKSwKLSAg ICAgICAgc3RkOjpjaHJvbm86OmR1cmF0aW9uX2Nhc3Q8c3RkOjpjaHJvbm86Om1pY3Jvc2Vjb25k cz4oc3RkOjpjaHJvbm86OmR1cmF0aW9uPGRvdWJsZT4oaW50ZXJ2YWwpKSwgcHJpb3JpdHkpOwor ICAgIGdTaGFyZWRUaW1lci5zY2hlZHVsZUFmdGVyRGVsYXkoIltXZWJLaXRdIHNoYXJlZFRpbWVy VGltZW91dENhbGxiYWNrIiwgc3RkOjpmdW5jdGlvbjx2b2lkKCk+KHNoYXJlZFRpbWVyRmlyZWRG dW5jdGlvbiksIGRlbGF5LCBwcmlvcml0eSk7CiB9CiAKIHZvaWQgc3RvcFNoYXJlZFRpbWVyKCkK