Allocate arguments for lists

CHANGELOG.md

@@ -11,6 +11,7 @@
 * Fix several UBSan failures which did not appear to result in functional bugs ([#6649](https://github.com/realm/realm-core/pull/6649)).
 * Fix an out-of-bounds read in sectioned results when sectioned are removed by modifying all objects in that section to no longer appear in that section ([#6649](https://github.com/realm/realm-core/pull/6649), since v13.12.0)
 * Using both synchronous and asynchronous transactions on the same thread or scheduler could hit the assertion failure "!realm.is_in_transaction()" if one of the callbacks for an asynchronous transaction happened to be scheduled during a synchronous transaction ([#6659](https://github.com/realm/realm-core/issues/6659), since v11.8.0)
+* Fix the query parser needs to copy list of arguments and own the memory. ([#6674](https://github.com/realm/realm-core/pull/6674), since v12.5.0)
 
 ### Breaking changes
 * None.

src/realm/parser/driver.cpp

@@ -1246,103 +1246,13 @@ std::unique_ptr<Subexpr> ConstantNode::visit(ParserDriver* drv, DataType hint)
             }
             else if (drv->m_args.is_argument_list(arg_no)) {
                 std::vector<Mixed> mixed_list = drv->m_args.list_for_argument(arg_no);
-                std::unique_ptr<Value<Mixed>> values = std::make_unique<Value<Mixed>>();
-                constexpr bool is_list = true;
-                values->init(is_list, mixed_list.size());
-                size_t ndx = 0;
-                for (auto& val : mixed_list) {
-                    values->set(ndx++, val);
-                }
-                if (m_comp_type) {
-                    values->set_comparison_type(*m_comp_type);
-                }
-                ret = std::move(values);
+                ret = copy_list_of_args(mixed_list);
             }
             else {
                 auto type = drv->m_args.type_for_argument(arg_no);
                 explain_value_message =
                     util::format("argument %1 of type '%2'", explain_value_message, get_data_type_name(type));
-                switch (type) {
-                    case type_Int:
-                        ret = std::make_unique<Value<int64_t>>(drv->m_args.long_for_argument(arg_no));
-                        break;
-                    case type_String:
-                        ret = std::make_unique<ConstantStringValue>(drv->m_args.string_for_argument(arg_no));
-                        break;
-                    case type_Binary:
-                        ret = std::make_unique<ConstantBinaryValue>(drv->m_args.binary_for_argument(arg_no));
-                        break;
-                    case type_Bool:
-                        ret = std::make_unique<Value<Bool>>(drv->m_args.bool_for_argument(arg_no));
-                        break;
-                    case type_Float:
-                        ret = std::make_unique<Value<float>>(drv->m_args.float_for_argument(arg_no));
-                        break;
-                    case type_Double: {
-                        // In realm-js all number type arguments are returned as double. If we don't cast to the
-                        // expected type, we would in many cases miss the option to use the optimized query node
-                        // instead of the general Compare class.
-                        double val = drv->m_args.double_for_argument(arg_no);
-                        switch (hint) {
-                            case type_Int:
-                            case type_Bool: {
-                                int64_t int_val = int64_t(val);
-                                // Only return an integer if it precisely represents val
-                                if (double(int_val) == val)
-                                    ret = std::make_unique<Value<int64_t>>(int_val);
-                                else
-                                    ret = std::make_unique<Value<double>>(val);
-                                break;
-                            }
-                            case type_Float:
-                                ret = std::make_unique<Value<float>>(float(val));
-                                break;
-                            default:
-                                ret = std::make_unique<Value<double>>(val);
-                                break;
-                        }
-                        break;
-                    }
-                    case type_Timestamp: {
-                        try {
-                            ret = std::make_unique<Value<Timestamp>>(drv->m_args.timestamp_for_argument(arg_no));
-                        }
-                        catch (const std::exception&) {
-                            ret = std::make_unique<Value<ObjectId>>(drv->m_args.objectid_for_argument(arg_no));
-                        }
-                        break;
-                    }
-                    case type_ObjectId: {
-                        try {
-                            ret = std::make_unique<Value<ObjectId>>(drv->m_args.objectid_for_argument(arg_no));
-                        }
-                        catch (const std::exception&) {
-                            ret = std::make_unique<Value<Timestamp>>(drv->m_args.timestamp_for_argument(arg_no));
-                        }
-                        break;
-                    }
-                    case type_Decimal:
-                        ret = std::make_unique<Value<Decimal128>>(drv->m_args.decimal128_for_argument(arg_no));
-                        break;
-                    case type_UUID:
-                        ret = std::make_unique<Value<UUID>>(drv->m_args.uuid_for_argument(arg_no));
-                        break;
-                    case type_Link:
-                        ret = std::make_unique<Value<ObjKey>>(drv->m_args.object_index_for_argument(arg_no));
-                        break;
-                    case type_TypedLink:
-                        if (hint == type_Mixed || hint == type_Link || hint == type_TypedLink) {
-                            ret = std::make_unique<Value<ObjLink>>(drv->m_args.objlink_for_argument(arg_no));
-                            break;
-                        }
-                        explain_value_message =
-                            util::format("%1 which links to %2", explain_value_message,
-                                         print_pretty_objlink(drv->m_args.objlink_for_argument(arg_no),
-                                                              drv->m_base_table->get_parent_group()));
-                        break;
-                    default:
-                        break;
-                }
+                ret = copy_arg(drv, type, arg_no, hint, explain_value_message);
             }
             break;
         }
@@ -1355,6 +1265,92 @@ std::unique_ptr<Subexpr> ConstantNode::visit(ParserDriver* drv, DataType hint)
     return ret;
 }
 
+std::unique_ptr<ConstantMixedList> ConstantNode::copy_list_of_args(std::vector<Mixed>& mixed_args)
+{
+    std::unique_ptr<ConstantMixedList> args_in_list = std::make_unique<ConstantMixedList>(mixed_args.size());
+    size_t ndx = 0;
+    for (const auto& mixed : mixed_args) {
+        args_in_list->set(ndx++, mixed);
+    }
+    if (m_comp_type) {
+        args_in_list->set_comparison_type(*m_comp_type);
+    }
+    return args_in_list;
+}
+
+std::unique_ptr<Subexpr> ConstantNode::copy_arg(ParserDriver* drv, DataType type, size_t arg_no, DataType hint,
+                                                std::string& err)
+{
+    switch (type) {
+        case type_Int:
+            return std::make_unique<Value<int64_t>>(drv->m_args.long_for_argument(arg_no));
+        case type_String:
+            return std::make_unique<ConstantStringValue>(drv->m_args.string_for_argument(arg_no));
+        case type_Binary:
+            return std::make_unique<ConstantBinaryValue>(drv->m_args.binary_for_argument(arg_no));
+        case type_Bool:
+            return std::make_unique<Value<Bool>>(drv->m_args.bool_for_argument(arg_no));
+        case type_Float:
+            return std::make_unique<Value<float>>(drv->m_args.float_for_argument(arg_no));
+        case type_Double: {
+            // In realm-js all number type arguments are returned as double. If we don't cast to the
+            // expected type, we would in many cases miss the option to use the optimized query node
+            // instead of the general Compare class.
+            double val = drv->m_args.double_for_argument(arg_no);
+            switch (hint) {
+                case type_Int:
+                case type_Bool: {
+                    int64_t int_val = int64_t(val);
+                    // Only return an integer if it precisely represents val
+                    if (double(int_val) == val)
+                        return std::make_unique<Value<int64_t>>(int_val);
+                    else
+                        return std::make_unique<Value<double>>(val);
+                }
+                case type_Float:
+                    return std::make_unique<Value<float>>(float(val));
+                default:
+                    return std::make_unique<Value<double>>(val);
+            }
+            break;
+        }
+        case type_Timestamp: {
+            try {
+                return std::make_unique<Value<Timestamp>>(drv->m_args.timestamp_for_argument(arg_no));
+            }
+            catch (const std::exception&) {
+                return std::make_unique<Value<ObjectId>>(drv->m_args.objectid_for_argument(arg_no));
+            }
+        }
+        case type_ObjectId: {
+            try {
+                return std::make_unique<Value<ObjectId>>(drv->m_args.objectid_for_argument(arg_no));
+            }
+            catch (const std::exception&) {
+                return std::make_unique<Value<Timestamp>>(drv->m_args.timestamp_for_argument(arg_no));
+            }
+            break;
+        }
+        case type_Decimal:
+            return std::make_unique<Value<Decimal128>>(drv->m_args.decimal128_for_argument(arg_no));
+        case type_UUID:
+            return std::make_unique<Value<UUID>>(drv->m_args.uuid_for_argument(arg_no));
+        case type_Link:
+            return std::make_unique<Value<ObjKey>>(drv->m_args.object_index_for_argument(arg_no));
+        case type_TypedLink:
+            if (hint == type_Mixed || hint == type_Link || hint == type_TypedLink) {
+                return std::make_unique<Value<ObjLink>>(drv->m_args.objlink_for_argument(arg_no));
+            }
+            err = util::format("%1 which links to %2", err,
+                               print_pretty_objlink(drv->m_args.objlink_for_argument(arg_no),
+                                                    drv->m_base_table->get_parent_group()));
+            break;
+        default:
+            break;
+    }
+    return nullptr;
+}
+
 #if REALM_ENABLE_GEOSPATIAL
 GeospatialNode::GeospatialNode(GeospatialNode::Box, GeoPoint& p1, GeoPoint& p2)
     : m_geo{Geospatial{GeoBox{p1, p2}}}

src/realm/parser/driver.hpp

@@ -191,6 +191,9 @@ class ConstantNode : public ValueNode {
     {
         target_table = table_name.substr(1, table_name.size() - 2);
     }
+
+    std::unique_ptr<ConstantMixedList> copy_list_of_args(std::vector<Mixed>&);
+    std::unique_ptr<Subexpr> copy_arg(ParserDriver*, DataType, size_t, DataType, std::string&);
     std::unique_ptr<Subexpr> visit(ParserDriver*, DataType) override;
     util::Optional<ExpressionComparisonType> m_comp_type;
     std::string target_table;

test/object-store/c_api/c_api.cpp

@@ -2373,6 +2373,41 @@ TEST_CASE("C API", "[c_api]") {
                 CHECK_ERR_CAT(RLM_ERR_INDEX_OUT_OF_BOUNDS, (RLM_ERR_CAT_INVALID_ARG | RLM_ERR_CAT_LOGIC));
             }
 
+            SECTION("string in list") {
+                char foo[] = "foo";
+                realm_value_t str = rlm_str_val(foo);
+                realm_value_t list_arg[2] = {str, rlm_str_val("bar")};
+
+                write([&]() {
+                    CHECK(realm_set_value(obj1.get(), foo_properties["string"], rlm_str_val("foo"), false));
+                });
+
+                static const size_t num_args = 1;
+                realm_query_arg_t args[num_args] = {realm_query_arg_t{1, false, &str}};
+                realm_query_arg_t* arg_list_simple = &args[0];
+
+                realm_query_arg_t args_in_list[num_args] = {realm_query_arg_t{2, true, &list_arg[0]}};
+                realm_query_arg_t* arg_list = &args_in_list[0];
+
+                auto q_string_single_param =
+                    cptr_checked(realm_query_parse(realm, class_foo.key, "string == $0", num_args, arg_list_simple));
+                auto q_string_in_list =
+                    cptr_checked(realm_query_parse(realm, class_foo.key, "string IN $0", num_args, arg_list));
+
+                // changing the value for one of the parameters passed should not change the result of the query.
+                // essentially we must assure that core is copying all the arguments passed inside the list (like for
+                // normal query arguments), and after realm_query_parse completes any modification of the memory that
+                // was used to store the parameter does not impact in any way core.
+                char* s = foo;
+                s[0] = 'a';
+                size_t count, count_list;
+
+                CHECK(checked(realm_query_count(q_string_single_param.get(), &count)));
+                CHECK(1 == count);
+                CHECK(checked(realm_query_count(q_string_in_list.get(), &count_list)));
+                CHECK(1 == count_list);
+            }
+
             SECTION("decimal NaN") {
                 realm_value_t decimal = rlm_decimal_nan();