Bug 69669 – WebKit2/Shared/APIClient.h complains that array subscript is below array bounds

• Component: WebKit2
• Status: NEW

[Rafael Brandao]

I'm getting the following warning trying to build WebKit2 (compiling Source/WebKit2/UIProcess/WebContext.cpp):

cc1plus: warnings being treated as errors
../../../Source/WebKit2/Shared/APIClient.h: In member function 'void WebKit::WebContext::initializeInjectedBundleClient(const WKContextInjectedBundleClient*)':
../../../Source/WebKit2/Shared/APIClient.h:52:13: error: array subscript is below array bounds

I'm using openSUSE 11.4 (x86_64) and g++ (SUSE Linux) 4.5.1 20101208 [gcc-4_5-branch revision 167585]. I wonder if someone else have the same problem.

[Rafael Brandao]

Created attachment 110225 [details]
Just ignored the warning on APIClient

[Darin Adler]

Comment on attachment 110225 [details]
Just ignored the warning on APIClient

View in context: https://bugs.webkit.org/attachment.cgi?id=110225&action=review

Could you be more specific about what the warning is? Maybe it’s fixable instead of something we need to turn off.

> Source/WebKit2/Shared/APIClient.h:40
> +#pragma GCC diagnostic ignored "-Warray-bounds"

I don’t think this is the best practice for using a compiler-specific pragma. In the past we have used #if around them.

> Source/WebKit2/Shared/APIClient.h:55
> +#pragma GCC diagnostic warning "-Warray-bounds"

It’s not good to unconditionally turn a warning on even if it wasn’t on before; instead you should use push/pop.

[Rafael Brandao]

Created attachment 110674 [details]
Added a check for non-negative value for array's index

I've googled a bit about this warning and I've found out that sometimes the compiler make some optimizations on the code and the access on arrays may be negative values after these changes (sources: http://www.mail-archive.com/[e-mail]/msg46397.html and http://gcc.gnu.org/ml/gcc/2009-09/msg00270.html). So I've just added a check for non-negative values for that index and it has stopped complaining. :)

[Alexis Menard (darktears)]

Comment on attachment 110674 [details]
Added a check for non-negative value for array's index

View in context: https://bugs.webkit.org/attachment.cgi?id=110674&action=review

> Source/WebKit2/Shared/APIClient.h:51
> +        if (client && client->version < currentVersion && client->version >= 0)

Can currentVersion be a unsigned?

[Alexis Menard (darktears)]

(In reply to comment #4)
> (From update of attachment 110674 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=110674&action=review
> 
> > Source/WebKit2/Shared/APIClient.h:51
> > +        if (client && client->version < currentVersion && client->version >= 0)
> 
> Can currentVersion be a unsigned?

And client->version obviously. Not sure if that's possible but it looks like it could be a better fix.

[Rafael Brandao]

(In reply to comment #4)
> (From update of attachment 110674 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=110674&action=review
> 
> > Source/WebKit2/Shared/APIClient.h:51
> > +        if (client && client->version < currentVersion && client->version >= 0)
> 
> Can currentVersion be a unsigned?

I've tried to change it to be unsigned, but then the compiler started to complain about comparison of unsigned expression < 0 being always false. The expression it is talking about is "client->version < currentVersion" when currentVersion is 0 which is possible. So I would just get into another warning.

[Anders Carlsson]

Comment on attachment 110674 [details]
Added a check for non-negative value for array's index

Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.