Skip to content

Commit 73d44ec

Browse files
[Fix] Correctly set testcase build url and gn args for fuzzing on batch (#5013)
Fix missing testcase metadata when fuzz task is running on batch (or any non-local execution). #### Context Some build-related environment variables (`build_url`, `build_key` and `gn_args_path`) are set during the build setup on fuzz task main stage. These should be retrieved during postprocess to set the testcase's metadata, which is used to display the build url and the gn args config section on the clusterfuzz UI. However, for non-local execution, these variables are not propagated through the `uworker_output` proto and are missing in postprocess, so any testcase created during fuzzing on batch is not displaying these values. #### Changes * Added the `build_url`, `build_key` and `gn_args` fields to the fuzz task output message. * Added the logic to store these fields and use them to set the initial testcase metadata. #### Tests * Testing on development environment is ineffective as it is not currently running fuzz tasks on batch. I deployed to dev anyway and confirmed that, at least, the local execution of fuzz tasks is still working. * Testing on prod (deployed this PR to internal) * Fuzzing hours stable after deploy: https://screenshot.googleplex.com/7JfregzBR8F6MRA.png * Logs for fuzz tasks executing on batch: https://cloudlogging.app.goo.gl/5xXpLjUdwzK6wTeR9 (main) and https://cloudlogging.app.goo.gl/nX832G1782asdMDXA (postprocess) * Logs for fuzz tasks executing on GCE VMs: https://cloudlogging.app.goo.gl/61nvxR1BwQrQnvXr8 (main) and https://cloudlogging.app.goo.gl/6yoQSvU8r1mUVxsN7 (postprocess) * Evidence of testcases creates after the deploy containing the build url and GN config args: * GCE VMs (local exec): https://clusterfuzz.com/testcase-detail/6746169883426816 * Batch: https://clusterfuzz.com/testcase-detail/6451774202249216 * Build URL: https://screenshot.googleplex.com/45n7M75hEPoEsNZ.png * GN Configs: https://screenshot.googleplex.com/9r3SxRPRQRyC4tv.png Bug: b/441128474 Related issue: #4914
1 parent 5386621 commit 73d44ec

File tree

6 files changed

+130
-44
lines changed

6 files changed

+130
-44
lines changed

src/clusterfuzz/_internal/bot/tasks/utasks/fuzz_task.py

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1002,17 +1002,18 @@ def create_testcase(group: uworker_msg_pb2.FuzzTaskCrashGroup,
10021002
fully_qualified_fuzzer_name: str):
10031003
"""Create a testcase based on crash."""
10041004
crash = group.main_crash
1005+
fuzz_task_output = uworker_output.fuzz_task_output
10051006
comment = (f'Fuzzer {fully_qualified_fuzzer_name} generated testcase crashed '
10061007
f'in {crash.crash_time} seconds '
1007-
f'(r{uworker_output.fuzz_task_output.crash_revision})')
1008+
f'(r{fuzz_task_output.crash_revision})')
10081009
testcase_id = data_handler.store_testcase(
10091010
crash=crash,
10101011
fuzzed_keys=crash.fuzzed_key or None,
10111012
minimized_keys=get_fixed_or_minimized_key(group.one_time_crasher_flag),
10121013
regression=get_regression(group.one_time_crasher_flag),
10131014
fixed=get_fixed_or_minimized_key(group.one_time_crasher_flag),
10141015
one_time_crasher_flag=group.one_time_crasher_flag,
1015-
crash_revision=int(uworker_output.fuzz_task_output.crash_revision),
1016+
crash_revision=int(fuzz_task_output.crash_revision),
10161017
comment=comment,
10171018
absolute_path=crash.absolute_path,
10181019
fuzzer_name=uworker_input.fuzzer_name,
@@ -1036,6 +1037,15 @@ def create_testcase(group: uworker_msg_pb2.FuzzTaskCrashGroup,
10361037
events.TestcaseCreationEvent(
10371038
testcase=testcase, creation_origin=events.TestcaseOrigin.FUZZ_TASK))
10381039

1040+
# Add build metadata to testcase. This is needed due to some build env vars
1041+
# not being propagated if utask is not executed locally (e.g., BUILD_URL).
1042+
data_handler.set_build_metadata_to_testcase(
1043+
testcase,
1044+
build_key=fuzz_task_output.build_key,
1045+
build_url=fuzz_task_output.build_url,
1046+
gn_args=fuzz_task_output.gn_args,
1047+
update=True)
1048+
10391049
if group.context.fuzzer_metadata:
10401050
for key, value in group.context.fuzzer_metadata.items():
10411051
testcase.set_metadata(key, value, update_testcase=False)
@@ -1879,6 +1889,7 @@ def run(self):
18791889
fuzz_target = self.fuzz_target.binary if self.fuzz_target else None
18801890
build_setup_result = build_manager.setup_build(
18811891
environment.get_value('APP_REVISION'), fuzz_target=fuzz_target)
1892+
_add_build_metadata_to_output(self.fuzz_task_output)
18821893

18831894
engine_impl = engine.get(self.fuzzer.name)
18841895
if engine_impl and build_setup_result:
@@ -2241,6 +2252,14 @@ def _to_engine_output(output: str, crash_path: str, return_code: int,
22412252
return engine_output
22422253

22432254

2255+
def _add_build_metadata_to_output(
2256+
fuzz_task_output: uworker_msg_pb2.FuzzTaskOutput) -> None:
2257+
"""Add build metadata from environment to fuzz task output."""
2258+
fuzz_task_output.build_key = environment.get_value('BUILD_KEY', '')
2259+
fuzz_task_output.build_url = environment.get_value('BUILD_URL', '')
2260+
fuzz_task_output.gn_args = data_handler.get_filtered_gn_args() or ''
2261+
2262+
22442263
def _upload_engine_output(engine_output):
22452264
timestamp = uworker_io.proto_timestamp_to_timestamp(engine_output.timestamp)
22462265
testcase_manager.upload_log(engine_output.output.decode(),

src/clusterfuzz/_internal/datastore/data_handler.py

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -834,16 +834,8 @@ def store_testcase(crash, fuzzed_keys, minimized_keys, regression, fixed,
834834
return testcase_id
835835

836836

837-
def set_initial_testcase_metadata(testcase):
838-
"""Set various testcase metadata fields during testcase initialization."""
839-
build_key = environment.get_value('BUILD_KEY')
840-
if build_key:
841-
testcase.set_metadata('build_key', build_key, update_testcase=False)
842-
843-
build_url = environment.get_value('BUILD_URL')
844-
if build_url:
845-
testcase.set_metadata('build_url', build_url, update_testcase=False)
846-
837+
def get_filtered_gn_args() -> str | None:
838+
"""Return filtered GN args based on filepath GN_ARGS_PATH."""
847839
gn_args_path = environment.get_value('GN_ARGS_PATH', '')
848840
if gn_args_path:
849841
gn_args = utils.read_data_from_file(
@@ -856,8 +848,41 @@ def set_initial_testcase_metadata(testcase):
856848
if not GOMA_DIR_LINE_REGEX.match(line)
857849
]
858850
filtered_gn_args = '\n'.join(filtered_gn_args_lines)
859-
testcase.set_metadata('gn_args', filtered_gn_args, update_testcase=False)
851+
return filtered_gn_args
852+
return None
853+
854+
855+
def set_build_metadata_to_testcase(testcase: data_types.Testcase,
856+
build_key: str | None = None,
857+
build_url: str | None = None,
858+
gn_args: str | None = None,
859+
update: bool = False):
860+
"""Set testcase metadata fields related to the build metadata."""
861+
dirty_flag = False
862+
build_key = (
863+
environment.get_value('BUILD_KEY') if build_key is None else build_key)
864+
if build_key and not testcase.get_metadata('build_key'):
865+
dirty_flag = True
866+
testcase.set_metadata('build_key', build_key, update_testcase=False)
867+
868+
build_url = (
869+
environment.get_value('BUILD_URL') if build_url is None else build_url)
870+
if build_url and not testcase.get_metadata('build_url'):
871+
dirty_flag = True
872+
testcase.set_metadata('build_url', build_url, update_testcase=False)
873+
874+
gn_args = get_filtered_gn_args() if gn_args is None else gn_args
875+
if gn_args and not testcase.get_metadata('gn_args'):
876+
dirty_flag = True
877+
testcase.set_metadata('gn_args', gn_args, update_testcase=False)
860878

879+
if update and dirty_flag:
880+
testcase.put()
881+
882+
883+
def set_initial_testcase_metadata(testcase: data_types.Testcase) -> None:
884+
"""Set various testcase metadata fields during testcase initialization."""
885+
set_build_metadata_to_testcase(testcase)
861886
testcase.platform = environment.platform().lower()
862887
testcase.platform_id = environment.get_platform_id()
863888

src/clusterfuzz/_internal/protos/uworker_msg.proto

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,9 @@ message FuzzTaskOutput {
261261
optional BuildData build_data = 14;
262262
optional int64 app_revision = 15;
263263
repeated EngineOutput engine_outputs = 16;
264+
optional string build_key = 17;
265+
optional string build_url = 18;
266+
optional string gn_args = 19;
264267
}
265268

266269
message MinimizeTaskOutput {

src/clusterfuzz/_internal/protos/uworker_msg_pb2.py

Lines changed: 26 additions & 26 deletions
Large diffs are not rendered by default.

src/clusterfuzz/_internal/protos/uworker_msg_pb2.pyi

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1250,6 +1250,9 @@ class FuzzTaskOutput(google.protobuf.message.Message):
12501250
BUILD_DATA_FIELD_NUMBER: builtins.int
12511251
APP_REVISION_FIELD_NUMBER: builtins.int
12521252
ENGINE_OUTPUTS_FIELD_NUMBER: builtins.int
1253+
BUILD_KEY_FIELD_NUMBER: builtins.int
1254+
BUILD_URL_FIELD_NUMBER: builtins.int
1255+
GN_ARGS_FIELD_NUMBER: builtins.int
12531256
fully_qualified_fuzzer_name: builtins.str
12541257
"""TODO(metzman): Remove this since tworkers should know what this is based on
12551258
the input.
@@ -1272,6 +1275,9 @@ class FuzzTaskOutput(google.protobuf.message.Message):
12721275
app_revision: builtins.int
12731276
@property
12741277
def engine_outputs(self) -> google.protobuf.internal.containers.RepeatedCompositeFieldContainer[global___EngineOutput]: ...
1278+
build_key: builtins.str
1279+
build_url: builtins.str
1280+
gn_args: builtins.str
12751281
def __init__(
12761282
self,
12771283
*,
@@ -1288,14 +1294,21 @@ class FuzzTaskOutput(google.protobuf.message.Message):
12881294
build_data: global___BuildData | None = ...,
12891295
app_revision: builtins.int | None = ...,
12901296
engine_outputs: collections.abc.Iterable[global___EngineOutput] | None = ...,
1297+
build_key: builtins.str | None = ...,
1298+
build_url: builtins.str | None = ...,
1299+
gn_args: builtins.str | None = ...,
12911300
) -> None: ...
1292-
def HasField(self, field_name: typing_extensions.Literal["_app_revision", b"_app_revision", "_build_data", b"_build_data", "_crash_revision", b"_crash_revision", "_fully_qualified_fuzzer_name", b"_fully_qualified_fuzzer_name", "_fuzzer_revision", b"_fuzzer_revision", "_fuzzer_run_results", b"_fuzzer_run_results", "_job_run_timestamp", b"_job_run_timestamp", "_new_targets_count", b"_new_targets_count", "_testcases_executed", b"_testcases_executed", "app_revision", b"app_revision", "build_data", b"build_data", "crash_revision", b"crash_revision", "fully_qualified_fuzzer_name", b"fully_qualified_fuzzer_name", "fuzzer_revision", b"fuzzer_revision", "fuzzer_run_results", b"fuzzer_run_results", "job_run_timestamp", b"job_run_timestamp", "new_targets_count", b"new_targets_count", "testcases_executed", b"testcases_executed"]) -> builtins.bool: ...
1293-
def ClearField(self, field_name: typing_extensions.Literal["_app_revision", b"_app_revision", "_build_data", b"_build_data", "_crash_revision", b"_crash_revision", "_fully_qualified_fuzzer_name", b"_fully_qualified_fuzzer_name", "_fuzzer_revision", b"_fuzzer_revision", "_fuzzer_run_results", b"_fuzzer_run_results", "_job_run_timestamp", b"_job_run_timestamp", "_new_targets_count", b"_new_targets_count", "_testcases_executed", b"_testcases_executed", "app_revision", b"app_revision", "build_data", b"build_data", "crash_groups", b"crash_groups", "crash_revision", b"crash_revision", "engine_outputs", b"engine_outputs", "fully_qualified_fuzzer_name", b"fully_qualified_fuzzer_name", "fuzz_targets", b"fuzz_targets", "fuzzer_revision", b"fuzzer_revision", "fuzzer_run_results", b"fuzzer_run_results", "job_run_timestamp", b"job_run_timestamp", "new_targets_count", b"new_targets_count", "testcase_run_jsons", b"testcase_run_jsons", "testcases_executed", b"testcases_executed"]) -> None: ...
1301+
def HasField(self, field_name: typing_extensions.Literal["_app_revision", b"_app_revision", "_build_data", b"_build_data", "_build_key", b"_build_key", "_build_url", b"_build_url", "_crash_revision", b"_crash_revision", "_fully_qualified_fuzzer_name", b"_fully_qualified_fuzzer_name", "_fuzzer_revision", b"_fuzzer_revision", "_fuzzer_run_results", b"_fuzzer_run_results", "_gn_args", b"_gn_args", "_job_run_timestamp", b"_job_run_timestamp", "_new_targets_count", b"_new_targets_count", "_testcases_executed", b"_testcases_executed", "app_revision", b"app_revision", "build_data", b"build_data", "build_key", b"build_key", "build_url", b"build_url", "crash_revision", b"crash_revision", "fully_qualified_fuzzer_name", b"fully_qualified_fuzzer_name", "fuzzer_revision", b"fuzzer_revision", "fuzzer_run_results", b"fuzzer_run_results", "gn_args", b"gn_args", "job_run_timestamp", b"job_run_timestamp", "new_targets_count", b"new_targets_count", "testcases_executed", b"testcases_executed"]) -> builtins.bool: ...
1302+
def ClearField(self, field_name: typing_extensions.Literal["_app_revision", b"_app_revision", "_build_data", b"_build_data", "_build_key", b"_build_key", "_build_url", b"_build_url", "_crash_revision", b"_crash_revision", "_fully_qualified_fuzzer_name", b"_fully_qualified_fuzzer_name", "_fuzzer_revision", b"_fuzzer_revision", "_fuzzer_run_results", b"_fuzzer_run_results", "_gn_args", b"_gn_args", "_job_run_timestamp", b"_job_run_timestamp", "_new_targets_count", b"_new_targets_count", "_testcases_executed", b"_testcases_executed", "app_revision", b"app_revision", "build_data", b"build_data", "build_key", b"build_key", "build_url", b"build_url", "crash_groups", b"crash_groups", "crash_revision", b"crash_revision", "engine_outputs", b"engine_outputs", "fully_qualified_fuzzer_name", b"fully_qualified_fuzzer_name", "fuzz_targets", b"fuzz_targets", "fuzzer_revision", b"fuzzer_revision", "fuzzer_run_results", b"fuzzer_run_results", "gn_args", b"gn_args", "job_run_timestamp", b"job_run_timestamp", "new_targets_count", b"new_targets_count", "testcase_run_jsons", b"testcase_run_jsons", "testcases_executed", b"testcases_executed"]) -> None: ...
12941303
@typing.overload
12951304
def WhichOneof(self, oneof_group: typing_extensions.Literal["_app_revision", b"_app_revision"]) -> typing_extensions.Literal["app_revision"] | None: ...
12961305
@typing.overload
12971306
def WhichOneof(self, oneof_group: typing_extensions.Literal["_build_data", b"_build_data"]) -> typing_extensions.Literal["build_data"] | None: ...
12981307
@typing.overload
1308+
def WhichOneof(self, oneof_group: typing_extensions.Literal["_build_key", b"_build_key"]) -> typing_extensions.Literal["build_key"] | None: ...
1309+
@typing.overload
1310+
def WhichOneof(self, oneof_group: typing_extensions.Literal["_build_url", b"_build_url"]) -> typing_extensions.Literal["build_url"] | None: ...
1311+
@typing.overload
12991312
def WhichOneof(self, oneof_group: typing_extensions.Literal["_crash_revision", b"_crash_revision"]) -> typing_extensions.Literal["crash_revision"] | None: ...
13001313
@typing.overload
13011314
def WhichOneof(self, oneof_group: typing_extensions.Literal["_fully_qualified_fuzzer_name", b"_fully_qualified_fuzzer_name"]) -> typing_extensions.Literal["fully_qualified_fuzzer_name"] | None: ...
@@ -1304,6 +1317,8 @@ class FuzzTaskOutput(google.protobuf.message.Message):
13041317
@typing.overload
13051318
def WhichOneof(self, oneof_group: typing_extensions.Literal["_fuzzer_run_results", b"_fuzzer_run_results"]) -> typing_extensions.Literal["fuzzer_run_results"] | None: ...
13061319
@typing.overload
1320+
def WhichOneof(self, oneof_group: typing_extensions.Literal["_gn_args", b"_gn_args"]) -> typing_extensions.Literal["gn_args"] | None: ...
1321+
@typing.overload
13071322
def WhichOneof(self, oneof_group: typing_extensions.Literal["_job_run_timestamp", b"_job_run_timestamp"]) -> typing_extensions.Literal["job_run_timestamp"] | None: ...
13081323
@typing.overload
13091324
def WhichOneof(self, oneof_group: typing_extensions.Literal["_new_targets_count", b"_new_targets_count"]) -> typing_extensions.Literal["new_targets_count"] | None: ...

src/clusterfuzz/_internal/tests/core/datastore/data_handler_test.py

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,11 @@ def setUp(self):
4040
test_utils.set_up_pyfakefs(self)
4141
helpers.patch_environ(self)
4242

43-
def test_set(self):
44-
"""Test set everything."""
43+
def test_testcase_metadata_from_env(self):
44+
"""Tests the initial testcase metadata set from the env vars."""
4545
os.environ['FAIL_RETRIES'] = '3'
4646
os.environ['FAIL_WAIT'] = '3'
4747
os.environ['BUILD_KEY'] = 'build_key_value'
48-
os.environ['BUILD_KEY'] = 'build_key_value'
4948
os.environ['BUILD_URL'] = 'build_url_value'
5049
os.environ['APP_DIR'] = 'app_dir_value'
5150
os.environ['GN_ARGS_PATH'] = 'app_dir_value/args.gn'
@@ -66,6 +65,31 @@ def test_set(self):
6665
'is_asan = true\nuse_goma = true\nv8_enable_verify_heap = true',
6766
metadata['gn_args'])
6867

68+
def test_testcase_metadata_from_args(self):
69+
"""Tests that testcase build metadata is set from args."""
70+
build_key = 'build_key_test'
71+
build_url = 'build_url_test'
72+
os.environ['GN_ARGS_PATH'] = 'app_dir_value/args.gn'
73+
self.fs.create_file(
74+
'app_dir_value/args.gn',
75+
contents=('is_asan = true\n'
76+
'goma_dir = /home/user/goma\n'
77+
'use_goma = false\n'
78+
'v8_enable_verify_heap = true'))
79+
gn_args = data_handler.get_filtered_gn_args()
80+
del os.environ['GN_ARGS_PATH']
81+
82+
testcase = data_types.Testcase()
83+
data_handler.set_build_metadata_to_testcase(
84+
testcase, build_key=build_key, build_url=build_url, gn_args=gn_args)
85+
metadata = json.loads(testcase.additional_metadata)
86+
87+
self.assertEqual('build_key_test', metadata['build_key'])
88+
self.assertEqual('build_url_test', metadata['build_url'])
89+
self.assertEqual(
90+
'is_asan = true\nuse_goma = false\nv8_enable_verify_heap = true',
91+
metadata['gn_args'])
92+
6993

7094
@test_utils.with_cloud_emulators('datastore')
7195
class DataHandlerTest(unittest.TestCase):

0 commit comments

Comments
 (0)