Fix up GCC analyzer

Author: kddnewton

Makefile

@@ -11,7 +11,7 @@ FUZZ_OUTPUT_DIR = $(shell pwd)/fuzz/output
 SOEXT := $(shell ruby -e 'puts RbConfig::CONFIG["SOEXT"]')
 
 CPPFLAGS := -Iinclude
-CFLAGS := -g -O2 -std=c99 -Wall -Werror -Wextra -Wpedantic -Wundef -Wconversion -Wno-missing-braces -fPIC -fvisibility=hidden
+CFLAGS := -g -O2 -std=c99 -Wall -Werror -Wextra -Wpedantic -Wundef -Wconversion -Wno-missing-braces -fPIC -fvisibility=hidden $(CFLAGS)
 CC := cc
 WASI_SDK_PATH := /opt/wasi-sdk
 

ext/prism/extension.c

@@ -82,7 +82,9 @@ build_options_scopes(pm_options_t *options, VALUE scopes) {
 
     // Initialize the scopes array.
     size_t scopes_count = RARRAY_LEN(scopes);
-    pm_options_scopes_init(options, scopes_count);
+    if (!pm_options_scopes_init(options, scopes_count)) {
+        rb_raise(rb_eNoMemError, "failed to allocate memory");
+    }
 
     // Iterate over the scopes and add them to the options.
     for (size_t scope_index = 0; scope_index < scopes_count; scope_index++) {
@@ -97,7 +99,9 @@ build_options_scopes(pm_options_t *options, VALUE scopes) {
         // Initialize the scope array.
         size_t locals_count = RARRAY_LEN(scope);
         pm_options_scope_t *options_scope = &options->scopes[scope_index];
-        pm_options_scope_init(options_scope, locals_count);
+        if (!pm_options_scope_init(options_scope, locals_count)) {
+            rb_raise(rb_eNoMemError, "failed to allocate memory");
+        }
 
         // Iterate over the locals and add them to the scope.
         for (size_t local_index = 0; local_index < locals_count; local_index++) {

include/prism/options.h

@@ -129,8 +129,9 @@ PRISM_EXPORTED_FUNCTION bool pm_options_version_set(pm_options_t *options, const
  *
  * @param options The options struct to initialize the scopes array on.
  * @param scopes_count The number of scopes to allocate.
+ * @return Whether or not the scopes array was initialized successfully.
  */
-PRISM_EXPORTED_FUNCTION void pm_options_scopes_init(pm_options_t *options, size_t scopes_count);
+PRISM_EXPORTED_FUNCTION bool pm_options_scopes_init(pm_options_t *options, size_t scopes_count);
 
 /**
  * Return a pointer to the scope at the given index within the given options.
@@ -147,8 +148,9 @@ PRISM_EXPORTED_FUNCTION const pm_options_scope_t * pm_options_scope_get(const pm
  *
  * @param scope The scope struct to initialize.
  * @param locals_count The number of locals to allocate.
+ * @return Whether or not the scope was initialized successfully.
  */
-PRISM_EXPORTED_FUNCTION void pm_options_scope_init(pm_options_scope_t *scope, size_t locals_count);
+PRISM_EXPORTED_FUNCTION bool pm_options_scope_init(pm_options_scope_t *scope, size_t locals_count);
 
 /**
  * Return a pointer to the local at the given index within the given scope.

include/prism/util/pm_constant_pool.h

@@ -186,7 +186,7 @@ pm_constant_id_t pm_constant_pool_insert_shared(pm_constant_pool_t *pool, const
  * @param length The length of the constant.
  * @return The id of the constant.
  */
-pm_constant_id_t pm_constant_pool_insert_owned(pm_constant_pool_t *pool, const uint8_t *start, size_t length);
+pm_constant_id_t pm_constant_pool_insert_owned(pm_constant_pool_t *pool, uint8_t *start, size_t length);
 
 /**
  * Insert a constant into a constant pool from memory that is constant. Returns

src/diagnostic.c

@@ -389,13 +389,14 @@ pm_diagnostic_list_append_format(pm_list_t *list, const uint8_t *start, const ui
  */
 void
 pm_diagnostic_list_free(pm_list_t *list) {
-    pm_list_node_t *node, *next;
+    pm_diagnostic_t *node = (pm_diagnostic_t *) list->head;
 
-    for (node = list->head; node != NULL; node = next) {
-        next = node->next;
-        pm_diagnostic_t *diagnostic = (pm_diagnostic_t *) node;
+    while (node != NULL) {
+        pm_diagnostic_t *next = (pm_diagnostic_t *) node->node.next;
 
-        if (diagnostic->owned) free((void *) diagnostic->message);
-        free(diagnostic);
+        if (node->owned) free((void *) node->message);
+        free(node);
+
+        node = next;
     }
 }

src/options.c

@@ -64,14 +64,22 @@ pm_options_version_set(pm_options_t *options, const char *version, size_t length
     return false;
 }
 
+// For some reason, GCC analyzer thinks we're leaking allocated scopes and
+// locals here, even though we definitely aren't. This is a false positive.
+// Ideally we wouldn't need to suppress this.
+#if defined(__GNUC__) && (__GNUC__ >= 10)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wanalyzer-malloc-leak"
+#endif
+
 /**
  * Allocate and zero out the scopes array on the given options struct.
  */
-PRISM_EXPORTED_FUNCTION void
+PRISM_EXPORTED_FUNCTION bool
 pm_options_scopes_init(pm_options_t *options, size_t scopes_count) {
     options->scopes_count = scopes_count;
     options->scopes = calloc(scopes_count, sizeof(pm_options_scope_t));
-    if (options->scopes == NULL) abort();
+    return options->scopes != NULL;
 }
 
 /**
@@ -86,11 +94,11 @@ pm_options_scope_get(const pm_options_t *options, size_t index) {
  * Create a new options scope struct. This will hold a set of locals that are in
  * scope surrounding the code that is being parsed.
  */
-PRISM_EXPORTED_FUNCTION void
+PRISM_EXPORTED_FUNCTION bool
 pm_options_scope_init(pm_options_scope_t *scope, size_t locals_count) {
     scope->locals_count = locals_count;
     scope->locals = calloc(locals_count, sizeof(pm_string_t));
-    if (scope->locals == NULL) abort();
+    return scope->locals != NULL;
 }
 
 /**
@@ -192,14 +200,17 @@ pm_options_read(pm_options_t *options, const char *data) {
     data += 4;
 
     if (scopes_count > 0) {
-        pm_options_scopes_init(options, scopes_count);
+        if (!pm_options_scopes_init(options, scopes_count)) return;
 
         for (size_t scope_index = 0; scope_index < scopes_count; scope_index++) {
             uint32_t locals_count = pm_options_read_u32(data);
             data += 4;
 
             pm_options_scope_t *scope = &options->scopes[scope_index];
-            pm_options_scope_init(scope, locals_count);
+            if (!pm_options_scope_init(scope, locals_count)) {
+                pm_options_free(options);
+                return;
+            }
 
             for (size_t local_index = 0; local_index < locals_count; local_index++) {
                 uint32_t local_length = pm_options_read_u32(data);
@@ -211,3 +222,7 @@ pm_options_read(pm_options_t *options, const char *data) {
         }
     }
 }
+
+#if defined(__GNUC__) && (__GNUC__ >= 10)
+#pragma GCC diagnostic pop
+#endif

src/prism.c

@@ -600,7 +600,7 @@ pm_parser_constant_id_location(pm_parser_t *parser, const uint8_t *start, const
  * Retrieve the constant pool id for the given string.
  */
 static inline pm_constant_id_t
-pm_parser_constant_id_owned(pm_parser_t *parser, const uint8_t *start, size_t length) {
+pm_parser_constant_id_owned(pm_parser_t *parser, uint8_t *start, size_t length) {
     return pm_constant_pool_insert_owned(&parser->constant_pool, start, length);
 }
 
@@ -6409,7 +6409,7 @@ pm_parser_local_add_token(pm_parser_t *parser, pm_token_t *token) {
  * Add a local variable from an owned string to the current scope.
  */
 static pm_constant_id_t
-pm_parser_local_add_owned(pm_parser_t *parser, const uint8_t *start, size_t length) {
+pm_parser_local_add_owned(pm_parser_t *parser, uint8_t *start, size_t length) {
     pm_constant_id_t constant_id = pm_parser_constant_id_owned(parser, start, length);
     if (constant_id != 0) pm_parser_local_add(parser, constant_id);
     return constant_id;
@@ -16945,7 +16945,7 @@ parse_regular_expression_named_captures(pm_parser_t *parser, const pm_string_t *
                 memcpy(memory, source, length);
                 // This silences clang analyzer warning about leak of memory pointed by `memory`.
                 // NOLINTNEXTLINE(clang-analyzer-*)
-                name = pm_parser_constant_id_owned(parser, (const uint8_t *) memory, length);
+                name = pm_parser_constant_id_owned(parser, (uint8_t *) memory, length);
 
                 if (pm_token_is_numbered_parameter(source, source + length)) {
                     const pm_location_t *location = &call->receiver->location;
@@ -18021,11 +18021,11 @@ pm_parser_init(pm_parser_t *parser, const uint8_t *source, size_t size, const pm
                 const uint8_t *source = pm_string_source(local);
                 size_t length = pm_string_length(local);
 
-                uint8_t *allocated = malloc(length);
+                void *allocated = malloc(length);
                 if (allocated == NULL) continue;
 
-                memcpy((void *) allocated, source, length);
-                pm_parser_local_add_owned(parser, allocated, length);
+                memcpy(allocated, source, length);
+                pm_parser_local_add_owned(parser, (uint8_t *) allocated, length);
             }
         }
     }
@@ -18235,8 +18235,9 @@ typedef struct {
 static inline pm_error_t *
 pm_parser_errors_format_sort(const pm_parser_t *parser, const pm_list_t *error_list, const pm_newline_list_t *newline_list) {
     pm_error_t *errors = calloc(error_list->size, sizeof(pm_error_t));
-    int32_t start_line = parser->start_line;
+    if (errors == NULL) return NULL;
 
+    int32_t start_line = parser->start_line;
     for (pm_diagnostic_t *error = (pm_diagnostic_t *) error_list->head; error != NULL; error = (pm_diagnostic_t *) error->node.next) {
         pm_line_column_t start = pm_newline_list_line_column(newline_list, error->location.start, start_line);
         pm_line_column_t end = pm_newline_list_line_column(newline_list, error->location.end, start_line);
@@ -18316,7 +18317,9 @@ pm_parser_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, bool col
     // insertion sort into a newly allocated array.
     const int32_t start_line = parser->start_line;
     const pm_newline_list_t *newline_list = &parser->newline_list;
+
     pm_error_t *errors = pm_parser_errors_format_sort(parser, error_list, newline_list);
+    if (errors == NULL) return;
 
     // Now we're going to determine how we're going to format line numbers and
     // blank lines based on the maximum number of digits in the line numbers

src/util/pm_buffer.c

@@ -47,7 +47,7 @@ pm_buffer_length(const pm_buffer_t *buffer) {
 /**
  * Append the given amount of space to the buffer.
  */
-static inline void
+static inline bool
 pm_buffer_append_length(pm_buffer_t *buffer, size_t length) {
     size_t next_length = buffer->length + length;
 
@@ -61,9 +61,11 @@ pm_buffer_append_length(pm_buffer_t *buffer, size_t length) {
         }
 
         buffer->value = realloc(buffer->value, buffer->capacity);
+        if (buffer->value == NULL) return false;
     }
 
     buffer->length = next_length;
+    return true;
 }
 
 /**
@@ -72,8 +74,9 @@ pm_buffer_append_length(pm_buffer_t *buffer, size_t length) {
 static inline void
 pm_buffer_append(pm_buffer_t *buffer, const void *source, size_t length) {
     size_t cursor = buffer->length;
-    pm_buffer_append_length(buffer, length);
-    memcpy(buffer->value + cursor, source, length);
+    if (pm_buffer_append_length(buffer, length)) {
+        memcpy(buffer->value + cursor, source, length);
+    }
 }
 
 /**
@@ -82,8 +85,9 @@ pm_buffer_append(pm_buffer_t *buffer, const void *source, size_t length) {
 void
 pm_buffer_append_zeroes(pm_buffer_t *buffer, size_t length) {
     size_t cursor = buffer->length;
-    pm_buffer_append_length(buffer, length);
-    memset(buffer->value + cursor, 0, length);
+    if (pm_buffer_append_length(buffer, length)) {
+        memset(buffer->value + cursor, 0, length);
+    }
 }
 
 /**
@@ -100,13 +104,12 @@ pm_buffer_append_format(pm_buffer_t *buffer, const char *format, ...) {
     size_t length = (size_t) (result + 1);
 
     size_t cursor = buffer->length;
-    pm_buffer_append_length(buffer, length);
-
-    va_start(arguments, format);
-    vsnprintf(buffer->value + cursor, length, format, arguments);
-    va_end(arguments);
-
-    buffer->length--;
+    if (pm_buffer_append_length(buffer, length)) {
+        va_start(arguments, format);
+        vsnprintf(buffer->value + cursor, length, format, arguments);
+        va_end(arguments);
+        buffer->length--;
+    }
 }
 
 /**
@@ -236,9 +239,10 @@ pm_buffer_append_source(pm_buffer_t *buffer, const uint8_t *source, size_t lengt
 void
 pm_buffer_prepend_string(pm_buffer_t *buffer, const char *value, size_t length) {
     size_t cursor = buffer->length;
-    pm_buffer_append_length(buffer, length);
-    memmove(buffer->value + length, buffer->value, cursor);
-    memcpy(buffer->value, value, length);
+    if (pm_buffer_append_length(buffer, length)) {
+        memmove(buffer->value + length, buffer->value, cursor);
+        memcpy(buffer->value, value, length);
+    }
 }
 
 /**

src/util/pm_constant_pool.c

@@ -287,7 +287,7 @@ pm_constant_pool_insert_shared(pm_constant_pool_t *pool, const uint8_t *start, s
  * potential calls to resize fail.
  */
 pm_constant_id_t
-pm_constant_pool_insert_owned(pm_constant_pool_t *pool, const uint8_t *start, size_t length) {
+pm_constant_pool_insert_owned(pm_constant_pool_t *pool, uint8_t *start, size_t length) {
     return pm_constant_pool_insert(pool, start, length, PM_CONSTANT_POOL_BUCKET_OWNED);
 }
 

src/util/pm_newline_list.c

@@ -30,9 +30,10 @@ pm_newline_list_append(pm_newline_list_t *list, const uint8_t *cursor) {
 
         list->capacity = (list->capacity * 3) / 2;
         list->offsets = (size_t *) calloc(list->capacity, sizeof(size_t));
+        if (list->offsets == NULL) return false;
+
         memcpy(list->offsets, original_offsets, list->size * sizeof(size_t));
         free(original_offsets);
-        if (list->offsets == NULL) return false;
     }
 
     assert(*cursor == '\n');