[gs-bugs] [Bug 698722] - Ghostscript - Erroneous Range-Check causes wrong arithmetic results

[b***@artifex.com]

http://bugs.ghostscript.com/show_bug.cgi?id=698722

Bug ID: 698722
Summary: Erroneous Range-Check causes wrong arithmetic results
Product: Ghostscript
Version: master
Hardware: PC
OS: Linux
Status: UNCONFIRMED
Severity: normal
Priority: P4
Component: General
Assignee: ghostpdl-***@artifex.com
Reporter: bg+***@nerilex.org
QA Contact: gs-***@ghostscript.com
Word Size: ---

Created attachment 14436
--> http://bugs.ghostscript.com/attachment.cgi?id=14436&action=edit
patch to fix this issue

The Problem:

9223372036854775808 2 mul =

results in:

-9223372036854775808


During multiplication (and likely other opertaions), the result is checked to
fit inside the integer range by constructs like:

double ab = (double)op[-1].value.intval * op->value.intval;
if (ab > (double)MAX_PS_INT) /* (double)0x7fffffffffffffff */
make_real(op - 1, ab);
else if (ab < (double)MIN_PS_INT) /* (double)(int64_t)0x8000000000000000 */
make_real(op - 1, ab);
else
op[-1].value.intval = (ps_int)ab;

Due to the behaviour of C casts, we may get:

MAX_PS_INT == 9223372036854775807

and

(double)MAX_PS_INT == 9223372036854775808

Using

(double)((ps_int)ab) == ab

as check may be more helpful.

[b***@artifex.com]

http://bugs.ghostscript.com/show_bug.cgi?id=698722

--- Comment #1 from bg nerilex <bg+***@nerilex.org> ---
Created attachment 14437
--> http://bugs.ghostscript.com/attachment.cgi?id=14437&action=edit
postscript code to test the issue

[b***@artifex.com]

http://bugs.ghostscript.com/show_bug.cgi?id=698722

Ken Sharp <***@artifex.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Ever confirmed|0 |1
Status|UNCONFIRMED |IN_PROGRESS
Assignee|ghostpdl-***@artifex.com |***@artifex.com
Component|General |PS Interpreter

--- Comment #2 from Ken Sharp <***@artifex.com> ---
I see the problem, but your proposed patch has problems I'm afraid. Firstly
you;ve dropped the CPSI check, we need that to maintain compatibility with the
Adobe CPSI interpreter, which is essential for passing several test suites.

In addition, the cast to int and back only (I believe) works when ints are
32-bits. Because a 64-bt signed int has 63 bits of integer precision while a
double only has 52. This results in returning a real number more often than
necessary.

[b***@artifex.com]

http://bugs.ghostscript.com/show_bug.cgi?id=698722

--- Comment #3 from bg nerilex <bg+***@nerilex.org> ---
Hi,

in my test I only used powers of 2, probably this is why I did not trip over
the issue with 52-bit mantissa.

But that may already be a problem if the product of two integers does not fit
in 52 bits we may already loose precision (and convert back to integer with
reduced precision). I thought of using long double.

Sorry, for dropping the CPSI check. Probably I did not understand that stuff
entirely.

[b***@artifex.com]

http://bugs.ghostscript.com/show_bug.cgi?id=698722

--- Comment #4 from Ken Sharp <***@artifex.com> ---
(In reply to bg nerilex from comment #3)

Post by b***@artifex.com
Sorry, for dropping the CPSI check. Probably I did not understand that stuff
entirely.

There's no need to apologise for anything, I'm glad you pointed out the
existing problem.

I'm currently looking at ways to implement this check robustly and preferably
with as small a performance cost as possible. Essentially we simply need to
identify the overflow and return a real number instead of an integer. There are
a couple of techniques for this, but I need to implement and test one.

[b***@artifex.com]

http://bugs.ghostscript.com/show_bug.cgi?id=698722

Ken Sharp <***@artifex.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|IN_PROGRESS |RESOLVED
Resolution|--- |FIXED

--- Comment #5 from Ken Sharp <***@artifex.com> ---
Fixed in commit ea81364fdda57ce2c82007a150add5949a1c5788