The address sanitizer highlighted this issue in our code base. It
looks like the code is currently grabbing a pointer to a temporary
object and then performing operations on it.
I printed some information right before the asan crash:
eigensolver address: 0x7f0ad95032f0
eigensolver size: 4528
eig_vecs_ ptr: 0x7f0ad95045e0
eig_vecs_ offset: 4848
This shows that `eig_vecs_` points past the end of `eigensolver`. In
other words, it points at the temporary object created by the
`eigensolver.eigenvectors()` call.
Compare the docs for `.eigenvalues()`:
https://eigen.tuxfamily.org/dox/classEigen_1_1EigenSolver.html#a0f507ad7ab14797882f474ca8f2773e7
to the docs for `.eigenvectors()`:
https://eigen.tuxfamily.org/dox/classEigen_1_1EigenSolver.html#a66288022802172e3ee059283b26201d7
The difference in return types is interesting. `.eigenvalues()`
returns a reference. But `.eigenvectors()` returns a matrix.
This patch here fixes the problem by saving the temporary object and
then grabbing a pointer into it.
This is a curated snippet of the original asan failure:
==12==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7fc633704640 at pc 0x7fc64f7f1593 bp 0x7ffe8875fc90 sp 0x7ffe8875fc88
READ of size 8 at 0x7fc633704640 thread T0
#0 0x7fc64f7f1592 in cv::usac::EssentialMinimalSolverStewenius5ptsImpl::estimate(std::__1::vector<int, std::__1::allocator<int> > const&, std::__1::vector<cv::Mat, std::__1::allocator<cv::Mat> >&) const /proc/self/cwd/external/com_github_opencv_opencv/modules/calib3d/src/usac/essential_solver.cpp:181:48
#1 0x7fc64f915d92 in cv::usac::EssentialEstimatorImpl::estimateModels(std::__1::vector<int, std::__1::allocator<int> > const&, std::__1::vector<cv::Mat, std::__1::allocator<cv::Mat> >&) const /proc/self/cwd/external/com_github_opencv_opencv/modules/calib3d/src/usac/estimator.cpp:110:46
#2 0x7fc64fa74fb0 in cv::usac::Ransac::run(cv::Ptr<cv::usac::RansacOutput>&) /proc/self/cwd/external/com_github_opencv_opencv/modules/calib3d/src/usac/ransac_solvers.cpp:152:58
#3 0x7fc64fa6cd8e in cv::usac::run(cv::Ptr<cv::usac::Model const> const&, cv::_InputArray const&, cv::_InputArray const&, int, cv::Ptr<cv::usac::RansacOutput>&, cv::_InputArray const&, cv::_InputArray const&, cv::_InputArray const&, cv::_InputArray const&) /proc/self/cwd/external/com_github_opencv_opencv/modules/calib3d/src/usac/ransac_solvers.cpp:1010:16
#4 0x7fc64fa6fb46 in cv::usac::findEssentialMat(cv::_InputArray const&, cv::_InputArray const&, cv::_InputArray const&, int, double, double, cv::_OutputArray const&) /proc/self/cwd/external/com_github_opencv_opencv/modules/calib3d/src/usac/ransac_solvers.cpp:527:9
#5 0x7fc64f3b5522 in cv::findEssentialMat(cv::_InputArray const&, cv::_InputArray const&, cv::_InputArray const&, int, double, double, int, cv::_OutputArray const&) /proc/self/cwd/external/com_github_opencv_opencv/modules/calib3d/src/five-point.cpp:437:16
#6 0x7fc64f3b7e00 in cv::findEssentialMat(cv::_InputArray const&, cv::_InputArray const&, cv::_InputArray const&, int, double, double, cv::_OutputArray const&) /proc/self/cwd/external/com_github_opencv_opencv/modules/calib3d/src/five-point.cpp:486:12
...
Address 0x7fc633704640 is located in stack of thread T0 at offset 17984 in frame
#0 0x7fc64f7ed4ff in cv::usac::EssentialMinimalSolverStewenius5ptsImpl::estimate(std::__1::vector<int, std::__1::allocator<int> > const&, std::__1::vector<cv::Mat, std::__1::allocator<cv::Mat> >&) const /proc/self/cwd/external/com_github_opencv_opencv/modules/calib3d/src/usac/essential_solver.cpp:36
This frame has 63 object(s):
[32, 56) 'coefficients' (line 38)
[96, 384) 'ee' (line 55)
...
[13040, 17568) 'eigensolver' (line 142)
[17824, 17840) 'ref.tmp518' (line 143)
[17856, 17872) 'ref.tmp523' (line 144)
[17888, 19488) 'ref.tmp524' (line 144) <== Memory access at offset 17984 is inside this variable
[19616, 19640) 'ref.tmp532' (line 169)
...
The crash report says that we're accessing a temporary object from
line 144 when we shouldn't be. Line 144 looks like this:
https://github.com/opencv/opencv/blob/4.6.0/modules/calib3d/src/usac/essential_solver.cpp#L144
const auto * const eig_vecs_ = (double *) eigensolver.eigenvectors().real().data();
We are using version 4.6.0 for this, but the problem is present on the
4.x branch.
Note that I am dropping the .real() call here. I think that is safe because
of the code further down (line 277 in the most recent version):
const int eig_i = 20 * i + 12; // eigen stores imaginary values too
The code appears to expect to have to skip doubles for the imaginary parts
of the complex numbers.
Admittedly, I couldn't find a test case that exercised this code path to
validate correctness.
Fix crash in ap3p #23607
### Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
- [ ] I agree to contribute to the project under Apache 2 License.
- [ ] 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
- [ ] 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
Patch to opencv_extra has the same branch name.
- [x] The feature is well documented and sample code can be built with the project CMake
Fixing typos in usac #23900
Just read and correct some typos in `usac`
### 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
- [ ] There is a reference to the original bug report and related work
- [ ] There is accuracy test, performance test and test data in opencv_extra repository, if applicable
Patch to opencv_extra has the same branch name.
- [ ] The feature is well documented and sample code can be built with the project CMake
Update USAC #23078
### Pull Request Readiness Checklist
- [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
Patch to opencv_extra has the same branch name.
- [x] The feature is well documented and sample code can be built with the project CMake
Resolves https://github.com/opencv/opencv/issues/23304
Fixes the incorrect pixel grid
Switches type to double to avoid precision loss as all callers use doubles
### 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
Patch to opencv_extra has the same branch name.
- [X] The feature is well documented and sample code can be built with the project CMake
Backport of #22992 to 3.4
### 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
Patch to opencv_extra has the same branch name.
- [x] The feature is well documented and sample code can be built with the project CMake
Add `estimateSE2(...)`, `estimateSE3(...)`, `estimateSIM2(...)`, `estimateSIM3(...)` for estimating an geometric transformation with rotation and translation (with scaling for SIM) using USAC: as alternative for `estimateAffinePartial2D` and `estimateAffine3D`.
Modified test module.
Remove unused variables.
Remove initializer of unused variable.
Add interfaces to accept UsacParams() and corresponding test codes.
Revise test code.
PartialNd removed
Umeyama rewritten for code quality & speed
comments & minors
rise number of points
fix, and +30% faster!
only one number should be that big
remove USAC code, leave fix only
big number
Usage of imread(): magic number 0, unchecked result
* docs: rewrite 0/1 to IMREAD_GRAYSCALE/IMREAD_COLOR in imread()
* samples, apps: rewrite 0/1 to IMREAD_GRAYSCALE/IMREAD_COLOR in imread()
* tests: rewrite 0/1 to IMREAD_GRAYSCALE/IMREAD_COLOR in imread()
* doc/py_tutorials: check imread() result
The current implementation overwrites the result rotation and translation in every iteration.
If SOLVEPNP_ITERATIVE was run as a refinement it will start from the incorrect initial
transformation thus degrading the final outcome.
