gpu: Avoid integer overflow when setting up client side buffers

gpu: Avoid integer overflow when setting up client side buffers

Use gpu::SafeAddInt32 to calculate the number of elements in the buffers
whilst avoiding overflow. The code is also refactored slightly to make
the setup of buffers conditional on them being supported via the new
SupportsClientSideBuffers() check.

BUG=629072
TEST=gpu_unittests
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/d05fe0bb566dbffcef374f314403c340e4181711
Cr-Commit-Position: refs/heads/master@{#409172}

[robert.bradford, 2016-07-22 14:47:50]

Description was changed from

==========
gpu: Avoid integer overflow when setting up client side buffers

Use gpu::SafeAddInt32 to calculate the number of elements in the buffers
whilst avoiding overflow. The code is also refactored slightly to make
the setup of buffers conditional on them being supported via the new
SupportsClientSideBuffers() check.

BUG=629072
TEST=gpu_unittests
==========

to

==========
gpu: Avoid integer overflow when setting up client side buffers

Use gpu::SafeAddInt32 to calculate the number of elements in the buffers
whilst avoiding overflow. The code is also refactored slightly to make
the setup of buffers conditional on them being supported via the new
SupportsClientSideBuffers() check.

BUG=629072
TEST=gpu_unittests
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[robert.bradford, 2016-07-22 14:49:09]

robert.bradford@intel.com changed reviewers:
+ kbr@chromium.org

[robert.bradford, 2016-07-22 14:49:09]

Hi Ken, is this what you had in mind for
https://bugs.chromium.org/p/chromium/issues/detail?id=629072#c7

[Ken Russell (switch to Gerrit), 2016-08-02 01:17:55]

kbr@chromium.org changed reviewers:
+ piman@chromium.org, zmo@chromium.org

[Ken Russell (switch to Gerrit), 2016-08-02 01:17:55]

Robert: thanks very much for this patch. We were all at SIGGRAPH this past week,
so sorry for the delay.

I'm not an OWNER of this code, but yes, thanks, this LGTM.

[piman, 2016-08-02 02:54:53]

LGTM, thanks!

[robert.bradford, 2016-08-02 10:17:30]

The CQ bit was checked by robert.bradford@intel.com

[commit-bot: I haz the power, 2016-08-02 10:17:39]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-02 11:19:32]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-08-02 11:21:12]

Description was changed from

==========
gpu: Avoid integer overflow when setting up client side buffers

Use gpu::SafeAddInt32 to calculate the number of elements in the buffers
whilst avoiding overflow. The code is also refactored slightly to make
the setup of buffers conditional on them being supported via the new
SupportsClientSideBuffers() check.

BUG=629072
TEST=gpu_unittests
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
gpu: Avoid integer overflow when setting up client side buffers

Use gpu::SafeAddInt32 to calculate the number of elements in the buffers
whilst avoiding overflow. The code is also refactored slightly to make
the setup of buffers conditional on them being supported via the new
SupportsClientSideBuffers() check.

BUG=629072
TEST=gpu_unittests
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/d05fe0bb566dbffcef374f314403c340e4181711
Cr-Commit-Position: refs/heads/master@{#409172}
==========

[commit-bot: I haz the power, 2016-08-02 11:21:13]

Patchset 1 (id:??) landed as
https://crrev.com/d05fe0bb566dbffcef374f314403c340e4181711
Cr-Commit-Position: refs/heads/master@{#409172}

[Zhenyao Mo, 2016-08-04 17:40:14]

https://codereview.chromium.org/2177513002/diff/1/gpu/command_buffer/client/g...
File gpu/command_buffer/client/gles2_implementation.cc (right):

https://codereview.chromium.org/2177513002/diff/1/gpu/command_buffer/client/g...
gpu/command_buffer/client/gles2_implementation.cc:4568: SafeAddInt32(first,
count, &num_elements);
We should generate INVALID_VALUE if SafeAddInt32 returns false.

https://codereview.chromium.org/2177513002/diff/1/gpu/command_buffer/client/g...
gpu/command_buffer/client/gles2_implementation.cc:5585: SafeAddInt32(first,
count, &num_elements);
Same here.

[Zhenyao Mo, 2016-08-04 17:41:44]

On 2016/08/04 17:40:14, Zhenyao Mo wrote:
>
https://codereview.chromium.org/2177513002/diff/1/gpu/command_buffer/client/g...
> File gpu/command_buffer/client/gles2_implementation.cc (right):
> 
>
https://codereview.chromium.org/2177513002/diff/1/gpu/command_buffer/client/g...
> gpu/command_buffer/client/gles2_implementation.cc:4568: SafeAddInt32(first,
> count, &num_elements);
> We should generate INVALID_VALUE if SafeAddInt32 returns false.
> 
>
https://codereview.chromium.org/2177513002/diff/1/gpu/command_buffer/client/g...
> gpu/command_buffer/client/gles2_implementation.cc:5585: SafeAddInt32(first,
> count, &num_elements);
> Same here.

Sorry about the delayed after-the-fact review.  Just came back from Siggraph and
vacation.  I'll upload the CL to deal with the situation I pointed out.

Thanks for fixing the overflow.