Introspection: emit messages inline in the binaries instead of introducing a ptr+len indirection

Significantly simplifies the parsing

I dropped the old code path, we can add it back if we want to maintain backward compatibility

Author: Tpt

.github/workflows/ci.yml

@@ -697,8 +697,7 @@ jobs:
         with:
           save-if: ${{ github.ref == 'refs/heads/main' || contains(github.event.pull_request.labels.*.name, 'CI-save-pr-cache') }}
       - run: python -m pip install --upgrade pip && pip install nox[uv]
-        # TODO test will be fixed in https://github.com/PyO3/pyo3/pull/5450
-#      - run: nox -s test-introspection
+      - run: nox -s test-introspection
     env:
       CARGO_BUILD_TARGET: ${{ matrix.platform.rust-target }}
 

newsfragments/5450.changed.md

@@ -0,0 +1 @@
+Introspection: change the way introspection data is emitted in the binaries to avoid a pointer indirection and simplify parsing.

pyo3-introspection/src/introspection.rs

@@ -10,10 +10,11 @@ use goblin::mach::{Mach, MachO, SingleArch};
 use goblin::pe::PE;
 use goblin::Object;
 use serde::Deserialize;
+use serde_json::Deserializer;
 use std::cmp::Ordering;
 use std::collections::HashMap;
-use std::fs;
 use std::path::Path;
+use std::{fs, str};
 
 /// Introspect a cdylib built with PyO3 and returns the definition of a Python module.
 ///
