Merge of #673 - Various cleanups

* Avoid two unnecessary allocations in `handle_split_dwarf`. * Couple of other minor changes.
rust-lang · Jan 6, 2025 · 016f80a · 016f80a
2 parents f8cc6ac + 62c5764
commit 016f80a
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 22 deletions.
diff --git a/src/symbolize/gimli/elf.rs b/src/symbolize/gimli/elf.rs
@@ -317,7 +317,7 @@ impl<'a> Object<'a> {
         let section = self.section_header(".gnu_debuglink")?;
         let data = section.data(self.endian, self.data).ok()?;
         let len = data.iter().position(|x| *x == 0)?;
-        let filename = &data[..len];
+        let filename = OsStr::from_bytes(&data[..len]);
         let offset = (len + 1 + 3) & !3;
         let crc_bytes = data
             .get(offset..offset + 4)
@@ -332,7 +332,7 @@ impl<'a> Object<'a> {
         let section = self.section_header(".gnu_debugaltlink")?;
         let data = section.data(self.endian, self.data).ok()?;
         let len = data.iter().position(|x| *x == 0)?;
-        let filename = &data[..len];
+        let filename = OsStr::from_bytes(&data[..len]);
         let build_id = &data[len + 1..];
         let path_sup = locate_debugaltlink(path, filename, build_id)?;
         Some((path_sup, build_id))
@@ -460,12 +460,12 @@ fn locate_build_id(build_id: &[u8]) -> Option<PathBuf> {
 /// gdb also allows the user to customize the debug search path, but we don't.
 ///
 /// gdb also supports debuginfod, but we don't yet.
-fn locate_debuglink(path: &Path, filename: &[u8]) -> Option<PathBuf> {
+fn locate_debuglink(path: &Path, filename: &OsStr) -> Option<PathBuf> {
     let path = fs::canonicalize(path).ok()?;
     let parent = path.parent()?;
     let mut f =
         PathBuf::with_capacity(DEBUG_PATH.len() + parent.as_os_str().len() + filename.len() + 2);
-    let filename = Path::new(OsStr::from_bytes(filename));
+    let filename = Path::new(filename);
 
     // Try "/parent/filename" if it differs from "path"
     f.push(parent);
@@ -509,8 +509,8 @@ fn locate_debuglink(path: &Path, filename: &[u8]) -> Option<PathBuf> {
 /// gdb also allows the user to customize the debug search path, but we don't.
 ///
 /// gdb also supports debuginfod, but we don't yet.
-fn locate_debugaltlink(path: &Path, filename: &[u8], build_id: &[u8]) -> Option<PathBuf> {
-    let filename = Path::new(OsStr::from_bytes(filename));
+fn locate_debugaltlink(path: &Path, filename: &OsStr, build_id: &[u8]) -> Option<PathBuf> {
+    let filename = Path::new(filename);
     if filename.is_absolute() {
         if filename.is_file() {
             return Some(filename.into());
@@ -528,11 +528,6 @@ fn locate_debugaltlink(path: &Path, filename: &[u8], build_id: &[u8]) -> Option<
     locate_build_id(build_id)
 }
 
-fn convert_path<R: gimli::Reader>(r: &R) -> Result<PathBuf, gimli::Error> {
-    let bytes = r.to_slice()?;
-    Ok(PathBuf::from(OsStr::from_bytes(&bytes)))
-}
-
 pub(super) fn handle_split_dwarf<'data>(
     package: Option<&gimli::DwarfPackage<EndianSlice<'data, Endian>>>,
     stash: &'data Stash,
@@ -546,10 +541,10 @@ pub(super) fn handle_split_dwarf<'data>(
 
     let mut path = PathBuf::new();
     if let Some(p) = load.comp_dir.as_ref() {
-        path.push(convert_path(p).ok()?);
+        path.push(OsStr::from_bytes(&p));
     }
 
-    path.push(convert_path(load.path.as_ref()?).ok()?);
+    path.push(OsStr::from_bytes(&load.path.as_ref()?));
 
     if let Some(map_dwo) = super::mmap(&path) {
         let map_dwo = stash.cache_mmap(map_dwo);

diff --git a/src/symbolize/gimli/libs_aix.rs b/src/symbolize/gimli/libs_aix.rs
@@ -1,17 +1,25 @@
 use super::mystd::env;
 use super::mystd::ffi::OsStr;
-use super::mystd::io::Error;
 use super::mystd::os::unix::prelude::*;
 use super::xcoff;
 use super::{Library, LibrarySegment};
 use alloc::borrow::ToOwned;
 use alloc::vec;
 use alloc::vec::Vec;
-use core::ffi::CStr;
+use core::ffi::{c_int, CStr};
 use core::mem;
 
 const EXE_IMAGE_BASE: u64 = 0x100000000;
 
+extern "C" {
+    #[link_name = "_Errno"]
+    fn errno_location() -> *mut c_int;
+}
+
+fn errno() -> i32 {
+    unsafe { (*errno_location()) as i32 }
+}
+
 /// On AIX, we use `loadquery` with `L_GETINFO` flag to query libraries mmapped.
 /// See https://www.ibm.com/docs/en/aix/7.2?topic=l-loadquery-subroutine for
 /// detailed information of `loadquery`.
@@ -28,15 +36,14 @@ pub(super) fn native_libraries() -> Vec<Library> {
             {
                 break;
             } else {
-                match Error::last_os_error().raw_os_error() {
-                    Some(libc::ENOMEM) => {
+                match errno() {
+                    libc::ENOMEM => {
                         buffer.resize(buffer.len() * 2, mem::zeroed::<libc::ld_info>());
                     }
-                    Some(_) => {
+                    _ => {
                         // If other error occurs, return empty libraries.
                         return Vec::new();
                     }
-                    _ => unreachable!(),
                 }
             }
         }

diff --git a/src/symbolize/gimli/xcoff.rs b/src/symbolize/gimli/xcoff.rs
@@ -1,4 +1,4 @@
-use super::mystd::ffi::{OsStr, OsString};
+use super::mystd::ffi::OsStr;
 use super::mystd::os::unix::ffi::OsStrExt;
 use super::mystd::path::Path;
 use super::{gimli, Context, Endian, EndianSlice, Mapping, Stash};
@@ -19,7 +19,7 @@ type Xcoff = object::xcoff::FileHeader32;
 type Xcoff = object::xcoff::FileHeader64;
 
 impl Mapping {
-    pub fn new(path: &Path, member_name: &OsString) -> Option<Mapping> {
+    pub fn new(path: &Path, member_name: &OsStr) -> Option<Mapping> {
         let map = super::mmap(path)?;
         Mapping::mk(map, |data, stash| {
             if member_name.is_empty() {
@@ -81,7 +81,7 @@ pub fn parse_xcoff(data: &[u8]) -> Option<Image> {
     }
 }
 
-pub fn parse_image(path: &Path, member_name: &OsString) -> Option<Image> {
+pub fn parse_image(path: &Path, member_name: &OsStr) -> Option<Image> {
     let map = super::mmap(path)?;
     let data = map.deref();
     if member_name.is_empty() {