Skip to content

Commit

Permalink
Handle issue metadata properly (google#4174)
Browse files Browse the repository at this point in the history
Since it's a heterogenous dictionary, we must store it as a string
representation of a json dict (as unwieldy as this is, it's the only
correct way to do this).

Fixes:
https://pantheon.corp.google.com/errors/detail/CODtycKwtN-pGA;filter=%5B%22Type%22%5D;time=P30D;locations=global?e=-13802955&invt=AbZgCw&mods=logs_tg_prod&project=google.com:cluster-fuzz

```
Bar chart

Sample stack trace
Parsed
Raw
TypeError: bad argument type for built-in operation

at .update ( <frozen _collections_abc>:949 )
at .find_fixed_range ( /mnt/scratch0/clusterfuzz/src/clusterfuzz/_internal/bot/tasks/utasks/progression_task.py:567 )
at .utask_main ( /mnt/scratch0/clusterfuzz/src/clusterfuzz/_internal/bot/tasks/utasks/progression_task.py:671 )
at .uworker_main_no_io ( /mnt/scratch0/clusterfuzz/src/clusterfuzz/_internal/bot/tasks/utasks/__init__.py:211 )
at .execute_locally ( /mnt/scratch0/clusterfuzz/src/clusterfuzz/_internal/bot/tasks/task_types.py:63 )
at .execute ( /mnt/scratch0/clusterfuzz/src/clusterfuzz/_internal/bot/tasks/task_types.py:127 )
at .run_command ( /mnt/scratch0/clusterfuzz/src/clusterfuzz/_internal/bot/tasks/commands.py:218 )
at .process_command_impl ( /mnt/scratch0/clusterfuzz/src/clusterfuzz/_internal/bot/tasks/commands.py:429 )
at .wrapper ( /mnt/scratch0/clusterfuzz/src/clusterfuzz/_internal/bot/tasks/commands.py:159 )
at .process_command ( /mnt/scratch0/clusterfuzz/src/clusterfuzz/_internal/bot/tasks/commands.py:248 )
at .task_loop ( /mnt/scratch0/clusterfuzz/src/python/bot/startup/run_bot.py:146 )
```
  • Loading branch information
jonathanmetzman authored Aug 16, 2024
1 parent 5f093bf commit d6ce764
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 32 deletions.
9 changes: 6 additions & 3 deletions src/clusterfuzz/_internal/bot/tasks/utasks/analyze_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
"""Analyze task for handling user uploads."""

import datetime
import json
from typing import Dict
from typing import Optional

from clusterfuzz._internal.base import tasks
Expand All @@ -36,7 +38,8 @@
from clusterfuzz._internal.system import environment


def _add_default_issue_metadata(testcase, fuzz_target_metadata):
def _add_default_issue_metadata(testcase: data_types.Testcase,
fuzz_target_metadata: Dict):
"""Adds the default issue metadata (e.g. components, labels) to testcase."""
testcase_metadata = testcase.get_metadata()
for key, default_value in fuzz_target_metadata.items():
Expand Down Expand Up @@ -420,7 +423,7 @@ def utask_main(uworker_input):
analyze_task_output=analyze_task_output,
test_timeout=test_timeout,
crash_time=crash_time,
issue_metadata=fuzz_target_metadata)
issue_metadata=json.dumps(fuzz_target_metadata))


def test_for_reproducibility(fuzz_target, testcase, testcase_file_path, state,
Expand Down Expand Up @@ -565,7 +568,7 @@ def utask_postprocess(output):
testcase_upload_metadata.security_flag = testcase.security_flag
testcase_upload_metadata.put()

_add_default_issue_metadata(testcase, output.issue_metadata)
_add_default_issue_metadata(testcase, json.loads(output.issue_metadata))
logs.info('Creating post-analyze tasks.')

# Create tasks to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import collections
import datetime
import json
import os
import random
import shutil
Expand Down Expand Up @@ -842,7 +843,7 @@ def _process_corpus_crashes(output: uworker_msg_pb2.Output): # pylint: disable=
corpus_pruning_output.fuzzer_binary_name)

if output.issue_metadata:
for key, value in output.issue_metadata.items():
for key, value in json.loads(output.issue_metadata).items():
testcase.set_metadata(key, value, update_testcase=False)

testcase.put()
Expand Down Expand Up @@ -1032,7 +1033,7 @@ def utask_main(uworker_input):
crash_revision=result.revision,
crashes=_extract_corpus_crashes(result),
corpus_backup_uploaded=bool(result.coverage_info.corpus_location)),
issue_metadata=issue_metadata)
issue_metadata=json.dumps(issue_metadata))
_fill_cross_pollination_stats(result.cross_pollination_stats,
uworker_output)
except Exception as e:
Expand Down
20 changes: 11 additions & 9 deletions src/clusterfuzz/_internal/bot/tasks/utasks/progression_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
# limitations under the License.
"""Test to see if test cases are fixed."""

import json
import time
from typing import Dict
from typing import List

