[SERVER-70885] Ensure complex SBE expressions properly manage lifetime of stack values Created: 27/Oct/22  Updated: 29/Oct/23  Resolved: 14/Nov/22

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 6.2.0-rc0

Type: Task Priority: Major - P3
Reporter: Alberto Massari Assignee: Zixuan Zhuang
Resolution: Fixed Votes: 0
Labels: pm2697-m2
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Duplicate
is duplicated by SERVER-70582 [CQF] Sampling CE may cause a MONGO_U... Closed
Backwards Compatibility: Fully Compatible
Sprint: QE 2022-11-28
Participants:
Story Points: 10

 Description   

When an expression is retrieving data from a value that is owned by the stack, it should either ensure that it has a lifetime bigger than the lifetime of the return value, or make it a stack-owned copy.

Example:

TEST_F(SBEBuiltinExtractSubArrayTest, MemoryManagement) {
    auto array = makeArray(BSON_ARRAY("Item#1"
                                      << "Item#2"
                                      << "Item#3"
                                      << "Item#4"));
 
    // Run getElement(0) on the subarray holding just the last entry in the above array.
    auto extractFromSubArrayExpr = makeE<EFunction>(
        "getElement",
        makeEs(makeE<EFunction>(
                   "extractSubArray",
                                makeEs(makeE<EConstant>(array.first, array.second),
                                       makeE<EConstant>(value::TypeTags::NumberInt32, 1),
                                       makeE<EConstant>(value::TypeTags::NumberInt32, 2))),
               makeE<EConstant>(value::TypeTags::NumberInt32, 0)));
 
    auto compiledExpr = compileExpression(*extractFromSubArrayExpr);
 
    auto [tag, value] = runCompiledExpression(compiledExpr.get());
    std::cout << std::make_pair(tag, value) << std::endl;
    ASSERT_TRUE(value::isString(tag));
    ASSERT_EQ("Item#3", value::getRawStringView(tag, value));
}

The extractSubArray instruction creates a new stack-owned array, and getElement returns a shared value that points to deleted memory by the time runCompiledExpression attempts to copy.

 	KernelBase.dll!wil::details::DebugBreak(void)	Unknown
 	db_sbe_test.exe!mongo::invariantFailed(const char * expr, const char * file, unsigned int line) Line 142	C++
 	[Inline Frame] db_sbe_test.exe!mongo::invariantWithLocation(const bool &) Line 74	C++
 	db_sbe_test.exe!mongo::sbe::value::makeBigString(mongo::StringData input) Line 1110	C++
 	db_sbe_test.exe!mongo::sbe::value::copyValue(mongo::sbe::value::TypeTags tag, unsigned __int64 val) Line 1452	C++
>	[Inline Frame] db_sbe_test.exe!mongo::sbe::EExpressionTestFixture::runCompiledExpression(const mongo::sbe::vm::CodeFragment *) Line 85	C++
 	db_sbe_test.exe!mongo::sbe::UnitTest_SuiteNameSBEBuiltinExtractSubArrayTestTestNameMemoryManagement::_doTest() Line 240	C++



 Comments   
Comment by Githook User [ 14/Nov/22 ]

Author:

{'name': 'Zixuan Zhuang', 'email': 'zixuan.zhuang@mongodb.com', 'username': 'leozzx'}

Message: SERVER-70885 Fix getElement/getField memory bug for stack owned value
Branch: master
https://github.com/mongodb/mongo/commit/d9109360d4c6da049775ba1a146865045d581710

Comment by Mihai Andrei [ 08/Nov/22 ]

Yes, adding it

Comment by Kyle Suarez [ 08/Nov/22 ]

mihai.andrei@mongodb.com, shall we add this to the SBE Perf project and consider it for sprint planning today?

Comment by Ana Meza [ 08/Nov/22 ]

kyle.suarez@mongodb.com , just wondering if someone from the sbe perf team can pick this up for the next sprint? 

Generated at Thu Feb 08 06:17:24 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.