Bug 43358 – Fix warning in JavaScriptCore/wtf/ByteArray.h

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Csaba Osztrogonác]

../../../JavaScriptCore/wtf/ByteArray.h:66: warning: array subscript is above array bounds

unsigned char get(unsigned index) const
{
    ASSERT(index < m_size);
    return m_data[index];  < -- line 66
}

[Gabor Loki]

Created attachment 64018 [details]
Fix the array subscript is above array bounds warning in ByteArray on ARM

The proposed patch replaces the m_data array to a zero-sized one, in order to skip the 'array subscript is above array bounds' warning check. I think the reason of this warning should be very similar to this GCC's bug: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37861

This fix works fine with GCC. I hope the other build-bots will be happy as well ;)

[Geoffrey Garen]

Comment on attachment 64018 [details]
Fix the array subscript is above array bounds warning in ByteArray on ARM

r=me

[WebKit Review Bot]

Attachment 64018 [details] did not build on win:
Build output: http://queues.webkit.org/results/3734035

[Gabor Loki]

> Build output: http://queues.webkit.org/results/3734035

Well, not much to say about it. :D
Tomorrow I will get someone to test this patch on Win as well.

[Csaba Osztrogonác]

Comment on attachment 64018 [details]
Fix the array subscript is above array bounds warning in ByteArray on ARM

cq- -ed not to break windows build

----  Comment #3 From  WebKit Review Bot   2010-08-10 11:09:50 PST   (-) [reply] -------
Attachment 64018 [details] did not build on win:
Build output: http://queues.webkit.org/results/3734035

[Gabor Loki]

Comment on attachment 64018 [details]
Fix the array subscript is above array bounds warning in ByteArray on ARM

Unfortunately, the vc++ complains about the zero-sized array: http://msdn.microsoft.com/en-us/library/79wf64bc.aspx

[Gabor Loki]

Created attachment 64081 [details]
Fix the array subscript is above array bounds warning in ByteArray on ARM.

This patch uses the offsetof method to compute the static size of the structure for creation.

It looks like if the creation of an array is in the same context where the usage of the same array is unrolled, the GCC will shoot this false positive warning. I would like to create a small example which demonstrates this warning for the GCC community.

[Csaba Osztrogonác]

(In reply to comment #7)
> Created an attachment (id=64081) [details]
> Fix the array subscript is above array bounds warning in ByteArray on ARM.

It isn't an ARM related warning, but a general GCC related warning,
we can find it on all our bots. (x86-linux, x86-windows too)

[Gabor Loki]

Comment on attachment 64081 [details]
Fix the array subscript is above array bounds warning in ByteArray on ARM.

> It isn't an ARM related warning, but a general GCC related warning,
> we can find it on all our bots. (x86-linux, x86-windows too)

Okay, I will remove the ARM keyword before landing (cq- is set).

[Geoffrey Garen]

Comment on attachment 64081 [details]
Fix the array subscript is above array bounds warning in ByteArray on ARM.

r=me

[WebKit Commit Bot]

Comment on attachment 64081 [details]
Fix the array subscript is above array bounds warning in ByteArray on ARM.

Clearing flags on attachment: 64081

Committed r65260: <http://trac.webkit.org/changeset/65260>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

It still fails on Qt Linux Release bot:

../../../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

Gabor, could you check it tomorrow?

[Csaba Osztrogonác]

(In reply to comment #13)
> It still fails on Qt Linux Release bot:
> 
> ../../../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
> 
> Gabor, could you check it tomorrow?

Filed a new bug for it: https://bugs.webkit.org/show_bug.cgi?id=44672