runtime: make string builtins binary-safe (slen-aware)#18
Merged
Conversation
Six VAL_STRING ops were using strlen/strcmp/strstr/strncmp and silently truncated at the first embedded NUL. Replaced with slen-aware variants: - gem_val_eq, gem_lt: memcmp + length compare - sort default cmp: same lexicographic memcmp pattern - sqlite3_bind_text: pass v.slen instead of -1 (sqlite was calling strlen) - path_join: explicit-length concat, no strcpy/strcat - str_replace: memcmp scan instead of strstr/strncmp Adds examples/93_binary_safe_strings.gem covering all six.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Six
VAL_STRINGops were usingstrlen/strcmp/strstr/strncmpand silently truncated at the first embedded NUL. Strings in Gem carry an explicitslenand are documented as binary-safe — these were violating that invariant.Found via a runtime audit. Memory-safety audit on the same pass came back clean (all reported findings were false positives — stb_ds idioms, arena-reset timing, deep-copy semantics on
sendwere all correct).Fixes
gem_val_eqruntime/gem_core.cmemcmp+ length comparegem_ltruntime/gem_ops.cmemcmpwith length tiebreaksortdefault cmpruntime/gem_builtins_collection.cgem_ltsqlite3_bind_textruntime/gem_builtins_sqlite.cv.sleninstead of-1(sqlite was callingstrleninternally)path_joinruntime/gem_builtins_io.cstrcpy/strcatstr_replaceruntime/gem_builtins_string.cmemcmpscan instead ofstrstr/strncmpSkipped
input()(gem_builtins_core.c:479) — reads interactive lines viafgets, embedded NULs aren't a sensible input there.Test plan
make test— 120 examples pass (was 119; new93_binary_safe_strings.gemcovers all six fixes with 12 assertions)\0:"x\0y" == "x"is nowfalse\0:"x\0y" < "x\0z"is nowtruesortordering past\0str_replacefinds patterns containing\0path_joinpreserves bytes past\0in either argumentmake buildclean — no compiler source changes, so no bootstrap needed