From 7548b04ef7a84572a795976483f9da8070b784f0 Mon Sep 17 00:00:00 2001 From: Scott Anderson <662325+scottanderson@users.noreply.github.com> Date: Mon, 2 Nov 2026 16:35:50 -0500 Subject: [PATCH 1/2] std: Decode std::mem::read_string through the current encoding read_string returned the raw bytes. Under a non-UTF-8 #pragma encoding, the UI then read them as UTF-8. Mac Roman "Hink" 0xD5 "s" showed as garbage, because 0xD5 took the "s" with it. read_string now returns UTF-8 text. It has a new 4th parameter, encoding. An empty encoding means the #pragma encoding. An [[encoding]] attribute does not apply, because read_string reads no field. It returns the raw bytes, as before, when: - the encoding is "UTF-8" - no encoding is set - the host set no codec "UTF-8" is the only spelling that gives raw bytes. Pass it to read binary data under a non-UTF-8 pragma. Bytes that do not decode stop the pattern with E0004. The comment on read_string named an endian parameter. It has none. The 3rd parameter is the section. --- lib/source/pl/lib/std/mem.cpp | 37 ++++- tests/CMakeLists.txt | 4 + .../test_pattern_read_string.hpp | 151 ++++++++++++++++++ tests/source/tests.cpp | 5 + 4 files changed, 193 insertions(+), 4 deletions(-) create mode 100644 tests/include/test_patterns/test_pattern_read_string.hpp diff --git a/lib/source/pl/lib/std/mem.cpp b/lib/source/pl/lib/std/mem.cpp index 67529ef9..f4884682 100644 --- a/lib/source/pl/lib/std/mem.cpp +++ b/lib/source/pl/lib/std/mem.cpp @@ -37,6 +37,29 @@ namespace pl::lib::libstd::mem { core::err::E0012.throwError("Invalid section id.", "Only custom sections can be modified through std::mem."); } + /** + * @brief Decodes bytes read from the data into UTF-8 text + * @param ctx The evaluator, which holds the host's codec + * @param bytes The bytes to decode + * @param encoding The encoding to decode with + * @return The decoded text. The bytes unchanged under UTF-8, under no encoding, or with no codec set. + */ + static std::string decodeString(::pl::core::Evaluator *ctx, std::string bytes, const std::string &encoding) { + // An empty name makes the host's codec use its own default, which is not always UTF-8. + if (encoding.empty() || encoding == "UTF-8") + return bytes; + + const auto &codec = ctx->getStringEncodeDecode(); + if (codec == nullptr) + return bytes; + + auto decoded = codec->decode({ reinterpret_cast(bytes.data()), bytes.size() }, encoding); + if (decoded.bytesConsumed != bytes.size()) + core::err::E0004.throwError(fmt::format("invalid byte sequence for encoding '{}'", encoding)); + + return std::move(decoded.text); + } + static std::optional findSequence(::pl::core::Evaluator *ctx, u64 occurrenceIndex, u64 offsetFrom, u64 offsetTo, u64 section, const std::vector &sequence) { u32 occurrences = 0; const u64 bufferSize = ctx->getSectionSize(section); @@ -168,17 +191,23 @@ namespace pl::lib::libstd::mem { return hlp::signExtend(size * 8, value); }); - /* read_string(address, size, endian, section) */ - runtime.addFunction(nsStdMem, "read_string", FunctionParameterCount::between(2, 3), [](Evaluator *ctx, auto params) -> std::optional { + /* read_string(address, size, section, encoding) */ + runtime.addFunction(nsStdMem, "read_string", FunctionParameterCount::between(2, 4), [](Evaluator *ctx, auto params) -> std::optional { const auto address = u64(params[0].toUnsigned()); const auto size = std::size_t(params[1].toSigned()); - u64 section = params.size() == 3 ? u64(params[2].toUnsigned()) : ctx->getUserSectionId(); + u64 section = params.size() >= 3 ? u64(params[2].toUnsigned()) : ctx->getUserSectionId(); section = validateReadableSection(ctx, section); + auto encoding = params.size() == 4 ? params[3].toString(false) : std::string(); + + // No field is read, so no [[encoding]] attribute applies. Only #pragma encoding does. + if (encoding.empty()) + encoding = ctx->getDefaultEncoding(); + std::string result(size, '\x00'); ctx->readData(address, result.data(), size, section); - return result; + return decodeString(ctx, std::move(result), encoding); }); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 292b15f5..fc5aa1fe 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -90,6 +90,10 @@ set(AVAILABLE_TESTS #RedeclarationInFuncBodyFail #FuncParamAndBodyRedeclarationFail AllowedRedefinitions + ReadStringNoPragma + ReadStringDecoded + ReadStringNoCodec + ReadStringUndecodableFail #GlobalContinueFail #FunctionContinueFail #GlobalBreakFail diff --git a/tests/include/test_patterns/test_pattern_read_string.hpp b/tests/include/test_patterns/test_pattern_read_string.hpp new file mode 100644 index 00000000..e07c6f06 --- /dev/null +++ b/tests/include/test_patterns/test_pattern_read_string.hpp @@ -0,0 +1,151 @@ +#pragma once + +#include "test_pattern.hpp" + +#include +#include + +namespace pl::test { + + /** + * @brief A predictable codec for the std::mem::read_string tests + * + * Under "latin1", a byte gives the code point of the same value. A byte above 0x7F + * gives two UTF-8 bytes, which raw bytes cannot give. Under "ascii", a byte above + * 0x7F is malformed. It counts its calls, so a test can show that it was not used. + */ + class ReadStringCodec : public core::StringEncodeDecode { + public: + [[nodiscard]] core::DecodeResult decode(std::span bytes, std::string_view encoding, + std::optional) const override { + m_decodeCount += 1; + + core::DecodeResult result; + for (const u8 byte : bytes) { + if (byte >= 0x80 && encoding == "ascii") { + result.stopReason = core::DecodeStop::MalformedBytes; + return result; + } + + if (byte < 0x80) { + result.text += char(byte); + } else { + result.text += char(0xC0 | (byte >> 6)); + result.text += char(0x80 | (byte & 0x3F)); + } + + result.bytesConsumed += 1; + result.codepointCount += 1; + } + + return result; + } + + [[nodiscard]] std::optional> encode(std::string_view text, std::string_view) const override { + return std::vector(text.begin(), text.end()); + } + + [[nodiscard]] std::vector encodeLossy(std::string_view text, std::string_view) const override { + return std::vector(text.begin(), text.end()); + } + + [[nodiscard]] size_t getDecodeCount() const { return m_decodeCount; } + + private: + mutable size_t m_decodeCount = 0; + }; + + /** + * @brief With no #pragma encoding, read_string returns the raw bytes and does not call the codec + * + * The test data starts with the PNG signature 89 50 4E 47. The 0x89 is not valid UTF-8. + */ + class TestPatternReadStringNoPragma : public TestPattern { + public: + TestPatternReadStringNoPragma(core::Evaluator *evaluator) : TestPattern(evaluator, "ReadStringNoPragma") { } + ~TestPatternReadStringNoPragma() override = default; + + void setup() override { + m_codec = std::make_shared(); + m_runtime->setStringEncodeDecode(m_codec); + } + + [[nodiscard]] std::string getSourceCode() const override { + return R"( + std::assert(builtin::std::mem::read_string(0, 4) == "\x89PNG", "no pragma did not give raw bytes"); + std::assert(builtin::std::mem::read_string(0, 4, 0, "") == "\x89PNG", "an empty encoding did not give raw bytes"); + )"; + } + + [[nodiscard]] bool runChecks(const std::vector> &) const override { + return m_codec->getDecodeCount() == 0; + } + + private: + std::shared_ptr m_codec; + }; + + /** + * @brief Under a non-UTF-8 #pragma encoding, read_string decodes, and "UTF-8" gives raw bytes + */ + class TestPatternReadStringDecoded : public TestPattern { + public: + TestPatternReadStringDecoded(core::Evaluator *evaluator) : TestPattern(evaluator, "ReadStringDecoded") { } + ~TestPatternReadStringDecoded() override = default; + + void setup() override { + m_runtime->setStringEncodeDecode(std::make_shared()); + } + + [[nodiscard]] std::string getSourceCode() const override { + return R"( + #pragma encoding latin1 + + std::assert(builtin::std::mem::read_string(0, 4) == "\xC2\x89PNG", "the pragma's encoding was not used"); + std::assert(builtin::std::mem::read_string(0, 4, 0) == "\xC2\x89PNG", "the pragma's encoding was not used with a section"); + std::assert(builtin::std::mem::read_string(0, 4, 0, "") == "\xC2\x89PNG", "an empty encoding did not use the pragma's encoding"); + std::assert(builtin::std::mem::read_string(0, 4, 0, "UTF-8") == "\x89PNG", "UTF-8 did not give raw bytes"); + std::assert(builtin::std::mem::read_string(0, 0) == "", "an empty read did not give an empty string"); + )"; + } + }; + + /** + * @brief With no codec set, read_string returns the raw bytes under any #pragma encoding + */ + class TestPatternReadStringNoCodec : public TestPattern { + public: + TestPatternReadStringNoCodec(core::Evaluator *evaluator) : TestPattern(evaluator, "ReadStringNoCodec") { } + ~TestPatternReadStringNoCodec() override = default; + + [[nodiscard]] std::string getSourceCode() const override { + return R"( + #pragma encoding latin1 + + std::assert(builtin::std::mem::read_string(0, 4) == "\x89PNG", "no codec did not give raw bytes"); + )"; + } + }; + + /** + * @brief Bytes that do not decode under the encoding stop the pattern + */ + class TestPatternReadStringUndecodableFail : public TestPattern { + public: + TestPatternReadStringUndecodableFail(core::Evaluator *evaluator) : TestPattern(evaluator, "ReadStringUndecodableFail", Mode::Failing) { } + ~TestPatternReadStringUndecodableFail() override = default; + + void setup() override { + m_runtime->setStringEncodeDecode(std::make_shared()); + } + + [[nodiscard]] std::string getSourceCode() const override { + return R"( + #pragma encoding ascii + + str value = builtin::std::mem::read_string(0, 4); + )"; + } + }; + +} diff --git a/tests/source/tests.cpp b/tests/source/tests.cpp index 0e6510ef..41ed7f21 100644 --- a/tests/source/tests.cpp +++ b/tests/source/tests.cpp @@ -42,6 +42,7 @@ #include "test_patterns/test_pattern_in_variable_defaults.hpp" #include "test_patterns/test_pattern_set_entries_sorted_consistency.hpp" #include "test_patterns/test_pattern_allowed_redefinitions.hpp" +#include "test_patterns/test_pattern_read_string.hpp" static pl::core::Evaluator s_evaluator; @@ -133,6 +134,10 @@ std::array Tests = { // TEST(RedeclarationInFuncBodyFail), // TEST(FuncParamAndBodyRedeclarationFail), TEST(AllowedRedefinitions), + TEST(ReadStringNoPragma), + TEST(ReadStringDecoded), + TEST(ReadStringNoCodec), + TEST(ReadStringUndecodableFail), // TEST(GlobalContinueFail), // TEST(FunctionContinueFail), // TEST(GlobalBreakFail), From fb6f0aa49572e35199086794e15d4cb45402b1f1 Mon Sep 17 00:00:00 2001 From: Scott Anderson <662325+scottanderson@users.noreply.github.com> Date: Mon, 2 Nov 2026 18:01:57 -0500 Subject: [PATCH 2/2] fix: Do not crash when an out variable cannot be read Under a non-UTF-8 #pragma encoding, a str out variable crashed the process. Its value was decoded through the codec again, but a str variable holds text, not data bytes. When that text did not decode, the error came from inside a scope guard's destructor, and that ended the process. A str in the heap or in pattern-local storage no longer goes through the codec. An error while an out variable is read is now logged, and that variable is left out. --- lib/include/pl/patterns/pattern_string.hpp | 22 +++++- lib/source/pl/core/evaluator.cpp | 7 +- tests/CMakeLists.txt | 2 + .../test_pattern_out_variables.hpp | 77 +++++++++++++++++++ tests/source/tests.cpp | 3 + 5 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 tests/include/test_patterns/test_pattern_out_variables.hpp diff --git a/lib/include/pl/patterns/pattern_string.hpp b/lib/include/pl/patterns/pattern_string.hpp index e7e69b40..b5c10631 100644 --- a/lib/include/pl/patterns/pattern_string.hpp +++ b/lib/include/pl/patterns/pattern_string.hpp @@ -54,7 +54,7 @@ namespace pl::ptrn { std::string buffer(size, '\x00'); evaluator->readData(this->getOffset(), buffer.data(), size, this->getSection()); - if (const auto &codec = evaluator->getStringEncodeDecode(); codec != nullptr) { + if (const auto codec = this->getCodec(); codec != nullptr) { const auto encoding = this->getEncodingName(); auto decoded = codec->decode({ reinterpret_cast(buffer.data()), buffer.size() }, encoding); if (decoded.bytesConsumed != buffer.size()) @@ -70,7 +70,7 @@ namespace pl::ptrn { if (auto stringValue = std::get_if(&value); stringValue != nullptr) { std::vector bytes; - if (const auto &codec = this->getEvaluator()->getStringEncodeDecode(); codec != nullptr) { + if (const auto codec = this->getCodec(); codec != nullptr) { const auto encoding = this->getEncodingName(); auto encoded = codec->encode(*stringValue, encoding); if (!encoded.has_value()) @@ -100,7 +100,7 @@ namespace pl::ptrn { void setValueLossy(const std::string &value) { std::vector bytes; - if (const auto &codec = this->getEvaluator()->getStringEncodeDecode(); codec != nullptr) + if (const auto codec = this->getCodec(); codec != nullptr) bytes = codec->encodeLossy(value, this->getEncodingName()); else bytes = { value.begin(), value.end() }; @@ -148,7 +148,7 @@ namespace pl::ptrn { if (auto formatted = Pattern::callUserFormatFunc(buffer); formatted.has_value()) return *formatted; - const auto &codec = evaluator->getStringEncodeDecode(); + const auto codec = this->getCodec(); // No codec configured keeps the old raw-byte display, unescaped by any // encoding. A configured codec reports an undecodable buffer as invalid, @@ -209,6 +209,20 @@ namespace pl::ptrn { return result; } + private: + /** + * @brief Gets the codec that converts this string's bytes + * @return The evaluator's codec, or nullptr when there is none or this is a variable + * @note A str variable holds the text assigned to it, not data bytes. Decoding it again would garble it. + */ + [[nodiscard]] std::shared_ptr getCodec() const { + const auto section = this->getSection(); + if (section == HeapSectionId || section == PatternLocalSectionId) + return nullptr; + + return this->getEvaluator()->getStringEncodeDecode(); + } + }; } \ No newline at end of file diff --git a/lib/source/pl/core/evaluator.cpp b/lib/source/pl/core/evaluator.cpp index 647808d2..58e1e238 100644 --- a/lib/source/pl/core/evaluator.cpp +++ b/lib/source/pl/core/evaluator.cpp @@ -1088,7 +1088,12 @@ namespace pl::core { this->m_mainSectionEditsAllowed = false; for (const auto &[name, pattern] : this->m_outVariables) { - this->m_outVariableValues.insert({ name, pattern->getValue() }); + // This runs in a destructor, so an exception that leaves it ends the process. + try { + this->m_outVariableValues.insert({ name, pattern->getValue() }); + } catch (const std::exception &e) { + this->m_console.log(LogConsole::Level::Error, fmt::format("Cannot read out variable '{}': {}", name, e.what())); + } } }; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index fc5aa1fe..6b5aa374 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -94,6 +94,8 @@ set(AVAILABLE_TESTS ReadStringDecoded ReadStringNoCodec ReadStringUndecodableFail + OutVariableEncoding + OutVariableError #GlobalContinueFail #FunctionContinueFail #GlobalBreakFail diff --git a/tests/include/test_patterns/test_pattern_out_variables.hpp b/tests/include/test_patterns/test_pattern_out_variables.hpp new file mode 100644 index 00000000..613a0c0e --- /dev/null +++ b/tests/include/test_patterns/test_pattern_out_variables.hpp @@ -0,0 +1,77 @@ +#pragma once + +#include "test_pattern.hpp" +#include "test_pattern_read_string.hpp" + +#include + +namespace pl::test { + + /** + * @brief A str out variable keeps the text assigned to it under a non-UTF-8 #pragma encoding + * + * Under "ascii" the test codec rejects a byte above 0x7F. A str out variable must not go + * through it, because it holds text, not data bytes. + */ + class TestPatternOutVariableEncoding : public TestPattern { + public: + TestPatternOutVariableEncoding(core::Evaluator *evaluator) : TestPattern(evaluator, "OutVariableEncoding") { } + ~TestPatternOutVariableEncoding() override = default; + + void setup() override { + m_runtime->setStringEncodeDecode(std::make_shared()); + } + + [[nodiscard]] std::string getSourceCode() const override { + return R"( + #pragma encoding ascii + + str text out; + str raw out; + text = "café"; + raw = builtin::std::mem::read_string(0, 4, 0, "UTF-8"); + )"; + } + + [[nodiscard]] bool runChecks(const std::vector> &) const override { + const auto outVariables = m_runtime->getOutVariables(); + + if (outVariables.at("text").toString(false) != "caf\xC3\xA9") + throw std::runtime_error(fmt::format("text was '{}'", outVariables.at("text").toString(false))); + if (outVariables.at("raw").toString(false) != "\x89PNG") + throw std::runtime_error(fmt::format("raw was '{}'", outVariables.at("raw").toString(false))); + + return true; + } + }; + + /** + * @brief An out variable whose value cannot be read is left out, and does not end the process + */ + class TestPatternOutVariableError : public TestPattern { + public: + TestPatternOutVariableError(core::Evaluator *evaluator) : TestPattern(evaluator, "OutVariableError") { } + ~TestPatternOutVariableError() override = default; + + [[nodiscard]] std::string getSourceCode() const override { + return R"( + fn fail(auto value) { + builtin::std::error("no value"); + }; + + using Broken = u8 [[transform("fail")]]; + + Broken broken out; + u8 good out; + good = 1; + )"; + } + + [[nodiscard]] bool runChecks(const std::vector> &) const override { + const auto outVariables = m_runtime->getOutVariables(); + + return !outVariables.contains("broken") && outVariables.contains("good"); + } + }; + +} diff --git a/tests/source/tests.cpp b/tests/source/tests.cpp index 41ed7f21..9845d07d 100644 --- a/tests/source/tests.cpp +++ b/tests/source/tests.cpp @@ -43,6 +43,7 @@ #include "test_patterns/test_pattern_set_entries_sorted_consistency.hpp" #include "test_patterns/test_pattern_allowed_redefinitions.hpp" #include "test_patterns/test_pattern_read_string.hpp" +#include "test_patterns/test_pattern_out_variables.hpp" static pl::core::Evaluator s_evaluator; @@ -138,6 +139,8 @@ std::array Tests = { TEST(ReadStringDecoded), TEST(ReadStringNoCodec), TEST(ReadStringUndecodableFail), + TEST(OutVariableEncoding), + TEST(OutVariableError), // TEST(GlobalContinueFail), // TEST(FunctionContinueFail), // TEST(GlobalBreakFail),