[webkit-reviews] review denied: [Bug 59477] Loss of precission errors throughout the codebase spotted by GCC 4.6 : [Attachment 91136] precission.diff

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied Xan Lopez
<xan.lopez at gmail.com>'s request for review:
Bug 59477: Loss of precission errors throughout the codebase spotted by GCC 4.6
https://bugs.webkit.org/show_bug.cgi?id=59477

Attachment 91136: precission.diff
https://bugs.webkit.org/attachment.cgi?id=91136&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=91136&action=review

I think that this should be split into parts that domain experts could look at.


> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2102
> +    SwitchInfo info = { static_cast<uint32_t>(instructions().size()), type
};

I think that this patch is making things worse in that once this warning is
enabled in some other compiler, we won't see the issues. It's not obvious to me
that malicious code can't create 4 billion instructions.

> Source/JavaScriptCore/wtf/unicode/UTF8.cpp:236
> +static const UChar32 offsetsFromUTF8[6] = { 0x00000000UL, 0x00003080UL,
0x000E2080UL, 0x03C82080UL, static_cast<const UChar32>(0xFA082080UL),
static_cast<const UChar32>(0x82082080UL) };

Can this be fixed by not using UChar32? These are not Unicode characters, these
are magic numbers.

> Source/WebCore/plugins/PluginPackage.cpp:348
> +	   static_cast<unsigned int>(m_lastModified)

We don't use "unsigned int", we use "unsigned".