from clusterfuzz._internal.base import bisection
Expand Down Expand Up @@ -297,15 +299,15 @@ def _check_fixed_for_custom_binary(testcase: data_types.Testcase,
return uworker_msg_pb2.Output(progression_task_output=progression_task_output) # pylint: disable=no-member


def _update_issue_metadata(testcase, metadata):
def _update_issue_metadata(testcase: data_types.Testcase, metadata: Dict):
if not metadata:
return

for key, value in metadata.items():
old_value = testcase.get_metadata(key)
if old_value != value:
logs.info('Updating issue metadata for {} from {} to {}.'.format(
key, old_value, value))
logs.info(
f'Updating issue metadata for {key} from {old_value} to {value}.')
testcase.set_metadata(key, value)


Expand Down Expand Up @@ -570,7 +572,7 @@ def find_fixed_range(uworker_input):
last_tested_crash_stacktrace)
return uworker_msg_pb2.Output(
progression_task_output=progression_task_output,
issue_metadata=issue_metadata)
issue_metadata=json.dumps(issue_metadata))

# Verify that we do crash in the min revision. This is assumed to be true
# while we are doing the bisect.
Expand All @@ -585,7 +587,7 @@ def find_fixed_range(uworker_input):
progression_task_output.crash_revision = int(min_revision)
return uworker_msg_pb2.Output( # pylint: disable=no-member
progression_task_output=progression_task_output,
issue_metadata=issue_metadata,
issue_metadata=json.dumps(issue_metadata),
error_message=error_message,
error_type=uworker_msg_pb2.ErrorType.PROGRESSION_NO_CRASH) # pylint: disable=no-member

Expand All @@ -607,7 +609,7 @@ def find_fixed_range(uworker_input):
progression_task_output.max_revision = int(max_revision)
return uworker_msg_pb2.Output( # pylint: disable=no-member
progression_task_output=progression_task_output,
issue_metadata=issue_metadata)
issue_metadata=json.dumps(issue_metadata))

# Occasionally, we get into this bad state. It seems to be related to test
# cases with flaky stacks, but the exact cause is unknown.
Expand All @@ -622,7 +624,7 @@ def find_fixed_range(uworker_input):
progression_task_output.last_progression_max = int(last_progression_max)
return uworker_msg_pb2.Output( # pylint: disable=no-member
progression_task_output=progression_task_output,
issue_metadata=issue_metadata,
issue_metadata=json.dumps(issue_metadata),
error_type=uworker_msg_pb2.ErrorType.PROGRESSION_BAD_STATE_MIN_MAX) # pylint: disable=no-member

# Test the middle revision of our range.
Expand Down Expand Up @@ -662,7 +664,7 @@ def find_fixed_range(uworker_input):
progression_task_output.last_progression_max = last_progression_max
return uworker_msg_pb2.Output( # pylint: disable=no-member
error_message=error_message,
issue_metadata=issue_metadata,
issue_metadata=json.dumps(issue_metadata),
progression_task_output=progression_task_output,
error_type=uworker_msg_pb2.ErrorType.PROGRESSION_TIMEOUT) # pylint: disable=no-member

Expand Down Expand Up @@ -705,7 +707,7 @@ def utask_postprocess(output: uworker_msg_pb2.Output): # pylint: disable=no-mem
task_output = None

if output.issue_metadata:
_update_issue_metadata(testcase, output.issue_metadata)
_update_issue_metadata(testcase, json.loads(output.issue_metadata))

if output.HasField('progression_task_output'):
task_output = output.progression_task_output
Expand Down
2 changes: 1 addition & 1 deletion src/clusterfuzz/_internal/protos/uworker_msg.proto
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,6 @@ message Output {
optional SymbolizeTaskOutput symbolize_task_output = 13;
optional VariantTaskOutput variant_task_output = 14;
optional CorpusPruningTaskOutput corpus_pruning_task_output = 16;
map<string,string> issue_metadata = 17;
optional string issue_metadata = 20;
optional string error_message = 15;
}
12 changes: 4 additions & 8 deletions src/clusterfuzz/_internal/protos/uworker_msg_pb2.py

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"""Tests for uworker_io."""

import datetime
import json
import os
import tempfile
import unittest
Expand Down Expand Up @@ -347,15 +348,6 @@ def test_list_update(self):
deserialized = uworker_io.deserialize_uworker_input(wire_format)
self.assertEqual(deserialized.analyze_task_input.bad_revisions, [0, 1])

def test_map_update(self):
"""Tests that updating a map works."""
output = uworker_msg_pb2.Output(issue_metadata={'a': 'b', 'c': 'd'})
output.issue_metadata.clear()
output.issue_metadata.update({'e': 'f'})
wire_format = uworker_io.serialize_uworker_output(output)
deserialized = uworker_io.deserialize_uworker_output(wire_format)
self.assertEqual(deserialized.issue_metadata, {'e': 'f'})

def test_submessage_references(self):
"""Tests that updating a submessage works both when directly reading from
uworker_input and from reading from it once it has been serialized and
Expand All @@ -381,3 +373,17 @@ def test_unset_a_message_field(self):
wire_format = uworker_io.serialize_uworker_input(uworker_input)
deserialized = uworker_io.deserialize_uworker_input(wire_format)
self.assertFalse(deserialized.HasField('analyze_task_input'))

def test_issue_metadata_field(self):
"""Tests issue_metadata a serialized json string."""
metadata = {
'sam': 1,
'i': 2.1,
'am': {
'rhyme': [89]
},
}
output = uworker_msg_pb2.Output(issue_metadata=json.dumps(metadata))
wire_format = uworker_io.serialize_uworker_input(output)
deserialized = uworker_io.deserialize_uworker_output(wire_format)
self.assertEqual(json.loads(deserialized.issue_metadata), metadata)

0 comments on commit d6ce764

Please sign in to comment.