gcc warning: ../../../JavaScriptCore/wtf/ByteArray.h: In member function ‘bool WebCore::FELighting::drawLighting(WebCore::CanvasPixelArray*, int, int)’: ../../../JavaScriptCore/wtf/ByteArray.h:66: warning: array subscript is above array bounds inlined getter functions used in drawLighting() cause this warning: ALWAYS_INLINE int upLeftPixelValue(); ALWAYS_INLINE int upPixelValue(); ALWAYS_INLINE int upRightPixelValue(); ALWAYS_INLINE int leftPixelValue(); ALWAYS_INLINE int centerPixelValue(); ALWAYS_INLINE int rightPixelValue(); ALWAYS_INLINE int downLeftPixelValue(); ALWAYS_INLINE int downPixelValue(); ALWAYS_INLINE int downRightPixelValue(); eg: ALWAYS_INLINE int FELighting::LightingData::upLeftPixelValue() { return static_cast<int>(pixels->get(offset - widthMultipliedByPixelSize - cPixelSize + cAlphaChannelOffset)); } How do you guarantee avoiding incorrect array subscripts? Zoltan, could you check and fix this bug, please? It would be the last step before enabling -Werror for QtWebKit x86-Linux build.
Created attachment 66218 [details] possibly fix possibly fix for EWS bots without changelog, please don't review
> possibly fix for EWS bots without changelog, please don't review It is not a good solution. This will fail with msvc! See: http://msdn.microsoft.com/en-us/library/79wf64bc.aspx
(In reply to comment #2) > > possibly fix for EWS bots without changelog, please don't review > > It is not a good solution. This will fail with msvc! > See: http://msdn.microsoft.com/en-us/library/79wf64bc.aspx It is an other idea, I try unsized array instead of zero sized. It should work now. I hope. :)
Practically it is the same trick. MSVC 6.0 and MSVC 2003 say the same C4200 warning on it. I've tried it before. Only the MSVC 2010 allows something: http://msdn.microsoft.com/en-us/library/b6fae073.aspx So, I think it is not a good idea to use an unsized array with several compiler specific guards in order to avoid a bogus warning of GCC.
Attachment 66218 [details] did not build on win: Build output: http://queues.webkit.org/results/3939031
(In reply to comment #4) > Practically it is the same trick. > MSVC 6.0 and MSVC 2003 say the same C4200 warning on it. I've tried it before. > > Only the MSVC 2010 allows something: http://msdn.microsoft.com/en-us/library/b6fae073.aspx > > So, I think it is not a good idea to use an unsized array with several compiler specific guards in order to avoid a bogus warning of GCC. Unsized arrays are supported by all MSVC * Visual Studio 2010 * Visual Studio 2008 * Visual Studio 2005 * Visual Studio .NET 2003 But http://webkit.org/building/tools.html say that _only_ VS2005 is supported for building WebKit. Additionally the C99 standard [ISO/IEC 9899:1999] introduces flexible array members into the language, it should be supported by all compiler. Unfortunately I don't know what the problem is on Win EWS until https://bugs.webkit.org/show_bug.cgi?id=39199 is fixed.
Created attachment 66300 [details] possibly hack to make MSVC and GCC happy Test for EWS bots, without changelog, please don't review.
Ossy, I suggest you to use the following form (or a similar one) to avoid the warnings: +#if COMPILER(MSVC) +// These compilers do not support zero-sized array +#define DECLARATION_SIZE_OF_BYTEARRAY 0xffff +#else +#define DECLARATION_SIZE_OF_BYTEARRAY 0 +#endif ... -unsigned char m_data[sizeof(size_t)]; +unsigned char m_data[DECLARATION_SIZE_OF_BYTEARRAY];
Comment on attachment 66300 [details] possibly hack to make MSVC and GCC happy Status bubbles are green, I'll add a changlog and upload again.
Created attachment 66348 [details] possibly fix for Win-EWS warning C4200: nonstandard extension used : zero-sized array in struct/union Cannot generate copy-ctor or copy-assignment operator when UDT contains a zero-sized array I think a possibly fix is defining private copy constructor and private operator= . MSVC can't generate them, I think nobody can generate correct copy constructor for this class. So we can implement them if we need it. I think we shouldn't copying a ByteArray object, so I prefer never used functions. Please don't review it, I would like to wait Windows EWS. If it will be green, I'll upload the patch with changelog for review.
Attachment 66348 [details] did not build on mac: Build output: http://queues.webkit.org/results/3927051
Comment on attachment 66348 [details] possibly fix for Win-EWS I'm going to fix it on Mac.
Created attachment 66364 [details] proposed fix First, I tried to use defined but not implemented copy constructor and operator=. And then I tried to implement them with ASSERT_NOT_REACHED(), but MSVC still warns. I propose to use maximal array size for MSVC and unsized array for other compilers. It won't cause memory problems, because you can create a ByteArray only with its create method, which absolutely ignores the sized of m_data array.
Comment on attachment 66364 [details] proposed fix Clearing flags on attachment: 66364 Committed r66729: <http://trac.webkit.org/changeset/66729>
All reviewed patches have been landed. Closing bug.