Hardening and bugfixes

CMakeLists.txt

@@ -46,8 +46,10 @@ endif()
 if (ENABLE_UBSAN)
   # 'alignment' is excluded on purpose: the EtherCAT frame layer reads/writes packed buffers at
   # unaligned offsets (supported on the x86/Cortex-M targets). The other UB checks stay on and abort.
-  add_compile_options(-fsanitize=undefined -fno-sanitize=alignment -fno-sanitize-recover=undefined)
-  add_link_options(-fsanitize=undefined)
+  # float-cast-overflow is not in gcc's -fsanitize=undefined, and its no-recover needs naming too.
+  add_compile_options(-fsanitize=undefined,float-cast-overflow -fno-sanitize=alignment
+                      -fno-sanitize-recover=undefined,float-cast-overflow)
+  add_link_options(-fsanitize=undefined,float-cast-overflow)
 endif()
 
 

examples/master/gateway/emitter.cc

@@ -62,6 +62,7 @@ int main(int argc, char* argv[])
     // Local mailbox to generate and process messages
     mailbox::request::Mailbox mailbox;
     mailbox.recv_size = 128;
+    mailbox.send_size = 128;
 
     // Frame to send/rec on the UDP socket
     uint8_t frame[ETH_MTU_SIZE];

lib/CMakeLists.txt

@@ -7,6 +7,7 @@ set(KICKCAT_SOURCES
   ${CMAKE_CURRENT_SOURCE_DIR}/src/TapSocket.cc
   ${CMAKE_CURRENT_SOURCE_DIR}/src/SIIParser.cc
 
+  ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/Filesystem.cc
   ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/SoftPll.cc
   ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/Time.cc
   ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/Timer.cc
@@ -30,6 +31,7 @@ if (KICKOS)
     ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/KickOS/Mutex.cc
     ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/KickOS/ConditionVariable.cc
     ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/KickOS/SharedMemory.cc
+    ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/KickOS/Filesystem.cc
   )
   set(OS_LIBRARIES )
   # Selects the KickOS os_types branch in types.h, for the library and its consumers.
@@ -38,6 +40,7 @@ elseif (NUTTX)
   # NuttX is POSIX enough to share the Unix time backend (clock_gettime/clock_nanosleep).
   set(OS_LIB_SOURCES
     ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/Unix/Time.cc
+    ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/Unix/Filesystem.cc
     ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/NuttX/Socket.cc
   )
   set(OS_LIBRARIES )
@@ -46,6 +49,7 @@ elseif(PIKEOS)
     ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/PikeOS/Socket.cc
     ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/PikeOS/Time.cc
     ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/PikeOS/ErrorCategory.cc
+    ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/Unix/Filesystem.cc
   )
 
   set(OS_LIBRARIES )
@@ -59,6 +63,7 @@ elseif (UNIX)
     ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/Unix/SharedMemory.cc
     ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/Unix/ConditionVariable.cc
     ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/Unix/Thread.cc
+    ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/Unix/Filesystem.cc
   )
   set(OS_LIBRARIES pthread rt)
 elseif (WIN32)
@@ -69,6 +74,7 @@ elseif (WIN32)
     ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/Windows/Time.cc
     ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/Unix/Mutex.cc
     ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/Unix/ConditionVariable.cc
+    ${CMAKE_CURRENT_SOURCE_DIR}/src/OS/Windows/Filesystem.cc
   )
 
   find_package(npcap CONFIG REQUIRED)

lib/include/kickcat/CoE/mailbox/request.h

