Skip to content

Bug fixes#2

Merged
rsachetto merged 1 commit into
masterfrom
bugfixes
May 20, 2026
Merged

Bug fixes#2
rsachetto merged 1 commit into
masterfrom
bugfixes

Conversation

@rsachetto
Copy link
Copy Markdown
Owner


  • File: inotify_helpers.c

  • Problem: In check_for_model_file_changes, assigning the return value of read() to size_t length caused an implicit cast to a very large unsigned number on read error (-1), leading to stack overflow. Additionally, attempting to break out of the infinite loop inside a _Noreturn function generated compiler warnings and undefined runtime behavior.

  • Fix:

    • Changed length to ssize_t and validated length < 0.
    • Added an #include <errno.h> check: if errno == EINTR (a transient interrupt), the loop safely continues.
    • Implemented automatic recreation/recovery: on any unrecoverable error, the old descriptor is closed, the hash map resources are freed, a new inotify_init() instance is obtained, and it dynamically re-registers file watches for all currently loaded models, allowing monitoring to continue seamlessly without terminating the thread or the shell.
  • File: commands.c

  • Problem: In unloadall, iterating forward by index through the loaded_models hash map while invoking unload (which deletes entries) caused subsequent keys to shift/reorder within the internal stb_ds dynamic array, leading to skipped deletions or out-of-bounds memory accesses.

  • Fix:

    • Replaced the indexed loop with a robust while (shlen(shell_state->loaded_models) > 0) loop, always unloading the first element (index 0) until empty.
  • File: commands.c

  • Problem: Unloading the active model left a dangling pointer in shell_state->current_model pointing to freed memory, leading to potential use-after-free (UAF) crashes in subsequent shell commands.

  • Fix:

    • Added a safety check in unload to explicitly set shell_state->current_model = NULL if the last remaining model is being unloaded.
  • File: commands.c

  • Problem: Assigning empty or invalid values in the shell (e.g. setodevalue v = ) caused parser errors that returned either NULL or an empty program array. Directly dereferencing program[0] resulted in an immediate segmentation fault due to accessing NULL memory.

  • Fix:

    • Added a defensive check if(program == NULL || arrlen(program) == 0) to gracefully return false on parser failure/empty input instead of crashing.
  • File: parser.c

  • Problem: The global scope checking flag propagation was nested inside case ast_ode_stmt: / case ast_global_stmt: but wrapped in an impossible if(src->tag == ast_assignment_stmt) condition, resulting in dead code. This caused global variables in standard assignment statements to not propagate their .global flag, leading to compiler local-variable shadow-declaration bugs in generated C.

  • Fix:

    • Moved the flag propagation logic to its correct block inside case ast_assignment_stmt::
      bool var_is_global = shgeti(p->global_scope, id_name) != -1;
      if(var_is_global) {
          src->assignment_stmt.name->identifier.global = true;
      }
    • Cleaned up the unreachable condition in case ast_ode_stmt:/case ast_global_stmt: to directly check the tag's presence in global_scope.

---

* **File**: [inotify_helpers.c](file:///home/sachetto/Projects/odecompiler/src/inotify_helpers.c)
* **Problem**: In `check_for_model_file_changes`, assigning the return value of `read()` to `size_t length` caused an implicit cast to a very large unsigned number on read error (`-1`), leading to stack overflow. Additionally, attempting to break out of the infinite loop inside a `_Noreturn` function generated compiler warnings and undefined runtime behavior.
* **Fix**:
  - Changed `length` to `ssize_t` and validated `length < 0`.
  - Added an `#include <errno.h>` check: if `errno == EINTR` (a transient interrupt), the loop safely `continue`s.
  - Implemented automatic recreation/recovery: on any unrecoverable error, the old descriptor is closed, the hash map resources are freed, a new `inotify_init()` instance is obtained, and it dynamically re-registers file watches for all currently loaded models, allowing monitoring to continue seamlessly without terminating the thread or the shell.

* **File**: [commands.c](file:///home/sachetto/Projects/odecompiler/src/commands.c)
* **Problem**: In `unloadall`, iterating forward by index through the `loaded_models` hash map while invoking `unload` (which deletes entries) caused subsequent keys to shift/reorder within the internal `stb_ds` dynamic array, leading to skipped deletions or out-of-bounds memory accesses.
* **Fix**:
  - Replaced the indexed loop with a robust `while (shlen(shell_state->loaded_models) > 0)` loop, always unloading the first element (index 0) until empty.

* **File**: [commands.c](file:///home/sachetto/Projects/odecompiler/src/commands.c)
* **Problem**: Unloading the active model left a dangling pointer in `shell_state->current_model` pointing to freed memory, leading to potential use-after-free (UAF) crashes in subsequent shell commands.
* **Fix**:
  - Added a safety check in `unload` to explicitly set `shell_state->current_model = NULL` if the last remaining model is being unloaded.

* **File**: [commands.c](file:///home/sachetto/Projects/odecompiler/src/commands.c)
* **Problem**: Assigning empty or invalid values in the shell (e.g. `setodevalue v = `) caused parser errors that returned either `NULL` or an empty program array. Directly dereferencing `program[0]` resulted in an immediate segmentation fault due to accessing `NULL` memory.
* **Fix**:
  - Added a defensive check `if(program == NULL || arrlen(program) == 0)` to gracefully return `false` on parser failure/empty input instead of crashing.

* **File**: [parser.c](file:///home/sachetto/Projects/odecompiler/src/compiler/parser.c)
* **Problem**: The global scope checking flag propagation was nested inside `case ast_ode_stmt:` / `case ast_global_stmt:` but wrapped in an impossible `if(src->tag == ast_assignment_stmt)` condition, resulting in dead code. This caused global variables in standard assignment statements to not propagate their `.global` flag, leading to compiler local-variable shadow-declaration bugs in generated C.
* **Fix**:
  - Moved the flag propagation logic to its correct block inside `case ast_assignment_stmt:`:
    ```c
    bool var_is_global = shgeti(p->global_scope, id_name) != -1;
    if(var_is_global) {
        src->assignment_stmt.name->identifier.global = true;
    }
    ```
  - Cleaned up the unreachable condition in `case ast_ode_stmt:`/`case ast_global_stmt:` to directly check the tag's presence in `global_scope`.

---
@rsachetto rsachetto merged commit 7f9620a into master May 20, 2026
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant