feat(image): allocate each table at its own length, told to the plugins

core/src/drivers/plugin_driver.c

@@ -591,6 +591,122 @@ int plugin_driver_load_config(plugin_driver_t *driver, const char *config_file)
 }
 
 // Send to plugin init function all args
+/**
+ * The fourteen table lengths, for a plugin that asked to be told.
+ *
+ * Read once per plugin_driver_init() rather than cached: the image is
+ * reallocated on every program load, and a cached copy is exactly the stale
+ * state this symbol exists to prevent.
+ */
+static uint32_t image_sizes_snapshot(uint32_t *out, uint32_t cap)
+{
+    uint32_t n = 0;
+    for (int id = 0; id < IMAGE_TABLE_COUNT && n < cap; ++id)
+    {
+        out[n++] = image_table_capacity((image_table_id_t)id);
+    }
+    return n;
+}
+
+/**
+ * Hand one plugin the fourteen lengths, before its init() runs.
+ *
+ * Returns 0 when the plugin has nothing to be told or accepted them, and
+ * non-zero when it refused -- which fails the plugin exactly as a failed
+ * init() does, because a plugin that cannot make sense of the image it is
+ * about to be handed should not be handed it.
+ */
+static int deliver_image_sizes(plugin_instance_t *plugin)
+{
+    uint32_t sizes[IMAGE_TABLE_COUNT];
+    const uint32_t count = image_sizes_snapshot(sizes, IMAGE_TABLE_COUNT);
+
+    if (plugin->config.type == PLUGIN_TYPE_NATIVE && plugin->native_plugin &&
+        plugin->native_plugin->set_image_sizes)
+    {
+        const int rc = plugin->native_plugin->set_image_sizes(sizes, count);
+        if (rc != 0)
+        {
+            log_error("Plugin '%s' refused the image sizes (returned %d)", plugin->config.name, rc);
+            return rc;
+        }
+    }
+
+    if (plugin->config.type == PLUGIN_TYPE_PYTHON && plugin->python_plugin &&
+        plugin->python_plugin->pFuncSetImageSizes)
+    {
+        PyObject *list = PyList_New((Py_ssize_t)count);
+        if (!list)
+        {
+            PyErr_Clear();
+            log_error("Plugin '%s': could not build the image size list", plugin->config.name);
+            return -1;
+        }
+        for (uint32_t i = 0; i < count; ++i)
+        {
+            /* PyList_SetItem steals the reference, so a failed PyLong_FromLong
+             * is the only leak to worry about and it cannot happen for a
+             * uint32_t that already exists. */
+            PyList_SetItem(list, (Py_ssize_t)i, PyLong_FromUnsignedLong(sizes[i]));
+        }
+        PyObject *result =
+            PyObject_CallFunctionObjArgs(plugin->python_plugin->pFuncSetImageSizes, list, NULL);
+        Py_DECREF(list);
+        if (!result)
+        {
+            PyErr_Print();
+            log_error("Plugin '%s' raised in set_image_sizes", plugin->config.name);
+            return -1;
+        }
+        Py_DECREF(result);
+    }
+
+    return 0;
+}
+
+bool plugin_driver_all_understand_per_table_sizes(plugin_driver_t *driver,
+                                                  const char **first_without)
+{
+    if (first_without)
+        *first_without = NULL;
+    if (!driver)
+        return false;
+
+    for (int i = 0; i < driver->plugin_count; i++)
+    {
+        plugin_instance_t *plugin = &driver->plugins[i];
+        bool understands          = false;
+
+        /* A DEGRADED PLUGIN DOES NOT GET A VOTE.
+         *
+         * It failed to load, so plugin_driver_init skips it in every branch:
+         * it never receives runtime args and never touches the image. Letting
+         * it answer "no" would mean one box missing Npcap, where the EtherCAT
+         * plugin degrades, silently costs every OTHER plugin its per-table
+         * image -- a downgrade with no relation to anything that will actually
+         * read the tables.
+         *
+         * DISABLED plugins still vote, deliberately: plugin_driver_init
+         * initialises them regardless of the enabled flag, so a disabled
+         * plugin does hold the base pointers and does read the image. */
+        if (plugin->degraded)
+            continue;
+
+        if (plugin->config.type == PLUGIN_TYPE_NATIVE)
+            understands = plugin->native_plugin && plugin->native_plugin->set_image_sizes;
+        else if (plugin->config.type == PLUGIN_TYPE_PYTHON)
+            understands = plugin->python_plugin && plugin->python_plugin->pFuncSetImageSizes;
+
+        if (!understands)
+        {
+            if (first_without)
+                *first_without = plugin->config.name;
+            return false;
+        }
+    }
+    return true;
+}
+
 int plugin_driver_init(plugin_driver_t *driver)
 {
     if (!driver)
@@ -632,6 +748,18 @@ int plugin_driver_init(plugin_driver_t *driver)
                 }
                 return -1;
             }
