diff options
author | Ian Boros <puppyofkosh@gmail.com> | 2019-04-10 13:37:57 -0400 |
---|---|---|
committer | Ian Boros <puppyofkosh@gmail.com> | 2019-04-11 11:28:12 -0400 |
commit | 0be76c660815bfdf898dec790905eab6f0b70f0a (patch) | |
tree | e1612410f5cbc4a2b9fa5837abc5987911fd1d12 | |
parent | 2602fb6991469c7573534c2011d6ce3dae075682 (diff) | |
download | mongo-0be76c660815bfdf898dec790905eab6f0b70f0a.tar.gz |
SERVER-39721 Fix unary minus on value which can be min int
-rw-r--r-- | src/mongo/db/update/push_node.cpp | 21 | ||||
-rw-r--r-- | src/mongo/db/update/push_node_test.cpp | 20 |
2 files changed, 33 insertions, 8 deletions
diff --git a/src/mongo/db/update/push_node.cpp b/src/mongo/db/update/push_node.cpp index 8cbb7d9d892..e58c74f9e22 100644 --- a/src/mongo/db/update/push_node.cpp +++ b/src/mongo/db/update/push_node.cpp @@ -82,6 +82,16 @@ Status checkSortClause(const BSONObj& sortObject) { return Status::OK(); } +/** + * std::abs(LLONG_MIN) results in undefined behavior on 2's complement systems because the + * absolute value of LLONG_MIN cannot be represented in a 'long long'. + * + * If the input is LLONG_MIN, will return std::abs(LLONG_MIN + 1). + */ +long long safeApproximateAbs(long long val) { + return val == std::numeric_limits<decltype(val)>::min() ? std::abs(val + 1) : std::abs(val); +} + } // namespace Status PushNode::init(BSONElement modExpr, const boost::intrusive_ptr<ExpressionContext>& expCtx) { @@ -246,8 +256,8 @@ ModifierNode::ModifyResult PushNode::insertElementsWithPosition( auto insertAfter = getNthChild(*array, position.get() - 1); invariant(insertAfter.addSiblingRight(firstElementToInsert)); result = ModifyResult::kNormalUpdate; - } else if (position.get() < 0 && -position.get() < arraySize) { - auto insertAfter = getNthChild(*array, arraySize - (-position.get()) - 1); + } else if (position.get() < 0 && safeApproximateAbs(position.get()) < arraySize) { + auto insertAfter = getNthChild(*array, arraySize - safeApproximateAbs(position.get()) - 1); invariant(insertAfter.addSiblingRight(firstElementToInsert)); result = ModifyResult::kNormalUpdate; } else { @@ -297,12 +307,7 @@ ModifierNode::ModifyResult PushNode::performPush(mutablebson::Element* element, } if (_slice) { - // std::abs(LLONG_MIN) results in undefined behavior on 2's complement systems because the - // absolute value of LLONG_MIN cannot be represented in a 'long long'. - const auto sliceAbs = - _slice.get() == std::numeric_limits<decltype(_slice)::value_type>::min() - ? std::abs(_slice.get() + 1) - : std::abs(_slice.get()); + const auto sliceAbs = safeApproximateAbs(_slice.get()); while (static_cast<long long>(countChildren(*element)) > sliceAbs) { result = ModifyResult::kNormalUpdate; diff --git a/src/mongo/db/update/push_node_test.cpp b/src/mongo/db/update/push_node_test.cpp index 6f2cbff78e6..704108d96ce 100644 --- a/src/mongo/db/update/push_node_test.cpp +++ b/src/mongo/db/update/push_node_test.cpp @@ -1024,5 +1024,25 @@ TEST_F(PushNodeTest, ApplyMultipleElementsPushWithNegativePosition) { ASSERT_EQUALS("{a}", getModifiedPaths()); } +TEST_F(PushNodeTest, PushWithMinIntAsPosition) { + auto update = + BSON("$push" << BSON("a" << BSON("$each" << BSON_ARRAY(5) << "$position" + << std::numeric_limits<long long>::min()))); + boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest()); + PushNode node; + ASSERT_OK(node.init(update["$push"]["a"], expCtx)); + + mutablebson::Document doc(fromjson("{a: [0, 1, 2, 3, 4]}")); + setPathTaken("a"); + addIndexedPath("a"); + auto result = node.apply(getApplyParams(doc.root()["a"])); + ASSERT_FALSE(result.noop); + ASSERT_TRUE(result.indexesAffected); + ASSERT_EQUALS(fromjson("{a: [5, 0, 1, 2, 3, 4]}"), doc); + ASSERT_FALSE(doc.isInPlaceModeEnabled()); + ASSERT_EQUALS(fromjson("{$set: {a: [5, 0, 1, 2, 3, 4]}}"), getLogDoc()); + ASSERT_EQUALS("{a}", getModifiedPaths()); +} + } // namespace } // namespace mongo |