Bug 691876 - typo in imagesmooth.c ?
typo in imagesmooth.c ?
Product: MuPDF
Classification: Unclassified
Component: fitz
PC Windows 7
: P4 normal
Assigned To: Robin Watts
Bug traffic
Depends on:
  Show dependency treegraph
Reported: 2011-01-10 22:37 UTC by zeniko
Modified: 2011-01-17 11:46 UTC (History)
1 user (show)

See Also:
Word Size: ---

DEBUG_SCALING dump (753.89 KB, text/plain)
2011-01-12 14:20 UTC, zeniko

Note You need to log in before you can comment on or make changes to this bug.
Description zeniko 2011-01-10 22:37:27 UTC
Could http://code.google.com/p/sumatrapdf/source/diff?spec=svn2625&r=2625&format=side&path=/trunk/mupdf/draw/imagesmooth.c#sc_svn2614_928 be a typo? It's currently apparently possible for min to become less than src_w at line 928 which leads to a negative index at line 934 and thus a potential read access violation. Making that change didn't even regress anything in my test suite.
Comment 1 Robin Watts 2011-01-12 00:28:35 UTC
No, it's correct as is - or at least should be.

It shouldn't be possible for min to be less than src_w. If it is, then there is clearly something going on that I haven't considered.

If you are in a position to reproduce the crash, I'd very much like to know the values of the arguments to scale_single_col (and the values of the members of weights).

Or even better, build with 'DEBUG_SCALING' defined and get me a dump of the output for the scale operation that crashes.

Comment 2 zeniko 2011-01-12 14:20:14 UTC
Created attachment 7116 [details]

The attached dump has been produced while rendering the first page of the document downloadable from http://code.google.com/p/sumatrapdf/issues/detail?id=1162 .
Comment 3 zeniko 2011-01-12 14:25:43 UTC
BTW: You should be able to reproduce the potential read access violation yourself, even if it doesn't lead to a crash: just set a conditional breakpoint at line 928 on "(src_w-min)*n>src_w".
Comment 4 Robin Watts 2011-01-12 18:13:10 UTC
Having looked more closely at the code, I do:

   src_w = (src_w-1)*n;

earlier, hence src_w has already been multiplied by n. You are therefore absolutely correct, and I shouldn't be multiplying src_w by n again. Your fix was ideal. I've asked Tor to push it to the repo.

Thanks for this!
Comment 5 Robin Watts 2011-01-17 11:46:40 UTC
Tor has now recorded this patch, so closing. Thanks all.