Info

The uassert check in the StorageEngineImpl constructor reads the moved 'options' variable. This seems to be a violation of the use-after-move rule in C++. This check is only relevant to storage engines without --directoryperdb support.

Top User Comments

acm commented on Wed, 22 Jul 2020 18:04:36 +0000: benety.goh, yes, of course. xgen-internal-githook commented on Wed, 22 Jul 2020 18:03:48 +0000: Author: {'name': 'Bynn Lee', 'email': 'bynn.lee@mongodb.com', 'username': 'bynn'} Message: SERVER-49229 use-after-move: 'options' argument in StorageEngineImpl constructor Branch: master https://github.com/mongodb/mongo/commit/db78b73d0b1cda09831163e9283e7ff5adf90fa7 benety.goh commented on Wed, 22 Jul 2020 15:30:53 +0000: We tried running patch build a week ago before assigning this ticket and found numerous violations of the clan-tidy rule. We'll include the link to the patch build in a separate comment. acm commented on Wed, 22 Jul 2020 15:21:05 +0000: benety.goh - You had mentioned above that you would prefer not to enable the clang-tidy check as part of this ticket but instead to file a new ticket, so I think we should file that. On the other hand, it might be trivial to try to patch build adding the new check once the above CR lands? If it patch builds green, you could just commit it right away. If it doesn't patch build green, you could look at where the errors are reported and that might indicate a team who should pick up the ball. Overall, I want the set of checks to be something that we view has having common ownership: if a bug is found, and clang-tidy could have caught it, I'd prefer that the work to resolve the defect include eliminating remaining instances of that detectable defect along with locking in detection by enabling the new check. Otherwise, it will only be the SDP team trying to move this forward, and that won't scale. acm commented on Thu, 2 Jul 2020 12:18:27 +0000: benety.goh - Fine by me to do it under a different ticket if there is a reasonable path to actually getting it done promptly. I was very excited that we got clang-tidy stood up in CI under SERVER-27984, and it had been my fervent hope that teams would of their own initiative expand the set of checks as bugs like this one were found. So far that has not been the case, and we are currently spending several hours looking only for bugprone-unused-raii. benety.goh commented on Wed, 1 Jul 2020 16:36:14 +0000: acm, I'm in favor of adding this to the list of checks in clang-tidy in a separate SERVER ticket. acm commented on Wed, 1 Jul 2020 15:26:58 +0000: benety.goh - Our toolchain clang-tidy has an explicit checker for this error: $ /opt/mongodbtoolchain/v3/bin/clang-tidy -checks=* -list-checks | grep use-after-move bugprone-use-after-move Perhaps, rather than just making the limited change to fix this one instance, it would be worthwhile to expand the list of checks to include this, and see if 1) clang-tidy would have caught this bug and 2) if so, whether there are other instances that should be repaired. Doing so would allow us to permanently add that check to our clang-tidy pass in CI. benety.goh commented on Wed, 1 Jul 2020 15:15:04 +0000: This issue was introduced in SERVER-38600 when we started to initialize the _options member variable using a move rather than a copy.

Steps to Reproduce