mirror of
https://github.com/opencv/opencv.git
synced 2026-07-21 19:33:03 +04:00
Merge pull request #29447 from Adel-Ayoub:fix/matexpr-mul-scalar-lifetime
core: fix use-after-scope when Mat::mul() is given a scalar #29447 ### Summary `cv::Mat::mul()` called with a scalar returns a `MatExpr` that reads a dead stack slot when it is evaluated. In a normal (non-instrumented) build this produces silently wrong values as soon as the slot is reused: ```cpp static cv::MatExpr makeExpr(const cv::Mat& m) { return m.mul(7); // 7.0 is a temporary double in THIS frame } cv::Mat matrix(2, 3, CV_32FC1, cv::Scalar(3.0f)); cv::MatExpr expr = makeExpr(matrix); // ... any further calls reuse the dead frame ... cv::Mat result = expr; // observed: all 0, expected: all 21 ``` Under AddressSanitizer this is the `stack-use-after-scope` reported in #23577, with the same stack trace (`cvt64s` -> `convertAndUnrollScalar` -> `arithm_op` -> `multiply` -> `MatOp_Bin::assign`). Storing the expression is the documented lazy-evaluation usage of `MatExpr`; the argument is ordinary supported API usage (`mat.hpp` itself shows `Mat C = A.mul(5/B);`). ### Root cause A scalar argument binds to `_InputArray(const double& val)`, which records the **address** of the temporary with kind `MATX`: ```cpp inline _InputArray::_InputArray(const double& val) { init(FIXED_TYPE + FIXED_SIZE + MATX + CV_64F + ACCESS_READ, &val, Size(1,1)); } ``` `Mat::mul()` then parks `m.getMat()` inside the returned `MatExpr`. For `MATX` kind, `getMat_()` returns a non-owning, non-refcounted header over that stack memory (`return Mat(sz, flags, obj);`). The temporary dies at the end of the full expression, but the `MatExpr` keeps the header, and `MatOp_Bin::assign()` later feeds it to `cv::multiply()`. `Matx`/`Vec` arguments take the same path. `Mat::mul()` is the only `MatExpr` factory in `matrix_expressions.cpp` that takes an `InputArray`; every other scalar operand there is stored by value in the `Scalar` member (`e.s`), so no other expression path can capture a stack pointer this way. ### Fix Snapshot the operand with `clone()` unless it is a `Mat`/`UMat`, which keep the current zero-copy behaviour: their headers are refcounted and already safe to defer. Any other `InputArray` kind (a scalar, `Matx`, `Vec`, `std::vector`, an evaluated expression) is a potentially non-owning view, so it is copied once at expression construction, off any hot path. ### Test Adds `Core_MatExpr.mul_scalar_use_after_scope_23577` to `modules/core/test/test_operations.cpp`. It builds the expression in a helper frame and overwrites the stack before evaluating; the helpers are called through volatile function pointers so they cannot be inlined, which makes the stale read deterministic. The test fails before the fix (result is all 0 instead of all 21) and passes after. It is self-contained: no opencv_extra data is needed. Verified locally on macOS/AArch64 (Apple clang 17, Release): full `opencv_test_core` passes, and the AddressSanitizer reproducer from the issue is clean after the fix. Fixes #23577. ### Pull Request Readiness Checklist See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request - [x] I agree to contribute to the project under Apache 2 License. - [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV - [x] The PR is proposed to the proper branch - [x] There is a reference to the original bug report and related work - [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Self-contained accuracy regression test in `modules/core/test/test_operations.cpp`; no opencv_extra data required. No performance test: no existing perf test covers `Mat::mul` expression construction, and the copy happens once at expression construction, only for non-`Mat`/`UMat` operands. - [ ] The feature is well documented and sample code can be built with the project CMake N/A - bug fix, no new API.
This commit is contained in:
committed by
GitHub
parent
fcc3418cd7
commit
e9289fc7c4
@@ -1738,8 +1738,16 @@ MatExpr Mat::mul(InputArray m, double scale) const
|
||||
{
|
||||
CV_INSTRUMENT_REGION();
|
||||
|
||||
Mat b = m.getMat();
|
||||
// Unless the argument is a refcounted Mat/UMat, the header returned by getMat() may be
|
||||
// a non-owning view of caller memory (e.g. a scalar bound to _InputArray(const double&),
|
||||
// a Matx or a Vec) that the returned MatExpr can outlive, so snapshot it.
|
||||
// See https://github.com/opencv/opencv/issues/23577
|
||||
if( !m.isMat() && !m.isUMat() )
|
||||
b = b.clone();
|
||||
|
||||
MatExpr e;
|
||||
MatOp_Bin::makeExpr(e, '*', *this, m.getMat(), scale);
|
||||
MatOp_Bin::makeExpr(e, '*', *this, b, scale);
|
||||
return e;
|
||||
}
|
||||
|
||||
|
||||
@@ -1561,6 +1561,37 @@ TEST(Core_MatExpr, empty_check_15760)
|
||||
EXPECT_THROW(Mat c = Mat().cross(Mat()), cv::Exception);
|
||||
}
|
||||
|
||||
// https://github.com/opencv/opencv/issues/23577
|
||||
// A scalar passed to Mat::mul() binds to _InputArray(const double&), i.e. to a temporary
|
||||
// double on the caller's stack. The returned MatExpr used to keep a Mat header pointing at
|
||||
// that stack slot after it died. The helpers are called through volatile function pointers
|
||||
// so they cannot be inlined, which makes the stale-stack read deterministic.
|
||||
MatExpr makeScalarMulExpr(const Mat& m, double scale)
|
||||
{
|
||||
return m.mul(scale);
|
||||
}
|
||||
|
||||
void overwriteStackFrame()
|
||||
{
|
||||
volatile double buf[256];
|
||||
for (int i = 0; i < 256; i++)
|
||||
buf[i] = -1.0;
|
||||
(void)buf;
|
||||
}
|
||||
|
||||
TEST(Core_MatExpr, mul_scalar_use_after_scope_23577)
|
||||
{
|
||||
MatExpr (*volatile makeExprFn)(const Mat&, double) = makeScalarMulExpr;
|
||||
void (*volatile overwriteFn)() = overwriteStackFrame;
|
||||
|
||||
Mat m(2, 3, CV_32FC1, Scalar::all(3.0f));
|
||||
MatExpr e = makeExprFn(m, 7.0);
|
||||
overwriteFn();
|
||||
Mat res = e;
|
||||
|
||||
EXPECT_EQ(0, cvtest::norm(res, Mat(2, 3, CV_32FC1, Scalar::all(21.0f)), NORM_INF));
|
||||
}
|
||||
|
||||
TEST(Core_Arithm, scalar_handling_19599) // https://github.com/opencv/opencv/issues/19599 (OpenCV 4.x+ only)
|
||||
{
|
||||
Mat a(1, 1, CV_32F, Scalar::all(1));
|
||||
|
||||
Reference in New Issue
Block a user