-
Notifications
You must be signed in to change notification settings - Fork 39
fix(ci): separate non-sanitized binary for valgrind and fix parse-error leaks #202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
fc0a592
b98e632
a5cddc9
3f22561
a1a313f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| .vscode | ||
| brainrot | ||
| brainrot-valgrind | ||
| lang.tab.c | ||
| lang.tab.h | ||
| lex.yy.c | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,9 @@ PYTHON := python3 | |
| EMCC := emcc | ||
|
|
||
| # Compiler and linker flags | ||
| CFLAGS := -Wall -Wextra -Wpedantic -Werror -O2 -Wuninitialized -fsanitize=address,undefined -fno-omit-frame-pointer -g | ||
| SANITIZER_FLAGS := -fsanitize=address,undefined | ||
| CFLAGS := -Wall -Wextra -Wpedantic -Werror -O2 -Wuninitialized $(SANITIZER_FLAGS) -fno-omit-frame-pointer -g | ||
| VALGRIND_CFLAGS := $(filter-out $(SANITIZER_FLAGS),$(CFLAGS)) | ||
| LDFLAGS := -lfl -lm -ldl -rdynamic | ||
| SO_CFLAGS := -fPIC -shared | ||
|
|
||
|
|
@@ -24,6 +26,7 @@ STDROT_LIB := libstdrot.so | |
|
|
||
| # Output files | ||
| TARGET := brainrot | ||
| VALGRIND_TARGET := brainrot-valgrind | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should-fix: |
||
| BISON_OUTPUT := lang.tab.c | ||
| FLEX_OUTPUT := lex.yy.c | ||
|
|
||
|
|
@@ -91,6 +94,11 @@ $(TARGET): $(ALL_SRCS) $(STDROT_LIB) | |
| $(CC) $(CFLAGS) -o $@ $(ALL_SRCS) $(LDFLAGS) | ||
| @echo "Skibidi toilet: $(TARGET) compiled with max gyatt." | ||
|
|
||
| # Valgrind executable build (built without ASan/UBSan for Valgrind compatibility) | ||
| $(VALGRIND_TARGET): $(ALL_SRCS) $(STDROT_LIB) | ||
| $(CC) $(VALGRIND_CFLAGS) -o $@ $(ALL_SRCS) $(LDFLAGS) | ||
| @echo "brainrot-valgrind compiled without sanitizers." | ||
|
|
||
| # WebAssembly build: stdrot sources go straight into the same binary | ||
| # instead of a separate $(STDROT_LIB), so this does NOT depend on it. | ||
| .PHONY: wasm | ||
|
|
@@ -118,15 +126,15 @@ test: ensure-stdrot $(TARGET) | |
| # Clean build artifacts | ||
| .PHONY: clean | ||
| clean: | ||
| rm -f $(TARGET) $(STDROT_LIB) $(GENERATED_SRCS) lang.tab.h | ||
| rm -f $(TARGET) $(VALGRIND_TARGET) $(STDROT_LIB) $(GENERATED_SRCS) lang.tab.h | ||
| rm -f $(WASM_TARGET) $(WASM_JS) | ||
| rm -f *.o | ||
| @echo "Blud cleaned up the mess like a true sigma coder." | ||
|
|
||
| # Run Valgrind on all .brainrot tests | ||
| .PHONY: valgrind | ||
| valgrind: ensure-stdrot $(TARGET) | ||
| @./run_valgrind_tests.sh | ||
| valgrind: ensure-stdrot $(VALGRIND_TARGET) | ||
| @./run_valgrind_tests.sh ./$(VALGRIND_TARGET) | ||
| @echo "Valgrind check done. If anything was sus, it'll show up with a non-zero exit code. No cap." | ||
|
|
||
| # Install target | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,32 +1,54 @@ | ||
| #!/bin/bash | ||
|
|
||
| TARGET="${1:-./brainrot-valgrind}" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. MAJOR. Default is a binary CONTRIBUTING now documents
Fail closed here: |
||
|
|
||
| if ! command -v valgrind >/dev/null 2>&1; then | ||
| echo "Error: valgrind is not installed or not in PATH" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| if [[ ! -x "$TARGET" ]]; then | ||
| echo "Error: Valgrind target '$TARGET' is not executable" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| for f in test_cases/*.brainrot; do | ||
| echo "Running Valgrind on $f..." | ||
| base=$(basename "$f" .brainrot) | ||
|
|
||
| case "$base" in | ||
| slorp_int) input="42" ;; | ||
| slorp_short) input="69" ;; | ||
| slorp_float) input="3.14" ;; | ||
| slorp_double) input="3.141592" ;; | ||
| slorp_char) input="c" ;; | ||
| slorp_string) input="skibidi bop bop yes yes" ;; | ||
| *) input="" ;; | ||
| slorp_int) input="42" ;; | ||
| slorp_short) input="69" ;; | ||
| slorp_float) input="3.14" ;; | ||
| slorp_double) input="3.141592" ;; | ||
| slorp_char) input="c" ;; | ||
| slorp_string) input="skibidi bop bop yes yes" ;; | ||
| native_call_self_init) input="42" ;; | ||
| native_call_loop) input=$'1\n2\n3' ;; | ||
| native_call_string_arg) input=$'skibidi\nq' ;; | ||
| native_call_do_while) input=$'5\n50\n6\n150' ;; | ||
| *) input="" ;; | ||
| esac | ||
|
|
||
| if [[ -n "$input" ]]; then | ||
| echo "$input" | valgrind --leak-check=full --error-exitcode=100 ./brainrot "$f" | ||
| echo "$input" | valgrind --leak-check=full --error-exitcode=100 "$TARGET" "$f" | ||
| else | ||
| valgrind --track-origins=yes --leak-check=full --error-exitcode=100 ./brainrot "$f" | ||
| valgrind --leak-check=full --error-exitcode=100 "$TARGET" "$f" | ||
| fi | ||
|
|
||
| valgrind_exit_code=$? # Capture only valgrind’s exit code | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. MAJOR. This is the same success policy that made #186 silent. The next lines only fail on valgrind exit code 100. Missing binary is 127. Valgrind not installed is 127. The ASan SIGSEGV that #186 caught was also not 100. That check is unchanged. Verified: missing binary → valgrind exit 127 → loop continues → script exit 0.
Treat 127, and any failure to start the target, as a hard error. Do not make "not 100" mean "no leaks." #186 was not "wrong binary." It was "the job can succeed without testing leaks." The sanitizer split removed one trigger. This check is the mechanism. |
||
|
|
||
| if [[ $valgrind_exit_code -eq 100 ]]; then | ||
| echo "Valgrind detected memory issues in $f" | ||
| exit 1 | ||
| fi | ||
| case $valgrind_exit_code in | ||
| 0|1) ;; | ||
| 100) | ||
| echo "Valgrind detected memory issues in $f" >&2 | ||
| exit 1 | ||
| ;; | ||
| *) | ||
| echo "Valgrind failed while running $f (exit $valgrind_exit_code)" >&2 | ||
| exit 1 | ||
| ;; | ||
| esac | ||
|
|
||
| echo | ||
| done | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This documents the broken path as a supported workflow. After
make,./brainrot-valgrinddoes not exist, andrun_valgrind_tests.shwill still "pass" because it only treats exit code 100 as failure.Either delete this alternative or make the script refuse to run without an executable target. Do not tell people to invoke a checker that fail-opens.