search

ryanqq / openjpeg

Automatically exported from code.google.com/p/openjpeg
Other
0 stars 0 forks source link

inline division might be wrong #231

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago 
There are certain functions and lines of code that do not properly implement 
operations like divisions. As it is in the source now, things will work for all 
values up to 31 bits, but it will break at the border cases.

Example 1:
static INLINE OPJ_UINT32  opj_uint_ceildiv(OPJ_UINT32  a, OPJ_UINT32  b) {
    return (a + b - 1) / b;
}

This looks perfectly fine at first sight, however when a and b are larger than 
31-bit the addition will overflow and cycle around. The subsequent division 
will be wrong. In plain numbers and using uint8 to demonstrate, suppose a = 250 
and b = 50. The division should yield 5 as result. However, in uint8 arithmetic 
the result becomes 0 (250 + 50 overflows to 44). The same things can happen 
with uint32.

I used the following code:
inline static uint32 UINT32_CEIL_DIV(const uint32 a, const uint32 b) { return 
(a / b) + (((a % b) == 0)?0:1); }

This should be correct, but it has the drawback that it requires a branch 
instruction, which might be less performant (but correct nevertheless).

Original issue reported on code.google.com by anto...@gmail.com on 16 Jul 2013 at 1:02

GoogleCodeExporter commented 9 years ago 
(from tim bruylants)

Original comment by anto...@gmail.com on 16 Jul 2013 at 1:13

GoogleCodeExporter commented 9 years ago 
This is interesting and could be demonstrated by:
https://code.google.com/p/openjpeg/issues/detail?id=225#c42

However I am not able to do it from my main machine. will continue exploring.

Original comment by mathieu.malaterre on 27 Feb 2014 at 2:47

GoogleCodeExporter commented 9 years ago

Original comment by mathieu.malaterre on 4 Mar 2014 at 9:15

GoogleCodeExporter commented 9 years ago 
I could not reproduce the actual segfault on any machine, but on 32bits 
machine, I could see some weird stuff going on without patch:

http://git.ghostscript.com/?p=user/zeniko/ghostpdl.git;a=commitdiff;h=ea6e58fa7b
f818b917b22c2b2652fcb9b684a925

Original comment by mathieu.malaterre on 14 Mar 2014 at 11:54

GoogleCodeExporter commented 9 years ago 
This issue was updated by revision r2733.

Original comment by mathieu.malaterre on 14 Mar 2014 at 11:59