https://git.ghostscript.com/?p=mupdf.git;a=commit;h=82df2631d7d0446b206ea6b434ea609b6c28b0e8 The original fix contains this change: - if (i0 < 0 || i1 < 0) + if (i0 < 0 || i1 < 0 || (i0+i1) < 0) Since signed integer overflow is undefined behavior, compiler has the right to assume it won't happen. The previous i0 <0 || i1 < 0 check makes sure that i0 and i1 are both greater than zero, the last check then considered always true. That's what has happened when I were preparing update for Debian. If compiled with build=debug the reproducer file no longer crashes but if compiled with build=release, the reproducer still crashes. I ended up using this patch in Debian: - if (i0 < 0 || i1 < 0) + if (i0 < 0 || i1 < 0 || __builtin_add_overflow_p(i0, i1, (int)0))
(In reply to Kan-Ru Chen from comment #0) > https://git.ghostscript.com/?p=mupdf.git;a=commit; > h=82df2631d7d0446b206ea6b434ea609b6c28b0e8 > > The original fix contains this change: > > - if (i0 < 0 || i1 < 0) > + if (i0 < 0 || i1 < 0 || (i0+i1) < 0) > > Since signed integer overflow is undefined behavior, compiler has the right > to assume it won't happen. The previous i0 <0 || i1 < 0 check makes sure > that i0 and i1 are both greater than zero, the last check then considered > always true. I mean, always *false*.
So instead of || ((i0+i1) < 0 use: || ((i0+i1) & (1<<(8*sizeof(int)-1)) maybe? Yuck.
Perhaps: if (i0 < 0 || i1 < 0 || i0 > (INT_MAX - i1))
(In reply to Joseph Heenan from comment #3) > Perhaps: if (i0 < 0 || i1 < 0 || i0 > (INT_MAX - i1)) Ah, yes, that's nicer. Thanks.
I have a fix awaiting review.
Fixed in: commit d18bc728e46c5a5708f14d27c2b6c44e1d0c3232 Author: Robin Watts <robin.watts@artifex.com> Date: Tue Nov 7 17:53:40 2017 +0000 Bug 698704: Fix for overflow check failing due to 'clever' compiler. Adopt Josephs suggested fix for arithmetic overflow. Thanks to Kan-Ru Chen for spotting the problem.