Fix FMCallExpression undefined shift behaviour.

Fix FMCallExpression undefined shift behaviour.

When determining which params should be an object and which are a value it is
possible to overflow the int on the shift comparision (if there are more then
32 arguments).

This never happens in practise as it's a controlled list of method calls which
we pass objects for. Cap the check at 32 for the shifting so it doesn't
overflow. We can revisit and extend the value later if we ever have an internal
formcalc method that needs an object in a position greater then 32.

BUG=chromium:603490
Committed: https://pdfium.googlesource.com/pdfium/+/e85f971fe9ba628e46bcb0709d5da4368c15d0b0

[dsinclair, 2016-08-03 14:54:12]

The CQ bit was checked by dsinclair@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-03 14:54:17]

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

[dsinclair, 2016-08-03 14:55:14]

dsinclair@chromium.org changed reviewers:
+ thestig@chromium.org, weili@chromium.org

[dsinclair, 2016-08-03 14:55:15]

PTAL.

https://codereview.chromium.org/2206253002/diff/1/xfa/fxfa/fm2js/xfa_simpleex...
File xfa/fxfa/fm2js/xfa_simpleexpression_unittest.cpp (right):

https://codereview.chromium.org/2206253002/diff/1/xfa/fxfa/fm2js/xfa_simpleex...
xfa/fxfa/fm2js/xfa_simpleexpression_unittest.cpp:14: new
CXFA_FMIdentifierExpressionn(0, CFX_WideStringC(L"sign"));
I'll fix the double n's in a followup.

[commit-bot: I haz the power, 2016-08-03 15:05:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-03 15:05:39]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2016-08-03 16:43:23]

https://codereview.chromium.org/2206253002/diff/1/xfa/fxfa/fm2js/xfa_simpleex...
File xfa/fxfa/fm2js/xfa_simpleexpression.cpp (right):

https://codereview.chromium.org/2206253002/diff/1/xfa/fxfa/fm2js/xfa_simpleex...
xfa/fxfa/fm2js/xfa_simpleexpression.cpp:543: for (int i = 0; i <
m_pArguments->GetSize(); ++i) {
So is it the case that m_pArguments->GetSize() can be over 32 normally?

https://codereview.chromium.org/2206253002/diff/1/xfa/fxfa/fm2js/xfa_simpleex...
File xfa/fxfa/fm2js/xfa_simpleexpression_unittest.cpp (right):

https://codereview.chromium.org/2206253002/diff/1/xfa/fxfa/fm2js/xfa_simpleex...
xfa/fxfa/fm2js/xfa_simpleexpression_unittest.cpp:8: 
extra blank line?

https://codereview.chromium.org/2206253002/diff/1/xfa/fxfa/fm2js/xfa_simpleex...
xfa/fxfa/fm2js/xfa_simpleexpression_unittest.cpp:13:
CXFA_FMIdentifierExpressionn* exp =
std::unique_ptr + release() below?

And some day, std::move()...

https://codereview.chromium.org/2206253002/diff/1/xfa/fxfa/fm2js/xfa_simpleex...
xfa/fxfa/fm2js/xfa_simpleexpression_unittest.cpp:16:
CFX_ArrayTemplate<CXFA_FMSimpleExpression*>* args =
Ditto.

[dsinclair, 2016-08-03 16:55:28]

The CQ bit was checked by dsinclair@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-03 16:55:31]

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

[dsinclair, 2016-08-03 16:55:36]

https://codereview.chromium.org/2206253002/diff/1/xfa/fxfa/fm2js/xfa_simpleex...
File xfa/fxfa/fm2js/xfa_simpleexpression.cpp (right):

https://codereview.chromium.org/2206253002/diff/1/xfa/fxfa/fm2js/xfa_simpleex...
xfa/fxfa/fm2js/xfa_simpleexpression.cpp:543: for (int i = 0; i <
m_pArguments->GetSize(); ++i) {
On 2016/08/03 16:43:23, Lei Zhang wrote:
> So is it the case that m_pArguments->GetSize() can be over 32 normally?

The spec for formcalc doesn't mention a limit on the number of arguments just
that there are required and optional arguments. I left the ability to have more
then 32 in because I wasn't sure if there were methods were you could pass n
optional arguments and have it use them.

https://codereview.chromium.org/2206253002/diff/1/xfa/fxfa/fm2js/xfa_simpleex...
File xfa/fxfa/fm2js/xfa_simpleexpression_unittest.cpp (right):

https://codereview.chromium.org/2206253002/diff/1/xfa/fxfa/fm2js/xfa_simpleex...
xfa/fxfa/fm2js/xfa_simpleexpression_unittest.cpp:8: 
On 2016/08/03 16:43:23, Lei Zhang wrote:
> extra blank line?

Done.

https://codereview.chromium.org/2206253002/diff/1/xfa/fxfa/fm2js/xfa_simpleex...
xfa/fxfa/fm2js/xfa_simpleexpression_unittest.cpp:13:
CXFA_FMIdentifierExpressionn* exp =
On 2016/08/03 16:43:23, Lei Zhang wrote:
> std::unique_ptr + release() below?
> 
> And some day, std::move()...

Done.

https://codereview.chromium.org/2206253002/diff/1/xfa/fxfa/fm2js/xfa_simpleex...
xfa/fxfa/fm2js/xfa_simpleexpression_unittest.cpp:16:
CFX_ArrayTemplate<CXFA_FMSimpleExpression*>* args =
On 2016/08/03 16:43:23, Lei Zhang wrote:
> Ditto.

Done.

[Lei Zhang, 2016-08-03 16:57:58]

lgtm

[dsinclair, 2016-08-03 17:00:47]

The CQ bit was unchecked by dsinclair@chromium.org

[dsinclair, 2016-08-03 17:00:47]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2016-08-03 17:00:51]

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-03 17:08:18]

Description was changed from

==========
Fix FMCallExpression undefined shift behaviour.

When determining which params should be an object and which are a value it is
possible to overflow the int on the shift comparision (if there are more then
32 arguments).

This never happens in practise as it's a controlled list of method calls which
we pass objects for. Cap the check at 32 for the shifting so it doesn't
overflow. We can revisit and extend the value later if we ever have an internal
formcalc method that needs an object in a position greater then 32.

BUG=chromium:603490
==========

to

==========
Fix FMCallExpression undefined shift behaviour.

When determining which params should be an object and which are a value it is
possible to overflow the int on the shift comparision (if there are more then
32 arguments).

This never happens in practise as it's a controlled list of method calls which
we pass objects for. Cap the check at 32 for the shifting so it doesn't
overflow. We can revisit and extend the value later if we ever have an internal
formcalc method that needs an object in a position greater then 32.

BUG=chromium:603490

Committed:
https://pdfium.googlesource.com/pdfium/+/e85f971fe9ba628e46bcb0709d5da4368c15...
==========

[commit-bot: I haz the power, 2016-08-03 17:08:19]

Committed patchset #2 (id:20001) as
https://pdfium.googlesource.com/pdfium/+/e85f971fe9ba628e46bcb0709d5da4368c15...