+            /* BEFORE init(), because init() is where a plugin copies the
+             * base pointers by value and decides how big everything is.
+             * Delivering afterwards leaves a window, once per load, in which
+             * the plugin holds new pointers and previous sizes. */
+            if (deliver_image_sizes(plugin) != 0)
+            {
+                Py_DECREF(args);
+                if (have_gil)
+                    PyGILState_Release(local_gstate);
+                return -1;
+            }
+
             // Call the Python init function with proper capsule
             PyObject *result =
                 PyObject_CallFunctionObjArgs(plugin->python_plugin->pFuncInit, args, NULL);
@@ -671,6 +799,15 @@ int plugin_driver_init(plugin_driver_t *driver)
                 return -1;
             }
 
+            /* BEFORE init(), for the same reason as the Python path above. */
+            if (deliver_image_sizes(plugin) != 0)
+            {
+                free_structured_args(args);
+                if (have_gil)
+                    PyGILState_Release(local_gstate);
+                return -1;
+            }
+
             // Call the native init function
             int result = plugin->native_plugin->init(args);
             if (result != 0)
@@ -1120,12 +1257,19 @@ void *generate_structured_args_with_driver(plugin_type_t type, plugin_driver_t *
            sizeof(driver->plugins[plugin_index].config.plugin_related_config_path));
 
     // Initialize buffer size info
-    /* The allocated size, not a compile-time constant. Plugins bounds-check
-     * against this field -- ethercat_io.c refuses a byte_index at or above it,
-     * s7comm derives every clamp from it -- so it has to describe the image
-     * that actually exists. It describes all fourteen tables because they are
-     * all allocated at the same count; see image_sizes_largest() for why the
-     * ABI leaves no room for anything else. */
+    /* THE SMALLEST OF THE FOURTEEN, not the length they all share.
+     *
+     * The tables no longer have one length, and this field cannot say so --
+     * CON06 keeps the struct's offsets fixed. The minimum is the only safe
+     * single number for a consumer that has not been told they can differ:
+     * bounding by it refuses an index, where bounding by the largest reads
+     * past every shorter table.
+     *
+     * ethercat_io.c and s7comm no longer derive their clamps from this field;
+     * they export set_image_sizes and bound by the table each access actually
+     * addresses, falling back here only when the sizes were never delivered.
+     * plugin_types.h carries the same statement for plugin authors, and
+     * journal_buffer.h for the runtime's own copy. */
     args->buffer_size     = (int)image_tables_capacity();
     args->bits_per_buffer = 8;
 
@@ -1335,6 +1479,10 @@ int python_plugin_get_symbols(plugin_instance_t *plugin)
         // start_loop is optional
         Py_XDECREF(py_binds->pFuncStart);
         py_binds->pFuncStart = NULL;
+        /* A failed PyObject_GetAttrString leaves an AttributeError SET, and an
+         * optional lookup does not return, so it has to be cleared here or the
+         * next CPython call reports this absence as its own failure. */
+        PyErr_Clear();
     }
 
     py_binds->pFuncStop = PyObject_GetAttrString(py_binds->pModule, "stop_loop");
@@ -1343,6 +1491,23 @@ int python_plugin_get_symbols(plugin_instance_t *plugin)
         // stop_loop is optional
         Py_XDECREF(py_binds->pFuncStop);
         py_binds->pFuncStop = NULL;
