Auto merge of #113695 - bjorn3:fix_rlib_cdylib_metadata_handling, r=pnkfelix,petrochenkov

Verify that all crate sources are in sync

This ensures that rustc will not attempt to link against a cdylib as if it is a rust dylib when an rlib for the same crate is available. Previously rustc didn't actually check if any further formats of a crate which has been loaded are of the same version and if they are actually valid. This caused a cdylib to be interpreted as rust dylib as soon as the corresponding rlib was loaded. As cdylibs don't export any rust symbols, linking would fail if rustc decides to link against the cdylib rather than the rlib.

Two crates depended on the previous behavior by separately compiling a test crate as both rlib and dylib. These have been changed to capture their original spirit to the best of my ability while still working when rustc verifies that all crates are in sync. It is unlikely that build systems depend on the current behavior and in any case we are taking a lot of measures to ensure that any change to either the source or the compilation options (including crate type) results in rustc rejecting it as incompatible. We merely didn't do this check here for now obsolete perf reasons.

Fixes https://github.com/rust-lang/rust/issues/10786
Fixes https://github.com/rust-lang/rust/issues/82151
Fixes https://github.com/rust-lang/rust/issues/82972
Closes https://github.com/bevy-cheatbook/bevy-cheatbook/issues/114

1 files changed, 25 insertions, 24 deletions

diff --git a/compiler/rustc_metadata/src/locator.rs b/compiler/rustc_metadata/src/locator.rs
index a1511c4b570..44195996762 100644
--- a/compiler/rustc_metadata/src/locator.rs
+++ b/compiler/rustc_metadata/src/locator.rs
@@ -511,7 +511,7 @@ impl<'a> CrateLocator<'a> {
             rlib: self.extract_one(rlibs, CrateFlavor::Rlib, &mut slot)?,
             dylib: self.extract_one(dylibs, CrateFlavor::Dylib, &mut slot)?,
         };
-        Ok(slot.map(|(svh, metadata)| (svh, Library { source, metadata })))
+        Ok(slot.map(|(svh, metadata, _)| (svh, Library { source, metadata })))
     }
 
     fn needs_crate_flavor(&self, flavor: CrateFlavor) -> bool {
@@ -535,11 +535,13 @@ impl<'a> CrateLocator<'a> {
     // read the metadata from it if `*slot` is `None`. If the metadata couldn't
     // be read, it is assumed that the file isn't a valid rust library (no
     // errors are emitted).
+    //
+    // The `PathBuf` in `slot` will only be used for diagnostic purposes.
     fn extract_one(
         &mut self,
         m: FxHashMap<PathBuf, PathKind>,
         flavor: CrateFlavor,
-        slot: &mut Option<(Svh, MetadataBlob)>,
+        slot: &mut Option<(Svh, MetadataBlob, PathBuf)>,
     ) -> Result<Option<(PathBuf, PathKind)>, CrateError> {
         // If we are producing an rlib, and we've already loaded metadata, then
         // we should not attempt to discover further crate sources (unless we're
@@ -550,16 +552,9 @@ impl<'a> CrateLocator<'a> {
         //
         // See also #68149 which provides more detail on why emitting the
         // dependency on the rlib is a bad thing.
-        //
-        // We currently do not verify that these other sources are even in sync,
-        // and this is arguably a bug (see #10786), but because reading metadata
-        // is quite slow (especially from dylibs) we currently do not read it
-        // from the other crate sources.
         if slot.is_some() {
             if m.is_empty() || !self.needs_crate_flavor(flavor) {
                 return Ok(None);
-            } else if m.len() == 1 {
-                return Ok(Some(m.into_iter().next().unwrap()));
             }
         }
 
@@ -610,8 +605,7 @@ impl<'a> CrateLocator<'a> {
                         candidates,
                     ));
                 }
-                err_data = Some(vec![ret.as_ref().unwrap().0.clone()]);
-                *slot = None;
+                err_data = Some(vec![slot.take().unwrap().2]);
             }
             if let Some(candidates) = &mut err_data {
                 candidates.push(lib);
@@ -644,7 +638,7 @@ impl<'a> CrateLocator<'a> {
                     continue;
                 }
             }
-            *slot = Some((hash, metadata));
+            *slot = Some((hash, metadata, lib.clone()));
             ret = Some((lib, kind));
         }
 
@@ -814,19 +808,26 @@ fn get_metadata_section<'p>(
             let compressed_len = u32::from_be_bytes(len_bytes) as usize;
 
             // Header is okay -> inflate the actual metadata
-            let compressed_bytes = &buf[data_start..(data_start + compressed_len)];
-            debug!("inflating {} bytes of compressed metadata", compressed_bytes.len());
-            // Assume the decompressed data will be at least the size of the compressed data, so we
-            // don't have to grow the buffer as much.
-            let mut inflated = Vec::with_capacity(compressed_bytes.len());
-            FrameDecoder::new(compressed_bytes).read_to_end(&mut inflated).map_err(|_| {
-                MetadataError::LoadFailure(format!(
-                    "failed to decompress metadata: {}",
-                    filename.display()
-                ))
-            })?;
+            let compressed_bytes = buf.slice(|buf| &buf[data_start..(data_start + compressed_len)]);
+            if &compressed_bytes[..cmp::min(METADATA_HEADER.len(), compressed_bytes.len())]
+                == METADATA_HEADER
+            {
+                // The metadata was not actually compressed.
+                compressed_bytes
+            } else {
+                debug!("inflating {} bytes of compressed metadata", compressed_bytes.len());
+                // Assume the decompressed data will be at least the size of the compressed data, so we
+                // don't have to grow the buffer as much.
+                let mut inflated = Vec::with_capacity(compressed_bytes.len());
+                FrameDecoder::new(&*compressed_bytes).read_to_end(&mut inflated).map_err(|_| {
+                    MetadataError::LoadFailure(format!(
+                        "failed to decompress metadata: {}",
+                        filename.display()
+                    ))
+                })?;
 
-            slice_owned(inflated, Deref::deref)
+                slice_owned(inflated, Deref::deref)
+            }
         }
         CrateFlavor::Rmeta => {
             // mmap the file, because only a small fraction of it is read.