diff --git a/src/clusterfuzz/_internal/platforms/android/adb.py b/src/clusterfuzz/_internal/platforms/android/adb.py index 47c59b31c72..0776ecfeb26 100755 --- a/src/clusterfuzz/_internal/platforms/android/adb.py +++ b/src/clusterfuzz/_internal/platforms/android/adb.py @@ -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. + """ # 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') @@ -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() diff --git a/src/clusterfuzz/_internal/platforms/android/device.py b/src/clusterfuzz/_internal/platforms/android/device.py index 0c8d81ca686..6cf783e5a57 100755 --- a/src/clusterfuzz/_internal/platforms/android/device.py +++ b/src/clusterfuzz/_internal/platforms/android/device.py @@ -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() diff --git a/src/clusterfuzz/_internal/platforms/android/sanitizer.py b/src/clusterfuzz/_internal/platforms/android/sanitizer.py index 832f6a4a577..c602df87221 100644 --- a/src/clusterfuzz/_internal/platforms/android/sanitizer.py +++ b/src/clusterfuzz/_internal/platforms/android/sanitizer.py @@ -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. + 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() @@ -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.', @@ -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 diff --git a/src/clusterfuzz/_internal/tests/core/platforms/android/device_test.py b/src/clusterfuzz/_internal/tests/core/platforms/android/device_test.py index e10051fb7ef..ebb6ba11a26 100644 --- a/src/clusterfuzz/_internal/tests/core/platforms/android/device_test.py +++ b/src/clusterfuzz/_internal/tests/core/platforms/android/device_test.py @@ -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 + 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()