Fix memory leak of node pairs/items arrays on allocation failure in the loader

src/loader.c

@@ -455,6 +455,9 @@ yaml_parser_load_sequence(yaml_parser_t *parser, yaml_event_t *event,
 error:
     yaml_free(tag);
     yaml_free(event->data.sequence_start.anchor);
+    /* The items array has not been attached to the document yet, so the
+     * document cleanup will not free it. */
+    yaml_free(items.start);
     return 0;
 }
 
@@ -523,6 +526,9 @@ yaml_parser_load_mapping(yaml_parser_t *parser, yaml_event_t *event,
 error:
     yaml_free(tag);
     yaml_free(event->data.mapping_start.anchor);
+    /* The pairs array has not been attached to the document yet, so the
+     * document cleanup will not free it. */
+    yaml_free(pairs.start);
     return 0;
 }
 

tests/CMakeLists.txt

@@ -27,3 +27,22 @@ add_test(NAME version COMMAND test-version)
 add_test(NAME reader COMMAND test-reader)
 add_test(NAME nesting COMMAND test-nesting)
 
+# Regression test for allocation-failure handling in the loader: the
+# library sources are compiled into the test with the allocator renamed,
+# so the test can inject allocation failures and count outstanding blocks
+# portably, without LD_PRELOAD or linker-specific tricks.
+foreach(src IN LISTS SRCS)
+  list(APPEND SRCS_ABS ${PROJECT_SOURCE_DIR}/${src})
+endforeach()
+
+add_executable(test-load-error test-load-error.c test-alloc.c ${SRCS_ABS})
+target_compile_definitions(test-load-error PRIVATE
+  HAVE_CONFIG_H YAML_DECLARE_STATIC)
+target_include_directories(test-load-error PRIVATE
+  ${PROJECT_SOURCE_DIR}/include
+  ${CMAKE_BINARY_DIR}/include
+  ${PROJECT_SOURCE_DIR}/src)
+set_property(SOURCE ${SRCS_ABS} APPEND PROPERTY COMPILE_DEFINITIONS
+  malloc=test_malloc realloc=test_realloc free=test_free strdup=test_strdup)
+add_test(NAME load-error COMMAND test-load-error)
+

tests/test-alloc.c

@@ -0,0 +1,63 @@
+/* Counting/fault-injecting allocator implementation. Compiled without the
+ * malloc/realloc/free renaming, so it can forward to the real allocator. */
+#include "test-alloc.h"
+
+#include <stdlib.h>
+#include <string.h>
+
+long test_alloc_calls = 0;
+long test_alloc_outstanding = 0;
+long test_alloc_fail_at = -1;
+
+void *test_malloc(size_t size) {
+    void *ptr;
+
+    test_alloc_calls++;
+    if (test_alloc_fail_at >= 0 && test_alloc_calls == test_alloc_fail_at) {
+        return NULL;
+    }
+    ptr = malloc(size);
+    if (ptr) {
+        test_alloc_outstanding++;
+    }
+    return ptr;
+}
+
+void *test_realloc(void *ptr, size_t size) {
+    void *new_ptr;
+
+    test_alloc_calls++;
+    if (test_alloc_fail_at >= 0 && test_alloc_calls == test_alloc_fail_at) {
+        return NULL;
+    }
+    new_ptr = realloc(ptr, size);
+    if (new_ptr && !ptr) {
+        test_alloc_outstanding++;
+    }
+    return new_ptr;
+}
+
+void test_free(void *ptr) {
+    if (ptr) {
+        test_alloc_outstanding--;
+    }
+    free(ptr);
+}
+
+/* strdup() allocates behind the library's back; route it through the
+ * counting allocator as well. */
+char *test_strdup(const char *s) {
+    size_t len = strlen(s) + 1;
+    char *copy = (char *)test_malloc(len);
+
+    if (copy) {
+        memcpy(copy, s, len);
+    }
+    return copy;
+}
+
+void test_alloc_reset(void) {
+    test_alloc_calls = 0;
+    test_alloc_outstanding = 0;
+    test_alloc_fail_at = -1;
+}

tests/test-alloc.h