@@ -9,7 +9,7 @@ namespace kickcat::mailbox::request
     class SDOMessage final : public AbstractMessage
     {
     public:
-        SDOMessage(uint16_t mailbox_size, uint16_t index, uint8_t subindex, bool CA, uint8_t request, void* data, uint32_t* data_size, nanoseconds timeout);
+        SDOMessage(uint16_t mbx_recv_size, uint16_t mbx_send_size, uint16_t index, uint8_t subindex, bool CA, uint8_t request, void* data, uint32_t* data_size, nanoseconds timeout);
         virtual ~SDOMessage() = default;
 
         ProcessingResult process(uint8_t const* received) override;
@@ -33,7 +33,7 @@ namespace kickcat::mailbox::request
     class SDOInformationMessage final : public AbstractMessage
     {
     public:
-        SDOInformationMessage(uint16_t mailbox_size, uint8_t request, void* data, uint32_t* data_size, uint32_t request_payload_size, nanoseconds timeout);
+        SDOInformationMessage(uint16_t mbx_recv_size, uint16_t mbx_send_size, uint8_t request, void* data, uint32_t* data_size, uint32_t request_payload_size, nanoseconds timeout);
         virtual ~SDOInformationMessage() = default;
 
         ProcessingResult process(uint8_t const* received) override;

lib/include/kickcat/CoE/protocol.h

@@ -178,6 +178,22 @@ namespace kickcat::CoE
                 uint16_t  access;
             } __attribute__((__packed__));
             std::string toString(EntryDescription const& entry_description);
+
+            /// \brief Size of the CoE service a response to this opcode occupies, headers included.
+            /// \details A server builds its answer in the buffer the request arrived in, past the
+            ///          request it parsed, so a request sized to its own length cannot hold it. The
+            ///          mailbox header is the mailbox layer's own and is not counted here.
+            constexpr std::size_t responseSize(uint16_t opcode)
+            {
+                constexpr std::size_t headers = sizeof(Header) + sizeof(ServiceDataInfo);
+                switch (opcode)
+                {
+                    case GET_OD_LIST_REQ: { return headers + sizeof(ListType) + 5 * sizeof(uint16_t); }
+                    case GET_OD_REQ:      { return headers + sizeof(ObjectDescription);               }
+                    case GET_ED_REQ:      { return headers + sizeof(EntryDescription);                }
+                    default:              { return headers + sizeof(uint32_t); } // abort code
+                }
+            }
         }
 
         namespace abort

lib/include/kickcat/Mailbox.h

