fix: address PR review — fuzz target fixes, UBSan, CI improvements
Critical fixes: - Add missing #include <string.h> to 5 fuzz targets (wouldn't compile with GCC14+/Clang16+) - Fix fuzz_metadata_from_buf.c OOB read: guard size >= sizeof(int) + FILE_METADATA_WIRE_SIZE - Add Clang compiler check for ENABLE_FUZZ in CMakeLists.txt (fail fast at configure time) - Add fuzz-build CI job (install clang, build all 6 fuzz targets with ENABLE_FUZZ=ON) Warning fixes: - Add UBSan to sanitizer CI matrix (address + undefined) - Remove tautological test_property_glob_consistency (pure/deterministic function) - Use fixed seed srand(42) instead of srand(time(NULL)) for reproducible property tests - Add valid-header + truncated-instructions delta robustness test (exercises instruction-loop error paths) - Fix lcov --remove to exclude '*/_deps/*' (xxhash coverage pollution) - Add comment explaining FASTSYNC_UNDER_VALGRIND skip in test_file.c - Update .gitignore for build-*/ directories
This commit is contained in:
@@ -42,7 +42,7 @@ jobs:
|
||||
needs: lint
|
||||
strategy:
|
||||
matrix:
|
||||
sanitizer: [address]
|
||||
sanitizer: [address, undefined]
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
@@ -56,6 +56,23 @@ jobs:
|
||||
- name: Unit Tests
|
||||
run: ctest --test-dir build-${{ matrix.sanitizer }} --output-on-failure
|
||||
|
||||
fuzz-build:
|
||||
runs-on: ubuntu-latest
|
||||
container: gitea.tap-tap.win/taptap/fastsync-ci:v7
|
||||
needs: lint
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
|
||||
- name: Install clang
|
||||
run: apt-get update -qq && apt-get install -y -qq clang
|
||||
|
||||
- name: Configure (clang + fuzz)
|
||||
run: CC=clang CXX=clang++ cmake -B build-fuzz -S . -DENABLE_FUZZ=ON
|
||||
|
||||
- name: Build fuzz targets
|
||||
run: cmake --build build-fuzz -j$(nproc)
|
||||
|
||||
coverage:
|
||||
runs-on: ubuntu-latest
|
||||
container: gitea.tap-tap.win/taptap/fastsync-ci:v7
|
||||
@@ -79,7 +96,7 @@ jobs:
|
||||
- name: Coverage Report
|
||||
run: |
|
||||
lcov --capture --directory build --output-file coverage.info --branch-coverage
|
||||
lcov --remove coverage.info '/usr/*' '*/tests/*' --output-file coverage.info --branch-coverage --ignore-errors unused
|
||||
lcov --remove coverage.info '/usr/*' '*/tests/*' '*/_deps/*' --output-file coverage.info --branch-coverage --ignore-errors unused
|
||||
lcov --list coverage.info
|
||||
|
||||
valgrind:
|
||||
|
||||
@@ -4,3 +4,4 @@ test_data/
|
||||
__pycache__/
|
||||
build-asan
|
||||
coverage.info
|
||||
build-*/
|
||||
|
||||
+10
-4
@@ -9,8 +9,8 @@ set(CMAKE_C_STANDARD_REQUIRED ON)
|
||||
add_compile_options(-Wall -g -O3)
|
||||
|
||||
# --- Sanitizer option ---
|
||||
set(SANITIZER "none" CACHE STRING "Sanitizer to enable (address, thread, none)")
|
||||
set_property(CACHE SANITIZER PROPERTY STRINGS address thread none)
|
||||
set(SANITIZER "none" CACHE STRING "Sanitizer to enable (address, thread, undefined, none)")
|
||||
set_property(CACHE SANITIZER PROPERTY STRINGS address thread undefined none)
|
||||
|
||||
if(SANITIZER STREQUAL "address")
|
||||
add_compile_options(-fsanitize=address -fno-omit-frame-pointer -g)
|
||||
@@ -18,8 +18,11 @@ if(SANITIZER STREQUAL "address")
|
||||
elseif(SANITIZER STREQUAL "thread")
|
||||
add_compile_options(-fsanitize=thread -fno-omit-frame-pointer -g)
|
||||
add_link_options(-fsanitize=thread)
|
||||
elseif(SANITIZER STREQUAL "undefined")
|
||||
add_compile_options(-fsanitize=undefined -fno-omit-frame-pointer -g)
|
||||
add_link_options(-fsanitize=undefined)
|
||||
elseif(NOT SANITIZER STREQUAL "none")
|
||||
message(FATAL_ERROR "Unknown sanitizer: ${SANITIZER}. Supported values: address, thread, none")
|
||||
message(FATAL_ERROR "Unknown sanitizer: ${SANITIZER}. Supported values: address, thread, undefined, none")
|
||||
endif()
|
||||
|
||||
# --- Strict warnings option ---
|
||||
@@ -84,7 +87,10 @@ add_test(NAME unit_all COMMAND tests)
|
||||
# --- Fuzz targets (requires clang) ---
|
||||
option(ENABLE_FUZZ "Build fuzz targets (requires clang)" OFF)
|
||||
if(ENABLE_FUZZ)
|
||||
file(GLOB FUZZ_SRCS "tests/fuzz/*.c")
|
||||
if(NOT CMAKE_C_COMPILER_ID MATCHES "Clang")
|
||||
message(FATAL_ERROR "ENABLE_FUZZ requires Clang (compiler is ${CMAKE_C_COMPILER_ID})")
|
||||
endif()
|
||||
file(GLOB FUZZ_SRCS "tests/fuzz/*.c")
|
||||
foreach(FUZZ_SRC ${FUZZ_SRCS})
|
||||
get_filename_component(FUZZ_NAME ${FUZZ_SRC} NAME_WE)
|
||||
add_executable(${FUZZ_NAME} ${FUZZ_SRC} ${SHARED_SRCS})
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
#include "data.h"
|
||||
#include <stdint.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
|
||||
int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
|
||||
if (size == 0)
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
#include "data.h"
|
||||
#include <stdint.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
|
||||
int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
|
||||
if (size == 0)
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
#include "data.h"
|
||||
#include <stdint.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
|
||||
int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
|
||||
if (size == 0)
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
#include "data.h"
|
||||
#include <stdint.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
|
||||
int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
|
||||
if (size == 0)
|
||||
|
||||
@@ -2,9 +2,10 @@
|
||||
#include "file.h"
|
||||
#include <stdint.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
|
||||
int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
|
||||
if (size < sizeof(int))
|
||||
if (size < sizeof(int) + FILE_METADATA_WIRE_SIZE)
|
||||
return 0;
|
||||
|
||||
char* buf = malloc(size);
|
||||
|
||||
@@ -272,6 +272,11 @@ void test_file() {
|
||||
test_to_disk_creates_dirs();
|
||||
test_file_content_to_buffer();
|
||||
if (!getenv("FASTSYNC_UNDER_VALGRIND")) {
|
||||
// Fork tests are skipped under valgrind because the parent process runs
|
||||
// orders of magnitude slower than the child (parent is instrumented, child
|
||||
// is not), which causes pipe-based protocol handshake timeouts. The parent
|
||||
// process itself has zero valgrind errors -- the failures are all in the
|
||||
// forked children where inherited allocations are reported as leaks.
|
||||
test_file_send_receive();
|
||||
test_file_send_no_path();
|
||||
}
|
||||
|
||||
+1
-18
@@ -10,7 +10,6 @@
|
||||
#include <stdlib.h>
|
||||
#include <sys/stat.h>
|
||||
#include <unistd.h>
|
||||
#include <time.h>
|
||||
|
||||
static Data* random_data(int min_size, int max_size) {
|
||||
int size = min_size + rand() % (max_size - min_size + 1);
|
||||
@@ -21,7 +20,7 @@ static Data* random_data(int min_size, int max_size) {
|
||||
}
|
||||
|
||||
static void test_property_compress_roundtrip() {
|
||||
srand((unsigned)time(NULL));
|
||||
srand(42);
|
||||
for (int iter = 0; iter < 10; iter++) {
|
||||
Data* original = random_data(1, 10000);
|
||||
EXPECT_NOT_NULL(original);
|
||||
@@ -114,24 +113,8 @@ static void test_property_chunk_roundtrip() {
|
||||
}
|
||||
}
|
||||
|
||||
static void test_property_glob_consistency() {
|
||||
const char* patterns[] = {"*.txt", "test_*", "*.c", "foo", "*.??", "a*b*c"};
|
||||
const char* strings[] = {"test.txt", "foo.c", "bar", "abc", "aXbYcZ", ""};
|
||||
int n_patterns = sizeof(patterns) / sizeof(patterns[0]);
|
||||
int n_strings = sizeof(strings) / sizeof(strings[0]);
|
||||
|
||||
for (int p = 0; p < n_patterns; p++) {
|
||||
for (int s = 0; s < n_strings; s++) {
|
||||
bool first = glob_match(patterns[p], strings[s]);
|
||||
bool second = glob_match(patterns[p], strings[s]);
|
||||
EXPECT_TRUE(first == second);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void test_property() {
|
||||
test_property_compress_roundtrip();
|
||||
test_property_delta_roundtrip();
|
||||
test_property_chunk_roundtrip();
|
||||
test_property_glob_consistency();
|
||||
}
|
||||
|
||||
@@ -126,6 +126,35 @@ static void test_delta_signature_deserialize_truncated() {
|
||||
delta_signature_destroy(sig);
|
||||
}
|
||||
|
||||
static void test_delta_deserialize_truncated_instructions() {
|
||||
// Create a real delta with 2 LITERAL instructions, serialize, then
|
||||
// truncate after the header so the instruction-loop error paths are
|
||||
// exercised (earlier tests with tiny buffers die at the 12-byte
|
||||
// header guard and never reach the instruction decoder).
|
||||
char old_data[4096], new_data[4096];
|
||||
for (int i = 0; i < 4096; i++) {
|
||||
old_data[i] = (char)(i % 256);
|
||||
new_data[i] = old_data[i];
|
||||
}
|
||||
// Two small changes to produce 2 LITERAL instructions
|
||||
new_data[100] = 'X';
|
||||
new_data[200] = 'Y';
|
||||
|
||||
DeltaSignature* sig = delta_signature_create(old_data, 4096, 1024);
|
||||
Delta* delta = delta_compute(new_data, 4096, sig, 1024);
|
||||
Data* serialized = delta_serialize(delta);
|
||||
EXPECT_NOT_NULL(serialized);
|
||||
|
||||
// Truncate to include the header (12 bytes) + partial first instruction
|
||||
serialized->size = 14;
|
||||
const Delta* result = delta_deserialize(serialized);
|
||||
EXPECT_NULL(result);
|
||||
|
||||
data_destroy(serialized);
|
||||
delta_destroy(delta);
|
||||
delta_signature_destroy(sig);
|
||||
}
|
||||
|
||||
static void test_delta_apply_null() {
|
||||
const void* result = delta_apply(NULL, 0, NULL, 0);
|
||||
EXPECT_NULL(result);
|
||||
@@ -177,6 +206,7 @@ void test_robustness() {
|
||||
test_delta_deserialize_truncated();
|
||||
test_delta_deserialize_empty();
|
||||
test_delta_deserialize_garbage();
|
||||
test_delta_deserialize_truncated_instructions();
|
||||
test_delta_signature_deserialize_truncated();
|
||||
test_delta_apply_null();
|
||||
test_protocol_receive_n_data_closed_pipe();
|
||||
|
||||
Reference in New Issue
Block a user