revise arithmetic with const

there's a problem with out of bounds values, for example:

	vips relational_const k2.jpg x.v equal 1000

actually finds pixels == 255, since 1000 is saturated converted to 255
before the test starts.

This patch reworks arithmetic against const values to fix this.
This commit is contained in:
John Cupitt 2019-08-21 10:35:48 +01:00
parent 1de2947d51
commit ed2054dbbc
7 changed files with 100 additions and 163 deletions

View File

@ -9,6 +9,8 @@
- loaders use "minimise" to close input files earlier - loaders use "minimise" to close input files earlier
- integrate support for oss-fuzz [omira-sch] - integrate support for oss-fuzz [omira-sch]
- add vips_switch() / vips_case() ... fast many-way ifthenelse - add vips_switch() / vips_case() ... fast many-way ifthenelse
- better const handling for arithmetic operators fixes comparisons against out
of range values
9/7/19 started 8.8.2 9/7/19 started 8.8.2
- better early shutdown in readers - better early shutdown in readers

View File

@ -458,14 +458,11 @@ vips_boolean_const_build( VipsObject *object )
{ {
VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( object ); VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( object );
VipsUnary *unary = (VipsUnary *) object; VipsUnary *unary = (VipsUnary *) object;
VipsUnaryConst *uconst = (VipsUnaryConst *) object;
if( unary->in && if( unary->in &&
vips_check_noncomplex( class->nickname, unary->in ) ) vips_check_noncomplex( class->nickname, unary->in ) )
return( -1 ); return( -1 );
uconst->const_format = VIPS_FORMAT_INT;
if( VIPS_OBJECT_CLASS( vips_boolean_const_parent_class )-> if( VIPS_OBJECT_CLASS( vips_boolean_const_parent_class )->
build( object ) ) build( object ) )
return( -1 ); return( -1 );
@ -474,9 +471,9 @@ vips_boolean_const_build( VipsObject *object )
} }
#define LOOPC( TYPE, OP ) { \ #define LOOPC( TYPE, OP ) { \
TYPE *p = (TYPE *) in[0]; \ TYPE * restrict p = (TYPE *) in[0]; \
TYPE *q = (TYPE *) out; \ TYPE * restrict q = (TYPE *) out; \
int *c = (int *) uconst->c_ready; \ int * restrict c = uconst->c_int; \
\ \
for( i = 0, x = 0; x < width; x++ ) \ for( i = 0, x = 0; x < width; x++ ) \
for( b = 0; b < bands; b++, i++ ) \ for( b = 0; b < bands; b++, i++ ) \
@ -484,9 +481,9 @@ vips_boolean_const_build( VipsObject *object )
} }
#define FLOOPC( TYPE, OP ) { \ #define FLOOPC( TYPE, OP ) { \
TYPE *p = (TYPE *) in[0]; \ TYPE * restrict p = (TYPE *) in[0]; \
int *q = (int *) out; \ int * restrict q = (int *) out; \
int *c = (int *) uconst->c_ready; \ int * restrict c = uconst->c_int; \
\ \
for( i = 0, x = 0; x < width; x++ ) \ for( i = 0, x = 0; x < width; x++ ) \
for( b = 0; b < bands; b++, i++ ) \ for( b = 0; b < bands; b++, i++ ) \

View File

