Fix memory allocator mismatch in error message handling

[dirtyfilthy]

Replace strdup() calls with sqlite3_mprintf() to ensure consistent memory management. Error messages allocated with strdup() (standard malloc) were being freed with sqlite3_free(), causing crashes.

Fixes #2

Description

This PR fixes a memory allocator mismatch bug that caused crashes when executing invalid Cypher queries. Error messages were allocated using strdup() (standard C library malloc()) but freed using sqlite3_free() (SQLite's allocator), resulting in free(): invalid pointer crashes.

Related Issues

Closes #2

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
• 📝 Documentation update
• 🔧 Build/CI configuration change
• ♻️ Code refactoring (no functional changes)
• ⚡ Performance improvement
• ✅ Test improvement

Changes Made

• Replaced strdup("Query string is NULL") with sqlite3_mprintf("%s", "Query string is NULL") at line 172
• Replaced strdup("Failed to create lexer") with sqlite3_mprintf("%s", "Failed to create lexer") at line 177
• Replaced strdup(pParser->zErrorMsg) with sqlite3_mprintf("%s", pParser->zErrorMsg) at line 184
• Added condition to only copy error message when parsing fails (!pParser->pAst && check)

Testing

• All existing tests pass (make test)
• Added new tests for new functionality
• Tested with Valgrind for memory leaks
• Tested with AddressSanitizer
• Manual testing performed
• Performance testing performed (if applicable)

Test Evidence

Before fix:

Program received signal SIGABRT, Aborted.
#7  0x0000fffff67ed8f8 [PAC] in sqlite3_free () from /lib/aarch64-linux-gnu/libsqlite3.so.0
#8  0x0000fffff6566b40 [PAC] in cypherExecuteSqlFunc (context=0xdcac48, argc=1, argv=0xdcac78) at cypher/cypher-executor-sql.c:64

Valgrind output (before fix):

==600085== Invalid free() / delete / delete[] / realloc()
==600085==    at 0x48885F8: free (vg_replace_malloc.c:989)
==600085==    by 0x6B9D8F7: sqlite3_free (in /lib/aarch64-linux-gnu/libsqlite3.so.0)
==600085==    by 0x6E26B3F: cypherExecuteSqlFunc (cypher-executor-sql.c:64)
==600085==  Address 0x6f233e8 is 8 bytes before a block of size 79 alloc'd
==600085==    at 0x4885558: malloc (vg_replace_malloc.c:446)
==600085==    by 0x4AD50E7: strdup (strdup.c:42)
==600085==    by 0x6E33613: cypherParse (cypher-parser.c:184)

After fix:

• All tests pass without crashes
• Invalid queries return proper error messages instead of crashing
• Valgrind shows no memory errors related to this issue
• Valid queries continue to work correctly

Documentation

• Updated API documentation
• Updated README.md (if needed)
• Updated CHANGELOG.md
• Added code comments where necessary
• Updated examples (if applicable)

Checklist

• My code follows the project's code style guidelines
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• My changes generate no new warnings
• I have checked for memory leaks
• I have updated the documentation accordingly
• My changes are compatible with the supported platforms (Linux x86_64)

Breaking Changes

N/A

Performance Impact

• No performance impact
• Performance improved
• Performance slightly degraded (justified because...)
• Performance significantly degraded (needs discussion)

Screenshots / Examples

Minimal reproduction (before fix):

CREATE VIRTUAL TABLE IF NOT EXISTS graph USING graph();
SELECT cypher_execute('RETURN 1');  -- Crashes with free(): invalid pointer

After fix:

CREATE VIRTUAL TABLE IF NOT EXISTS graph USING graph();
SELECT cypher_execute('RETURN 1');  -- Returns: "Expected MATCH, CREATE, MERGE, SET, or DELETE near 'RETURN' at line 1 column 1"

Additional Context

The issue was identified using Valgrind, which clearly showed the allocator mismatch:

• Memory allocated with strdup() (standard malloc())
• Freed with sqlite3_free() (SQLite's allocator)
• This caused undefined behavior and crashes

The fix ensures all error message allocations use SQLite's allocator (sqlite3_mprintf) which is compatible with sqlite3_free().

Reviewer Notes

Please verify:

1. The three strdup() replacements are correct
2. The condition change (!pParser->pAst &&) is appropriate
3. No other locations in the codebase have similar allocator mismatches

---

For Maintainers:

• Code review completed
• All CI checks passing
• Documentation reviewed
• Version number updated (if needed)
• CHANGELOG.md updated

[gabewillen]

@dirtyfilthy Our first contribution! Thanks I'm actually working on the memory leaks right now in the new MERGE feature. Thanks for knocking this out!