@@ -49,8 +49,10 @@ namespace kickcat::mailbox::request
     class AbstractMessage
     {
     public:
-        /// \param mailbox_size Size of the mailbox the message is targeted to (required to adapt internal buffer)
-        AbstractMessage(uint16_t mailbox_size, nanoseconds timeout);
+        /// \param mbx_recv_size Slave receive mailbox size: sizes the internal buffer
+        /// \param mbx_send_size Slave send mailbox size. A mailbox may be asymmetric, so this is
+        ///                      not mbx_recv_size.
+        AbstractMessage(uint16_t mbx_recv_size, uint16_t mbx_send_size, nanoseconds timeout);
         virtual ~AbstractMessage() = default;
 
         // set message counter (aka session handle)
@@ -78,6 +80,7 @@ namespace kickcat::mailbox::request
         std::vector<uint8_t> data_;     // data of the message (send and gateway rec)
         mailbox::Header* header_;       // pointer on the mailbox header in data
         uint32_t status_;               // message current status
+        uint16_t send_size_;            // valid bytes in the buffer given to process() - not data_.size()
 
     private:
         nanoseconds timeout_;           // Max time to handle the message. Relative time before sending, absolute time after. 0 means no timeout
@@ -89,7 +92,7 @@ namespace kickcat::mailbox::request
     class GatewayMessage final : public AbstractMessage
     {
     public:
-        GatewayMessage(uint16_t mailbox_size, uint8_t const* raw_message, uint16_t gateway_index, nanoseconds timeout);
+        GatewayMessage(uint16_t mbx_recv_size, uint16_t mbx_send_size, uint8_t const* raw_message, uint16_t gateway_index, nanoseconds timeout);
 
         /// \brief Build a GatewayMessage that is already completed: the reply is already in hand,
         ///        so there is no bus round-trip. Used by synchronous dispatch paths (e.g. the master OD,
@@ -110,15 +113,15 @@ namespace kickcat::mailbox::request
     /// \brief Request mailbox - it orchestrates the emission and the processing of messages (for master)
     struct Mailbox
     {
-        uint16_t recv_offset;
-        uint16_t recv_size;
-        uint16_t send_offset;
-        uint16_t send_size;
-
-        bool can_read;      // data available on the slave
-        bool can_write;     // free space for a new message on the slave
-        uint8_t counter{0}; // session handle, from 1 to 7
-        bool toggle;        // for SDO segmented transfer
+        uint16_t recv_offset{0};
+        uint16_t recv_size{0};      // slave receive mailbox: bounds what the master writes
+        uint16_t send_offset{0};
+        uint16_t send_size{0};      // slave send mailbox: bounds what a reply carries
+
+        bool can_read{false};       // data available on the slave
+        bool can_write{false};      // free space for a new message on the slave
+        uint8_t counter{0};         // session handle, from 1 to 7
+        bool toggle{false};         // for SDO segmented transfer
 
         //
         void generateSMConfig(SyncManager::Register SM[2]);

lib/include/kickcat/OS/Filesystem.h

@@ -0,0 +1,71 @@
+#ifndef KICKCAT_OS_FILESYSTEM_H
+#define KICKCAT_OS_FILESYSTEM_H
+
+#include <cstddef>
+#include <cstdint>
+#include <string>
+#include <vector>
+
+namespace kickcat::filesystem
+{
+    // Thin wrappers over the platform's own file API, deliberately not <filesystem>/<fstream>:
+    // EmulatedESC is compiled for the embedded targets, whose C++ export has neither, and on MinGW
+    // those two headers bind the binary to libstdc++ symbols the runtime DLL it finds may not
+    // export (a load-time STATUS_ENTRYPOINT_NOT_FOUND, seen in CI).
+    //
+    // Paths are byte strings separated by '/', on every platform: the Win32 API accepts '/' too.
+    // Nothing here resolves symlinks, expands '..' or makes a path absolute.
+
+    struct Entry
+    {
+        std::string name;           // leaf name, not a path
+        bool        is_directory;
+    };
+
+    /// \return true if the path exists, whatever its kind.
+    bool exists(std::string const& path);
+
+    /// \return true if the path exists and is a directory.
+    bool isDirectory(std::string const& path);
+
+    /// \brief Delete one file.
+    /// \return true if it was deleted, false if it was already absent.
+    bool removeFile(std::string const& path);
+
+    /// \brief Create one directory. The parent must already exist.
+    /// \return true if it was created, false if it was already there.
+    bool createDirectory(std::string const& path);
+
+    /// \brief Delete one directory, which must be empty.
+    /// \return true if it was deleted, false if it was already absent.
+    bool removeDirectory(std::string const& path);
+
+    /// \brief One directory level, in whatever order the OS reports. '.' and '..' are not listed.
+    std::vector<Entry> list(std::string const& path);
+
+    /// \brief Every file below directory, as paths prefixed with directory. Directories themselves
+    ///        are not listed, and the order is unspecified.
+    std::vector<std::string> listFilesRecursive(std::string const& directory);
+
+    /// \return everything before the last separator, empty if the path has none.
+    std::string parent(std::string const& path);
+
+    /// \return the leaf name: everything after the last separator.
+    std::string filename(std::string const& path);
+
+    /// \return the last '.' of the leaf name and what follows, empty if the leaf has none.
+    std::string extension(std::string const& path);
+
+    /// \brief Append name to directory, inserting a separator only where one is missing. An empty
+    ///        directory yields name unchanged, so joining onto parent() of a bare filename works.
+    std::string join(std::string const& directory, std::string const& name);
+
+    /// \brief Read a whole file.
+    std::vector<uint8_t> readFile(std::string const& path);
+
+    /// \brief Create or truncate a file and write it whole.
+    void writeFile(std::string const& path, void const* data, std::size_t size);
+    void writeFile(std::string const& path, std::string const& content);
+}
+
+#endif

lib/include/kickcat/utils/math.h

@@ -0,0 +1,111 @@
+#ifndef KICKCAT_UTILS_MATH_H
+#define KICKCAT_UTILS_MATH_H
+
+#include <cstdint>
+#include <limits>
+#include <type_traits>
+
+namespace kickcat
+{
+    // Freestanding min/max/abs/round/convert helpers that pull no <algorithm>/<cmath>/<cstdlib>.
+    // In a subdirectory on purpose: lib/include/kickcat is itself on the include path, so a math.h
+    // sitting directly in it is what every translation unit here would get for #include <math.h>.
+    // Those standard headers are broken or mutually conflicting on some embedded C++ exports (e.g.
+    // NuttX on arm-none-eabi, where cxx/cmath references an absent ::nextafterl and <cstdlib>
+    // collides with the toolchain's stdlib.h on div_t) so code compiled for those targets uses these
+    // instead. <limits> is required of a freestanding implementation, so it is safe here.
+
+    template<typename T>
+    constexpr T clamp(T v, T lo, T hi)
+    {
+        if (v < lo)
+        {
+            return lo;
+        }
+        if (v > hi)
+        {
+            return hi;
+        }
+        return v;
+    }
+
+    // Only NaN compares unequal to itself, and <cmath> is one of the headers this file avoids.
+    constexpr bool is_nan(double v)
+    {
+        return v != v;
+    }
+
+    template<typename T>
+    constexpr T abs_value(T v)
+    {
+        if (v < 0)
+        {
+            return -v;
+        }
+        return v;
+    }
+
+    /// \brief Convert a real value to T, clamping it into [min, max] reduced into T's range.
+    /// \details static_cast<T>(double) is undefined behaviour outside T's range, for a narrower
+    ///          floating type as much as for an integer. Bounds the destination cannot hold give a
+    ///          meaningless result, never undefined behaviour.
+    template<typename T>
+    constexpr T saturate(double v, double min, double max)
+    {
+        static_assert(std::is_arithmetic_v<T>, "saturate() converts to an arithmetic type");
+
+        // lowest(), not min(): for a floating T the latter is the smallest positive normal.
+        // Beyond 32 bits these images are not exact - numeric_limits<int64_t>::max() rounds up to
+        // 2^63 - so they are thresholds to compare against, never values to convert back.
+        double const lowest  = static_cast<double>(std::numeric_limits<T>::lowest());
+        double const highest = static_cast<double>(std::numeric_limits<T>::max());
+        double const first   = clamp(min, lowest, highest);
+        double const second  = clamp(max, lowest, highest);
+
+        // Transposed bounds still describe the interval between them; rejecting them belongs to the
+        // caller-facing API (see Drive::setLimits).
+        double lo = first;
+        double hi = second;
+        if (second < first)
+        {
+            lo = second;
+            hi = first;
+        }
+
+        if (v >= hi)
+        {
+            if (hi >= highest)
+            {
+                return std::numeric_limits<T>::max();
+            }
+            return static_cast<T>(hi);
+        }
+
+        if (v <= lo)
+        {
+            if (lo <= lowest)
+            {
+                return std::numeric_limits<T>::lowest();
+            }
+            return static_cast<T>(lo);
+        }
+
+        return static_cast<T>(v);
+    }
+
+    /// \brief Round to the nearest integer, saturating: the destination range is the only bound a
+    ///        bare double to int64_t conversion has, and exceeding it would be undefined.
+    constexpr int64_t round_to_int(double v)
+    {
+        constexpr double lowest  = static_cast<double>(std::numeric_limits<int64_t>::lowest());
+        constexpr double highest = static_cast<double>(std::numeric_limits<int64_t>::max());
+
+        if (v < 0.0)
+        {
+            return saturate<int64_t>(v - 0.5, lowest, highest);
+        }
+        return saturate<int64_t>(v + 0.5, lowest, highest);
+    }
+}
+
+#endif