Fixed 64-bit integer truncation issues in shader translator.

Fixed 64-bit integer truncation issues in shader translator.

This is an incompatible API change, but one which is necessary in
order to improve correctness of the code. The API version in
ShaderLang.h is updated and, unfortunately, the define renamed to
something less ambiguous due to conflicts on some Android buildbots.
Temporary patches in Chromium and WebKit will be landed separately to
support this upgrade.

BUG=403,404,405,406,407,408,409
Committed: https://code.google.com/p/angleproject/source/detail?r=1826

[kbr1, 2013-02-07 03:27:41]

Please review this change to fix 64-bit truncation issues throughout the shader
translator. Separate Chromium and WebKit changes are needed in order to support
upgrading ANGLE across this incompatible API change.

[jschuh, 2013-02-07 14:16:54]

lgtm on the security end with some minor non-security nits.

https://codereview.appspot.com/7300058/diff/1/include/GLSLANG/ShaderLang.h
File include/GLSLANG/ShaderLang.h (right):

https://codereview.appspot.com/7300058/diff/1/include/GLSLANG/ShaderLang.h#ne...
include/GLSLANG/ShaderLang.h:283: size_t numStrings,
Any reason for dropping const?

https://codereview.appspot.com/7300058/diff/1/src/compiler/ShHandle.h
File src/compiler/ShHandle.h (right):

https://codereview.appspot.com/7300058/diff/1/src/compiler/ShHandle.h#newcode64
src/compiler/ShHandle.h:64: size_t numStrings,
Any reason for dropping const?

https://codereview.appspot.com/7300058/diff/1/src/compiler/ShaderLang.cpp
File src/compiler/ShaderLang.cpp (right):

https://codereview.appspot.com/7300058/diff/1/src/compiler/ShaderLang.cpp#new...
src/compiler/ShaderLang.cpp:72: size_t activeUniformAndAttribLength = 1 +
MAX_SYMBOL_NAME_LEN;
Should be const for clarity?

https://codereview.appspot.com/7300058/diff/1/src/compiler/ShaderLang.cpp#new...
src/compiler/ShaderLang.cpp:79: size_t maxMappedNameLength = 1 +
MAX_SYMBOL_NAME_LEN;
same here.

[nicolas, 2013-02-07 16:21:58]

Looks fine otherwise.

https://codereview.appspot.com/7300058/diff/1/include/GLSLANG/ShaderLang.h
File include/GLSLANG/ShaderLang.h (right):

https://codereview.appspot.com/7300058/diff/1/include/GLSLANG/ShaderLang.h#ne...
include/GLSLANG/ShaderLang.h:283: size_t numStrings,
Since this is the number of strings, and not their size in memory or anything
like that, size_t seems massive overkill. There are several more cases like
this.

[Alok Priyadarshi, 2013-02-07 21:34:10]

lgtm

[kbr1, 2013-02-12 00:57:23]

https://codereview.appspot.com/7300058/diff/1/include/GLSLANG/ShaderLang.h
File include/GLSLANG/ShaderLang.h (right):

https://codereview.appspot.com/7300058/diff/1/include/GLSLANG/ShaderLang.h#ne...
include/GLSLANG/ShaderLang.h:283: size_t numStrings,
On 2013/02/07 14:16:54, jschuh wrote:
> Any reason for dropping const?

It is not very useful for scalar arguments such as integers. Also, the OpenGL
API glCompileShader, which this function mirrors, doesn't use it.

https://codereview.appspot.com/7300058/diff/1/include/GLSLANG/ShaderLang.h#ne...
include/GLSLANG/ShaderLang.h:283: size_t numStrings,
On 2013/02/07 16:21:59, nicolas wrote:
> Since this is the number of strings, and not their size in memory or anything
> like that, size_t seems massive overkill. There are several more cases like
> this.

I agree that a 64-bit value is overkill, but glShaderSource takes GLsizei for
this argument, and the point of this patch is to eliminate all integer
truncation issues in this API.

[nicolas, 2013-02-12 01:42:07]

On 2013/02/12 00:57:23, kbr1 wrote:
> It is not very useful for scalar arguments such as integers. Also, the OpenGL
> API glCompileShader, which this function mirrors, doesn't use it.
> 
> 
> I agree that a 64-bit value is overkill, but glShaderSource takes GLsizei for
> this argument, and the point of this patch is to eliminate all integer
> truncation issues in this API.

Ah, yes, makes sense to mirror the API functions. LGTM.