mirror of
https://github.com/hanzoai/datastore.git
synced 2026-09-26 13:55:07 +00:00
Fix AST fuzzer silently burning the corpus and exiting early with success
The client-side AST fuzzer loop swallowed every exception thrown during a fuzzing step: the `catch (...)` handler recorded the error, the loop bottom reset it, and fuzzing continued. When the failure mode is persistent - most importantly a dead server, where the reconnection attempt inside `processParsedSingleQuery` throws instantly - the client sprints through the remaining corpus at hundreds of steps per second without contacting the server and exits with code 0, which the job reports as "Fuzzer exited with success". Evidence from `AST fuzzer (amd_debug, targeted)` on PR #109061 (https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=109061&sha=1419f6b7e95c81c812f945c06333998b92d56ad3&name_0=PR&name_1=AST+fuzzer+%28amd_debug%2C+targeted%29): `fuzzer.log` records 68972 fuzzing steps, of which 52837 produced no outcome (silent exception), while `server.log` shows the server received only 209 queries within a 47-second window before dying. On PR #106250 (https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=106250&sha=latest&name_0=PR&name_1=AST+fuzzer+%28amd_debug%2C+targeted%29) a 5-file corpus with ~20 SELECT statements (about 20000 fuzzing steps, over an hour of work at the measured debug-build rate of 2-6 steps/s) "completed" in 74 seconds and the job passed. Overall, 13-33% of targeted AST fuzzer runs per month finish in under 8 minutes of their 30-minute budget. Client fixes (`programs/client/FuzzLoop.cpp`): - Print thrown exceptions in the fuzzing-step catch handlers instead of swallowing them silently, so `fuzzer.log` shows what actually happened. - After a thrown exception, check the connection via `tryToReconnect` (the same check `processASTFuzzerStep` does for errors received without throwing) and stop fuzzing when the server is gone instead of uselessly sprinting through the rest of the corpus. - Register query parameters generated by `QueryFuzzer` (from `getLastQueryParameters`) for client-side substitution, mirroring the server-side fuzzer in `executeASTFuzzerQueries`; previously any fuzzed query containing a substituted `{name:type}` parameter failed with "Substitution ... is not set" before reaching the server. Job fix (`ci/jobs/scripts/fuzzer/run-fuzzer.sh`): - When the AST fuzzer walks the whole corpus and exits 0 with a meaningful part of the FUZZ_TIME_LIMIT budget remaining, restart it (each pass uses a fresh random seed) instead of ending the job after a couple of minutes. This matters for the targeted variant, whose corpus is a handful of test files. The restart is gated on a server liveness probe so a dead server still terminates the job for classification.
This commit is contained in:
@@ -250,6 +250,14 @@ bool Client::processWithASTFuzzer(std::string_view full_query)
|
||||
if (fuzz_step > 0)
|
||||
{
|
||||
fuzzer.fuzzMain(ast_to_process);
|
||||
|
||||
/// The fuzzer can substitute fragments that contain {name:type} query parameters
|
||||
/// (e.g. from the column_like pool) and generates values for them. Register the
|
||||
/// values for client-side parameter substitution, like the server-side fuzzer
|
||||
/// does (see executeASTFuzzerQueries); otherwise every such query fails with
|
||||
/// "Substitution ... is not set" before even reaching the server.
|
||||
for (const auto & [name, value] : fuzzer.getLastQueryParameters())
|
||||
query_parameters.insert_or_assign(name, value);
|
||||
}
|
||||
|
||||
query_to_execute = ast_to_process->formatForErrorMessage();
|
||||
@@ -373,6 +381,8 @@ bool Client::processWithASTFuzzer(std::string_view full_query)
|
||||
{
|
||||
if (!ast_to_process)
|
||||
fmt::print(stderr, "Error while forming new query: {}\n", getCurrentExceptionMessage(true));
|
||||
else
|
||||
fmt::print(stderr, "Client-side exception on processing query '{}': {}\n", query_to_execute, getCurrentExceptionMessage(false));
|
||||
|
||||
// Some functions (e.g. protocol parsers) don't throw, but
|
||||
// set last_exception instead, so we'll also do it here for
|
||||
@@ -382,6 +392,15 @@ bool Client::processWithASTFuzzer(std::string_view full_query)
|
||||
client_exception
|
||||
= std::make_unique<Exception>(getCurrentExceptionMessageAndPattern(print_stack_trace), getCurrentExceptionCode());
|
||||
have_error = true;
|
||||
|
||||
// The exception may mean that the connection to the server was lost (e.g. the server
|
||||
// died and the connection attempt throws). processASTFuzzerStep performs this check
|
||||
// for errors received without throwing, but thrown exceptions bypass it. Without the
|
||||
// check every subsequent step fails instantly the same way, so the fuzzer silently
|
||||
// burns through the rest of the corpus in seconds and exits with code 0 as if all
|
||||
// the queries were fuzzed.
|
||||
if (!tryToReconnect(1, 10))
|
||||
return false;
|
||||
}
|
||||
|
||||
#if USE_BUZZHOUSE
|
||||
@@ -467,9 +486,15 @@ bool Client::processWithASTFuzzer(std::string_view full_query)
|
||||
}
|
||||
catch (...)
|
||||
{
|
||||
fmt::print(stderr, "Client-side exception on processing query '{}': {}\n", query_to_execute, getCurrentExceptionMessage(false));
|
||||
|
||||
client_exception
|
||||
= std::make_unique<Exception>(getCurrentExceptionMessageAndPattern(print_stack_trace), getCurrentExceptionCode());
|
||||
have_error = true;
|
||||
|
||||
// See the comment on the same check in the main fuzzing loop above.
|
||||
if (!tryToReconnect(1, 10))
|
||||
return false;
|
||||
}
|
||||
|
||||
if (have_error)
|
||||
|
||||
Reference in New Issue
Block a user