@ -338,14 +338,11 @@ vips_math2_const_build( VipsObject *object )
{ {
VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( object ); VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( object );
VipsUnary *unary = (VipsUnary *) object; VipsUnary *unary = (VipsUnary *) object;
VipsUnaryConst *uconst = (VipsUnaryConst *) object;
if( unary->in && if( unary->in &&
vips_check_noncomplex( class->nickname, unary->in ) ) vips_check_noncomplex( class->nickname, unary->in ) )
return( -1 ); return( -1 );
uconst->const_format = VIPS_FORMAT_DOUBLE;
if( VIPS_OBJECT_CLASS( vips_math2_const_parent_class )-> if( VIPS_OBJECT_CLASS( vips_math2_const_parent_class )->
build( object ) ) build( object ) )
return( -1 ); return( -1 );
@ -356,7 +353,7 @@ vips_math2_const_build( VipsObject *object )
#define LOOPC( IN, OUT, OP ) { \ #define LOOPC( IN, OUT, OP ) { \
IN * restrict p = (IN *) in[0]; \ IN * restrict p = (IN *) in[0]; \
OUT * restrict q = (OUT *) out; \ OUT * restrict q = (OUT *) out; \
double * restrict c = (double *) uconst->c_ready; \ double * restrict c = uconst->c_double; \
\ \
for( i = 0, x = 0; x < width; x++ ) \ for( i = 0, x = 0; x < width; x++ ) \
for( b = 0; b < bands; b++, i++ ) \ for( b = 0; b < bands; b++, i++ ) \

View File

@ -457,25 +457,18 @@ typedef VipsUnaryConstClass VipsRelationalConstClass;
G_DEFINE_TYPE( VipsRelationalConst, G_DEFINE_TYPE( VipsRelationalConst,
vips_relational_const, VIPS_TYPE_UNARY_CONST ); vips_relational_const, VIPS_TYPE_UNARY_CONST );
static int #define RLOOPCI( TYPE, OP ) { \
vips_relational_const_build( VipsObject *object ) TYPE * restrict p = (TYPE *) in[0]; \
{ int * restrict c = uconst->c_int; \
VipsUnary *unary = (VipsUnary *) object; \
VipsUnaryConst *uconst = (VipsUnaryConst *) object; for( i = 0, x = 0; x < width; x++ ) \
for( b = 0; b < bands; b++, i++ ) \
if( unary->in ) out[i] = (p[i] OP c[b]) ? 255 : 0; \
uconst->const_format = unary->in->BandFmt;
if( VIPS_OBJECT_CLASS( vips_relational_const_parent_class )->
build( object ) )
return( -1 );
return( 0 );
} }
#define RLOOPC( TYPE, OP ) { \ #define RLOOPCF( TYPE, OP ) { \
TYPE * restrict p = (TYPE *) in[0]; \ TYPE * restrict p = (TYPE *) in[0]; \
TYPE * restrict c = (TYPE *) uconst->c_ready; \ double * restrict c = uconst->c_double; \
\ \
for( i = 0, x = 0; x < width; x++ ) \ for( i = 0, x = 0; x < width; x++ ) \
for( b = 0; b < bands; b++, i++ ) \ for( b = 0; b < bands; b++, i++ ) \
@ -486,7 +479,7 @@ vips_relational_const_build( VipsObject *object )
TYPE * restrict p = (TYPE *) in[0]; \ TYPE * restrict p = (TYPE *) in[0]; \
\ \
for( i = 0, x = 0; x < width; x++ ) { \ for( i = 0, x = 0; x < width; x++ ) { \
TYPE * restrict c = (TYPE *) uconst->c_ready; \ double * restrict c = uconst->c_double; \
\ \
for( b = 0; b < bands; b++, i++ ) { \ for( b = 0; b < bands; b++, i++ ) { \
out[i] = OP( p[0], p[1], c[0], c[1]) ? 255 : 0; \ out[i] = OP( p[0], p[1], c[0], c[1]) ? 255 : 0; \
@ -505,32 +498,64 @@ vips_relational_const_buffer( VipsArithmetic *arithmetic,
VipsRelationalConst *rconst = (VipsRelationalConst *) arithmetic; VipsRelationalConst *rconst = (VipsRelationalConst *) arithmetic;
VipsImage *im = arithmetic->ready[0]; VipsImage *im = arithmetic->ready[0];
int bands = im->Bands; int bands = im->Bands;
gboolean is_int = uconst->is_int &&
vips_band_format_isint( im->BandFmt );
int i, x, b; int i, x, b;
switch( rconst->relational ) { switch( rconst->relational ) {
case VIPS_OPERATION_RELATIONAL_EQUAL: case VIPS_OPERATION_RELATIONAL_EQUAL:
SWITCH( RLOOPC, CLOOPC, ==, CEQUAL ); if( is_int ) {
SWITCH( RLOOPCI, CLOOPC, ==, CEQUAL );
}
else {
SWITCH( RLOOPCF, CLOOPC, ==, CEQUAL );
}
break; break;
case VIPS_OPERATION_RELATIONAL_NOTEQ: case VIPS_OPERATION_RELATIONAL_NOTEQ:
SWITCH( RLOOPC, CLOOPC, !=, CNOTEQ ); if( is_int ) {
SWITCH( RLOOPCI, CLOOPC, !=, CNOTEQ );
}
else {
SWITCH( RLOOPCF, CLOOPC, !=, CNOTEQ );
}
break; break;
case VIPS_OPERATION_RELATIONAL_LESS: case VIPS_OPERATION_RELATIONAL_LESS:
SWITCH( RLOOPC, CLOOPC, <, CLESS ); if( is_int ) {
SWITCH( RLOOPCI, CLOOPC, <, CLESS );
}
else {
SWITCH( RLOOPCF, CLOOPC, <, CLESS );
}
break; break;
case VIPS_OPERATION_RELATIONAL_LESSEQ: case VIPS_OPERATION_RELATIONAL_LESSEQ:
SWITCH( RLOOPC, CLOOPC, <=, CLESSEQ ); if( is_int ) {
SWITCH( RLOOPCI, CLOOPC, <=, CLESSEQ );
}
else {
SWITCH( RLOOPCF, CLOOPC, <=, CLESSEQ );
}
break; break;
case VIPS_OPERATION_RELATIONAL_MORE: case VIPS_OPERATION_RELATIONAL_MORE:
SWITCH( RLOOPC, CLOOPC, >, CMORE ); if( is_int ) {
SWITCH( RLOOPCI, CLOOPC, >, CMORE );
}
else {
SWITCH( RLOOPCF, CLOOPC, >, CMORE );
}
break; break;
case VIPS_OPERATION_RELATIONAL_MOREEQ: case VIPS_OPERATION_RELATIONAL_MOREEQ:
SWITCH( RLOOPC, CLOOPC, >=, CMOREEQ ); if( is_int ) {
SWITCH( RLOOPCI, CLOOPC, >=, CMOREEQ );
}
else {
SWITCH( RLOOPCF, CLOOPC, >=, CMOREEQ );
}
break; break;
default: default:
@ -551,7 +576,6 @@ vips_relational_const_class_init( VipsRelationalConstClass *class )
object_class->nickname = "relational_const"; object_class->nickname = "relational_const";
object_class->description = object_class->description =
_( "relational operations against a constant" ); _( "relational operations against a constant" );
object_class->build = vips_relational_const_build;
aclass->process_line = vips_relational_const_buffer; aclass->process_line = vips_relational_const_buffer;

View File

@ -238,15 +238,11 @@ vips_remainder_const_build( VipsObject *object )
{ {
VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( object ); VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( object );
VipsUnary *unary = (VipsUnary *) object; VipsUnary *unary = (VipsUnary *) object;
VipsUnaryConst *uconst = (VipsUnaryConst *) object;
if( unary->in && if( unary->in &&
vips_check_noncomplex( class->nickname, unary->in ) ) vips_check_noncomplex( class->nickname, unary->in ) )
return( -1 ); return( -1 );
if( unary->in )
uconst->const_format = unary->in->BandFmt;
if( VIPS_OBJECT_CLASS( vips_remainder_const_parent_class )-> if( VIPS_OBJECT_CLASS( vips_remainder_const_parent_class )->
build( object ) ) build( object ) )
return( -1 ); return( -1 );
@ -259,7 +255,7 @@ vips_remainder_const_build( VipsObject *object )
#define IREMAINDERCONST( TYPE ) { \ #define IREMAINDERCONST( TYPE ) { \
TYPE * restrict p = (TYPE *) in[0]; \ TYPE * restrict p = (TYPE *) in[0]; \
TYPE * restrict q = (TYPE *) out; \ TYPE * restrict q = (TYPE *) out; \
TYPE * restrict c = (TYPE *) uconst->c_ready; \ int * restrict c = uconst->c_int; \
\ \
for( i = 0, x = 0; x < width; x++ ) \ for( i = 0, x = 0; x < width; x++ ) \
for( b = 0; b < bands; b++, i++ ) \ for( b = 0; b < bands; b++, i++ ) \
@ -271,7 +267,7 @@ vips_remainder_const_build( VipsObject *object )
#define FREMAINDERCONST( TYPE ) { \ #define FREMAINDERCONST( TYPE ) { \
TYPE * restrict p = (TYPE *) in[0]; \ TYPE * restrict p = (TYPE *) in[0]; \
TYPE * restrict q = (TYPE *) out; \ TYPE * restrict q = (TYPE *) out; \
TYPE * restrict c = (TYPE *) uconst->c_ready; \ int * restrict c = uconst->c_int; \
\ \
for( i = 0, x = 0; x < width; x++ ) \ for( i = 0, x = 0; x < width; x++ ) \
for( b = 0; b < bands; b++, i++ ) { \ for( b = 0; b < bands; b++, i++ ) { \

View File

@ -49,100 +49,6 @@
G_DEFINE_ABSTRACT_TYPE( VipsUnaryConst, vips_unary_const, VIPS_TYPE_UNARY ); G_DEFINE_ABSTRACT_TYPE( VipsUnaryConst, vips_unary_const, VIPS_TYPE_UNARY );
/* Cast a vector of double to a vector of TYPE, clipping to a range.
*/
#define CAST_CLIP( TYPE, N, X ) { \
TYPE * restrict tq = (TYPE *) q; \
\
for( i = 0; i < m; i++ ) { \
double v = p[VIPS_MIN( n - 1, i )]; \
\
tq[i] = (TYPE) VIPS_FCLIP( N, v, X ); \
} \
}
/* Cast a vector of double to a vector of TYPE.
*/
#define CAST( TYPE ) { \
TYPE * restrict tq = (TYPE *) q; \
\
for( i = 0; i < m; i++ ) \
tq[i] = (TYPE) p[VIPS_MIN( n - 1, i )]; \
}
/* Cast a vector of double to a complex vector of TYPE.
*/
#define CASTC( TYPE ) { \
TYPE * restrict tq = (TYPE *) q; \
\
for( i = 0; i < m; i++ ) { \
tq[0] = (TYPE) p[VIPS_MIN( n - 1, i )]; \
tq[1] = 0; \
\
tq += 2; \
} \
}
/* Cast a n-band vector of double to a m-band vector in another format.
*/
static VipsPel *
make_pixel( VipsObject *obj,
int m, VipsBandFormat fmt, int n, double * restrict p )
{
VipsPel *q;
int i;
if( !(q = VIPS_ARRAY( obj, m * vips_format_sizeof( fmt ), VipsPel )) )
return( NULL );
switch( fmt ) {
case VIPS_FORMAT_CHAR:
CAST_CLIP( signed char, SCHAR_MIN, SCHAR_MAX );
break;
case VIPS_FORMAT_UCHAR:
CAST_CLIP( unsigned char, 0, UCHAR_MAX );
break;
case VIPS_FORMAT_SHORT:
CAST_CLIP( signed short, SCHAR_MIN, SCHAR_MAX );
break;
case VIPS_FORMAT_USHORT:
CAST_CLIP( unsigned short, 0, USHRT_MAX );
break;
case VIPS_FORMAT_INT:
CAST_CLIP( signed int, INT_MIN, INT_MAX );
break;
case VIPS_FORMAT_UINT:
CAST_CLIP( unsigned int, 0, UINT_MAX );
break;
case VIPS_FORMAT_FLOAT:
CAST( float );
break;
case VIPS_FORMAT_DOUBLE:
CAST( double );
break;
case VIPS_FORMAT_COMPLEX:
CASTC( float );
break;
case VIPS_FORMAT_DPCOMPLEX:
CASTC( double );
break;
default:
g_assert_not_reached();
}
return( q );
}
static int static int
vips_unary_const_build( VipsObject *object ) vips_unary_const_build( VipsObject *object )
{ {
@ -161,27 +67,43 @@ vips_unary_const_build( VipsObject *object )
uconst->n = VIPS_MAX( uconst->n, unary->in->Bands ); uconst->n = VIPS_MAX( uconst->n, unary->in->Bands );
arithmetic->base_bands = uconst->n; arithmetic->base_bands = uconst->n;
if( unary->in && uconst->c ) { if( unary->in &&
uconst->c ) {
if( vips_check_vector( class->nickname, if( vips_check_vector( class->nickname,
uconst->c->n, unary->in ) ) uconst->c->n, unary->in ) )
return( -1 ); return( -1 );
} }
/* Some operations need the vector in the input type (eg. /* Some operations need int constants, for example boolean AND, SHIFT
* im_equal_vec() where the output type is always uchar and is useless * etc.
* for comparisons), some need it in the output type (eg.
* im_andimage_vec() where we want to get the double to an int so we
* can do bitwise-and without having to cast for each pixel), some
* need a fixed type (eg. im_powtra_vec(), where we want to keep it as
* double).
* *
* Therefore pass in the desired vector type as a param. * Some can use int constants as an optimisation, for example (x <
* 12). It depends on the value though: obviously (x < 12.5) should
* not use the int form.
*/ */
if( uconst->c ) {
int i;
if( uconst->c ) uconst->c_int = VIPS_ARRAY( object, uconst->n, int );
uconst->c_ready = make_pixel( (VipsObject *) uconst, uconst->c_double = VIPS_ARRAY( object, uconst->n, double );
uconst->n, uconst->const_format, if( !uconst->c_int ||
uconst->c->n, (double *) uconst->c->data ); !uconst->c_double )
return( -1 );
for( i = 0; i < uconst->n; i++ )
uconst->c_double[i] = ((double *) uconst->c->data)
[VIPS_MIN( i, uconst->c->n - 1)];
for( i = 0; i < uconst->n; i++ )
uconst->c_int[i] = uconst->c_double[i];
uconst->is_int = TRUE;
for( i = 0; i < uconst->n; i++ )
if( uconst->c_int[i] != uconst->c_double[i] ) {
uconst->is_int = FALSE;
break;
}
}
if( VIPS_OBJECT_CLASS( vips_unary_const_parent_class )-> if( VIPS_OBJECT_CLASS( vips_unary_const_parent_class )->
build( object ) ) build( object ) )

View File

@ -59,16 +59,15 @@ typedef struct _VipsUnaryConst {
*/ */
VipsArea *c; VipsArea *c;
/* The format the constant should be cast to. Subclasses set this /* Our constant expanded to match arith->ready in size. We need int
* ready for unaryconst's build method. * and double versions.
*/ *
VipsBandFormat const_format; * is_int is TRUE if the two arrays are equal for every element.
/* Our constant expanded to match arith->ready in size and
* const_format in type.
*/ */
int n; int n;
VipsPel *c_ready; int *c_int;
double *c_double;
gboolean is_int;
} VipsUnaryConst; } VipsUnaryConst;