@@ -272,11 +273,8 @@ fn find_introspection_chunks_in_elf(elf: &Elf<'_>, library_content: &[u8]) -> Re
             ensure!(u32::try_from(sym.st_shndx)? != SHN_XINDEX, "Section names length is greater than SHN_LORESERVE in ELF, this is not supported by PyO3 yet");
             let section_header = &elf.section_headers[sym.st_shndx];
             let data_offset = sym.st_value + section_header.sh_offset - section_header.sh_addr;
-            chunks.push(read_symbol_value_with_ptr_and_len(
+            chunks.push(deserialize_prefix(
                 &library_content[usize::try_from(data_offset).context("File offset overflow")?..],
-                0,
-                library_content,
-                elf.is_64,
             )?);
         }
     }
@@ -313,85 +311,40 @@ fn find_introspection_chunks_in_macho(
         {
             let section = &sections[nlist.n_sect - 1]; // Sections are counted from 1
             let data_offset = nlist.n_value + u64::from(section.offset) - section.addr;
-            chunks.push(read_symbol_value_with_ptr_and_len(
+            chunks.push(deserialize_prefix(
                 &library_content[usize::try_from(data_offset).context("File offset overflow")?..],
-                0,
-                library_content,
-                macho.is_64,
             )?);
         }
     }
     Ok(chunks)
 }
 
 fn find_introspection_chunks_in_pe(pe: &PE<'_>, library_content: &[u8]) -> Result<Vec<Chunk>> {
-    let rdata_data_section = pe
-        .sections
-        .iter()
-        .find(|section| section.name().unwrap_or_default() == ".rdata")
-        .context("No .rdata section found")?;
-    let rdata_shift = usize::try_from(pe.image_base).context("image_base overflow")?
-        + usize::try_from(rdata_data_section.virtual_address)
-            .context(".rdata virtual_address overflow")?
-        - usize::try_from(rdata_data_section.pointer_to_raw_data)
-            .context(".rdata pointer_to_raw_data overflow")?;
-
     let mut chunks = Vec::new();
     for export in &pe.exports {
         if is_introspection_symbol(export.name.unwrap_or_default()) {
-            chunks.push(read_symbol_value_with_ptr_and_len(
+            chunks.push(deserialize_prefix(
                 &library_content[export.offset.context("No symbol offset")?..],
-                rdata_shift,
-                library_content,
-                pe.is_64,
             )?);
         }
     }
     Ok(chunks)
 }
 
-fn read_symbol_value_with_ptr_and_len(
-    value_slice: &[u8],
-    shift: usize,
-    full_library_content: &[u8],
-    is_64: bool,
-) -> Result<Chunk> {
-    let (ptr, len) = if is_64 {
-        let (ptr, len) = value_slice[..16].split_at(8);
-        let ptr = usize::try_from(u64::from_le_bytes(
-            ptr.try_into().context("Too short symbol value")?,
-        ))
-        .context("Pointer overflow")?;
-        let len = usize::try_from(u64::from_le_bytes(
-            len.try_into().context("Too short symbol value")?,
-        ))
-        .context("Length overflow")?;
-        (ptr, len)
-    } else {
-        let (ptr, len) = value_slice[..8].split_at(4);
-        let ptr = usize::try_from(u32::from_le_bytes(
-            ptr.try_into().context("Too short symbol value")?,
-        ))
-        .context("Pointer overflow")?;
-        let len = usize::try_from(u32::from_le_bytes(
-            len.try_into().context("Too short symbol value")?,
-        ))
-        .context("Length overflow")?;
-        (ptr, len)
-    };
-    let chunk = &full_library_content[ptr - shift..ptr - shift + len];
-    serde_json::from_slice(chunk).with_context(|| {
-        format!(
-            "Failed to parse introspection chunk: '{}'",
-            String::from_utf8_lossy(chunk)
-        )
+fn deserialize_prefix(chunk: &[u8]) -> Result<Chunk> {
+    Chunk::deserialize(&mut Deserializer::from_slice(chunk)).with_context(|| {
+        // We take the first valid utf8 bytes, it's quite likely to be the actual chunk
+        // We use a 4096 upper bound for security
+        let chunk = str::from_utf8(&chunk[..4096])
+            .unwrap_or_else(|e| str::from_utf8(&chunk[..e.valid_up_to()]).unwrap_or_default());
+        format!("Failed to parse introspection chunk: '{chunk}'")
     })
 }
 
 fn is_introspection_symbol(name: &str) -> bool {
     name.strip_prefix('_')
         .unwrap_or(name)
-        .starts_with("PYO3_INTROSPECTION_0_")
+        .starts_with("PYO3_INTROSPECTION_1_")
 }
 
 #[derive(Deserialize)]

pyo3-macros-backend/src/introspection.rs

@@ -353,17 +353,10 @@ impl IntrospectionNode<'_> {
     fn emit(self, pyo3_crate_path: &PyO3CratePath) -> TokenStream {
         let mut content = ConcatenationBuilder::default();
         self.add_to_serialization(&mut content, pyo3_crate_path);
-        let content = content.into_token_stream(pyo3_crate_path);
-
-        let static_name = format_ident!("PYO3_INTROSPECTION_0_{}", unique_element_id());
-        // #[no_mangle] is required to make sure some linkers like Linux ones do not mangle the section name too.
-        quote! {
-            const _: () = {
-                #[used]
-                #[no_mangle]
-                static #static_name: &'static [u8] = #content;
-            };
-        }
+        content.into_static(
+            pyo3_crate_path,
+            format_ident!("PYO3_INTROSPECTION_1_{}", unique_element_id()),
+        )
     }
 
     fn add_to_serialization(
@@ -530,6 +523,24 @@ impl ConcatenationBuilder {
             }
         }
     }
+
+    fn into_static(self, pyo3_crate_path: &PyO3CratePath, ident: Ident) -> TokenStream {
+        let mut elements = self.elements;
+        if !self.current_string.is_empty() {
+            elements.push(ConcatenationBuilderElement::String(self.current_string));
+        }
+
+        // #[no_mangle] is required to make sure some linkers like Linux ones do not mangle the section name too.
+        quote! {
+            const _: () = {
+                const PIECES: &[&[u8]] = &[#(#elements , )*];
+                const PIECES_LEN: usize = #pyo3_crate_path::impl_::concat::combined_len(PIECES);
+                #[used]
+                #[no_mangle]
+                static #ident: [u8; PIECES_LEN] = #pyo3_crate_path::impl_::concat::combine_to_array::<PIECES_LEN>(PIECES);
+            };
+        }
+    }
 }
 
 enum ConcatenationBuilderElement {

tests/test_compile_error.rs

@@ -69,7 +69,7 @@ fn test_compile_errors() {
     t.compile_fail("tests/ui/invalid_pymodule_glob.rs");
     t.compile_fail("tests/ui/invalid_pymodule_trait.rs");
     t.compile_fail("tests/ui/invalid_pymodule_two_pymodule_init.rs");
-    #[cfg(feature = "experimental-async")]
+    #[cfg(all(feature = "experimental-async", not(feature = "experimental-inspect")))]
     #[cfg(any(not(Py_LIMITED_API), Py_3_10))] // to avoid PyFunctionArgument for &str
     t.compile_fail("tests/ui/invalid_cancel_handle.rs");
     t.pass("tests/ui/pymodule_missing_docs.rs");