130286 2014-03-14 22:04:28 -0700 Fix build: using integer absolute value function 'abs' when argument is of floating point type 2014-03-15 16:37:51 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 ddkilzer ddkilzer fpizlo mhahnenberg msaboff oldest_to_newest 990819 0 ddkilzer 2014-03-14 22:04:28 -0700 Fix the following build failure when using trunk clang (r203900): JavaScriptCore/assembler/MacroAssembler.h:992:17: error: using integer absolute value function 'abs' when argument is of floating point type [-Werror,-Wabsolute-value] value = abs(value); ^ JavaScriptCore/assembler/MacroAssembler.h:992:17: note: use function 'fabs' instead value = abs(value); ^~~ fabs 1 error generated. 990820 1 226808 ddkilzer 2014-03-14 22:07:02 -0700 Created attachment 226808 Patch v1 990821 2 226808 fpizlo 2014-03-14 22:10:12 -0700 Comment on attachment 226808 Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=226808&action=review R=me to fix build. Leaving cq? alone in case you want to land the fabs() change instead. Calling abs() and casting to int means we miss a *bunch* of blinding opportunities. > Source/JavaScriptCore/assembler/MacroAssembler.h:992 > - value = abs(value); > + value = abs(static_cast<int>(value)); I strongly suspect that we meant to call fabs instead. 990858 3 ddkilzer 2014-03-15 07:29:09 -0700 (In reply to comment #2) > (From update of attachment 226808 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=226808&action=review > > R=me to fix build. Leaving cq? alone in case you want to land the fabs() change instead. This is for a trunk version of clang we're not compiling with yet, so no need to rush a fix. > Calling abs() and casting to int means we miss a *bunch* of blinding opportunities. > > > Source/JavaScriptCore/assembler/MacroAssembler.h:992 > > - value = abs(value); > > + value = abs(static_cast<int>(value)); > > I strongly suspect that we meant to call fabs instead. That's why I posted a patch for review. :) I'll switch to using fabs() before I land the patch. Thanks! 990893 4 ddkilzer 2014-03-15 15:25:42 -0700 Committed r165683: <http://trac.webkit.org/changeset/165683> 990899 5 226808 darin 2014-03-15 16:37:33 -0700 Comment on attachment 226808 Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=226808&action=review > Source/JavaScriptCore/ChangeLog:10 > + JavaScriptCore/assembler/MacroAssembler.h:992:17: error: using integer absolute value function 'abs' when argument is of floating point type [-Werror,-Wabsolute-value] I am *so* glad they added this warning. Use of abs instead of fabs is something I keep catching in patch review. 226808 2014-03-14 22:07:02 -0700 2014-03-15 16:37:51 -0700 Patch v1 bug-130286-20140314220639.patch text/plain 2053 ddkilzer U3VidmVyc2lvbiBSZXZpc2lvbjogMTY1NDAzCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCA1 MjQ1MmUzNDYzYmUyMDM1Y2Q2NzEyODAxYjI3YzQwYzc4NmU3YTM1Li41YWNmMGY0NzczYTM0M2Yz MzllYTdjMmZjOGU3NmJlMTdlYWUwNTEzIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwyNCBAQAorMjAxNC0wMy0xNCAgRGF2aWQgS2lsemVyICA8ZGRraWx6ZXJAYXBwbGUuY29t PgorCisgICAgICAgIEZpeCBidWlsZDogdXNpbmcgaW50ZWdlciBhYnNvbHV0ZSB2YWx1ZSBmdW5j dGlvbiAnYWJzJyB3aGVuIGFyZ3VtZW50IGlzIG9mIGZsb2F0aW5nIHBvaW50IHR5cGUKKyAgICAg ICAgPGh0dHA6Ly93ZWJraXQub3JnL2IvMTMwMjg2PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIEZpeGVzIHRoZSBmb2xsb3dpbmcgYnVpbGQgZmFpbHVy ZSB1c2luZyB0cnVuayBjbGFuZzoKKworICAgICAgICAgICAgSmF2YVNjcmlwdENvcmUvYXNzZW1i bGVyL01hY3JvQXNzZW1ibGVyLmg6OTkyOjE3OiBlcnJvcjogdXNpbmcgaW50ZWdlciBhYnNvbHV0 ZSB2YWx1ZSBmdW5jdGlvbiAnYWJzJyB3aGVuIGFyZ3VtZW50IGlzIG9mIGZsb2F0aW5nIHBvaW50 IHR5cGUgWy1XZXJyb3IsLVdhYnNvbHV0ZS12YWx1ZV0KKyAgICAgICAgICAgICAgICAgICAgdmFs dWUgPSBhYnModmFsdWUpOworICAgICAgICAgICAgICAgICAgICAgICAgICAgIF4KKyAgICAgICAg ICAgIEphdmFTY3JpcHRDb3JlL2Fzc2VtYmxlci9NYWNyb0Fzc2VtYmxlci5oOjk5MjoxNzogbm90 ZTogdXNlIGZ1bmN0aW9uICdmYWJzJyBpbnN0ZWFkCisgICAgICAgICAgICAgICAgICAgIHZhbHVl ID0gYWJzKHZhbHVlKTsKKyAgICAgICAgICAgICAgICAgICAgICAgICAgICBefn4KKyAgICAgICAg ICAgICAgICAgICAgICAgICAgICBmYWJzCisKKyAgICAgICAgKiBhc3NlbWJsZXIvTWFjcm9Bc3Nl bWJsZXIuaDoKKyAgICAgICAgKEpTQzo6TWFjcm9Bc3NlbWJsZXI6OnNob3VsZEJsaW5kRG91Ymxl KTogVXNlIHN0YXRpY19jYXN0PGludD4oKQorICAgICAgICB0byBjb252ZXJ0IHRoZSBhcmd1bWVu dCB0byBhYnMoKS4KKwogMjAxNC0wMy0xMCAgRmlsaXAgUGl6bG8gIDxmcGl6bG9AYXBwbGUuY29t PgogCiAgICAgICAgIGpzYyBjb21tYW5kbGluZSBzaG91bGRuJ3QgaGF2ZSBhICJjb25zb2xlIiBi ZWNhdXNlIHRoYXQgY29uZnVzZXMgc29tZSB0ZXN0cyBpbnRvIHRoaW5raW5nCmRpZmYgLS1naXQg YS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvYXNzZW1ibGVyL01hY3JvQXNzZW1ibGVyLmggYi9Tb3Vy Y2UvSmF2YVNjcmlwdENvcmUvYXNzZW1ibGVyL01hY3JvQXNzZW1ibGVyLmgKaW5kZXggNjBiZTYw MGRjNzQ5YTM4MDgxN2FlZjZjOWQwOTJlOTBhNzRmOWFmMy4uOWUzZjMwMzRhYjQ2OWUxZGU1Njgy MzFlNWFmMDVlNjY2MGUxZDQ1NCAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL2Fz c2VtYmxlci9NYWNyb0Fzc2VtYmxlci5oCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9hc3Nl bWJsZXIvTWFjcm9Bc3NlbWJsZXIuaApAQCAtOTg5LDcgKzk4OSw3IEBAIHB1YmxpYzoKICAgICAg ICAgaWYgKGJpdHdpc2VfY2FzdDx1aW50NjRfdD4odmFsdWUgKiAxLjApICE9IGJpdHdpc2VfY2Fz dDx1aW50NjRfdD4odmFsdWUpKQogICAgICAgICAgICAgcmV0dXJuIHNob3VsZENvbnNpZGVyQmxp bmRpbmcoKTsKIAotICAgICAgICB2YWx1ZSA9IGFicyh2YWx1ZSk7CisgICAgICAgIHZhbHVlID0g YWJzKHN0YXRpY19jYXN0PGludD4odmFsdWUpKTsKICAgICAgICAgLy8gT25seSBhbGxvdyBhIGxp bWl0ZWQgc2V0IG9mIGZyYWN0aW9uYWwgY29tcG9uZW50cwogICAgICAgICBkb3VibGUgc2NhbGVk VmFsdWUgPSB2YWx1ZSAqIDg7CiAgICAgICAgIGlmIChzY2FsZWRWYWx1ZSAvIDggIT0gdmFsdWUp Cg==