Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions src/clusterfuzz/_internal/platforms/android/adb.py
Original file line number Diff line number Diff line change
Expand Up @@ -880,8 +880,15 @@ def write_command_line_file(command_line, app_path):
write_data_to_file(command_line_file_contents, command_line_path)


def write_data_to_file(contents, file_path):
"""Writes content to file."""
def write_data_to_file(contents, file_path, should_reboot=True):
"""Writes content to file.

Args:
contents: The string content to write.
file_path: The path to the file on the Android device.
should_reboot: Whether to reboot the device if a system file is modified.
Defaults to True.
Comment on lines +889 to +890
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's explain what a system file is, and no need to repeat the default argument value since that's self-documenting:

Suggested change
should_reboot: Whether to reboot the device if a system file is modified.
Defaults to True.
should_reboot: Whether to reboot the device after writing the file.
Only applies if `file_path` is under `/system`, since we need to
remount that partition as read-write first.

"""
# If this is a file in /system, we need to remount /system as read-write and
# after file is written, revert it back to read-only.
is_system_file = file_path.startswith('/system')
Expand All @@ -895,6 +902,6 @@ def write_data_to_file(contents, file_path):
# Make command line file is readable.
run_shell_command('chmod 0644 %s' % file_path, root=True)

if is_system_file:
if is_system_file and should_reboot:
reboot()
wait_until_fully_booted()
5 changes: 3 additions & 2 deletions src/clusterfuzz/_internal/platforms/android/device.py
Original file line number Diff line number Diff line change
Expand Up @@ -311,11 +311,12 @@ def initialize_device():
add_test_accounts_if_needed()

# Setup AddressSanitizer if needed.
sanitizer.setup_asan_if_needed()
asan_reboot_done = sanitizer.setup_asan_if_needed()

# Reboot device as above steps would need it and also it brings device in a
# good state.
reboot()
if not asan_reboot_done:
reboot()

# Make sure we are running as root after restart.
adb.run_as_root()
Expand Down
22 changes: 15 additions & 7 deletions src/clusterfuzz/_internal/platforms/android/sanitizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,25 +69,31 @@ def set_options(sanitizer_tool_name, sanitizer_options):
if not sanitizer_options_file_path:
return

adb.write_data_to_file(sanitizer_options, sanitizer_options_file_path)
# Skip reboot as the app will pick up the options file on restart.
Comment thread
jardondiego marked this conversation as resolved.
adb.write_data_to_file(
sanitizer_options, sanitizer_options_file_path, should_reboot=False)


def setup_asan_if_needed():
"""Set up asan on device."""
def setup_asan_if_needed() -> bool:
"""Set up asan on device.

Returns:
True if the device was rebooted or restarted during setup, False otherwise.
"""
if not environment.get_value('ASAN_DEVICE_SETUP'):
# Only do this step if explicitly enabled in the job type. This cannot be
# determined from libraries in application directory since they can go
# missing in a bad build, so we want to catch that.
return
return False

if settings.get_sanitizer_tool_name():
# If this is a sanitizer build, no need to setup ASAN (incompatible).
return
return False

app_directory = environment.get_value('APP_DIR')
if not app_directory:
# No app directory -> No ASAN runtime library. No work to do, bail out.
return
return False

# Initialize variables.
android_directory = environment.get_platform_resources_directory()
Expand All @@ -108,7 +114,7 @@ def setup_asan_if_needed():
result = process.run_and_wait()
if result.return_code:
logs.error('Failed to setup ASan on device.', output=result.output)
return
return False

logs.info(
'ASan device setup script successfully finished, waiting for boot.',
Expand All @@ -117,3 +123,5 @@ def setup_asan_if_needed():
# Wait until fully booted as otherwise shell restart followed by a quick
# reboot can trigger data corruption in /data/data.
adb.wait_until_fully_booted()

return True
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,55 @@
# limitations under the License.
"""Tests for device functions."""

import unittest

from clusterfuzz._internal.platforms.android import device
from clusterfuzz._internal.tests.test_libs import android_helpers


class InitializeEnvironmentTest(android_helpers.AndroidTest):
"""Tests for """
"""Tests for the device environment initialization process (`initialize_environment`)."""

def test(self):
"""Ensure that initialize_environment throws no exceptions."""
device.initialize_environment()


class InitializeDeviceRebootLogicTest(unittest.TestCase):
"""Tests the reboot batching logic in initialize_device."""

def setUp(self):
from clusterfuzz._internal.tests.test_libs import helpers
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move this to the top level, no need to make this import conditional on the test being run?

helpers.patch(self, [
'clusterfuzz._internal.system.environment.is_engine_fuzzer_job',
'clusterfuzz._internal.platforms.android.adb.setup_adb',
'clusterfuzz._internal.platforms.android.adb.run_as_root',
'clusterfuzz._internal.platforms.android.device.configure_system_build_properties',
'clusterfuzz._internal.platforms.android.device.configure_device_settings',
'clusterfuzz._internal.platforms.android.device.add_test_accounts_if_needed',
'clusterfuzz._internal.platforms.android.sanitizer.setup_asan_if_needed',
'clusterfuzz._internal.platforms.android.device.reboot',
'clusterfuzz._internal.platforms.android.wifi.configure',
'clusterfuzz._internal.platforms.android.device.setup_host_and_device_forwarder_if_needed',
'clusterfuzz._internal.platforms.android.settings.change_se_linux_to_permissive_mode',
'clusterfuzz._internal.platforms.android.app.wait_until_optimization_complete',
'clusterfuzz._internal.platforms.android.ui.clear_notifications',
'clusterfuzz._internal.platforms.android.ui.unlock_screen',
])
self.mock.is_engine_fuzzer_job.return_value = False

def test_reboot_if_asan_did_not_run(self):
"""Test that `initialize_device()` calls `reboot()` if the ASan setup
script did not."""
self.mock.setup_asan_if_needed.return_value = False

device.initialize_device()
self.mock.reboot.assert_called_once()

def test_no_reboot_if_asan_ran(self):
"""Test that `initialize_device()` skips calling `reboot()` if the ASan
setup script did."""
self.mock.setup_asan_if_needed.return_value = True

device.initialize_device()
self.mock.reboot.assert_not_called()
Loading