[SERVER-24272] StageBuilder should have an explicit invariant that calls to findIndexByKeyPattern() return non-null Created: 24/May/16 Updated: 27/Aug/16 Resolved: 03/Jun/16 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Querying |
| Affects Version/s: | None |
| Fix Version/s: | 3.3.8 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Coverity Collector User | Assignee: | David Storch |
| Resolution: | Done | Votes: | 0 |
| Labels: | coverity | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Backwards Compatibility: | Fully Compatible |
| Operating System: | ALL |
| Sprint: | Query 16 (06/24/16) |
| Participants: |
| Description |
|
Return value of function which returns null is dereferenced without checking Defect 99343 (STATIC_C)
File: /src/mongo/db/query/stage_builder.cpp
|
| Comments |
| Comment by Githook User [ 03/Jun/16 ] | |||||||
|
Author: {u'username': u'dstorch', u'name': u'David Storch', u'email': u'david.storch@10gen.com'}Message: | |||||||
| Comment by David Storch [ 03/Jun/16 ] | |||||||
|
Since query planning and stage building must happen under the collection lock (without yielding locks in between or during), any index that was available during planning must also be available when constructing PlanStages. Therefore, I believe Coverity's concern about a possible null dereference is a false positive. A null dereference would only be possible if a bug in the query planner resulted in us attempting to use a non-existent index, or if a problem with locking allowed an index to be dropped out from under us. However, there are a few places in stage_builder.cpp were we check for a null IndexDescriptor where null should be impossible:
Our code coverage tool confirms that our tests do not exercise the code inside this if block, increasing my confidence that it is in fact not reachable. These should be converted into explicit calls to invariant() ensuring that the IndexDescriptor is non-null. |