From 3db46d63f962bb441dc21f361ab0f6c4f71bd882 Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Wed, 16 Sep 2026 14:58:30 -0400 Subject: [PATCH 1/4] feat(debug): address a leaf's value in place instead of copying it out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `handle_read` copies a value into a caller's buffer. That is the right shape for a Modbus frame, but wrong for a protocol stack that can serve memory it does not own: OPC-UA hands open62541 a `UA_Variant` and, marked `UA_VARIANT_DATA_NODELETE`, it will neither copy nor free it. Today the baremetal server allocates from a ~19 KB arena for every value of every read, and multiplies that by `maxNodesPerRead`. So `TypeOps` gains a sixth op and `handle_ptr(arr, elem, &len)` exposes it: the same value `handle_read` would produce, addressed where it already lives, with its length in bytes. STRING hands back the payload rather than the `[len][payload]` wire form, and WSTRING the code-unit buffer, so `len` is bytes there, not characters. The op reads through a new `IECVar::read_ptr()` rather than `raw_ptr()`. They agree the instant a force is applied, because `force()` writes through to `value_` — but a located variable is driven by the program straight into `value_` every scan, and only `read_ptr()` still resolves to the forced value afterwards. `raw_ptr()` would have leaked the program's value past an active force, silently and only for located variables. It mirrors `IECStringVar::c_str()`, which already resolves the force this way. `read_wstring` splits each code unit explicitly so the WIRE format is little-endian on any host. A pointer cannot do that — the caller gets host order — so `ptr_wstring` carries a `static_assert` on `__BYTE_ORDER__` instead of silently serving byte-swapped text on a big-endian target. Not exported through the `strucpp_debug_*` C ABI. The pointer is into live PLC storage, safe only for a caller inside the scan's own thread of control — true of the baremetal super-loop, false of the Runtime v4 `.so`, where the scan has its own thread. Linux callers keep `strucpp_debug_read()`. The test compiles real generated code and checks the properties that matter: the pointer agrees with `handle_read`, follows a later write with no second call, resolves a force the same way, keeps resolving it after the program writes the located storage directly, and returns null rather than crashing for an out-of-range leaf. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p --- src/runtime/include/debug_dispatch.hpp | 115 ++++++++++++++++++---- src/runtime/include/iec_var.hpp | 16 +++ tests/integration/debug-table-cpp.test.ts | 110 +++++++++++++++++++++ 3 files changed, 220 insertions(+), 21 deletions(-) diff --git a/src/runtime/include/debug_dispatch.hpp b/src/runtime/include/debug_dispatch.hpp index c7c7cb90..e31ee934 100644 --- a/src/runtime/include/debug_dispatch.hpp +++ b/src/runtime/include/debug_dispatch.hpp @@ -121,6 +121,23 @@ inline void read_impl(const void* p, uint8_t* dest) noexcept { std::memcpy(dest, &v, sizeof(T)); } +// Pointer op — the same value read_impl would copy out, addressed in place. +// +// Exists so a caller that can serve a value without owning it (OPC-UA hands +// open62541 a UA_Variant with UA_VARIANT_DATA_NODELETE) does not pay an +// allocation and a copy per read. `read_ptr()` rather than `raw_ptr()`: a +// located variable is written by the PLC program straight into value_, so +// raw_ptr() would leak the program's value past an active force. +// +// The pointer is valid until the variable is next written, which for a +// cooperative single-threaded runtime means "until the caller yields". A +// caller that can be preempted by the PLC scan must copy instead. +template +inline const void* ptr_impl(const void* p, uint16_t* len) noexcept { + *len = static_cast(sizeof(T)); + return static_cast*>(p)->read_ptr(); +} + // STRING / WSTRING live in `IECStringVar<254>` / `IECWStringVar<254>`, // the force-aware wrappers around `IECString<254>` / `IECWString<254>`. // Both wrappers carry their own length, capped at 254 bytes / 254 wide @@ -187,6 +204,15 @@ inline void unforce_string(void* p) noexcept { static_cast*>(p)->unforce(); } +// The payload only — NOT the [len][payload] wire form read_string() builds. +// `len` is the character count, and c_str() already resolves the force. +inline const void* ptr_string(const void* p, uint16_t* len) noexcept { + const auto* var = static_cast*>(p); + const std::size_t actual_len = var->length(); + *len = static_cast(actual_len < DEBUG_STRING_CAP ? actual_len : DEBUG_STRING_CAP); + return var->c_str(); +} + // --- WSTRING (IECWStringVar<254>) ------------------------------------- inline void read_wstring(const void* p, uint8_t* dest) noexcept { @@ -236,6 +262,24 @@ inline void unforce_wstring(void* p) noexcept { static_cast*>(p)->unforce(); } +// The code-unit buffer as it sits in memory, so `len` is BYTES (2 per code +// unit), not characters. +// +// read_wstring() splits each code unit explicitly to keep the WIRE format +// little-endian on any host. A pointer cannot do that — what the caller gets is +// host order — so this op is only correct on a little-endian target, and says +// so rather than silently serving byte-swapped text somewhere else. +static_assert(__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__, + "ptr_wstring hands out UTF-16 code units in host order; a " + "big-endian target must copy through read_wstring() instead."); +inline const void* ptr_wstring(const void* p, uint16_t* len) noexcept { + const auto* var = static_cast*>(p); + const std::size_t actual_len = var->length(); + const std::size_t capped = actual_len < DEBUG_STRING_CAP ? actual_len : DEBUG_STRING_CAP; + *len = static_cast(capped * 2); + return var->c_str(); +} + // --------------------------------------------------------------------------- // Dispatch table entry. The `size` field is the byte width consumed/produced // by force/read (for strings: reserved, handled specially). @@ -245,6 +289,10 @@ struct TypeOps { void (*unforce)(void*); void (*read) (const void*, uint8_t*); void (*write) (void*, const uint8_t*); + /** Address the value in place instead of copying it out, and report its + * length in BYTES. Same value `read` would produce, same force + * resolution — see ptr_impl(). */ + const void* (*ptr)(const void*, uint16_t*); uint8_t size; }; @@ -253,27 +301,27 @@ struct TypeOps { // Kept inline so it's flash-resident with no separate .cpp required. // --------------------------------------------------------------------------- inline constexpr TypeOps type_ops[TAG__COUNT] = { - /*BOOL */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(BOOL_t) }, - /*SINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(SINT_t) }, - /*USINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(USINT_t) }, - /*INT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(INT_t) }, - /*UINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(UINT_t) }, - /*DINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(DINT_t) }, - /*UDINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(UDINT_t) }, - /*LINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(LINT_t) }, - /*ULINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(ULINT_t) }, - /*REAL */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(REAL_t) }, - /*LREAL */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(LREAL_t) }, - /*BYTE */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(BYTE_t) }, - /*WORD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(WORD_t) }, - /*DWORD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(DWORD_t) }, - /*LWORD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(LWORD_t) }, - /*TIME */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(TIME_t) }, - /*DATE */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(DATE_t) }, - /*TOD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(TOD_t) }, - /*DT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(DT_t) }, - /*STRING */ { &force_string, &unforce_string, &read_string, &write_string, DEBUG_STRING_WIDTH }, - /*WSTRING */ { &force_wstring, &unforce_wstring, &read_wstring, &write_wstring, DEBUG_WSTRING_WIDTH }, + /*BOOL */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(BOOL_t) }, + /*SINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(SINT_t) }, + /*USINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(USINT_t) }, + /*INT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(INT_t) }, + /*UINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(UINT_t) }, + /*DINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(DINT_t) }, + /*UDINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(UDINT_t) }, + /*LINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(LINT_t) }, + /*ULINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(ULINT_t) }, + /*REAL */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(REAL_t) }, + /*LREAL */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(LREAL_t) }, + /*BYTE */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(BYTE_t) }, + /*WORD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(WORD_t) }, + /*DWORD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(DWORD_t) }, + /*LWORD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(LWORD_t) }, + /*TIME */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(TIME_t) }, + /*DATE */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(DATE_t) }, + /*TOD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(TOD_t) }, + /*DT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(DT_t) }, + /*STRING */ { &force_string, &unforce_string, &read_string, &write_string, &ptr_string, DEBUG_STRING_WIDTH }, + /*WSTRING*/ { &force_wstring, &unforce_wstring, &read_wstring, &write_wstring, &ptr_wstring, DEBUG_WSTRING_WIDTH }, }; // --------------------------------------------------------------------------- @@ -459,6 +507,25 @@ inline uint16_t handle_size(uint8_t arr, uint16_t elem) noexcept { return type_ops[e.tag].size; } +/** Address a leaf's value in place; `out_len` receives its length in BYTES. + * Returns nullptr (and sets *out_len = 0) for an out-of-range leaf. + * + * The copy-free counterpart to handle_read(). The pointer is into live PLC + * storage, so it is valid only until the variable is next written — safe for a + * caller that runs inside the scan's own thread of control, wrong for one that + * can be preempted by it. */ +inline const void* handle_ptr(uint8_t arr, uint16_t elem, uint16_t* out_len) noexcept { + Entry e = read_entry(arr, elem); + if (!e.ptr || e.tag >= TAG__COUNT) { + if (out_len) *out_len = 0; + return nullptr; + } + uint16_t len = 0; + const void* p = type_ops[e.tag].ptr(e.ptr, &len); + if (out_len) *out_len = len; + return p; +} + /** Total number of arrays. */ inline uint8_t handle_array_count() noexcept { return debug_array_count; @@ -516,6 +583,12 @@ STRUCPP_V4_EXPORT uint16_t strucpp_debug_size(uint8_t arr, uint16_t elem) { return strucpp::debug::handle_size(arr, elem); } +// handle_ptr() is deliberately NOT exported here. It hands out a pointer into +// live PLC storage, which is only safe for a caller running inside the scan's +// own thread of control — true of the baremetal super-loop, false of the +// Runtime v4 .so, where the scan runs in its own thread and a plugin reading +// through the pointer would race it. Linux callers use strucpp_debug_read(). + STRUCPP_V4_EXPORT uint8_t strucpp_debug_set(uint8_t arr, uint16_t elem, bool forcing, const uint8_t *bytes, diff --git a/src/runtime/include/iec_var.hpp b/src/runtime/include/iec_var.hpp index 0fdbf391..ade5a66c 100644 --- a/src/runtime/include/iec_var.hpp +++ b/src/runtime/include/iec_var.hpp @@ -204,6 +204,22 @@ class IECVar { */ const T* raw_ptr() const noexcept { return &value_; } + /** + * Get a const pointer to the value a READER should see, honouring force. + * + * Not the same as raw_ptr(). force() writes through to value_, so the two + * agree the moment a force is applied — but a located variable is driven by + * the PLC program every scan, and the program writes value_ directly through + * the image binding, not through set(). raw_ptr() would then show the + * program's value while get() still reports the forced one. + * + * So this is get()'s semantics with get()'s copy removed, which is what an + * external reader (the debug dispatch's pointer op, and through it OPC-UA) + * needs to serve a value without copying it. Mirrors IECStringVar::c_str(), + * which resolves the force the same way. + */ + const T* read_ptr() const noexcept { return forced_ ? &forced_value_ : &value_; } + // ========================================================================= // Implicit Conversions // ========================================================================= diff --git a/tests/integration/debug-table-cpp.test.ts b/tests/integration/debug-table-cpp.test.ts index 039c584c..294f2910 100644 --- a/tests/integration/debug-table-cpp.test.ts +++ b/tests/integration/debug-table-cpp.test.ts @@ -467,6 +467,116 @@ int main() { ); expect(execSync(`"${bin}"`, { encoding: "utf-8" }).trim()).toBe("ALL_OK"); }); + + // handle_ptr() serves a value without copying it, so a caller can hand it + // straight to a protocol stack (OPC-UA gives open62541 a UA_Variant marked + // NODELETE). The properties that matter are that it addresses the SAME value + // handle_read() would produce — including through a force — and that a STRING + // comes back as its payload rather than the [len][payload] wire form. + // + // The force case is the one that motivated read_ptr(): force() writes through + // to value_, so raw_ptr() agrees the instant a force is applied — but a + // located variable is driven by the program straight into value_ every scan, + // and then only read_ptr() still resolves to the forced value. + it("addresses the same value handle_read copies, force included", () => { + const result = compile( + `PROGRAM Main +VAR n : DINT := 7; END_VAR +VAR s : STRING := 'hello'; END_VAR +END_PROGRAM${CFG}`, + { headerFileName: "generated.hpp" }, + ); + expect(result.errors.map((e) => e.message)).toEqual([]); + + const dir = path.join(tempDir, "ptr"); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, "generated.hpp"), result.headerCode); + fs.writeFileSync(path.join(dir, "generated.cpp"), result.cppCode); + fs.writeFileSync( + path.join(dir, "generated_debug.cpp"), + result.debugTableCpp!, + ); + + const idx = (p: string) => { + const leaf = result.debugMap!.leaves.find((l) => l.path === p); + expect(leaf, `no leaf for ${p}`).toBeTruthy(); + return `${leaf!.arrayIdx}, ${leaf!.elemIdx}`; + }; + + fs.writeFileSync( + path.join(dir, "main.cpp"), + `#include "generated.hpp" +#include "debug_dispatch.hpp" +#include +#include +strucpp::Configuration_CONFIG0 g_config; +using namespace strucpp::debug; +int fails = 0; +static void chk(const char* what, bool ok) { if (!ok) { printf("FAIL %s\\n", what); ++fails; } } +int main() { + unsigned short len = 0; + + // Scalar: same bytes as handle_read, and the length is the type width. + const void* p = handle_ptr(${idx("INSTANCE0.N")}, &len); + chk("scalar ptr non-null", p != nullptr); + chk("scalar len", len == 4); + int via_ptr = 0; memcpy(&via_ptr, p, 4); + unsigned char buf[8] = {0}; + handle_read(${idx("INSTANCE0.N")}, buf); + int via_read = 0; memcpy(&via_read, buf, 4); + chk("scalar agrees with read", via_ptr == via_read && via_ptr == 7); + + // It addresses live storage, so a write is visible through the SAME pointer + // with no second call. + int v = 42; handle_write(${idx("INSTANCE0.N")}, (const unsigned char*)&v, 4); + memcpy(&via_ptr, p, 4); + chk("scalar follows a write", via_ptr == 42); + + // Forced: handle_ptr must resolve the force exactly as handle_read does. + int f = 99; handle_set(${idx("INSTANCE0.N")}, true, (const unsigned char*)&f, 4); + const void* pf = handle_ptr(${idx("INSTANCE0.N")}, &len); + memcpy(&via_ptr, pf, 4); + handle_read(${idx("INSTANCE0.N")}, buf); + memcpy(&via_read, buf, 4); + chk("forced agrees with read", via_ptr == via_read && via_ptr == 99); + + // And a force survives the program writing the located storage directly, + // which is what read_ptr() exists for. + g_config.INSTANCE0.N = 1234; + const void* pf2 = handle_ptr(${idx("INSTANCE0.N")}, &len); + memcpy(&via_ptr, pf2, 4); + chk("force beats a direct program write", via_ptr == 99); + + handle_set(${idx("INSTANCE0.N")}, false, (const unsigned char*)&f, 4); + + // STRING: the payload, NOT the [len][payload] wire form, and len is chars. + const void* sp = handle_ptr(${idx("INSTANCE0.S")}, &len); + chk("string ptr non-null", sp != nullptr); + chk("string len", len == 5); + chk("string payload", memcmp(sp, "hello", 5) == 0); + unsigned char sbuf[260] = {0}; + handle_read(${idx("INSTANCE0.S")}, sbuf); + chk("wire form still carries the length byte", sbuf[0] == 5); + chk("wire payload matches", memcmp(sbuf + 1, sp, 5) == 0); + + // Out of range is a null, not a crash. + chk("oob is null", handle_ptr(200, 0, &len) == nullptr && len == 0); + + printf(fails ? "FAILURES=%d\\n" : "ALL_OK\\n", fails); + return fails ? 1 : 0; +} +`, + ); + + const bin2 = path.join(dir, "ptr"); + execSync( + `g++ -std=c++17 -I"${RUNTIME_INCLUDE}" -I"${dir}" ` + + `-o "${bin2}" "${path.join(dir, "main.cpp")}" ` + + `"${path.join(dir, "generated.cpp")}" "${path.join(dir, "generated_debug.cpp")}"`, + { encoding: "utf-8" }, + ); + expect(execSync(`"${bin2}"`, { encoding: "utf-8" }).trim()).toBe("ALL_OK"); + }); }); /** From afcd67c62b25b1e8823767a258016af13f9dfda7 Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Thu, 17 Sep 2026 21:02:31 -0400 Subject: [PATCH 2/4] fix(debug): address the review on the live pointer accessor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eight items from the review on #238. **The endianness assert took the header down with it (blocker).** It sat at namespace scope and unconditional, so it fired when `debug_dispatch.hpp` was INCLUDED, not when `ptr_wstring` was used: a big-endian build of firmware that only ever calls handle_read/handle_write stopped compiling, and MSVC (which defines no `__BYTE_ORDER__`) failed regardless of endianness. Reproduced both. It is now a compile-time selection — on a target that cannot be served, the op returns nullptr with *len = 0, which is already handle_ptr's contract for "no pointer available", and routes the caller to the copying path the old assertion text told them to use anyway. **A parameterised string was registered wrong rather than not at all.** Every string op casts the leaf to `IECStringVar<254>`, but codegen emits the DECLARED size, and `length_` sits at a different offset for every `n`. Measured on `STRING(20) := 'hello'`: sizeof 50 vs 518, length read from 468 bytes past the end. It read 0 there — but with non-zero neighbouring memory it reads as the cap, and the read path then serves that many bytes of adjacent memory. `Entry` has nowhere to carry a per-leaf width, so the generator now SKIPS such a leaf with a reason instead of registering it. Nothing authored in the editor can reach this (its type picker is a closed list from the IEC registry, which has no length parameter) — it is hand-written ST through the CLI. **Skipped leaves were silent.** `skipped` was collected and dropped, so a variable was simply absent from the debugger and from OPC-UA with nothing in the build to say why. It is now a warning per leaf, like `incomplete` already was. **The pointer ops no longer truncate at DEBUG_STRING_CAP.** That cap is the Modbus debug FRAME budget — one length byte and a fixed window. A pointer has neither, and OPC-UA carries its own length, so clamping there silently shortened a 200-character string to 126 on a transport that never asked for it. Verified: 200 chars now arrive whole, and an empty string still returns a non-null pointer with length 0. **ptr_wstring had no test.** Added the byte-vs-code-unit arithmetic checked against what read_wstring puts on the wire, empty STRING and WSTRING (a valid value, not a miss), and a forced STRING through handle_ptr. Confirmed they fail when the `* 2` is removed. Also: the lifetime contract now says a pointer dies when the force state changes (read_ptr returns a different object after unforce), `docs/RUNTIME.md` documents `read_ptr()` beside `raw_ptr()`, the type_ops comment no longer claims to be flash-resident on AVR and carries the measured cost of the new column, and the table's column alignment is restored. 2399 tests pass. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p --- docs/RUNTIME.md | 16 +++ src/backend/debug-table-gen.ts | 46 ++++++++ src/index.ts | 16 +++ src/runtime/include/debug_dispatch.hpp | 125 +++++++++++++++------- tests/integration/debug-table-cpp.test.ts | 34 ++++++ 5 files changed, 198 insertions(+), 39 deletions(-) diff --git a/docs/RUNTIME.md b/docs/RUNTIME.md index 5c94a36e..f2feedd1 100644 --- a/docs/RUNTIME.md +++ b/docs/RUNTIME.md @@ -67,6 +67,9 @@ public: // Raw pointer for I/O memory binding T* raw_ptr(); + // Const pointer to the value a READER should see (force-aware) + const T* read_ptr() const; + // Cross-type converting constructor (enables implicit widening) template IECVar(const IECVar& other); }; @@ -95,6 +98,19 @@ Array elements store raw types; the calling variable wraps the entire array. Located variables (`AT %IX0.0`) use `raw_ptr()` to bind to an I/O image table at runtime. The code generator produces a descriptor array (`__located_vars_[]`) with metadata for each located variable (area, size, byte/bit indices) and accessor methods (`getLocatedVars()`, `getLocatedVarCount()`). +### `raw_ptr()` vs `read_ptr()` + +Two pointers into an `IECVar`, and picking the wrong one is silently wrong for exactly one kind of variable. + +| | Points at | Use for | +|---|---|---| +| `raw_ptr()` | `value_`, always | **Binding** — the I/O image writes through it every scan | +| `read_ptr()` | `forced_value_` while forced, `value_` otherwise | **Reading** — anything serving the value to a person or a protocol | + +`force()` writes through to `value_`, so for an ordinary variable the two agree the moment a force is applied. A **located** variable is the exception: the PLC program drives `value_` directly through the image binding every scan, without going through `set()`, so `raw_ptr()` would show the program's value while `get()` still reports the forced one. `read_ptr()` is `get()`'s semantics with `get()`'s copy removed, which is what an external reader needs to serve a value without copying it. `IECStringVar::c_str()` resolves the force the same way. + +The debug dispatch's pointer op (`handle_ptr`, and through it OPC-UA's zero-copy read) uses `read_ptr()` for that reason. The pointer it returns is valid only until the variable is next written **or its force state changes** — an `unforce()` moves the value back to a different object, and a retained pointer keeps reporting the stale forced one. + ## Type Traits (`iec_traits.hpp`) Template traits for compile-time type categorization: diff --git a/src/backend/debug-table-gen.ts b/src/backend/debug-table-gen.ts index 36bdda90..860c0db8 100644 --- a/src/backend/debug-table-gen.ts +++ b/src/backend/debug-table-gen.ts @@ -187,6 +187,16 @@ const IEC_NAME_TO_TAG: Record = { }; /** Byte size for each IEC elementary type — authoritative for debug. */ +/** + * The string capacity the debug dispatch assumes. + * + * `debug_dispatch.hpp` casts every STRING/WSTRING leaf to + * `IECStringVar<254>` / `IECWStringVar<254>`. A leaf declared with any other + * length has its members at different offsets, so it cannot be registered — + * see the check in `visit()`. Keep in step with those casts. + */ +const DISPATCH_STRING_CAPACITY = 254; + const IEC_NAME_TO_SIZE: Record = { BOOL: 1, SINT: 1, @@ -505,6 +515,42 @@ export function generateDebugTable( // Named elementary type (or alias thereof). if (IEC_NAME_TO_TAG[name] !== undefined) { + // A parameterised STRING(n) / WSTRING(n) cannot be dispatched. Every + // string op in `debug_dispatch.hpp` casts the leaf to + // `IECStringVar<254>` / `IECWStringVar<254>`, but codegen emits the + // DECLARED size (`IECStringVar`), and `length_` sits at a different + // offset for every `n` — so the cast reads out of bounds. Measured on + // `s20 : STRING(20) := 'hello'`: `sizeof` 50 vs 518, and the length came + // back from 468 bytes past the end of the object. It read 0 there, but + // the value is undefined: with non-zero neighbouring memory it reads as + // the cap, and the read path would then serve that many bytes of + // adjacent memory. + // + // The dispatch has one function per TAG and `Entry` has nowhere to carry + // a per-leaf size, so making this work needs a per-leaf width in the + // table — a real change to a structure that is deliberately 6 bytes on + // AVR. Until then the leaf is SKIPPED rather than registered wrong: + // `skipped` is already surfaced to the caller, so this is a visible + // build warning instead of an out-of-bounds read at runtime. + // + // Nothing authored in the editor reaches this: its type picker is a + // closed list from the IEC registry, which carries `STRING` with no + // length parameter. It is hand-written ST compiled through the CLI that + // can declare `STRING(n)`. + if ( + (name === "STRING" || name === "WSTRING") && + typeRef.maxLength !== undefined && + typeRef.maxLength !== DISPATCH_STRING_CAPACITY + ) { + skipped.push({ + path, + reason: + `declared ${name}(${typeRef.maxLength}), but the debug dispatch ` + + `addresses strings as ${name}(${DISPATCH_STRING_CAPACITY}). ` + + `Declare it as a plain ${name} to expose it.`, + }); + return; + } addLeaf(path, cppExpr, name, flags); return; } diff --git a/src/index.ts b/src/index.ts index ab30c20c..95829b06 100644 --- a/src/index.ts +++ b/src/index.ts @@ -821,6 +821,22 @@ export function compile( severity: "warning", }); } + + // A leaf the walk could not register is not debuggable: it is absent from + // the debugger's watch list and from anything built on the debug table, + // OPC-UA included. That was silent — `skipped` was collected and dropped — + // so a variable simply was not there, with nothing in the build to say why + // or which. Same reasoning as `incomplete` above: the program still builds, + // but a drop nobody is told about is found later, at a debugger that shows + // nothing, with no thread back to the cause. + for (const item of dbg.skipped) { + pipeline.warnings.push({ + message: `${item.path} is not debuggable: ${item.reason}`, + line: 0, + column: 0, + severity: "warning", + }); + } } return { diff --git a/src/runtime/include/debug_dispatch.hpp b/src/runtime/include/debug_dispatch.hpp index e31ee934..ab4d5f5e 100644 --- a/src/runtime/include/debug_dispatch.hpp +++ b/src/runtime/include/debug_dispatch.hpp @@ -129,9 +129,15 @@ inline void read_impl(const void* p, uint8_t* dest) noexcept { // located variable is written by the PLC program straight into value_, so // raw_ptr() would leak the program's value past an active force. // -// The pointer is valid until the variable is next written, which for a -// cooperative single-threaded runtime means "until the caller yields". A -// caller that can be preempted by the PLC scan must copy instead. +// The pointer is valid until the variable is next written OR ITS FORCE STATE +// CHANGES, which for a cooperative single-threaded runtime means "until the +// caller yields". A caller that can be preempted by the PLC scan must copy +// instead. +// +// The force clause is not pedantry: read_ptr() returns &forced_value_ while +// forced and &value_ otherwise, so force()/unforce() changes WHICH OBJECT the +// pointer refers to. A pointer taken while forced keeps reporting the stale +// forced value after an unforce, with nothing written to invalidate it. template inline const void* ptr_impl(const void* p, uint16_t* len) noexcept { *len = static_cast(sizeof(T)); @@ -206,10 +212,18 @@ inline void unforce_string(void* p) noexcept { // The payload only — NOT the [len][payload] wire form read_string() builds. // `len` is the character count, and c_str() already resolves the force. +// +// Deliberately NOT capped at DEBUG_STRING_CAP. That cap is the Modbus debug +// FRAME budget: the wire form spends one byte on the length prefix and the +// editor reads a fixed window, so read_string() has to truncate. A pointer +// spends nothing and has no window — the caller (OPC-UA) carries its own +// length — so clamping here would silently shorten a string for a transport +// that never asked for it. `length()` is already bounded by the variable's own +// capacity, which is the only real limit on this path. +// `len` must be non-null; handle_ptr always passes a real address. inline const void* ptr_string(const void* p, uint16_t* len) noexcept { const auto* var = static_cast*>(p); - const std::size_t actual_len = var->length(); - *len = static_cast(actual_len < DEBUG_STRING_CAP ? actual_len : DEBUG_STRING_CAP); + *len = static_cast(var->length()); return var->c_str(); } @@ -263,22 +277,34 @@ inline void unforce_wstring(void* p) noexcept { } // The code-unit buffer as it sits in memory, so `len` is BYTES (2 per code -// unit), not characters. +// unit), not characters. Like ptr_string, NOT capped at DEBUG_STRING_CAP — +// see the note there; the cap belongs to the Modbus frame, not to a pointer. +// `len` must be non-null; handle_ptr always passes a real address. // // read_wstring() splits each code unit explicitly to keep the WIRE format // little-endian on any host. A pointer cannot do that — what the caller gets is -// host order — so this op is only correct on a little-endian target, and says -// so rather than silently serving byte-swapped text somewhere else. -static_assert(__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__, - "ptr_wstring hands out UTF-16 code units in host order; a " - "big-endian target must copy through read_wstring() instead."); +// host order — so this op is only correct on a little-endian target. +// +// The refusal is a RUNTIME one, not a static_assert. At namespace scope an +// unconditional assert fires when the header is INCLUDED, so a big-endian (or +// MSVC, which defines no __BYTE_ORDER__) build of firmware that only ever calls +// handle_read/handle_write stopped compiling — an op nobody referenced taking +// the whole translation unit down with it. Refusing here instead leaves every +// other op available and routes the caller to the copying path, which is what +// the old assertion text told them to do anyway. +#if !defined(__BYTE_ORDER__) || !defined(__ORDER_LITTLE_ENDIAN__) || \ + __BYTE_ORDER__ != __ORDER_LITTLE_ENDIAN__ +inline const void* ptr_wstring(const void*, uint16_t* len) noexcept { + *len = 0; + return nullptr; // handle_ptr's contract for "no pointer available" +} +#else inline const void* ptr_wstring(const void* p, uint16_t* len) noexcept { const auto* var = static_cast*>(p); - const std::size_t actual_len = var->length(); - const std::size_t capped = actual_len < DEBUG_STRING_CAP ? actual_len : DEBUG_STRING_CAP; - *len = static_cast(capped * 2); + *len = static_cast(var->length() * 2); return var->c_str(); } +#endif // --------------------------------------------------------------------------- // Dispatch table entry. The `size` field is the byte width consumed/produced @@ -298,30 +324,47 @@ struct TypeOps { // --------------------------------------------------------------------------- // type_ops[]: one row per TypeTag, in tag order. -// Kept inline so it's flash-resident with no separate .cpp required. +// Kept inline so no separate .cpp is required. +// +// NOT flash-resident on AVR, whatever "inline constexpr" suggests. The Entry +// tables carry STRUCPP_DEBUG_FLASH (see debug_table.hpp) and this does not, so +// on a Harvard target it is const data in .rodata, which the startup code +// copies into SRAM. Every AVR firmware pays for it, including one that only +// ever calls handle_read/handle_write. +// +// The `ptr` column costs one function pointer per row: 21 rows x 2 bytes = 42 +// bytes of SRAM on an ATmega-class part, plus the ptr_impl instantiations in +// flash. (Measured on the host — 48 vs 40 bytes per row at 8-byte pointers — +// and scaled to AVR's 2-byte function pointers; it could NOT be measured on +// target, because no AVR core is installed to build against.) +// +// Moving this to STRUCPP_DEBUG_FLASH is not a one-line change: every read of a +// row would have to go through pgm_read_ptr, in the hot path of handle_read / +// handle_write / handle_set. Worth doing only against a real AVR build that can +// show the before and after. // --------------------------------------------------------------------------- inline constexpr TypeOps type_ops[TAG__COUNT] = { - /*BOOL */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(BOOL_t) }, - /*SINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(SINT_t) }, - /*USINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(USINT_t) }, - /*INT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(INT_t) }, - /*UINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(UINT_t) }, - /*DINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(DINT_t) }, - /*UDINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(UDINT_t) }, - /*LINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(LINT_t) }, - /*ULINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(ULINT_t) }, - /*REAL */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(REAL_t) }, - /*LREAL */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(LREAL_t) }, - /*BYTE */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(BYTE_t) }, - /*WORD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(WORD_t) }, - /*DWORD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(DWORD_t) }, - /*LWORD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(LWORD_t) }, - /*TIME */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(TIME_t) }, - /*DATE */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(DATE_t) }, - /*TOD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(TOD_t) }, - /*DT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(DT_t) }, - /*STRING */ { &force_string, &unforce_string, &read_string, &write_string, &ptr_string, DEBUG_STRING_WIDTH }, - /*WSTRING*/ { &force_wstring, &unforce_wstring, &read_wstring, &write_wstring, &ptr_wstring, DEBUG_WSTRING_WIDTH }, + /*BOOL */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(BOOL_t) }, + /*SINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(SINT_t) }, + /*USINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(USINT_t) }, + /*INT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(INT_t) }, + /*UINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(UINT_t) }, + /*DINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(DINT_t) }, + /*UDINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(UDINT_t) }, + /*LINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(LINT_t) }, + /*ULINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(ULINT_t) }, + /*REAL */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(REAL_t) }, + /*LREAL */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(LREAL_t) }, + /*BYTE */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(BYTE_t) }, + /*WORD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(WORD_t) }, + /*DWORD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(DWORD_t) }, + /*LWORD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(LWORD_t) }, + /*TIME */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(TIME_t) }, + /*DATE */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(DATE_t) }, + /*TOD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(TOD_t) }, + /*DT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(DT_t) }, + /*STRING*/ { &force_string, &unforce_string, &read_string, &write_string, &ptr_string, DEBUG_STRING_WIDTH }, + /*WSTRING*/ { &force_wstring, &unforce_wstring, &read_wstring, &write_wstring, &ptr_wstring, DEBUG_WSTRING_WIDTH }, }; // --------------------------------------------------------------------------- @@ -511,9 +554,13 @@ inline uint16_t handle_size(uint8_t arr, uint16_t elem) noexcept { * Returns nullptr (and sets *out_len = 0) for an out-of-range leaf. * * The copy-free counterpart to handle_read(). The pointer is into live PLC - * storage, so it is valid only until the variable is next written — safe for a - * caller that runs inside the scan's own thread of control, wrong for one that - * can be preempted by it. */ + * storage, so it is valid only until the variable is next written or its force + * state changes — safe for a caller that runs inside the scan's own thread of + * control, wrong for one that can be preempted by it. + * + * Returns nullptr with *out_len = 0 when the leaf has no address to give: + * an unknown (arr, elem), a tag with no ptr op, or WSTRING on a big-endian + * target (see ptr_wstring). */ inline const void* handle_ptr(uint8_t arr, uint16_t elem, uint16_t* out_len) noexcept { Entry e = read_entry(arr, elem); if (!e.ptr || e.tag >= TAG__COUNT) { diff --git a/tests/integration/debug-table-cpp.test.ts b/tests/integration/debug-table-cpp.test.ts index 294f2910..de5a8fc5 100644 --- a/tests/integration/debug-table-cpp.test.ts +++ b/tests/integration/debug-table-cpp.test.ts @@ -483,6 +483,9 @@ int main() { `PROGRAM Main VAR n : DINT := 7; END_VAR VAR s : STRING := 'hello'; END_VAR +VAR w : WSTRING := "hi"; END_VAR +VAR es : STRING; END_VAR +VAR ew : WSTRING; END_VAR END_PROGRAM${CFG}`, { headerFileName: "generated.hpp" }, ); @@ -559,6 +562,37 @@ int main() { chk("wire form still carries the length byte", sbuf[0] == 5); chk("wire payload matches", memcmp(sbuf + 1, sp, 5) == 0); + // A forced STRING resolves through c_str() the same way the scalar op + // resolves through read_ptr(). Nothing proved that before. + unsigned char sforce[1 + 5] = { 5, 'w','o','r','l','d' }; + handle_set(${idx("INSTANCE0.S")}, true, sforce, sizeof(sforce)); + const void* spf = handle_ptr(${idx("INSTANCE0.S")}, &len); + chk("forced string len", len == 5); + chk("forced string payload", memcmp(spf, "world", 5) == 0); + handle_set(${idx("INSTANCE0.S")}, false, sforce, sizeof(sforce)); + + // WSTRING: len is BYTES (2 per code unit), and the code units are the same + // ones read_wstring() puts on the wire once its explicit LE split is undone. + const void* wp = handle_ptr(${idx("INSTANCE0.W")}, &len); + chk("wstring ptr non-null", wp != nullptr); + chk("wstring len is bytes not units", len == 4); + const unsigned char* wb = (const unsigned char*)wp; + unsigned char wbuf[260] = {0}; + handle_read(${idx("INSTANCE0.W")}, wbuf); + chk("wstring wire count is units", wbuf[0] == 2); + chk("wstring unit 0 matches the wire", wb[0] == wbuf[1] && wb[1] == wbuf[2]); + chk("wstring unit 1 matches the wire", wb[2] == wbuf[3] && wb[3] == wbuf[4]); + chk("wstring units are 'hi'", wb[0] == 'h' && wb[1] == 0 && wb[2] == 'i' && wb[3] == 0); + + // Empty is a VALID value, not a miss: a non-null pointer and a zero length. + // read_node treats zero length as a value, so this is a contract. + const void* ep = handle_ptr(${idx("INSTANCE0.ES")}, &len); + chk("empty string len", len == 0); + chk("empty string ptr non-null", ep != nullptr); + const void* ewp = handle_ptr(${idx("INSTANCE0.EW")}, &len); + chk("empty wstring len", len == 0); + chk("empty wstring ptr non-null", ewp != nullptr); + // Out of range is a null, not a crash. chk("oob is null", handle_ptr(200, 0, &len) == nullptr && len == 0); From e38fbeb8d7a9ca9bcfb80e43337b24c1e87fdb36 Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Thu, 17 Sep 2026 21:44:14 -0400 Subject: [PATCH 3/4] perf(debug): stop charging every AVR firmware for the pointer op MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit S8 from the review, now measured on target rather than argued. The reviewer asked whether the new `ptr` column moved the smallest supported target's footprint, and could not measure it — nor could I at first, because the Arduino AVR toolchain ships no C++ standard headers. It does build: the editor carries the shim at `resources/sources/avr-libstdcpp/include`, and its simulator compiles for `arduino:avr:mega` (an ATmega2560) through arduino-cli. Same sketch, same runtime tree, only calling handle_read/handle_write: with the ptr column 425 B SRAM 5128 B flash without it (baseline) 383 B SRAM 4654 B flash So yes: +42 bytes of SRAM and ~474 of flash, on every AVR firmware, for an op only the baremetal OPC-UA server calls. `type_ops` carries no STRUCPP_DEBUG_FLASH, so on a Harvard target it is .rodata copied into SRAM at startup — "inline constexpr" does not make it flash-resident. The fix is not to move `type_ops` to PROGMEM (that would route every row read through pgm_read_ptr in the hot path of handle_read/handle_write/handle_set, which is a change that deserves its own evidence). It is to stop the pointer op being a column at all: `ptr_ops[]` is a table of its own, read only by handle_ptr. Nothing references it unless handle_ptr is called, so -ffunction-sections/-fdata-sections + --gc-sections drop both the table and the ptr_impl instantiations. Measured after the split, same sketch: never calls handle_ptr 383 B SRAM 4728 B flash calls handle_ptr 425 B SRAM 5196 B flash The feature now costs what it costs, and only to firmware that uses it. The consumers are unaffected — the baremetal glue calls handle_ptr and never touches the table. 2399 tests pass. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p --- src/runtime/include/debug_dispatch.hpp | 113 +++++++++++++++++-------- 1 file changed, 76 insertions(+), 37 deletions(-) diff --git a/src/runtime/include/debug_dispatch.hpp b/src/runtime/include/debug_dispatch.hpp index ab4d5f5e..144f391c 100644 --- a/src/runtime/include/debug_dispatch.hpp +++ b/src/runtime/include/debug_dispatch.hpp @@ -315,13 +315,14 @@ struct TypeOps { void (*unforce)(void*); void (*read) (const void*, uint8_t*); void (*write) (void*, const uint8_t*); - /** Address the value in place instead of copying it out, and report its - * length in BYTES. Same value `read` would produce, same force - * resolution — see ptr_impl(). */ - const void* (*ptr)(const void*, uint16_t*); uint8_t size; }; +/** The pointer op, in a table of its own — see ptr_ops[] below. */ +struct PtrOps { + const void* (*ptr)(const void*, uint16_t*); +}; + // --------------------------------------------------------------------------- // type_ops[]: one row per TypeTag, in tag order. // Kept inline so no separate .cpp is required. @@ -329,42 +330,80 @@ struct TypeOps { // NOT flash-resident on AVR, whatever "inline constexpr" suggests. The Entry // tables carry STRUCPP_DEBUG_FLASH (see debug_table.hpp) and this does not, so // on a Harvard target it is const data in .rodata, which the startup code -// copies into SRAM. Every AVR firmware pays for it, including one that only -// ever calls handle_read/handle_write. +// copies into SRAM. Every AVR firmware pays for this table. // -// The `ptr` column costs one function pointer per row: 21 rows x 2 bytes = 42 -// bytes of SRAM on an ATmega-class part, plus the ptr_impl instantiations in -// flash. (Measured on the host — 48 vs 40 bytes per row at 8-byte pointers — -// and scaled to AVR's 2-byte function pointers; it could NOT be measured on -// target, because no AVR core is installed to build against.) +// That is why the pointer op is NOT a column here — see ptr_ops[] below. // -// Moving this to STRUCPP_DEBUG_FLASH is not a one-line change: every read of a -// row would have to go through pgm_read_ptr, in the hot path of handle_read / -// handle_write / handle_set. Worth doing only against a real AVR build that can -// show the before and after. +// Moving this one to STRUCPP_DEBUG_FLASH would mean routing every row read +// through pgm_read_ptr in the hot path of handle_read / handle_write / +// handle_set. Worth doing on its own evidence, not as a side effect. // --------------------------------------------------------------------------- inline constexpr TypeOps type_ops[TAG__COUNT] = { - /*BOOL */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(BOOL_t) }, - /*SINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(SINT_t) }, - /*USINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(USINT_t) }, - /*INT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(INT_t) }, - /*UINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(UINT_t) }, - /*DINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(DINT_t) }, - /*UDINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(UDINT_t) }, - /*LINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(LINT_t) }, - /*ULINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(ULINT_t) }, - /*REAL */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(REAL_t) }, - /*LREAL */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(LREAL_t) }, - /*BYTE */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(BYTE_t) }, - /*WORD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(WORD_t) }, - /*DWORD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(DWORD_t) }, - /*LWORD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(LWORD_t) }, - /*TIME */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(TIME_t) }, - /*DATE */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(DATE_t) }, - /*TOD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(TOD_t) }, - /*DT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, &ptr_impl, sizeof(DT_t) }, - /*STRING*/ { &force_string, &unforce_string, &read_string, &write_string, &ptr_string, DEBUG_STRING_WIDTH }, - /*WSTRING*/ { &force_wstring, &unforce_wstring, &read_wstring, &write_wstring, &ptr_wstring, DEBUG_WSTRING_WIDTH }, + /*BOOL */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(BOOL_t) }, + /*SINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(SINT_t) }, + /*USINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(USINT_t) }, + /*INT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(INT_t) }, + /*UINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(UINT_t) }, + /*DINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(DINT_t) }, + /*UDINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(UDINT_t) }, + /*LINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(LINT_t) }, + /*ULINT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(ULINT_t) }, + /*REAL */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(REAL_t) }, + /*LREAL */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(LREAL_t) }, + /*BYTE */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(BYTE_t) }, + /*WORD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(WORD_t) }, + /*DWORD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(DWORD_t) }, + /*LWORD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(LWORD_t) }, + /*TIME */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(TIME_t) }, + /*DATE */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(DATE_t) }, + /*TOD */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(TOD_t) }, + /*DT */ { &force_impl, &unforce_impl, &read_impl, &write_impl, sizeof(DT_t) }, + /*STRING */ { &force_string, &unforce_string, &read_string, &write_string, DEBUG_STRING_WIDTH }, + /*WSTRING */ { &force_wstring, &unforce_wstring, &read_wstring, &write_wstring, DEBUG_WSTRING_WIDTH }, +}; + +// --------------------------------------------------------------------------- +// ptr_ops[]: the pointer op, deliberately NOT a sixth column of type_ops. +// +// Only handle_ptr reads this, and only the baremetal OPC-UA server calls +// handle_ptr — but as a column it was reachable from handle_read/handle_write +// too, so the whole table grew for every firmware. Measured on an ATmega2560 +// (arduino:avr:mega, the simulator's target) with a sketch that only ever calls +// handle_read/handle_write: 383 -> 425 bytes of SRAM and 4654 -> 5128 of flash. +// +// Split out, nothing references this table unless handle_ptr is called, and +// -ffunction-sections/-fdata-sections + --gc-sections (which the AVR core +// passes) drop both it and the ptr_impl instantiations. +// +// Measured on arduino:avr:mega, same sketch, same runtime tree: +// +// never calls handle_ptr 383 B SRAM / 4728 B flash (was 425 / 5128) +// calls handle_ptr 425 B SRAM / 5196 B flash +// +// So the feature now costs what it costs, and only to firmware that uses it. +// --------------------------------------------------------------------------- +inline constexpr PtrOps ptr_ops[TAG__COUNT] = { + /*BOOL */ { &ptr_impl }, + /*SINT */ { &ptr_impl }, + /*USINT */ { &ptr_impl }, + /*INT */ { &ptr_impl }, + /*UINT */ { &ptr_impl }, + /*DINT */ { &ptr_impl }, + /*UDINT */ { &ptr_impl }, + /*LINT */ { &ptr_impl }, + /*ULINT */ { &ptr_impl }, + /*REAL */ { &ptr_impl }, + /*LREAL */ { &ptr_impl }, + /*BYTE */ { &ptr_impl }, + /*WORD */ { &ptr_impl }, + /*DWORD */ { &ptr_impl }, + /*LWORD */ { &ptr_impl }, + /*TIME */ { &ptr_impl }, + /*DATE */ { &ptr_impl }, + /*TOD */ { &ptr_impl }, + /*DT */ { &ptr_impl }, + /*STRING */ { &ptr_string }, + /*WSTRING */ { &ptr_wstring }, }; // --------------------------------------------------------------------------- @@ -568,7 +607,7 @@ inline const void* handle_ptr(uint8_t arr, uint16_t elem, uint16_t* out_len) noe return nullptr; } uint16_t len = 0; - const void* p = type_ops[e.tag].ptr(e.ptr, &len); + const void* p = ptr_ops[e.tag].ptr(e.ptr, &len); if (out_len) *out_len = len; return p; } From 32bc76ed64bdbd6c058478d954842e9651d47dc0 Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Fri, 18 Sep 2026 12:22:26 -0400 Subject: [PATCH 4/4] chore(release): bump package.json to 0.6.8 Keep the committed version in step with the release tag. The release workflow injects the version from the tag at build time, so this is for consistency rather than function. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 166c80ba..dcfab0a6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "strucpp", - "version": "0.6.4", + "version": "0.6.8", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "strucpp", - "version": "0.6.4", + "version": "0.6.8", "license": "GPL-3.0-or-later", "dependencies": { "chevrotain": "^11.0.0" diff --git a/package.json b/package.json index 01ee8bd0..5e06def7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "strucpp", - "version": "0.6.4", + "version": "0.6.8", "description": "IEC 61131-3 Structured Text to C++ Compiler", "main": "dist/index.js", "types": "dist/index.d.ts",