Skip RPATH patching on Schema v1 builds

Author: notvictorl

src/clusterfuzz/_internal/build_management/build_archive.py

@@ -66,6 +66,11 @@ def __init__(self, reader: archive.ArchiveReader):
     self._reader = reader
     self._fuzz_targets = None
 
+  @abc.abstractmethod
+  def archive_schema_version(self) -> int:
+    """Returns the schema version number for this archive."""
+    raise NotImplementedError
+
   def list_members(self) -> List[archive.ArchiveMemberInfo]:
     return self._reader.list_members()
 
@@ -162,6 +167,14 @@ class DefaultBuildArchive(BuildArchive):
   """Default class for handling builds. This should work with everything.
   """
 
+  def __init__(self, reader: archive.ArchiveReader):
+    super().__init__(reader)
+    self._fuzz_targets = {}
+
+  @override
+  def archive_schema_version(self) -> int:
+    return 0
+
   def get_path_for_target(self, fuzz_target: str) -> str:
     """Returns the path in the archive of the fuzz_target if found.
     This is needed because target name normalization means we're losing
@@ -344,8 +357,8 @@ def root_dir(self) -> str:
       self._root_dir = super().root_dir()  # pylint: disable=attribute-defined-outside-init
     return self._root_dir
 
+  @override
   def archive_schema_version(self) -> int:
-    """Returns the schema version number for this archive."""
     return self._archive_schema_version
 
   @override

src/clusterfuzz/_internal/build_management/build_manager.py

@@ -51,6 +51,11 @@
 # File name for storing current build revision.
 REVISION_FILE_NAME = 'REVISION'
 
+# File name for storing the archive schema version retrieved during unpacking.
+# The version is derived from clusterfuzz_manifest.json if it exists in the
+# archive, otherwise it defaults to 0.
+SCHEMA_VERSION_FILE_NAME = '.schema_version'
+
 # Various build type mapping strings.
 BUILD_TYPE_SUBSTRINGS = [
     '-beta', '-stable', '-debug', '-release', '-symbolized', '-extended_stable'
@@ -350,6 +355,7 @@ def __init__(self,
     # Every fetched build is a release one, except when SymbolizedBuild
     # explicitly downloads a debug build
     self._build_type = 'release'
+    self._schema_version = None
 
   def _reset_cwd(self):
     """Reset current working directory. Needed to clean up build
@@ -378,6 +384,48 @@ def _pre_setup(self):
     environment.set_value(self.env_prefix + 'APP_PATH', '')
     environment.set_value(self.env_prefix + 'APP_PATH_DEBUG', '')
 
+  def _schema_version_path(self):
+    """Returns the path to the schema version metadata file in `build_dir`."""
+    return os.path.join(self.build_dir, SCHEMA_VERSION_FILE_NAME)
+
+  def _read_schema_version_from_disk(self) -> int | None:
+    """Reads the schema version from `SCHEMA_VERSION_FILE_NAME` in `build_dir`.
+
+    Returns None if the file is missing or cannot be read, so callers can
+    distinguish "no version recorded yet" from a recorded version of 0.
+    """
+    schema_version_path = self._schema_version_path()
+    if not os.path.exists(schema_version_path):
+      return None
+    try:
+      return int(utils.read_data_from_file(schema_version_path) or 0)
+    except Exception as e:
+      logs.warning(
+          f'Failed to read schema version from {schema_version_path}: {e}')
+      return None
+
+  def _write_schema_version(self, schema_version):
+    """Writes schema version to `SCHEMA_VERSION_FILE_NAME` in `build_dir`."""
+    schema_version_path = self._schema_version_path()
+    try:
+      utils.write_data_to_file(str(schema_version), schema_version_path)
+    except Exception as e:
+      logs.warning(
+          f'Failed to write schema version to {schema_version_path}: {e}')
+
+  def _get_schema_version(self):
+    """Gets the schema version of the build, preferring the in-memory value and
+    falling back to `SCHEMA_VERSION_FILE_NAME` in `build_dir`. Defaults to 0
+    when no version has been recorded.
+
+    Note: For `SymbolizedBuild`, this naively looks for `.schema_version` at
+    `build_dir` and assumes both release and debug builds share the same schema
+    version. If they are different, the release build's schema version is used.
+    """
+    if self._schema_version is None:
+      self._schema_version = self._read_schema_version_from_disk()
+    return self._schema_version or 0
+
   def _patch_rpath(self, binary_path, instrumented_library_paths):
     """Patch rpaths of a binary to point to instrumented libraries"""
     rpaths = get_rpaths(binary_path)
