feat(io): size the process image per board, and refuse locations outside it

resources/sources/Baremetal/Baremetal.ino

@@ -261,14 +261,31 @@ void setup()
 // MAP EMPTY BUFFERS (for Modbus)
 // =============================================================================
 #ifdef MODBUS_ENABLED
+
+// Backing storage for discrete slots the PLC program did not claim.
+//
+// The analog and memory slots below alias straight into the Modbus banks,
+// which is what makes an unclaimed %QW readable over Modbus. The discrete
+// banks are bit-packed and cannot be aliased that way, so each unclaimed
+// bit needs a byte of its own -- this array is it.
+//
+// One static block rather than a malloc per slot: this used to call
+// malloc(1) once per unbound point, which on a board with a 15-slot
+// expansion backplane is ~480 one-byte allocations, each carrying its own
+// heap header (often 8 bytes, so ~8x the payload) and fragmenting the heap
+// before the program has run a single scan. A flat array costs exactly
+// MAX_DIGITAL_INPUT + MAX_DIGITAL_OUTPUT bytes, needs no allocator, and
+// cannot fail partway through and leave the image half-mapped
+// (openplc-editor#296).
+static IEC_BOOL empty_discrete[MAX_DIGITAL_INPUT + MAX_DIGITAL_OUTPUT] = {};
+
 void mapEmptyBuffers()
 {
     for (int i = 0; i < MAX_DIGITAL_OUTPUT; i++)
     {
         if (bool_output[i/8][i%8] == NULL)
         {
-            bool_output[i/8][i%8] = (IEC_BOOL *)malloc(sizeof(IEC_BOOL));
-            *bool_output[i/8][i%8] = 0;
+            bool_output[i/8][i%8] = &empty_discrete[i];
         }
     }
     for (int i = 0; i < MAX_ANALOG_OUTPUT; i++)
@@ -282,8 +299,8 @@ void mapEmptyBuffers()
     {
         if (bool_input[i/8][i%8] == NULL)
         {
-            bool_input[i/8][i%8] = (IEC_BOOL *)malloc(sizeof(IEC_BOOL));
-            *bool_input[i/8][i%8] = 0;
+            // Offset past the output half -- one array, two disjoint ranges.
+            bool_input[i/8][i%8] = &empty_discrete[MAX_DIGITAL_OUTPUT + i];
         }
     }
     for (int i = 0; i < MAX_ANALOG_INPUT; i++)

resources/sources/Baremetal/modbus_registers.cpp

@@ -9,7 +9,7 @@ Copyright (C) 2022 OpenPLC - Thiago Alves
 // In a debug-only build this whole TU compiles to nothing, saving flash/SRAM.
 #ifdef MODBUS_ENABLED
 
-bool init_mbregs(uint8_t size_holding, uint8_t size_dint_memory, uint8_t size_lint_memory, uint8_t size_coils, uint8_t size_inputregs, uint8_t size_inputstatus)
+bool init_mbregs(uint16_t size_holding, uint16_t size_dint_memory, uint16_t size_lint_memory, uint16_t size_coils, uint16_t size_inputregs, uint16_t size_inputstatus)
 {
     //Save sizes
     modbus.holding_size = size_holding;
@@ -62,9 +62,14 @@ bool init_mbregs(uint8_t size_holding, uint8_t size_dint_memory, uint8_t size_li
     return true;
 }
 
+// byte_addr is uint16_t, not uint8_t: addr is already a 16-bit Modbus
+// address, so addr/8 overflows a uint8_t past 2040 coils. The callers
+// bound `addr` against *_size before getting here, and *_size is now
+// itself 16-bit -- narrowing the index would put the truncation back one
+// step further down.
 bool get_discrete(uint16_t addr, bool regtype)
 {
-    uint8_t byte_addr = addr / 8;
+    uint16_t byte_addr = addr / 8;
     uint8_t bit_addr = addr % 8;
     if (regtype == COILS)
         return bitRead(modbus.coils[byte_addr], bit_addr);
@@ -74,7 +79,7 @@ bool get_discrete(uint16_t addr, bool regtype)
 
 void write_discrete(uint16_t addr, bool regtype, bool value)
 {
-    uint8_t byte_addr = addr / 8;
+    uint16_t byte_addr = addr / 8;
     uint8_t bit_addr = addr % 8;
     if (regtype == COILS)
         bitWrite(modbus.coils[byte_addr], bit_addr, value);
@@ -116,7 +121,9 @@ void readRegisters(uint16_t startreg, uint16_t numregs)
 
     uint16_t val;
     uint16_t i = 0;
-    uint8_t pos = 0;
+    // uint16_t, not uint8_t: pos indexes dint_memory/lint_memory, whose
+    // sizes come from the MAX_MEMORY_* macros and are no longer capped at 255.
+    uint16_t pos = 0;
 	while(numregs--)
     {
         if ((startreg + i) < modbus.holding_size)
@@ -177,7 +184,9 @@ void writeSingleRegister(uint16_t reg, uint16_t value)
         return;
     }
 
-    uint8_t pos = 0;
+    // uint16_t, not uint8_t: pos indexes dint_memory/lint_memory, whose
+    // sizes come from the MAX_MEMORY_* macros and are no longer capped at 255.
+    uint16_t pos = 0;
 
     if (reg < modbus.holding_size)
     {
@@ -254,7 +263,9 @@ void writeMultipleRegisters(uint16_t startreg, uint16_t numoutputs, uint8_t byte
 
     uint16_t value;
     uint16_t i = 0;
-    uint8_t pos = 0;
+    // uint16_t, not uint8_t: pos indexes dint_memory/lint_memory, whose
+    // sizes come from the MAX_MEMORY_* macros and are no longer capped at 255.
+    uint16_t pos = 0;
 	while(numoutputs--)
     {
         value = (uint16_t)mb_frame[7+i*2] << 8 | (uint16_t)mb_frame[8+i*2];
@@ -350,7 +361,12 @@ void readCoils(uint16_t startreg, uint16_t numregs)
 	while (numregs)
     {
         i = (totregs - numregs--) / 8;
-		if (get_discrete((uint8_t)startreg, COILS))
+		// No (uint8_t) cast on startreg: it is a 16-bit coil address, and
+		// truncating it aliased every coil above 255 onto a low one --
+		// FC 0x01 answered with the wrong bit and no error. Harmless while
+		// no board had more than 56 coils; reachable as soon as one does
+		// (openplc-editor#296). readInputStatus below never had the cast.
+		if (get_discrete(startreg, COILS))
 			bitSet(mb_frame[3+i], bitn);
 		else
 			bitClear(mb_frame[3+i], bitn);

resources/sources/Baremetal/modbus_registers.h

@@ -14,7 +14,9 @@ lives in modbus_frame.* because its slave id is shared by every build.
 
 #include "modbus_frame.h"
 
-bool init_mbregs(uint8_t size_holding, uint8_t size_dint_memory, uint8_t size_lint_memory, uint8_t size_coils, uint8_t size_inputregs, uint8_t size_inputstatus);
+// Sizes are uint16_t: they come straight from the MAX_* process-image
+// macros, which a board with an expansion backplane sizes past 255.
+bool init_mbregs(uint16_t size_holding, uint16_t size_dint_memory, uint16_t size_lint_memory, uint16_t size_coils, uint16_t size_inputregs, uint16_t size_inputstatus);
 bool get_discrete(uint16_t addr, bool regtype);
 void write_discrete(uint16_t addr, bool regtype, bool value);
 

resources/sources/Baremetal/modbus_types.h

@@ -56,21 +56,28 @@ protocol, transport, register and debug layers agree on the same contracts.
 // exceptions (0x01-0x04) nor 0x7E/0x81/0x82.
 #define MB_PLC_CTRL_REFUSED_SWITCH       0x86
 
-//Modbus registers struct
+// Modbus registers struct
+//
+// The *_size fields are uint16_t, not uint8_t: they are populated from the
+// MAX_* process-image macros, and a board with an expansion backplane sizes
+// those well past 255 (a 15-slot P1AM reaches 240 discrete points per
+// direction). As uint8_t the assignment in init_mbregs truncated silently --
+// 256 coils became 0 -- and the register map came up wrong with no
+// diagnostic anywhere (openplc-editor#296).
 struct MBinfo {
     uint8_t slaveid;
     uint16_t *holding;
-    uint8_t holding_size;
+    uint16_t holding_size;
     uint32_t *dint_memory;
-    uint8_t dint_memory_size;
+    uint16_t dint_memory_size;
     uint64_t *lint_memory;
-    uint8_t lint_memory_size;
+    uint16_t lint_memory_size;
     uint8_t *coils;
-    uint8_t coils_size;
+    uint16_t coils_size;
     uint16_t *input_regs;
-    uint8_t input_regs_size;
+    uint16_t input_regs_size;
     uint8_t *input_status;
-    uint8_t input_status_size;
+    uint16_t input_status_size;
 };
 
 //Function Codes

resources/sources/arduino/arduino_runtime_glue.cpp

@@ -117,6 +117,23 @@ static uint64_t gcd(uint64_t a, uint64_t b)
 
 // ---------------------------------------------------------------------------
 // I/O binding: walk locatedVars[] and bind to openplc.h buffer pointers
+//
+// Every slot write below is range-checked. locatedVars[] is authored from
+// whatever `AT %...` the user typed, and nothing in the descriptor itself
+// says how big this firmware's process image is -- so an address past the
+// end (`%QX7.0` on a 56-output image: byte_index 7 against bool_output[7][8])
+// used to write straight past the array and corrupt whatever followed it.
+// Only the DWord cases were guarded; the rest are now (openplc-editor#296).
+//
+// The editor rejects an out-of-range location before the build gets here,
+// so reaching a skip is not the expected path -- this is the backstop for a
+// hand-written .st, a project moved to a smaller board, or a stale build.
+// Dropping the binding leaves the slot NULL, which every HAL and the Modbus
+// glue already treat as "not wired" and step over.
+//
+// The bit-addressed buffers are declared [MAX/8][8], so the bound to check
+// is the FIRST dimension: an image whose digital count isn't a multiple of 8
+// rounds down, and the slots in the partial byte are unaddressable.
 // ---------------------------------------------------------------------------
 void runtime_bind_located_vars()
 {
@@ -129,10 +146,14 @@ void runtime_bind_located_vars()
         case LocatedArea::Input:
             switch (lv.size) {
             case LocatedSize::Bit:
-                bool_input[lv.byte_index][lv.bit_index] = (::IEC_BOOL*)lv.pointer;
+                if (lv.byte_index < (MAX_DIGITAL_INPUT / 8) && lv.bit_index < 8) {
+                    bool_input[lv.byte_index][lv.bit_index] = (::IEC_BOOL*)lv.pointer;
+                }
                 break;
             case LocatedSize::Word:
-                int_input[lv.byte_index] = (::IEC_UINT*)lv.pointer;
+                if (lv.byte_index < MAX_ANALOG_INPUT) {
+                    int_input[lv.byte_index] = (::IEC_UINT*)lv.pointer;
+                }
                 break;
 #if !defined(__AVR_ATmega328P__) && !defined(__AVR_ATmega168__) && !defined(__AVR_ATmega32U4__) && !defined(__AVR_ATmega16U4__)
             case LocatedSize::DWord:
@@ -157,10 +178,14 @@ void runtime_bind_located_vars()
         case LocatedArea::Output:
             switch (lv.size) {
             case LocatedSize::Bit:
-                bool_output[lv.byte_index][lv.bit_index] = (::IEC_BOOL*)lv.pointer;
+                if (lv.byte_index < (MAX_DIGITAL_OUTPUT / 8) && lv.bit_index < 8) {
+                    bool_output[lv.byte_index][lv.bit_index] = (::IEC_BOOL*)lv.pointer;
+                }
                 break;
             case LocatedSize::Word:
-                int_output[lv.byte_index] = (::IEC_UINT*)lv.pointer;
+                if (lv.byte_index < MAX_ANALOG_OUTPUT) {
+                    int_output[lv.byte_index] = (::IEC_UINT*)lv.pointer;
+                }
                 break;
 #if !defined(__AVR_ATmega328P__) && !defined(__AVR_ATmega168__) && !defined(__AVR_ATmega32U4__) && !defined(__AVR_ATmega16U4__)
             case LocatedSize::DWord:
@@ -183,13 +208,19 @@ void runtime_bind_located_vars()
 #if !defined(__AVR_ATmega328P__) && !defined(__AVR_ATmega168__) && !defined(__AVR_ATmega32U4__) && !defined(__AVR_ATmega16U4__)
             switch (lv.size) {
             case LocatedSize::Word:
-                int_memory[lv.byte_index] = (::IEC_UINT*)lv.pointer;
+                if (lv.byte_index < MAX_MEMORY_WORD) {
+                    int_memory[lv.byte_index] = (::IEC_UINT*)lv.pointer;
+                }
                 break;
             case LocatedSize::DWord:
-                dint_memory[lv.byte_index] = (::IEC_UDINT*)lv.pointer;
+                if (lv.byte_index < MAX_MEMORY_DWORD) {
+                    dint_memory[lv.byte_index] = (::IEC_UDINT*)lv.pointer;
+                }
                 break;
             case LocatedSize::LWord:
-                lint_memory[lv.byte_index] = (::IEC_ULINT*)lv.pointer;
+                if (lv.byte_index < MAX_MEMORY_LWORD) {
+                    lint_memory[lv.byte_index] = (::IEC_ULINT*)lv.pointer;
+                }
                 break;
             default: break;
             }

resources/sources/arduino/openplc.h

@@ -3,6 +3,26 @@
 
 #include <stdint.h>
 
+/* Pulled in HERE, not left to the includer, so the MAX_* overrides below
+ * are visible in every translation unit that sees this header.
+ *
+ * The sketch and every HAL include "openplc.h" BEFORE "defines.h" (see
+ * Baremetal.ino), so an override arriving through the includer would land
+ * after the buffer arrays had already been declared at the fallback sizes
+ * -- and the .ino would size bool_output[] differently from the HAL that
+ * walks it. Including it from inside the guard removes the ordering
+ * question entirely.
+ *
+ * Safe to include from the extern "C" blocks the HALs wrap this header
+ * in, and safe to reach many times per build: defines.h is generated for
+ * every target, holds nothing but object-like #defines, and re-including
+ * it only ever re-defines each macro to the identical token sequence,
+ * which C explicitly permits (C11 6.10.3p2). It carries no include guard
+ * of its own, and deliberately isn't given one here -- that would change
+ * the generated bytes for every board, and this change is meant to leave
+ * boards that declare no process image byte-for-byte as they were. */
+#include "defines.h"
+
 /*********************/
 /*  IEC Types defs   */
 /*********************/
@@ -27,16 +47,43 @@ typedef uint64_t   IEC_LWORD;
 typedef float    IEC_REAL;
 typedef double   IEC_LREAL;
 
-//OpenPLC Buffers Sizes
+/* OpenPLC Buffers Sizes
+ *
+ * Every MAX_* below is a FALLBACK, guarded with #ifndef: whatever
+ * defines.h already set wins. defines.h carries these only when the
+ * target's VPP manifest declared a `processImage`, which is how a board
+ * with a 15-slot expansion backplane gets an image big enough to address
+ * it instead of the one-size-fits-all numbers here (openplc-editor#296).
+ *
+ * The two branches exist because the small AVRs have 2 KB of SRAM and
+ * cannot carry the general-purpose image at all -- they get no %M area
+ * whatsoever. That is exactly why the capability that feeds defines.h is
+ * optional rather than defaulted: no single preset can answer for both
+ * sides of this #if, so a board that declares nothing keeps landing on
+ * the branch that has always been right for it. */
 #if defined(__AVR_ATmega328P__) || defined(__AVR_ATmega168__) || defined(__AVR_ATmega32U4__) || defined(__AVR_ATmega16U4__)
 
+#ifndef MAX_DIGITAL_INPUT
 #define MAX_DIGITAL_INPUT          8
+#endif
+#ifndef MAX_DIGITAL_OUTPUT
 #define MAX_DIGITAL_OUTPUT         32
+#endif
+#ifndef MAX_ANALOG_INPUT
 #define MAX_ANALOG_INPUT           6
+#endif
+#ifndef MAX_ANALOG_OUTPUT
 #define MAX_ANALOG_OUTPUT          32
+#endif
+#ifndef MAX_MEMORY_WORD
 #define MAX_MEMORY_WORD            0
+#endif
+#ifndef MAX_MEMORY_DWORD
 #define MAX_MEMORY_DWORD           0
+#endif
+#ifndef MAX_MEMORY_LWORD
 #define MAX_MEMORY_LWORD           0
+#endif
 
 extern IEC_BOOL *bool_input[MAX_DIGITAL_INPUT/8][8];
 extern IEC_BOOL *bool_output[MAX_DIGITAL_OUTPUT/8][8];
@@ -45,15 +92,33 @@ extern IEC_UINT *int_output[MAX_ANALOG_OUTPUT];
 
 #else
 
+#ifndef MAX_DIGITAL_INPUT
 #define MAX_DIGITAL_INPUT          56
+#endif
+#ifndef MAX_DIGITAL_OUTPUT
 #define MAX_DIGITAL_OUTPUT         56
+#endif
+#ifndef MAX_ANALOG_INPUT
 #define MAX_ANALOG_INPUT           32
+#endif
+#ifndef MAX_ANALOG_OUTPUT
 #define MAX_ANALOG_OUTPUT          32
+#endif
+#ifndef MAX_REAL_INPUT
 #define MAX_REAL_INPUT             32
+#endif
+#ifndef MAX_REAL_OUTPUT
 #define MAX_REAL_OUTPUT            32
+#endif
+#ifndef MAX_MEMORY_WORD
 #define MAX_MEMORY_WORD            20
+#endif
+#ifndef MAX_MEMORY_DWORD
 #define MAX_MEMORY_DWORD           20
+#endif
+#ifndef MAX_MEMORY_LWORD
 #define MAX_MEMORY_LWORD           20
+#endif
 
 extern IEC_BOOL *bool_input[MAX_DIGITAL_INPUT/8][8];
 extern IEC_BOOL *bool_output[MAX_DIGITAL_OUTPUT/8][8];