BFMatcher::match corrupts heap (Bug #3117)

By Mac Reiter:

I have a BFMatcher(NORM_HAMMING, true), and when I ask it to match two sets of descriptors:

vector<DMatch> matches;
matcher->match(desc[a], desc[b], matches);

matches comes back full of matches, but the heap has been corrupted -- presumably by writing past the end. Deleting matches will throw up a dialog (at least under VS2010 using the debug heap).

To fix this from my side, I can pre-size matches:

vector<DMatch> matches(min(desc[a].rows, desc[b].rows));
matcher->match(desc[a], desc[b], matches); // same as above

If I do that, the extra size seems to protect the heap.

This strongly suggests that BFMatcher is using raw pointer access into the vector's memory, and getting it wrong. If it was using the [] indexer, the debugging library should have caught the out-of-range access.

(It would appear that BRISK::detect has a similar error, but I will have to do more research to verify.)

I am unable to continue work on my contract job with this problem in place, so I have been trying to find workarounds or likely sources. The most likely that I have seen on the web is that the DLLs are compiled slightly differently than the main app. Both are x64, both are VS2010, both are debug builds (made sure I was using the 'd' suffix libraries for the debug mode of the project. There may be some subtle compilation switch that differs, but I'm not sure what it would be.

This also would explain why things work if I pre-allocate the vector -- my code does the allocation and my code does the deallocation, so there is no mismatch of heap expectations. But if I let OpenCV do the allocation and then I try to do the deallocation, mismatched heap expectations lead to explosions.

The general solution to this problem is to clearly define who is responsible for memory allocation. C++, at least in the presence of pre-compiled DLLs, does not help this situation at all. So the detectors and matchers and anything else that wants to take a reference to an STL container and resize() it to fit will have problems. One of the following two solutions will need to be settled on as an allocation strategy pretty much across the board:

1. OpenCV does not allocate. Difficult to manage, because the caller frequently has no way of knowing how much space will be needed, and asking OpenCV would (in many cases) require doing all of the work twice -- once to determine capacity, and once to store the results.
2. OpenCV allocates AND deallocates. Requires some kind of a dispose() method corresponding to each OpenCV method that provides internal allocation. Also complicated by answering the question of what to do with incoming containers that are already sized. If they are being re-used from a previous OpenCV-based allocation, all is well. But if they were pre-sized by the calling program, they should not be modified, and the function should probably fail in some way that leads the developer to understand that OpenCV must perform the actual allocation. I'm not sure how you would determine the difference between these two conditions, unless you give up on STL containers and make your own keypoint/match/feature/whatever containers that contain a magic key that only OpenCV initializes during creation.

Just my 2 cents worth...