From 32bb1c85026c152128a62c8fc225d4c7a3ed084c Mon Sep 17 00:00:00 2001 From: Eberhard Beilharz Date: Tue, 17 Feb 2026 17:39:27 +0100 Subject: [PATCH 1/2] fix(core): fix iterator in backspace handling This change replaces the reverse iterator loop that holds a stale iterator across `pop_back()` calls with a pattern that directly accesses `context.back()` on each iteration. This avoids undefined behavior from iterator invalidation when mutating the list or vector. While so far the previous code didn't show problems, it might still access released memory depending on the implementation. The documentation for `pop_back()` says "References and iterators to the erased element are invalidated", so the previous implementation was clearly wrong. Test-bot: skip --- core/src/ldml/ldml_processor.cpp | 8 ++++---- core/tests/unit/ldml/ldml.cpp | 21 ++++++++++----------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/core/src/ldml/ldml_processor.cpp b/core/src/ldml/ldml_processor.cpp index 63e7995273c..04dd15ab830 100644 --- a/core/src/ldml/ldml_processor.cpp +++ b/core/src/ldml/ldml_processor.cpp @@ -233,15 +233,15 @@ void ldml_event_state::emit_backspace() { // Find out what the last actual character was and remove it. // attempt to get the last char // TODO-LDML: emoji backspace - auto end = state->context().rbegin(); - while (end != state->context().rend()) { - if (end->type == KM_CORE_CT_CHAR) { + while (!state->context().empty()) { + auto end = state->context().back(); + if (end.type == KM_CORE_CT_CHAR) { actions.code_points_to_delete++; state->context().pop_back(); return; } // else loop again - assert(end->type != KM_CORE_CT_END); // inappropriate here. + assert(end.type != KM_CORE_CT_END); // inappropriate here. state->context().pop_back(); } /* diff --git a/core/tests/unit/ldml/ldml.cpp b/core/tests/unit/ldml/ldml.cpp index b040df7e76d..bfe4d03123e 100644 --- a/core/tests/unit/ldml/ldml.cpp +++ b/core/tests/unit/ldml/ldml.cpp @@ -157,21 +157,20 @@ apply_action( matched_text = true; // no text to match as context is empty. } // now, we need to simulate what ldml_processor::emit_backspace() is going to do. - auto end = context.rbegin(); - while (end != context.rend()) { - if (end->type == KM_CORE_CT_CHAR) { - test_assert(!matched_text); - test_assert_equal(end->character, ch); // expect popped char to be same as what's in context - matched_text = true; - context.pop_back(); - break; // exit on first real char - } - test_assert(end->type != KM_CORE_CT_END); // inappropriate here. + while (!context.empty()) { + if (context.back().type == KM_CORE_CT_CHAR) { + test_assert(!matched_text); + test_assert_equal(context.back().character, ch); // expect popped char to be same as what's in context + matched_text = true; context.pop_back(); + break; // exit on first real char } - test_assert(matched_text); + test_assert(context.back().type != KM_CORE_CT_END); // inappropriate here. + context.pop_back(); } + test_assert(matched_text); break; + } case KM_CORE_IT_PERSIST_OPT: std::cout << " + TODO-LDML: persist_opt()" << std::endl; break; From df2b675202732768b2830d5a2ee69c801b62f909 Mon Sep 17 00:00:00 2001 From: Eberhard Beilharz Date: Wed, 18 Feb 2026 16:08:59 +0100 Subject: [PATCH 2/2] fix(core): changed ldml tests to use context list instead of vector Production code uses a list of context items, so this change modifies the tests to also use a list instead of a vector to more closely match production code. Addresses code review comment. Test-bot: skip --- core/tests/unit/ldml/ldml.cpp | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/core/tests/unit/ldml/ldml.cpp b/core/tests/unit/ldml/ldml.cpp index bfe4d03123e..2acced00187 100644 --- a/core/tests/unit/ldml/ldml.cpp +++ b/core/tests/unit/ldml/ldml.cpp @@ -40,7 +40,7 @@ namespace { -void print_context(std::u16string &text_store, km_core_state *&test_state, std::vector &test_context); +void print_context(std::u16string &text_store, km_core_state *&test_state, std::list &test_context); bool g_beep_found = false; @@ -68,10 +68,10 @@ string_to_hex(const std::u16string &input) { void -copy_context_items_to_vector(km_core_context_item *citems, std::vector &vector) { - vector.clear(); +copy_context_items_to_list(km_core_context_item *citems, std::list &list) { + list.clear(); for(km_core_context_item *ci = citems; ci->type != KM_CORE_CT_END; ci++) { - vector.emplace_back(*ci); + list.emplace_back(*ci); } } @@ -81,9 +81,9 @@ apply_action( km_core_state *test_state, km_core_action_item const &act, std::u16string &text_store, - std::vector &context, + std::list &context, km::tests::LdmlTestSource &test_source, - std::vector &test_context) { + std::list &test_context) { // print_context(text_store, test_state, test_context); switch (act.type) { @@ -183,9 +183,9 @@ apply_action( // We replace the cached context with the current application context km_core_status status = context_items_from_utf16(text_store.c_str(), &new_context_items); test_assert(status == KM_CORE_STATUS_OK); - copy_context_items_to_vector(new_context_items, context); + copy_context_items_to_list(new_context_items, context); // also update the test context - copy_context_items_to_vector(new_context_items, test_context); + copy_context_items_to_list(new_context_items, test_context); // TODO-LDML: now we need to SET the core context! status = km_core_context_set(km_core_state_context(test_state), new_context_items); km_core_context_items_dispose(new_context_items); @@ -211,9 +211,9 @@ apply_actions( km_core_state *test_state, km_core_actions const *actions, std::u16string &text_store, - std::vector &context, + std::list &context, km::tests::LdmlTestSource &test_source, - std::vector &test_context + std::list &test_context ) { if(actions->do_alert) { @@ -244,7 +244,7 @@ apply_actions( } void -print_context(std::u16string &text_store, km_core_state *&test_state, std::vector &test_context) { +print_context(std::u16string &text_store, km_core_state *&test_state, std::list &test_context) { // Compare context and text store at each step - should be identical size_t n = 0; km_core_context_item *citems = nullptr; @@ -279,7 +279,7 @@ print_context(std::u16string &text_store, km_core_state *&test_state, std::vecto std::cout << "test_context : "; std::cout.fill('0'); - for (auto i = test_context.begin(); i < test_context.end(); i++) { + for (auto i = test_context.begin(); i != test_context.end(); i++) { switch (i->type) { case KM_CORE_CT_CHAR: std::cout << "U+" << std::setw(4) << std::hex << i->character << std::dec << " "; @@ -301,7 +301,7 @@ print_context(std::u16string &text_store, km_core_state *&test_state, std::vecto * verify the current context */ void -verify_context(std::u16string &text_store, km_core_state *&test_state, std::vector &test_context, bool fully_normalized_mode, bool normalization_enabled) { +verify_context(std::u16string &text_store, km_core_state *&test_state, std::list &test_context, bool fully_normalized_mode, bool normalization_enabled) { // Compare context and text store at each step - should be identical print_context(text_store, test_state, test_context); @@ -434,7 +434,7 @@ run_test(const km::core::path &source, const km::core::path &compiled, km::tests g_beep_found = false; - std::vector test_context; + std::list test_context; km_core_context_item *citems = nullptr; // setup test_context @@ -447,7 +447,7 @@ run_test(const km::core::path &source, const km::core::path &compiled, km::tests } // Make a copy of the setup context for the test - copy_context_items_to_vector(citems, test_context); + copy_context_items_to_list(citems, test_context); km_core_context_items_dispose(citems); // Setup baseline text store