+        /* A failed PyObject_GetAttrString leaves an AttributeError SET, and an
+         * optional lookup does not return, so it has to be cleared here or the
+         * next CPython call reports this absence as its own failure. */
+        PyErr_Clear();
+    }
+
+    py_binds->pFuncSetImageSizes = PyObject_GetAttrString(py_binds->pModule, "set_image_sizes");
+    if (!py_binds->pFuncSetImageSizes || !PyCallable_Check(py_binds->pFuncSetImageSizes))
+    {
+        /* Optional. PyErr_Clear() is not decoration: a failed
+         * PyObject_GetAttrString leaves an AttributeError SET, and the next
+         * CPython call that checks would report this absence as its own
+         * failure. The four required lookups above never hit it because they
+         * return on failure. */
+        Py_XDECREF(py_binds->pFuncSetImageSizes);
+        py_binds->pFuncSetImageSizes = NULL;
+        PyErr_Clear();
     }
 
     py_binds->pFuncCleanup = PyObject_GetAttrString(py_binds->pModule, "cleanup");
@@ -1351,6 +1516,10 @@ int python_plugin_get_symbols(plugin_instance_t *plugin)
         // cleanup is optional
         Py_XDECREF(py_binds->pFuncCleanup);
         py_binds->pFuncCleanup = NULL;
+        /* A failed PyObject_GetAttrString leaves an AttributeError SET, and an
+         * optional lookup does not return, so it has to be cleared here or the
+         * next CPython call reports this absence as its own failure. */
+        PyErr_Clear();
     }
 
     // Store the python binds in the plugin instance
@@ -1487,6 +1656,11 @@ int native_plugin_get_symbols(plugin_instance_t *plugin)
     native_bundle->retain_load  = (plugin_retain_load_func_t)dlsym(handle, "retain_load");
     native_bundle->retain_flush = (plugin_retain_flush_func_t)dlsym(handle, "retain_flush");
 
+    /* Optional, like the five above: NULL simply means this plugin does not
+     * understand per-table image sizes, and the run stays square for it. */
+    native_bundle->set_image_sizes =
+        (plugin_set_image_sizes_func_t)dlsym(handle, "set_image_sizes");
+
     // Store the native bundle and handle in the plugin instance
     plugin->native_plugin = native_bundle;
 

core/src/drivers/plugin_driver.h

