search

vedanshu95 / ceres-solver

Automatically exported from code.google.com/p/ceres-solver
Other
0 stars 0 forks source link

Segfault Fixes #72

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Here the patch that fixes two segfault problem in Ceres 1.3.0 tests.

Original issue reported on code.google.com by popov...@gmail.com on 7 Sep 2012 at 5:28

Attachments:

GoogleCodeExporter commented 9 years ago 
A bit fixed patch.

Original comment by popov...@gmail.com on 7 Sep 2012 at 5:44

Attachments:

GoogleCodeExporter commented 9 years ago 
Hi Sergey,
Thank you very much for the patch. Could you please  submit the patch via 
gerrit? That way you get the credit, we get the signed CLA and the worflow is 
simpler?

The details on using the gerrit workflow are at

http://code.google.com/p/ceres-solver/wiki/DevelopingCeres

Sameer

Original comment by sameerag...@google.com on 7 Sep 2012 at 5:49

GoogleCodeExporter commented 9 years ago 
Hi Sergey,
I have pushed your change to levenberg_marquardt_test.cc upstream. The changes 
to evaluator_test.cc should not be needed. Because we explicitly resize these 
vectors inside Evaluator::Evaluate. If they are being resized incorrectly, then 
we have a bigger problem. Could you send me a crash log of the failing test, as 
I am unable to reproduce it on my end.

Thanks,
Sameer

Original comment by sameerag...@google.com on 8 Sep 2012 at 12:07

GoogleCodeExporter commented 9 years ago 
Hi Sameer,

Thanks for the information. I have investigated the issue with debugger and 
found that evaluator_test crushes only in debug mode when residuals[0] is 
called for the vector of zero size (please, see the patch in attachment). This 
is debug assertion in std::vector::operator[], not a segfault actually :).

My build environment is Win7 32 bit, MSVC 10.

Regards,
Sergey

Original comment by popov...@gmail.com on 8 Sep 2012 at 9:32

Attachments:

GoogleCodeExporter commented 9 years ago 
evaluator_test error message

Original comment by popov...@gmail.com on 8 Sep 2012 at 3:07

Attachments:

GoogleCodeExporter commented 9 years ago 
Hi Sergey,
Thanks for following this up.
I put an explicit check for the lengths of these vectors, and I still can't 
trigger this assert.
Could you point me to the particular test case inside evaluator_test thats 
causing this failure?

Thanks,
Sameer

Original comment by sameerag...@google.com on 8 Sep 2012 at 7:06

GoogleCodeExporter commented 9 years ago 
Hi Sameer,

The evaluator_test failure is MSVC specific. I have found that 
std::vector<T>::operator[] implementation of the libstdc++ library that comes 
with GCC does not check subscript range. Probably this is the reason why you 
can't trigger the assert. The name of the test case that causing failure is 
"StaticEvaluateTest.MultipleParameterAndResidualBlocks". The patch.diff patch 
fixes the bug.

Regards,
Sergey

Original comment by popov...@gmail.com on 9 Sep 2012 at 12:41

GoogleCodeExporter commented 9 years ago 
Hi Sergey,
I understand that it is a MSVC specific failure. What I was curious about was, 
the conditions in the test code that were causing this failure. Thanks for 
tracking down the name the failing test. I will take a closer look.
Sameer

Original comment by sameerag...@google.com on 9 Sep 2012 at 2:12

GoogleCodeExporter commented 9 years ago 
okay, I think I was looking at your patch wrong earlier.
You are right about the fix. I am pushing the changes upstream.
Thanks for your patience.

Sameer

Original comment by sameerag...@google.com on 9 Sep 2012 at 2:18