@@ -0,0 +1,24 @@
+/* Counting/fault-injecting allocator used by test-load-error.
+ *
+ * The library sources are compiled with malloc/realloc/free renamed to
+ * test_malloc/test_realloc/test_free, while this implementation is compiled
+ * normally and forwards to the real allocator. That keeps the interception
+ * portable (no LD_PRELOAD, no linker tricks).
+ */
+#ifndef TEST_ALLOC_H
+#define TEST_ALLOC_H
+
+#include <stddef.h>
+
+extern long test_alloc_calls;    /* total malloc+realloc calls          */
+extern long test_alloc_outstanding; /* live blocks (alloc - free)       */
+extern long test_alloc_fail_at;  /* fail the Nth call; -1 disables     */
+
+void *test_malloc(size_t size);
+void *test_realloc(void *ptr, size_t size);
+void test_free(void *ptr);
+char *test_strdup(const char *s);
+
+void test_alloc_reset(void);
+
+#endif /* TEST_ALLOC_H */

tests/test-load-error.c

@@ -0,0 +1,95 @@
+/* Regression test for allocation-failure handling in the loader.
+ *
+ * Runs yaml_parser_load over documents that place mapping/sequence nodes
+ * on the document node-stack growth boundaries (node 17, 33, ...). When
+ * PUSH grows the node stack and the growth allocation fails, the freshly
+ * allocated node pairs/items array is not attached to the document yet,
+ * and the loader's error path must free it. Before that fix, these runs
+ * leaked one block each, which this test reports.
+ *
+ * The test fails one allocation per run, exhaustively, and verifies that
+ * every failure path leaves zero outstanding blocks.
+ */
+#include "test-alloc.h"
+
+#include <stdio.h>
+#include <string.h>
+#include <yaml.h>
+
+/* 24 empty mappings as sequence items: the root sequence is node 1 and
+ * item N is node N+1, so items 16 and 32 are mapping nodes composed on
+ * node-stack growth boundaries with a live pairs array. */
+static const char test_mappings[] =
+    "- {}\n- {}\n- {}\n- {}\n- {}\n- {}\n- {}\n- {}\n"
+    "- {}\n- {}\n- {}\n- {}\n- {}\n- {}\n- {}\n- {}\n"
+    "- {}\n- {}\n- {}\n- {}\n- {}\n- {}\n- {}\n- {}\n";
+
+/* Same layout for sequences: items 16 and 32 are sequence nodes with a
+ * live items array. */
+static const char test_sequences[] =
+    "- []\n- []\n- []\n- []\n- []\n- []\n- []\n- []\n"
+    "- []\n- []\n- []\n- []\n- []\n- []\n- []\n- []\n"
+    "- []\n- []\n- []\n- []\n- []\n- []\n- []\n- []\n";
+
+/* Load one document (or fail partway), then release everything the API
+ * contract leaves to the caller. Returns 1 on success, 0 on failure. */
+static int run_load(const char *doc, size_t len) {
+    yaml_parser_t parser;
+    yaml_document_t document;
+    int ok = 0;
+
+    if (!yaml_parser_initialize(&parser)) {
+        return 0;
+    }
+    yaml_parser_set_input_string(&parser, (const unsigned char *)doc, len);
+    if (yaml_parser_load(&parser, &document)) {
+        yaml_document_delete(&document);
+        ok = 1;
+    }
+    yaml_parser_delete(&parser);
+    return ok;
+}
+
+static int run_test(const char *name, const char *doc) {
+    size_t len = strlen(doc);
+    long fail_at;
+    long total_calls;
+
+    /* Clean run to determine the total number of allocation calls. */
+    test_alloc_reset();
+    if (!run_load(doc, len)) {
+        fprintf(stderr, "test-load-error: %s: clean run failed\n", name);
+        return 1;
+    }
+    if (test_alloc_outstanding != 0) {
+        fprintf(stderr, "test-load-error: %s: %ld block(s) leaked on success path\n",
+                name, test_alloc_outstanding);
+        return 1;
+    }
+    total_calls = test_alloc_calls;
+
+    for (fail_at = 1; fail_at <= total_calls; fail_at++) {
+        test_alloc_reset();
+        test_alloc_fail_at = fail_at;
+        run_load(doc, len); /* failure here is expected and fine */
+        test_alloc_fail_at = -1;
+
+        if (test_alloc_outstanding != 0) {
+            fprintf(stderr,
+                    "test-load-error: %s: %ld block(s) leaked when allocation %ld failed\n",
+                    name, test_alloc_outstanding, fail_at);
+            return 1;
+        }
+    }
+
+    printf("test-load-error: %s: OK (%ld failure points)\n", name, total_calls);
+    return 0;
+}
+
+int main(void) {
+    int failed = 0;
+
+    failed |= run_test("mappings", test_mappings);
+    failed |= run_test("sequences", test_sequences);
+    return failed;
+}