@@ -419,8 +467,13 @@ def _post_setup_success(self, update_revision=True):
 
     # Update rpaths if necessary (for e.g. instrumented libraries).
     instrumented_library_paths = environment.get_instrumented_libraries_paths()
-    if instrumented_library_paths:
-      self._patch_rpaths(instrumented_library_paths)
+    if not instrumented_library_paths:
+      return
+    if self._get_schema_version() > 0:
+      logs.info('Skipping RPATH patching for schema v1+ build.')
+      return
+
+    self._patch_rpaths(instrumented_library_paths)
 
   @contextlib.contextmanager
   def _download_and_open_build_archive(self, base_build_dir: str,
@@ -563,6 +616,25 @@ def _unpack_build(self,
             fuzz_target=fuzz_target_to_unpack,
             trusted=trusted)
 
+        schema_version = build.archive_schema_version()
+
+        # Sync in-memory schema version with disk if we haven't read it yet.
+        # This helps detect mismatches in shared build dirs (e.g.,
+        # SymbolizedBuild).
+        if self._schema_version is None:
+          self._schema_version = self._read_schema_version_from_disk()
+
+        if self._schema_version is None:
+          # First time unpacking: record the schema version.
+          self._write_schema_version(schema_version)
+          self._schema_version = schema_version
+        elif self._schema_version != schema_version:
+          # Following unpack: flag if the new archive version doesn't match.
+          logs.error(
+              f'Schema version mismatch for build in {self.build_dir}. '
+              f'Existing version is {self._schema_version}, but new archive '
+              f'{build_url} has version {schema_version}.')
+
         _emit_job_build_retrieval_metric(unpack_start_time, 'unpack',
                                          self._build_type)
 

src/clusterfuzz/_internal/tests/core/build_management/build_manager_test.py

@@ -1580,6 +1580,33 @@ def test_patch_rpaths_existing_msan(self):
     rpaths = build_manager.get_rpaths(os.environ['APP_PATH'])
     self.assertListEqual(['/msan/lib', '/msan/usr/lib'], rpaths)
 
+  def test_patch_rpaths_schema_v1(self):
+    """Tests that RPATH is not patched when schema version is > 0."""
+
+    def mock_unpack_build_schema_v1(actual_self,
+                                    base_build_dir,
+                                    build_dir,
+                                    url,
+                                    http_build_url=None):
+      self.mock_unpack_build('rpath_new', actual_self, base_build_dir,
+                             build_dir, url, http_build_url)
+      schema_version_path = os.path.join(build_dir,
+                                         build_manager.SCHEMA_VERSION_FILE_NAME)
+      utils.write_data_to_file('1', schema_version_path)
+      return True
+
+    self.mock._unpack_build.side_effect = mock_unpack_build_schema_v1
+
+    build = build_manager.RegularBuild(self.base_build_dir, 1337,
+                                       'chained-origins')
+    self.assertTrue(build.setup())
+    self.assertEqual(
+        os.path.join(self.base_build_dir, 'revisions', 'app'),
+        os.environ['APP_PATH'])
+
+    rpaths = build_manager.get_rpaths(os.environ['APP_PATH'])
+    self.assertListEqual([], rpaths)
+
 
 class SortBuildUrlsByRevisionTest(unittest.TestCase):
   """Test _sort_build_urls_by_revision."""
@@ -1969,3 +1996,184 @@ def test_is_discovery_non_fuzzing(self):
     self.mock.is_engine_fuzzer_job.return_value = False
     build = build_manager.Build('/base', 1, build_prefix='', fuzz_target=None)
     self.assertFalse(build.is_discovery)
+
+
+class GetSchemaVersionTest(fake_filesystem_unittest.TestCase):
+  """Tests _get_schema_version resolution."""
+
+  def setUp(self):
+    fake_filesystem_unittest.TestCase.setUp(self)
+    self.setUpPyfakefs()
+    self.base_build_dir = '/base_build_dir'
+    self.fs.create_dir(self.base_build_dir)
+
+  def test_regular_build(self):
+    """Tests reading schema version for regular build."""
+    build = build_manager.RegularBuild(self.base_build_dir, 1337,
+                                       'gs://dummy/url.zip')
+    self.fs.create_file(
+        os.path.join(build.build_dir, '.schema_version'), contents='1')
+    self.assertEqual(build._get_schema_version(), 1)
+
+  def test_symbolized_build(self):
+    """Tests reading schema version for symbolized build."""
+    build = build_manager.SymbolizedBuild(self.base_build_dir, 1337,
+                                          'gs://dummy/release.zip',
+                                          'gs://dummy/debug.zip')
+    self.fs.create_file(
+        os.path.join(build.build_dir, '.schema_version'), contents='2')
+    self.assertEqual(build._get_schema_version(), 2)
+
+  def test_missing_file(self):
+    """Tests that 0 is returned if file is missing."""
+    build = build_manager.RegularBuild(self.base_build_dir, 1337,
+                                       'gs://dummy/url.zip')
+    self.assertEqual(build._get_schema_version(), 0)
+
+  def test_get_schema_version_caching(self):
+    """Tests that _get_schema_version caches results."""
+    build = build_manager.RegularBuild(self.base_build_dir, 1337,
+                                       'gs://dummy/url.zip')
+    self.fs.create_file(
+        os.path.join(build.build_dir, '.schema_version'), contents='1')
+
+    with mock.patch(
+        'clusterfuzz._internal.base.utils.read_data_from_file',
+        return_value='1') as mock_read_call:
+      # First call should trigger read.
+      v1 = build._get_schema_version()
+
+      self.assertEqual(v1, 1)
+
+      # Second call should use cache.
+      v2 = build._get_schema_version()
+
+      self.assertEqual(v2, 1)
+      self.assertEqual(mock_read_call.call_count, 1)
+
+      # Clear call history
+      mock_read_call.reset_mock()
+
+      # Call again, should still be cached.
+      v3 = build._get_schema_version()
+
+      self.assertEqual(v3, 1)
+      self.assertEqual(mock_read_call.call_count, 0)
+
+
+class UnpackBuildSchemaVersionTest(fake_filesystem_unittest.TestCase):
+  """Tests schema version handling during _unpack_build."""
+
+  def setUp(self):
+    test_utils.set_up_pyfakefs(self)
+    test_helpers.patch(self, [
+        'clusterfuzz._internal.build_management.build_manager._make_space',
+        'clusterfuzz._internal.system.shell.clear_temp_directory',
+        'clusterfuzz._internal.build_management.build_manager.Build._open_build_archive',
+    ])
+    self.mock._make_space.return_value = True
+
+    test_helpers.patch_environ(self)
+    os.environ['BUILDS_DIR'] = '/builds'
+    os.environ['FAIL_RETRIES'] = '1'
+    os.environ['APP_NAME'] = FAKE_APP_NAME
+    os.environ['JOB_NAME'] = 'job'
+
+    self.base_build_dir = '/base_build_dir'
+    self.fs.create_dir(self.base_build_dir)
+
+    self.build = build_manager.RegularBuild(self.base_build_dir, 1337,
+                                            'gs://dummy/url.zip')
+    # build_dir for RegularBuild is base_build_dir/revisions
+    self.fs.create_dir(self.build.build_dir)
+
+  def mock_archive(self, schema_version):
+    archive_mock = mock.MagicMock()
+    archive_mock.unpacked_size.return_value = 100
+    archive_mock.archive_schema_version.return_value = schema_version
+    archive_mock.list_fuzz_targets.return_value = []
+    return archive_mock
+
+  def test_first_unpack_writes_version(self):
+    """Tests that the first unpack writes the schema version."""
+    archive = self.mock_archive(1)
+    self.mock._open_build_archive.return_value.__enter__.return_value = archive
+    schema_file = os.path.join(self.build.build_dir, '.schema_version')
+
+    result = self.build._unpack_build(self.base_build_dir, self.build.build_dir,
+                                      'gs://dummy/url.zip')
+
+    self.assertTrue(result)
+    self.assertTrue(os.path.exists(schema_file))
+    self.assertEqual(utils.read_data_from_file(schema_file), 1)
+    self.assertEqual(self.build._schema_version, 1)
+
+  def test_second_unpack_same_version(self):
+    """Tests that subsequent unpack with same version does not overwrite and doesn't log error."""
+    schema_file = os.path.join(self.build.build_dir, '.schema_version')
+    self.fs.create_file(schema_file, contents='1')
+    self.build._schema_version = 1
+
+    archive = self.mock_archive(1)
+    self.mock._open_build_archive.return_value.__enter__.return_value = archive
+
+    with mock.patch.object(self.build, '_write_schema_version') as mock_write:
+      with mock.patch(
+          'clusterfuzz._internal.metrics.logs.error') as mock_log_error:
+        result = self.build._unpack_build(
+            self.base_build_dir, self.build.build_dir, 'gs://dummy/url.zip')
+
+        self.assertTrue(result)
+        mock_write.assert_not_called()
+        mock_log_error.assert_not_called()
+
+  def test_second_unpack_different_version_logs_error(self):
+    """Tests that subsequent unpack with different version logs error and does not overwrite."""
+    schema_file = os.path.join(self.build.build_dir, '.schema_version')
+    self.fs.create_file(schema_file, contents='1')
+    self.build._schema_version = 1
+
+    archive = self.mock_archive(2)  # Second archive has version 2
+    self.mock._open_build_archive.return_value.__enter__.return_value = archive
+
+    with mock.patch.object(self.build, '_write_schema_version') as mock_write:
+      with mock.patch(
+          'clusterfuzz._internal.metrics.logs.error') as mock_log_error:
+        result = self.build._unpack_build(
+            self.base_build_dir, self.build.build_dir, 'gs://dummy/url.zip')
+
+        self.assertTrue(result)
+        mock_write.assert_not_called()
+        mock_log_error.assert_called_once()
+        error_msg = mock_log_error.call_args[0][0]
+        self.assertIn('Schema version mismatch', error_msg)
+        self.assertIn('1', error_msg)  # Existing
+        self.assertIn('2', error_msg)  # New
+
+  def test_unpack_reads_from_disk_if_not_in_memory(self):
+    """Tests that unpack reads from disk if _schema_version is not in memory."""
+    build = build_manager.SymbolizedBuild(self.base_build_dir, 1337,
+                                          'gs://dummy/release.zip',
+                                          'gs://dummy/debug.zip')
+    self.fs.create_dir(build.build_dir)
+    self.fs.create_dir(build.release_build_dir)
+    self.fs.create_dir(build.debug_build_dir)
+
+    schema_file = os.path.join(build.build_dir, '.schema_version')
+    self.fs.create_file(schema_file, contents='1')
+    build._schema_version = None  # Not in memory
+
+    archive = self.mock_archive(1)
+    self.mock._open_build_archive.return_value.__enter__.return_value = archive
+
+    with mock.patch.object(build, '_write_schema_version') as mock_write:
+      with mock.patch(
+          'clusterfuzz._internal.metrics.logs.error') as mock_log_error:
+        # Unpack debug build (simulating it running after release build, but with clean memory)
+        result = build._unpack_build(self.base_build_dir, build.debug_build_dir,
+                                     'gs://dummy/debug.zip')
+
+        self.assertTrue(result)
+        mock_write.assert_not_called()
+        self.assertEqual(build._schema_version, 1)  # Should be synced to memory
+        mock_log_error.assert_not_called()