Handle undefined shifts in CPDF_PSEngine.

Handle undefined shifts in CPDF_PSEngine.

BUG=chromium:641551

[Lei Zhang, 2016-10-12 17:00:54]

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

[commit-bot: I haz the power, 2016-10-12 17:01:02]

Dry run: 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-10-12 17:17:32]

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

[commit-bot: I haz the power, 2016-10-12 17:17:32]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2016-10-12 17:24:42]

thestig@chromium.org changed reviewers:
+ tsepez@chromium.org

[Tom Sepez, 2016-10-12 20:17:58]

https://codereview.chromium.org/2414473003/diff/1/core/fpdfapi/page/fpdf_page...
File core/fpdfapi/page/fpdf_page_func.cpp (right):

https://codereview.chromium.org/2414473003/diff/1/core/fpdfapi/page/fpdf_page...
core/fpdfapi/page/fpdf_page_func.cpp:346: if (shift < -31 || shift > 31)
http://en.cppreference.com/w/cpp/language/operator_arithmetic

Plucking out the C++11 sections, 

For signed and positive a, the value of a << b is a * 2b if it is representable
the return type, otherwise the behavior is undefined.	
For negative a, the behavior of a << b is undefined.

So there are still cases where this might be flagged.  It's pretty much
impossible to check this properly :( ...

The only way to do this, it would seem, is to use int64_t 
(which is how the safe_int packages handles this?)

[Lei Zhang, 2016-10-12 23:22:53]

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

[commit-bot: I haz the power, 2016-10-12 23:22:58]

Dry run: 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-10-12 23:29:47]

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

[commit-bot: I haz the power, 2016-10-12 23:29:48]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2016-10-12 23:42:14]

How's patch set 3?

[Tom Sepez, 2016-10-20 20:55:30]

let me see how I break it down

if (shift < -31) fail;
else if (shift < 0) i >> -shift is legal
else if (shift < 32) {
  if (i < 0) fail;
  else  { 
    ((int64_t)i) << shift is legal.
    if result fits in 32, fine else fail;
  }
} 
else fail

https://codereview.chromium.org/2414473003/diff/40001/core/fpdfapi/page/fpdf_...
File core/fpdfapi/page/fpdf_page_func.cpp (right):

https://codereview.chromium.org/2414473003/diff/40001/core/fpdfapi/page/fpdf_...
core/fpdfapi/page/fpdf_page_func.cpp:354: uint64_t unsigned_i = i;
think we want to do it as a signed64?

https://codereview.chromium.org/2414473003/diff/40001/core/fpdfapi/page/fpdf_...
core/fpdfapi/page/fpdf_page_func.cpp:358: unsigned_i >>= shift;
need a -shift here?

[Tom Sepez, 2016-10-20 21:01:57]

On 2016/10/20 20:55:30, Tom Sepez wrote:
> let me see how I break it down
> 
> if (shift < -31) fail;
> else if (shift < 0) i >> -shift is legal
> else if (shift < 32) {
>   if (i < 0) fail;
>   else  { 
>     ((int64_t)i) << shift is legal.
>     if result fits in 32, fine else fail;
>   }
> } 
> else fail
> 
>
https://codereview.chromium.org/2414473003/diff/40001/core/fpdfapi/page/fpdf_...
> File core/fpdfapi/page/fpdf_page_func.cpp (right):
> 
>
https://codereview.chromium.org/2414473003/diff/40001/core/fpdfapi/page/fpdf_...
> core/fpdfapi/page/fpdf_page_func.cpp:354: uint64_t unsigned_i = i;
> think we want to do it as a signed64?
> 
>
https://codereview.chromium.org/2414473003/diff/40001/core/fpdfapi/page/fpdf_...
> core/fpdfapi/page/fpdf_page_func.cpp:358: unsigned_i >>= shift;
> need a -shift here?

I think what we need to do is to implement safe << and >> for the integral types
in our safe math package.
Justin and I will take a stab at this, since no-one is ever going to get this
right on their own.

[Lei Zhang, 2016-10-20 21:07:39]

On 2016/10/20 21:01:57, Tom Sepez wrote:
> I think what we need to do is to implement safe << and >> for the integral
types
> in our safe math package.
> Justin and I will take a stab at this, since no-one is ever going to get this
> right on their own.

Sounds good. I'll just wait for that then.

[Tom Sepez, 2017-01-27 19:13:24]

On 2016/10/20 21:07:39, Lei Zhang (OOO) wrote:
> On 2016/10/20 21:01:57, Tom Sepez wrote:
> > I think what we need to do is to implement safe << and >> for the integral
> types
> > in our safe math package.
> > Justin and I will take a stab at this, since no-one is ever going to get
this
> > right on their own.
> 
> Sounds good. I'll just wait for that then.

Closing without landing, separate patch according to the discussion has landed.