From 4c7aa8645abd2ce9dc1e64088febb778b175c49e Mon Sep 17 00:00:00 2001 From: Alexander Alekhin Date: Tue, 14 Feb 2017 14:58:52 +0300 Subject: [PATCH] ocl: validate arguments in KernelArgs constructor - don't use undefined flag=0. It should be CONSTANT instead. - don't allow 'UMat* m=NULL' argument (except LOCAL/CONSTANT flags). This case is not handled well to provide NULL __global pointers. It is better to use '-D' macro defines instead (at least for performance) --- modules/core/src/arithm.cpp | 16 ++++++++-------- modules/core/src/ocl.cpp | 1 + modules/core/src/umatrix.cpp | 2 +- modules/imgproc/src/imgwarp.cpp | 4 ++-- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/modules/core/src/arithm.cpp b/modules/core/src/arithm.cpp index 4b1c67339b..9c2c126ffb 100644 --- a/modules/core/src/arithm.cpp +++ b/modules/core/src/arithm.cpp @@ -138,7 +138,7 @@ static bool ocl_binary_op(InputArray _src1, InputArray _src2, OutputArray _dst, convertAndUnrollScalar(src2sc, srctype, (uchar*)buf, 1); } - ocl::KernelArg scalararg = ocl::KernelArg(0, 0, 0, 0, buf, esz); + ocl::KernelArg scalararg = ocl::KernelArg(ocl::KernelArg::CONSTANT, 0, 0, 0, buf, esz); if( !haveMask ) k.args(src1arg, dstarg, scalararg); @@ -550,7 +550,7 @@ static bool ocl_arithm_op(InputArray _src1, InputArray _src2, OutputArray _dst, if( !src2sc.empty() ) convertAndUnrollScalar(src2sc, wtype, (uchar*)buf, 1); - ocl::KernelArg scalararg = ocl::KernelArg(0, 0, 0, 0, buf, esz); + ocl::KernelArg scalararg = ocl::KernelArg(ocl::KernelArg::CONSTANT, 0, 0, 0, buf, esz); if( !haveMask ) { @@ -558,7 +558,7 @@ static bool ocl_arithm_op(InputArray _src1, InputArray _src2, OutputArray _dst, k.args(src1arg, dstarg, scalararg); else if(n == 1) k.args(src1arg, dstarg, scalararg, - ocl::KernelArg(0, 0, 0, 0, usrdata_p, usrdata_esz)); + ocl::KernelArg(ocl::KernelArg::CONSTANT, 0, 0, 0, usrdata_p, usrdata_esz)); else CV_Error(Error::StsNotImplemented, "unsupported number of extra parameters"); } @@ -576,12 +576,12 @@ static bool ocl_arithm_op(InputArray _src1, InputArray _src2, OutputArray _dst, k.args(src1arg, src2arg, dstarg); else if (n == 1) k.args(src1arg, src2arg, dstarg, - ocl::KernelArg(0, 0, 0, 0, usrdata_p, usrdata_esz)); + ocl::KernelArg(ocl::KernelArg::CONSTANT, 0, 0, 0, usrdata_p, usrdata_esz)); else if (n == 3) k.args(src1arg, src2arg, dstarg, - ocl::KernelArg(0, 0, 0, 0, usrdata_p, usrdata_esz), - ocl::KernelArg(0, 0, 0, 0, usrdata_p + usrdata_esz, usrdata_esz), - ocl::KernelArg(0, 0, 0, 0, usrdata_p + usrdata_esz*2, usrdata_esz)); + ocl::KernelArg(ocl::KernelArg::CONSTANT, 0, 0, 0, usrdata_p, usrdata_esz), + ocl::KernelArg(ocl::KernelArg::CONSTANT, 0, 0, 0, usrdata_p + usrdata_esz, usrdata_esz), + ocl::KernelArg(ocl::KernelArg::CONSTANT, 0, 0, 0, usrdata_p + usrdata_esz*2, usrdata_esz)); else CV_Error(Error::StsNotImplemented, "unsupported number of extra parameters"); } @@ -1204,7 +1204,7 @@ static bool ocl_compare(InputArray _src1, InputArray _src2, OutputArray _dst, in convertAndUnrollScalar(Mat(1, 1, CV_32S, &ival), depth1, (uchar *)buf, kercn); } - ocl::KernelArg scalararg = ocl::KernelArg(0, 0, 0, 0, buf, esz); + ocl::KernelArg scalararg = ocl::KernelArg(ocl::KernelArg::CONSTANT, 0, 0, 0, buf, esz); k.args(ocl::KernelArg::ReadOnlyNoSize(src1, cn, kercn), ocl::KernelArg::WriteOnly(dst, cn, kercn), scalararg); diff --git a/modules/core/src/ocl.cpp b/modules/core/src/ocl.cpp index 60ecc69546..6197e5716d 100644 --- a/modules/core/src/ocl.cpp +++ b/modules/core/src/ocl.cpp @@ -3155,6 +3155,7 @@ KernelArg::KernelArg() KernelArg::KernelArg(int _flags, UMat* _m, int _wscale, int _iwscale, const void* _obj, size_t _sz) : flags(_flags), m(_m), obj(_obj), sz(_sz), wscale(_wscale), iwscale(_iwscale) { + CV_Assert(_flags == LOCAL || _flags == CONSTANT || _m != NULL); } KernelArg KernelArg::Constant(const Mat& m) diff --git a/modules/core/src/umatrix.cpp b/modules/core/src/umatrix.cpp index 5874be82d3..dd5b9a0c2c 100644 --- a/modules/core/src/umatrix.cpp +++ b/modules/core/src/umatrix.cpp @@ -986,7 +986,7 @@ UMat& UMat::setTo(InputArray _value, InputArray _mask) ocl::Kernel setK(haveMask ? "setMask" : "set", ocl::core::copyset_oclsrc, opts); if( !setK.empty() ) { - ocl::KernelArg scalararg(0, 0, 0, 0, buf, CV_ELEM_SIZE(d) * scalarcn); + ocl::KernelArg scalararg(ocl::KernelArg::CONSTANT, 0, 0, 0, buf, CV_ELEM_SIZE(d) * scalarcn); UMat mask; if( haveMask ) diff --git a/modules/imgproc/src/imgwarp.cpp b/modules/imgproc/src/imgwarp.cpp index 0fa520228e..2f0f2e9d4e 100644 --- a/modules/imgproc/src/imgwarp.cpp +++ b/modules/imgproc/src/imgwarp.cpp @@ -5814,7 +5814,7 @@ static bool ocl_warpTransform_cols4(InputArray _src, OutputArray _dst, InputArra matM.convertTo(M0, CV_32F); k.args(ocl::KernelArg::ReadOnly(src), ocl::KernelArg::WriteOnly(dst), ocl::KernelArg::PtrReadOnly(M0), - ocl::KernelArg(0, 0, 0, 0, borderBuf, CV_ELEM_SIZE(sctype))); + ocl::KernelArg(ocl::KernelArg::CONSTANT, 0, 0, 0, borderBuf, CV_ELEM_SIZE(sctype))); size_t globalThreads[2]; globalThreads[0] = (size_t)(dst.cols / 4); @@ -5913,7 +5913,7 @@ static bool ocl_warpTransform(InputArray _src, OutputArray _dst, InputArray _M0, matM.convertTo(M0, doubleSupport ? CV_64F : CV_32F); k.args(ocl::KernelArg::ReadOnly(src), ocl::KernelArg::WriteOnly(dst), ocl::KernelArg::PtrReadOnly(M0), - ocl::KernelArg(0, 0, 0, 0, borderBuf, CV_ELEM_SIZE(sctype))); + ocl::KernelArg(ocl::KernelArg::CONSTANT, 0, 0, 0, borderBuf, CV_ELEM_SIZE(sctype))); size_t globalThreads[2] = { (size_t)dst.cols, ((size_t)dst.rows + rowsPerWI - 1) / rowsPerWI }; return k.run(2, globalThreads, NULL, false);