@@ -82,6 +82,29 @@ typedef int (*plugin_retain_load_func_t)(const char *program_md5, uint16_t md5_l
  * assumed to commit inside save(), which is where durability belongs anyway. */
 typedef int (*plugin_retain_flush_func_t)(void);
 
+/* Optional, and its PRESENCE is the capability declaration (RTOP-284).
+ *
+ * The fourteen image tables no longer share one length, and
+ * `plugin_runtime_args_t` carries a single `buffer_size` that cannot say so.
+ * Rather than move that struct -- CON06 guarantees pre-compiled plugins keep
+ * their field offsets -- the sizes travel through a symbol the loader resolves
+ * with dlsym, exactly as it already does for execute_command, get_stats and
+ * the three retain_* hooks.
+ *
+ * Exporting it means "I understand per-table sizes". A plugin without it is
+ * not broken and is not refused; the runtime keeps the image SQUARE for that
+ * run instead, because a plugin bounding a byte index and a word index with
+ * the same number is only safe while the tables are equal.
+ *
+ * Called BEFORE init(), once per plugin_driver_init(). Self-contained on
+ * purpose -- no args -- which is what lets it run that early. A non-zero
+ * return fails the plugin exactly as a failed init() does.
+ *
+ * `sizes` is indexed by `image_table_id_t` and `count` is how many entries it
+ * carries, so a plugin built against an older enum reads the prefix it knows
+ * and ignores the rest. */
+typedef int (*plugin_set_image_sizes_func_t)(const uint32_t *sizes, uint32_t count);
+
 typedef struct
 {
     void *handle; // Handle to the loaded shared library
@@ -97,6 +120,9 @@ typedef struct
     plugin_retain_save_func_t  retain_save;
     plugin_retain_load_func_t  retain_load;
     plugin_retain_flush_func_t retain_flush;
+    /* Optional; NULL means this plugin does not understand per-table sizes and
+     * the image stays square for the run. */
+    plugin_set_image_sizes_func_t set_image_sizes;
 } plugin_funct_bundle_t;
 
 // Plugin instance structure
@@ -203,6 +229,26 @@ int plugin_driver_retain_load(plugin_instance_t *store, const char *program_md5,
                               uint8_t *out, uint16_t cap, uint16_t *out_len);
 int plugin_driver_retain_flush(plugin_instance_t *store);
 
+/**
+ * Does every loaded plugin understand per-table image sizes?
+ *
+ * Asked once per program load, BEFORE the image is allocated, because the
+ * answer decides how it is allocated. If any plugin says no, the image stays
+ * SQUARE for that run -- every table the same length.
+ *
+ * That is not a preference. The shipped VPP plugins bound a byte index into
+ * bool_output and a word index into int_output with the same `buffer_size`:
+ * the largest table would be an out-of-bounds read on the smaller ones, and
+ * the smallest would silently drop configured I/O. Only equal lengths keep one
+ * bound honest.
+ *
+ * `first_without` receives the name of the first plugin that does not, so the
+ * decision can be logged with a reason. It is the only way an operator can
+ * tell the two modes apart.
+ */
+bool plugin_driver_all_understand_per_table_sizes(plugin_driver_t *driver,
+                                                  const char **first_without);
+
 // Route a command to a specific plugin by name (for async commands like scan)
 int plugin_driver_execute_command(plugin_driver_t *driver, const char *plugin_name,
                                   const char *command_json, char *response, size_t response_size);

core/src/drivers/plugin_types.h

@@ -16,6 +16,16 @@
 #define PLUGIN_TYPES_H
 
 #include "../lib/iec_types.h"
+/* The image table identities, so a plugin receiving the sizes array can name
+ * the entries it indexes rather than counting positions (RTOP-284, B2).
+ * Publishing a type costs no ABI: no struct gains a field and no offset
+ * moves, which is what CON06 guarantees pre-compiled plugins.
+ *
+ * A plugin built against an OLDER runtime will not find this header, so
+ * anything that must work on both — a VPP package, which ships and is built
+ * independently of the runtime on the device — carries its own constants and
+ * keeps them in the order documented there. */
+#include "../plc_app/image_table_id.h"
 #include <pthread.h>
 #include <stdbool.h>
 #include <stdint.h>
@@ -229,7 +239,36 @@ typedef struct
     /* Plugin configuration */
     char plugin_specific_config_file_path[256];
 
-    /* Buffer size information */
+    /* THE SMALLEST TABLE, NOT THE ONLY ONE (RTOP-284).
+     *
+     * The fourteen image tables no longer share a length. This field cannot
+     * say that -- CON06 guarantees pre-compiled plugins keep their field
+     * offsets, so it does not move -- and it is now the MINIMUM of the
+     * fourteen rather than the length they all happened to have.
+     *
+     * The minimum is the only safe answer for a consumer that still reads one
+     * number: bounding by it refuses an index that would have run off the end
+     * of the shortest table, where bounding by the largest would have read
+     * past every table below it. Under-permissive, never over.
+     *
+     * A plugin that wants the truth exports `set_image_sizes` (plugin_driver.h)
+     * and receives all fourteen before its init() runs. When every loaded
+     * plugin does, the image is allocated per table; when any does not, it is
+     * kept square for that run and this field is again the length they all
+     * have.
+     *
+     * Not marked deprecated yet, and the reason is not what an earlier draft
+     * of this comment claimed. The runtime core does build with -Werror
+     * (core/src/CMakeLists.txt), but the plugins do not: they are configured
+     * by their own cmake invocation and the VPP packages by a plain Makefile,
+     * so the attribute would produce warnings there, not a build failure.
+     *
+     * It is deferred because the field is still the RIGHT thing to read: on a
+     * square run it is the length every table has, and it is the only bound a
+     * plugin that has not adopted set_image_sizes can use. Deprecating it now
+     * would warn at correct code, including in packages that ship
+     * independently and must keep working against older runtimes. The
+     * attribute goes in once the symbol is universal. */
     int buffer_size;
     int bits_per_buffer;
 

core/src/drivers/plugins/native/ethercat/CMakeLists.txt

@@ -153,6 +153,10 @@ set(PLUGIN_SOURCES
     ${CMAKE_CURRENT_SOURCE_DIR}/ethercat_proc.c
     ${CMAKE_CURRENT_SOURCE_DIR}/ethercat_iface_state.c
     ${OPENPLC_ROOT}/core/src/drivers/plugins/native/plugin_logger.c
+    # Exports set_image_sizes, which is how this plugin declares it
+    # understands per-table image sizes (RTOP-284). Without it the
+    # runtime keeps the image square for every run this plugin is in.
+    ${OPENPLC_ROOT}/core/src/drivers/plugins/native/plugin_image_sizes.c
 )
 
 # =============================================================================