Add FLATCC_ENFORCE_ALIGNED_EMPTY_VECTORS (#289)

Work around issue where other builders do not align empty vectors correctly.

Author: bkietz

CMakeLists.txt

@@ -77,6 +77,13 @@ option (FLATCC_DEBUG_VERIFY
 option (FLATCC_TRACE_VERIFY
     "assert on verify failure in runtime lib" OFF)
 
+# Some producers allow empty vectors to be misaligned.
+# The following setting will cause the verifier to require the index 0
+# position to be element aligned even if the vector is empty (otherwise that
+# position is only required to be aligned to the preceding size field).
+option (FLATCC_ENFORCE_ALIGNED_EMPTY_VECTORS
+    "verify includes full alignment check for empty vectors" OFF)
+
 # Reflection is the compilers ability to generate binary schema output
 # (.bfbs files). This requires using generated code from
 # `reflection.fbs`. During development it may not be possible to
@@ -141,6 +148,10 @@ if (FLATCC_TRACE_VERIFY)
     set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -DFLATCC_TRACE_VERIFY=1")
 endif()
 
+if (FLATCC_ENFORCE_ALIGNED_EMPTY_VECTORS)
+    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -DFLATCC_ENFORCE_ALIGNED_EMPTY_VECTORS=1")
+endif()
+
 
 if (FLATCC_REFLECTION)
     set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -DFLATCC_REFLECTION=1")

README.md

@@ -1164,18 +1164,20 @@ sufficient, but for example standard 32-bit Windows only allocates on an
 aligned fields.
 
 NOTE: as of August 2024 it has been discovered that C++ writer code has been
-aligning empty vectors to the size field only, even if elements where larger,
-like the double type of size 8. This would cause the FlatCC verifier to
-(correctly) reject these vectors because it would result in an invalid C pointer
-type on some architectures. However, because this has been in effect for over
-10 years, the consensus is to have verifiers tolerate this behaviour even if
-C++ will eventually fix this issue. The FlatCC verifier will be updated to
-accept such buffers by default with a compile time flag to enforce the strict
-behaviour as well. In principle the misaligned vectors can lead potentially
-lead to undefined behaviour in agressively optimized C compilers. As of now
-it appears to be safe to read such buffers on common platforms and it is
-preferably to avoid additional runtime reader overhead to deal with this.
-For more, see [FlatCC #287], [Google Flatbuffers #8374].
+aligning empty vectors to the size field only, even if elements require
+greater alignment like the double type which requires 8. This would cause the
+FlatCC verifier to (correctly) reject these vectors because it would result
+in an invalid C pointer type on some architectures. However, because this has
+been in effect for over 10 years, the consensus is to have verifiers tolerate
+this behaviour even if C++ will eventually fix this issue. The FlatCC
+verifier has been updated to accept such buffers by default with an optional
+compile time flag to enforce the strict behaviour as well
+(`FLATCC_ENFORCE_ALIGNED_EMPTY_VECTORS`). In principle the misaligned vectors
+can lead potentially lead to undefined behaviour in agressively optimized C
+compilers. As of now it appears to be safe to read such buffers on common
+platforms and it is preferable to avoid additional runtime reader overhead to
+deal with this. For more, see [FlatCC #287], [Google Flatbuffers #8374],
+[FlatCC #289].
 
 
 ### Potential Name Conflicts
@@ -2718,3 +2720,4 @@ See [Benchmarks]
 [WebKit Style]: https://webkit.org/code-style-guidelines/
 [FlatCC #287]: https://github.com/dvidelabs/flatcc/issues/287
 [Google Flatbuffers #8374]: https://github.com/google/flatbuffers/issues/8374
+[FlatCC #289]: https://github.com/dvidelabs/flatcc/issues/289

include/flatcc/flatcc_rtconfig.h

@@ -63,6 +63,15 @@ extern "C" {
 #define FLATCC_TRACE_VERIFY 0
 #endif
 
+/*
+ * Some producers allow empty vectors to be misaligned.
+ * The following setting will cause the verifier to require the index 0
+ * position to be element aligned even if the vector is empty (otherwise that
+ * position is only required to be aligned to the preceding size field).
+ */
+#if !defined(FLATCC_ENFORCE_ALIGNED_EMPTY_VECTORS)
+#define FLATCC_ENFORCE_ALIGNED_EMPTY_VECTORS 0
+#endif
 
 /*
  * Limit recursion level for tables. Actual level may be deeper

src/runtime/verifier.c

@@ -135,20 +135,6 @@ static inline int check_header(uoffset_t end, uoffset_t base, uoffset_t offset)
     return k > base && k + offset_size <= end && !(k & (offset_size - 1));
 }
 
-static inline int check_aligned_header(uoffset_t end, uoffset_t base, uoffset_t offset, uint16_t align)
-{
-    uoffset_t k = base + offset;
-
-    if (uoffset_size <= voffset_size && k + offset_size < k) {
-        return 0;
-    }
-    /* Alignment refers to element 0 and header must also be aligned. */
-    align = align < uoffset_size ? uoffset_size : align;
-
-    /* Note to self: the builder can also use the mask OR trick to propagate `min_align`. */
-    return k > base && k + offset_size <= end && !((k + offset_size) & ((offset_size - 1) | (align - 1u)));
-}
-
 static inline int verify_struct(uoffset_t end, uoffset_t base, uoffset_t offset, uoffset_t size, uint16_t align)
 {
     /* Structs can have zero size so `end` is a valid value. */
@@ -278,10 +264,17 @@ static inline int verify_vector(const void *buf, uoffset_t end, uoffset_t base,
 {
     uoffset_t n;
 
-    verify(check_aligned_header(end, base, offset, align), flatcc_verify_error_vector_header_out_of_range_or_unaligned);
+    verify(check_header(end, base, offset), flatcc_verify_error_vector_header_out_of_range_or_unaligned);
     base += offset;
+
     n = read_uoffset(buf, base);
     base += offset_size;
+
+#if !FLATCC_ENFORCE_ALIGNED_EMPTY_VECTORS
+    /* This is due to incorrect buffers from other builders than cannot easily be ignored. */
+    align = n == 0 ? uoffset_size : align;
+#endif
+    verify(!(base & ((align - 1u) | (uoffset_size - 1u))), flatcc_verify_error_vector_header_out_of_range_or_unaligned);
     /* `n * elem_size` can overflow uncontrollably otherwise. */
     verify(n <= max_count, flatcc_verify_error_vector_count_exceeds_representable_vector_size);
     verify(end - base >= n * elem_size, flatcc_verify_error_vector_out_of_range);