Fix a use after free bug in the tests.

The way the SystemTest fixture works is that it takes
a "FooProblem" object as a type, which contains a ceres::Problem
and a ceres::Solver::Options object.

The Options object also contains a linear_solver_ordering which
contains double* which refer to memory that is allocated when
a problem object is created.

So it is important that the lifetime of the ceres::Problem object
and the ceres::Solver::Options object be tied together. But we were
violating this by creating a FooProblem object on the stack, grabbing
its Options struct and passing it to the SystemTest fixture, which
would then create another instance of FooProblem, grab its Problem
object and copy the modified options struct into it.

In the case where a user provided ordering was being used,
this ordering would now be referring to memory allocated by the first
FooProblem object, which would cause Ceres's internal ApplyOrdering
function to fail.

The fix is ofcourse to Problem and Options object that are born
together.

Change-Id: I07c377a9d5fcabbb6c7ca8aa3460206ce045ffa9
This commit is contained in:
Sameer Agarwal
2018-04-20 11:02:48 -07:00
parent 4556eb9810
commit 865952821f
47 changed files with 452 additions and 353 deletions
+7 -12
View File
@@ -78,7 +78,7 @@ std::string ToString(const Solver::Options& options);
// It is assumed that the SystemTestProblem has an Solver::Options
// struct that contains the reference Solver configuration.
template <typename SystemTestProblem>
class SystemTest : public::testing::Test {
class SystemTest : public ::testing::Test {
protected:
virtual void SetUp() {
SystemTestProblem system_test_problem;
@@ -88,15 +88,10 @@ class SystemTest : public::testing::Test {
&expected_final_residuals_);
}
void RunSolverForConfigAndExpectResidualsMatch(const Solver::Options& options) {
LOG(INFO) << "Running solver configuration: "
<< ToString(options);
SystemTestProblem system_test_problem;
void RunSolverForConfigAndExpectResidualsMatch(const Solver::Options& options,
Problem* problem) {
std::vector<double> final_residuals;
SolveAndEvaluateFinalResiduals(
options,
system_test_problem.mutable_problem(),
&final_residuals);
SolveAndEvaluateFinalResiduals(options, problem, &final_residuals);
// We compare solutions by comparing their residual vectors. We do
// not compare parameter vectors because it is much more brittle
@@ -119,10 +114,10 @@ class SystemTest : public::testing::Test {
Solve(options, problem, &summary);
CHECK_NE(summary.termination_type, ceres::FAILURE);
problem->Evaluate(Problem::EvaluateOptions(),
NULL,
nullptr,
final_residuals,
NULL,
NULL);
nullptr,
nullptr);
}
std::vector<double> expected_final_residuals_;