From 5f9cd5bff221478fad732c2058809f3681c34af6 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sun, 2 Jul 2023 00:14:45 -0700 Subject: [PATCH 01/19] Track permissions as bytes Because we expect certain inputs, we don't need chars. The code also doesn't need to validate exact lengths. This reduces the generated code size slightly. --- .../gimli/parse_running_mmaps_unix.rs | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index 5803d5dc..8ffca4cc 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -20,7 +20,7 @@ pub(super) struct MapsEntry { /// x = execute /// s = shared /// p = private (copy on write) - perms: [char; 4], + perms: [u8; 4], /// Offset into the file (or "whatever"). offset: u64, /// device (major, minor) @@ -132,14 +132,12 @@ impl FromStr for MapsEntry { } else { return Err("Couldn't parse address range"); }; - let perms: [char; 4] = { - let mut chars = perms_str.chars(); - let mut c = || chars.next().ok_or("insufficient perms"); - let perms = [c()?, c()?, c()?, c()?]; - if chars.next().is_some() { - return Err("too many perms"); - } - perms + let perms = if let &[r, w, x, p, ..] = perms_str.as_bytes() { + // If a system in the future adds a 5th field to the permission list, + // there's no reason to assume previous fields were invalidated. + [r, w, x, p] + } else { + return Err("less than 4 perms"); }; let offset = hex64(offset_str)?; let dev = if let Some((major, minor)) = dev_str.split_once(':') { @@ -172,7 +170,7 @@ fn check_maps_entry_parsing_64bit() { .unwrap(), MapsEntry { address: (0xffffffffff600000, 0xffffffffff601000), - perms: ['-', '-', 'x', 'p'], + perms: *b"--xp", offset: 0x00000000, dev: (0x00, 0x00), inode: 0x0, @@ -187,7 +185,7 @@ fn check_maps_entry_parsing_64bit() { .unwrap(), MapsEntry { address: (0x7f5985f46000, 0x7f5985f48000), - perms: ['r', 'w', '-', 'p'], + perms: *b"rw-p", offset: 0x00039000, dev: (0x103, 0x06), inode: 0x76021795, @@ -200,7 +198,7 @@ fn check_maps_entry_parsing_64bit() { .unwrap(), MapsEntry { address: (0x35b1a21000, 0x35b1a22000), - perms: ['r', 'w', '-', 'p'], + perms: *b"rw-p", offset: 0x00000000, dev: (0x00, 0x00), inode: 0x0, @@ -224,7 +222,7 @@ fn check_maps_entry_parsing_32bit() { .unwrap(), MapsEntry { address: (0x08056000, 0x08077000), - perms: ['r', 'w', '-', 'p'], + perms: *b"rw-p", offset: 0x00000000, dev: (0x00, 0x00), inode: 0x0, @@ -239,7 +237,7 @@ fn check_maps_entry_parsing_32bit() { .unwrap(), MapsEntry { address: (0xb7c79000, 0xb7e02000), - perms: ['r', '-', '-', 'p'], + perms: *b"r--p", offset: 0x00000000, dev: (0x08, 0x01), inode: 0x60662705, @@ -252,7 +250,7 @@ fn check_maps_entry_parsing_32bit() { .unwrap(), MapsEntry { address: (0xb7e02000, 0xb7e03000), - perms: ['r', 'w', '-', 'p'], + perms: *b"rw-p", offset: 0x00000000, dev: (0x00, 0x00), inode: 0x0, From 515006b940cad74743dcbba7774aeef242a0b62a Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 15:34:53 -0700 Subject: [PATCH 02/19] Comment-out unused fields --- .../gimli/parse_running_mmaps_unix.rs | 72 +++++++++---------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index 8ffca4cc..f4665018 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -20,13 +20,13 @@ pub(super) struct MapsEntry { /// x = execute /// s = shared /// p = private (copy on write) - perms: [u8; 4], + // perms: [u8; 4], /// Offset into the file (or "whatever"). - offset: u64, + // offset: u64, /// device (major, minor) - dev: (usize, usize), + // dev: (usize, usize), /// inode on the device. 0 indicates that no inode is associated with the memory region (e.g. uninitalized data aka BSS). - inode: usize, + // inode: usize, /// Usually the file backing the mapping. /// /// Note: The man page for proc includes a note about "coordination" by @@ -132,28 +132,28 @@ impl FromStr for MapsEntry { } else { return Err("Couldn't parse address range"); }; - let perms = if let &[r, w, x, p, ..] = perms_str.as_bytes() { + let _perms = if let &[r, w, x, p, ..] = perms_str.as_bytes() { // If a system in the future adds a 5th field to the permission list, // there's no reason to assume previous fields were invalidated. [r, w, x, p] } else { return Err("less than 4 perms"); }; - let offset = hex64(offset_str)?; - let dev = if let Some((major, minor)) = dev_str.split_once(':') { + let _offset = hex(offset_str)?; + let _dev = if let Some((major, minor)) = dev_str.split_once(':') { (hex(major)?, hex(minor)?) } else { return Err("Couldn't parse dev"); }; - let inode = hex(inode_str)?; + let _inode = hex(inode_str)?; let pathname = pathname_str.into(); Ok(MapsEntry { address, - perms, - offset, - dev, - inode, + // perms, + // offset, + // dev, + // inode, pathname, }) } @@ -170,10 +170,10 @@ fn check_maps_entry_parsing_64bit() { .unwrap(), MapsEntry { address: (0xffffffffff600000, 0xffffffffff601000), - perms: *b"--xp", - offset: 0x00000000, - dev: (0x00, 0x00), - inode: 0x0, + // perms: *b"--xp", + // offset: 0x00000000, + // dev: (0x00, 0x00), + // inode: 0x0, pathname: "[vsyscall]".into(), } ); @@ -185,10 +185,10 @@ fn check_maps_entry_parsing_64bit() { .unwrap(), MapsEntry { address: (0x7f5985f46000, 0x7f5985f48000), - perms: *b"rw-p", - offset: 0x00039000, - dev: (0x103, 0x06), - inode: 0x76021795, + // perms: *b"rw-p", + // offset: 0x00039000, + // dev: (0x103, 0x06), + // inode: 0x76021795, pathname: "/usr/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2".into(), } ); @@ -198,10 +198,10 @@ fn check_maps_entry_parsing_64bit() { .unwrap(), MapsEntry { address: (0x35b1a21000, 0x35b1a22000), - perms: *b"rw-p", - offset: 0x00000000, - dev: (0x00, 0x00), - inode: 0x0, + // perms: *b"rw-p", + // offset: 0x00000000, + // dev: (0x00, 0x00), + // inode: 0x0, pathname: Default::default(), } ); @@ -222,10 +222,10 @@ fn check_maps_entry_parsing_32bit() { .unwrap(), MapsEntry { address: (0x08056000, 0x08077000), - perms: *b"rw-p", - offset: 0x00000000, - dev: (0x00, 0x00), - inode: 0x0, + // perms: *b"rw-p", + // offset: 0x00000000, + // dev: (0x00, 0x00), + // inode: 0x0, pathname: "[heap]".into(), } ); @@ -237,10 +237,10 @@ fn check_maps_entry_parsing_32bit() { .unwrap(), MapsEntry { address: (0xb7c79000, 0xb7e02000), - perms: *b"r--p", - offset: 0x00000000, - dev: (0x08, 0x01), - inode: 0x60662705, + // perms: *b"r--p", + // offset: 0x00000000, + // dev: (0x08, 0x01), + // inode: 0x60662705, pathname: "/usr/lib/locale/locale-archive".into(), } ); @@ -250,10 +250,10 @@ fn check_maps_entry_parsing_32bit() { .unwrap(), MapsEntry { address: (0xb7e02000, 0xb7e03000), - perms: *b"rw-p", - offset: 0x00000000, - dev: (0x00, 0x00), - inode: 0x0, + // perms: *b"rw-p", + // offset: 0x00000000, + // dev: (0x00, 0x00), + // inode: 0x0, pathname: Default::default(), } ); From fddf229b5a1a2afe34d930f07a2187af23cdfefa Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 15:55:38 -0700 Subject: [PATCH 03/19] Omit needless error strings --- .../gimli/parse_running_mmaps_unix.rs | 48 ++++++------------- 1 file changed, 15 insertions(+), 33 deletions(-) diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index f4665018..f63eabf4 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -96,54 +96,36 @@ impl FromStr for MapsEntry { // Note that paths may contain spaces, so we can't use `str::split` for parsing (until // Split::remainder is stabilized #77998). fn from_str(s: &str) -> Result { - let (range_str, s) = s.trim_start().split_once(' ').unwrap_or((s, "")); - if range_str.is_empty() { - return Err("Couldn't find address"); - } - - let (perms_str, s) = s.trim_start().split_once(' ').unwrap_or((s, "")); - if perms_str.is_empty() { - return Err("Couldn't find permissions"); - } - - let (offset_str, s) = s.trim_start().split_once(' ').unwrap_or((s, "")); - if offset_str.is_empty() { - return Err("Couldn't find offset"); - } - - let (dev_str, s) = s.trim_start().split_once(' ').unwrap_or((s, "")); - if dev_str.is_empty() { - return Err("Couldn't find dev"); - } - - let (inode_str, s) = s.trim_start().split_once(' ').unwrap_or((s, "")); - if inode_str.is_empty() { - return Err("Couldn't find inode"); - } - - // Pathname may be omitted in which case it will be empty - let pathname_str = s.trim_start(); - - let hex = |s| usize::from_str_radix(s, 16).map_err(|_| "Couldn't parse hex number"); - let hex64 = |s| u64::from_str_radix(s, 16).map_err(|_| "Couldn't parse hex number"); + let missing_field = "failed to find all map fields"; + let parse_err = "failed to parse all map fields"; + let mut parts = s + .split(' ') // space-separated fields + .filter(|s| s.len() > 0); // multiple spaces implies empty strings that need to be skipped. + let range_str = parts.next().ok_or(missing_field)?; + let perms_str = parts.next().ok_or(missing_field)?; + let offset_str = parts.next().ok_or(missing_field)?; + let dev_str = parts.next().ok_or(missing_field)?; + let inode_str = parts.next().ok_or(missing_field)?; + let pathname_str = parts.next().unwrap_or(""); // pathname may be omitted. + let hex = |s| usize::from_str_radix(s, 16).map_err(|_| parse_err); let address = if let Some((start, limit)) = range_str.split_once('-') { (hex(start)?, hex(limit)?) } else { - return Err("Couldn't parse address range"); + return Err(parse_err); }; let _perms = if let &[r, w, x, p, ..] = perms_str.as_bytes() { // If a system in the future adds a 5th field to the permission list, // there's no reason to assume previous fields were invalidated. [r, w, x, p] } else { - return Err("less than 4 perms"); + return Err(parse_err); }; let _offset = hex(offset_str)?; let _dev = if let Some((major, minor)) = dev_str.split_once(':') { (hex(major)?, hex(minor)?) } else { - return Err("Couldn't parse dev"); + return Err(parse_err); }; let _inode = hex(inode_str)?; let pathname = pathname_str.into(); From 281d75992567b25590d539c2744eec3ea20ca7c8 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 16:27:09 -0700 Subject: [PATCH 04/19] Use split_ascii_whitespace --- src/symbolize/gimli/parse_running_mmaps_unix.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index f63eabf4..f821117e 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -99,8 +99,7 @@ impl FromStr for MapsEntry { let missing_field = "failed to find all map fields"; let parse_err = "failed to parse all map fields"; let mut parts = s - .split(' ') // space-separated fields - .filter(|s| s.len() > 0); // multiple spaces implies empty strings that need to be skipped. + .split_ascii_whitespace(); let range_str = parts.next().ok_or(missing_field)?; let perms_str = parts.next().ok_or(missing_field)?; let offset_str = parts.next().ok_or(missing_field)?; From cd03f04e5ff26403795136765881360455e04c2d Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 16:36:33 -0700 Subject: [PATCH 05/19] Try not parsing the entire map --- .../gimli/parse_running_mmaps_unix.rs | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index f821117e..25d7aa21 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -113,20 +113,20 @@ impl FromStr for MapsEntry { } else { return Err(parse_err); }; - let _perms = if let &[r, w, x, p, ..] = perms_str.as_bytes() { - // If a system in the future adds a 5th field to the permission list, - // there's no reason to assume previous fields were invalidated. - [r, w, x, p] - } else { - return Err(parse_err); - }; - let _offset = hex(offset_str)?; - let _dev = if let Some((major, minor)) = dev_str.split_once(':') { - (hex(major)?, hex(minor)?) - } else { - return Err(parse_err); - }; - let _inode = hex(inode_str)?; + // let _perms = if let &[r, w, x, p, ..] = perms_str.as_bytes() { + // // If a system in the future adds a 5th field to the permission list, + // // there's no reason to assume previous fields were invalidated. + // [r, w, x, p] + // } else { + // return Err(parse_err); + // }; + // let _offset = hex(offset_str)?; + // let _dev = if let Some((major, minor)) = dev_str.split_once(':') { + // (hex(major)?, hex(minor)?) + // } else { + // return Err(parse_err); + // }; + // let _inode = hex(inode_str)?; let pathname = pathname_str.into(); Ok(MapsEntry { From b1e68fd1af04d5641c49633d166d5599b9322bd9 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 16:56:54 -0700 Subject: [PATCH 06/19] Revert "Try not parsing the entire map" This reverts commit 1166cc8b4927450dae10310bd9a65fd6adad4d53. --- .../gimli/parse_running_mmaps_unix.rs | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index 25d7aa21..f821117e 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -113,20 +113,20 @@ impl FromStr for MapsEntry { } else { return Err(parse_err); }; - // let _perms = if let &[r, w, x, p, ..] = perms_str.as_bytes() { - // // If a system in the future adds a 5th field to the permission list, - // // there's no reason to assume previous fields were invalidated. - // [r, w, x, p] - // } else { - // return Err(parse_err); - // }; - // let _offset = hex(offset_str)?; - // let _dev = if let Some((major, minor)) = dev_str.split_once(':') { - // (hex(major)?, hex(minor)?) - // } else { - // return Err(parse_err); - // }; - // let _inode = hex(inode_str)?; + let _perms = if let &[r, w, x, p, ..] = perms_str.as_bytes() { + // If a system in the future adds a 5th field to the permission list, + // there's no reason to assume previous fields were invalidated. + [r, w, x, p] + } else { + return Err(parse_err); + }; + let _offset = hex(offset_str)?; + let _dev = if let Some((major, minor)) = dev_str.split_once(':') { + (hex(major)?, hex(minor)?) + } else { + return Err(parse_err); + }; + let _inode = hex(inode_str)?; let pathname = pathname_str.into(); Ok(MapsEntry { From 11c4dc3f52eed1d43dc0c346ba1764a8dded7419 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 17:01:38 -0700 Subject: [PATCH 07/19] Inline, but less --- src/symbolize/gimli/parse_running_mmaps_unix.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index f821117e..b10fdb48 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -71,10 +71,12 @@ pub(super) fn parse_maps() -> Result, &'static str> { } impl MapsEntry { + #[inline] pub(super) fn pathname(&self) -> &OsString { &self.pathname } + #[inline] pub(super) fn ip_matches(&self, ip: usize) -> bool { self.address.0 <= ip && ip < self.address.1 } From 80420cb87eea08139d41409669830384dad78073 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 17:13:51 -0700 Subject: [PATCH 08/19] Revert "Track permissions as bytes" This reverts commit 25bddbf8dd06db638e91391248127ebbdf130b78. --- .../gimli/parse_running_mmaps_unix.rs | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index b10fdb48..55953882 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -20,7 +20,7 @@ pub(super) struct MapsEntry { /// x = execute /// s = shared /// p = private (copy on write) - // perms: [u8; 4], + perms: [char; 4], /// Offset into the file (or "whatever"). // offset: u64, /// device (major, minor) @@ -115,12 +115,14 @@ impl FromStr for MapsEntry { } else { return Err(parse_err); }; - let _perms = if let &[r, w, x, p, ..] = perms_str.as_bytes() { - // If a system in the future adds a 5th field to the permission list, - // there's no reason to assume previous fields were invalidated. - [r, w, x, p] - } else { - return Err(parse_err); + let perms: [char; 4] = { + let mut chars = perms_str.chars(); + let mut c = || chars.next().ok_or("insufficient perms"); + let perms = [c()?, c()?, c()?, c()?]; + if chars.next().is_some() { + return Err("too many perms"); + } + perms }; let _offset = hex(offset_str)?; let _dev = if let Some((major, minor)) = dev_str.split_once(':') { @@ -133,7 +135,7 @@ impl FromStr for MapsEntry { Ok(MapsEntry { address, - // perms, + perms, // offset, // dev, // inode, @@ -153,7 +155,7 @@ fn check_maps_entry_parsing_64bit() { .unwrap(), MapsEntry { address: (0xffffffffff600000, 0xffffffffff601000), - // perms: *b"--xp", + perms: ['-', '-', 'x', 'p'], // offset: 0x00000000, // dev: (0x00, 0x00), // inode: 0x0, @@ -168,7 +170,7 @@ fn check_maps_entry_parsing_64bit() { .unwrap(), MapsEntry { address: (0x7f5985f46000, 0x7f5985f48000), - // perms: *b"rw-p", + perms: ['r', 'w', '-', 'p'], // offset: 0x00039000, // dev: (0x103, 0x06), // inode: 0x76021795, @@ -181,7 +183,7 @@ fn check_maps_entry_parsing_64bit() { .unwrap(), MapsEntry { address: (0x35b1a21000, 0x35b1a22000), - // perms: *b"rw-p", + perms: ['r', 'w', '-', 'p'], // offset: 0x00000000, // dev: (0x00, 0x00), // inode: 0x0, @@ -205,7 +207,7 @@ fn check_maps_entry_parsing_32bit() { .unwrap(), MapsEntry { address: (0x08056000, 0x08077000), - // perms: *b"rw-p", + perms: ['r', 'w', '-', 'p'], // offset: 0x00000000, // dev: (0x00, 0x00), // inode: 0x0, @@ -220,7 +222,7 @@ fn check_maps_entry_parsing_32bit() { .unwrap(), MapsEntry { address: (0xb7c79000, 0xb7e02000), - // perms: *b"r--p", + perms: ['r', '-', '-', 'p'], // offset: 0x00000000, // dev: (0x08, 0x01), // inode: 0x60662705, @@ -233,7 +235,7 @@ fn check_maps_entry_parsing_32bit() { .unwrap(), MapsEntry { address: (0xb7e02000, 0xb7e03000), - // perms: *b"rw-p", + perms: ['r', 'w', '-', 'p'], // offset: 0x00000000, // dev: (0x00, 0x00), // inode: 0x0, From 3827d19fc98ca93cf3a4b47e7fef5bf80e20b7ce Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 17:24:58 -0700 Subject: [PATCH 09/19] Simply discard perms --- src/symbolize/gimli/parse_running_mmaps_unix.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index 55953882..169248cc 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -20,7 +20,7 @@ pub(super) struct MapsEntry { /// x = execute /// s = shared /// p = private (copy on write) - perms: [char; 4], + // perms: [char; 4], /// Offset into the file (or "whatever"). // offset: u64, /// device (major, minor) @@ -115,7 +115,7 @@ impl FromStr for MapsEntry { } else { return Err(parse_err); }; - let perms: [char; 4] = { + let _perms: [char; 4] = { let mut chars = perms_str.chars(); let mut c = || chars.next().ok_or("insufficient perms"); let perms = [c()?, c()?, c()?, c()?]; @@ -135,7 +135,7 @@ impl FromStr for MapsEntry { Ok(MapsEntry { address, - perms, + // perms, // offset, // dev, // inode, From fb390ad0ede0f73baf8ce5cdc6abf1fe3cacb3e0 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 17:26:37 -0700 Subject: [PATCH 10/19] In tests too --- src/symbolize/gimli/parse_running_mmaps_unix.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index 169248cc..79d6f34e 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -155,7 +155,7 @@ fn check_maps_entry_parsing_64bit() { .unwrap(), MapsEntry { address: (0xffffffffff600000, 0xffffffffff601000), - perms: ['-', '-', 'x', 'p'], + // perms: ['-', '-', 'x', 'p'], // offset: 0x00000000, // dev: (0x00, 0x00), // inode: 0x0, @@ -170,7 +170,7 @@ fn check_maps_entry_parsing_64bit() { .unwrap(), MapsEntry { address: (0x7f5985f46000, 0x7f5985f48000), - perms: ['r', 'w', '-', 'p'], + // perms: ['r', 'w', '-', 'p'], // offset: 0x00039000, // dev: (0x103, 0x06), // inode: 0x76021795, @@ -183,7 +183,7 @@ fn check_maps_entry_parsing_64bit() { .unwrap(), MapsEntry { address: (0x35b1a21000, 0x35b1a22000), - perms: ['r', 'w', '-', 'p'], + // perms: ['r', 'w', '-', 'p'], // offset: 0x00000000, // dev: (0x00, 0x00), // inode: 0x0, @@ -207,7 +207,7 @@ fn check_maps_entry_parsing_32bit() { .unwrap(), MapsEntry { address: (0x08056000, 0x08077000), - perms: ['r', 'w', '-', 'p'], + // perms: ['r', 'w', '-', 'p'], // offset: 0x00000000, // dev: (0x00, 0x00), // inode: 0x0, @@ -222,7 +222,7 @@ fn check_maps_entry_parsing_32bit() { .unwrap(), MapsEntry { address: (0xb7c79000, 0xb7e02000), - perms: ['r', '-', '-', 'p'], + // perms: ['r', '-', '-', 'p'], // offset: 0x00000000, // dev: (0x08, 0x01), // inode: 0x60662705, @@ -235,7 +235,7 @@ fn check_maps_entry_parsing_32bit() { .unwrap(), MapsEntry { address: (0xb7e02000, 0xb7e03000), - perms: ['r', 'w', '-', 'p'], + // perms: ['r', 'w', '-', 'p'], // offset: 0x00000000, // dev: (0x00, 0x00), // inode: 0x0, From ba1948700be9e1fb2180a3bb1e58db9b39f5ba00 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 17:38:21 -0700 Subject: [PATCH 11/19] Revert "Simply discard perms" This reverts commit 9c00af84ee412eeb0f36a95b3eaff233ede04db4. --- src/symbolize/gimli/parse_running_mmaps_unix.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index 79d6f34e..cd602178 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -20,7 +20,7 @@ pub(super) struct MapsEntry { /// x = execute /// s = shared /// p = private (copy on write) - // perms: [char; 4], + perms: [char; 4], /// Offset into the file (or "whatever"). // offset: u64, /// device (major, minor) @@ -115,7 +115,7 @@ impl FromStr for MapsEntry { } else { return Err(parse_err); }; - let _perms: [char; 4] = { + let perms: [char; 4] = { let mut chars = perms_str.chars(); let mut c = || chars.next().ok_or("insufficient perms"); let perms = [c()?, c()?, c()?, c()?]; @@ -135,7 +135,7 @@ impl FromStr for MapsEntry { Ok(MapsEntry { address, - // perms, + perms, // offset, // dev, // inode, From 0041f3334a5989eb4126502ce872685ae43e3410 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 17:38:30 -0700 Subject: [PATCH 12/19] Revert "In tests too" This reverts commit 1d63a5c5066c6dbcaf77f540799c96cdaeb5bf2f. --- src/symbolize/gimli/parse_running_mmaps_unix.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index cd602178..55953882 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -155,7 +155,7 @@ fn check_maps_entry_parsing_64bit() { .unwrap(), MapsEntry { address: (0xffffffffff600000, 0xffffffffff601000), - // perms: ['-', '-', 'x', 'p'], + perms: ['-', '-', 'x', 'p'], // offset: 0x00000000, // dev: (0x00, 0x00), // inode: 0x0, @@ -170,7 +170,7 @@ fn check_maps_entry_parsing_64bit() { .unwrap(), MapsEntry { address: (0x7f5985f46000, 0x7f5985f48000), - // perms: ['r', 'w', '-', 'p'], + perms: ['r', 'w', '-', 'p'], // offset: 0x00039000, // dev: (0x103, 0x06), // inode: 0x76021795, @@ -183,7 +183,7 @@ fn check_maps_entry_parsing_64bit() { .unwrap(), MapsEntry { address: (0x35b1a21000, 0x35b1a22000), - // perms: ['r', 'w', '-', 'p'], + perms: ['r', 'w', '-', 'p'], // offset: 0x00000000, // dev: (0x00, 0x00), // inode: 0x0, @@ -207,7 +207,7 @@ fn check_maps_entry_parsing_32bit() { .unwrap(), MapsEntry { address: (0x08056000, 0x08077000), - // perms: ['r', 'w', '-', 'p'], + perms: ['r', 'w', '-', 'p'], // offset: 0x00000000, // dev: (0x00, 0x00), // inode: 0x0, @@ -222,7 +222,7 @@ fn check_maps_entry_parsing_32bit() { .unwrap(), MapsEntry { address: (0xb7c79000, 0xb7e02000), - // perms: ['r', '-', '-', 'p'], + perms: ['r', '-', '-', 'p'], // offset: 0x00000000, // dev: (0x08, 0x01), // inode: 0x60662705, @@ -235,7 +235,7 @@ fn check_maps_entry_parsing_32bit() { .unwrap(), MapsEntry { address: (0xb7e02000, 0xb7e03000), - // perms: ['r', 'w', '-', 'p'], + perms: ['r', 'w', '-', 'p'], // offset: 0x00000000, // dev: (0x00, 0x00), // inode: 0x0, From 4d2bc88b6bb6e5c9100ca3460c5cc3d6a4269ba5 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 17:38:38 -0700 Subject: [PATCH 13/19] Revert "Revert "Track permissions as bytes"" This reverts commit 9bc29a0e9a35ff2b26e59caaba27ff990fe5f9d1. --- .../gimli/parse_running_mmaps_unix.rs | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index 55953882..b10fdb48 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -20,7 +20,7 @@ pub(super) struct MapsEntry { /// x = execute /// s = shared /// p = private (copy on write) - perms: [char; 4], + // perms: [u8; 4], /// Offset into the file (or "whatever"). // offset: u64, /// device (major, minor) @@ -115,14 +115,12 @@ impl FromStr for MapsEntry { } else { return Err(parse_err); }; - let perms: [char; 4] = { - let mut chars = perms_str.chars(); - let mut c = || chars.next().ok_or("insufficient perms"); - let perms = [c()?, c()?, c()?, c()?]; - if chars.next().is_some() { - return Err("too many perms"); - } - perms + let _perms = if let &[r, w, x, p, ..] = perms_str.as_bytes() { + // If a system in the future adds a 5th field to the permission list, + // there's no reason to assume previous fields were invalidated. + [r, w, x, p] + } else { + return Err(parse_err); }; let _offset = hex(offset_str)?; let _dev = if let Some((major, minor)) = dev_str.split_once(':') { @@ -135,7 +133,7 @@ impl FromStr for MapsEntry { Ok(MapsEntry { address, - perms, + // perms, // offset, // dev, // inode, @@ -155,7 +153,7 @@ fn check_maps_entry_parsing_64bit() { .unwrap(), MapsEntry { address: (0xffffffffff600000, 0xffffffffff601000), - perms: ['-', '-', 'x', 'p'], + // perms: *b"--xp", // offset: 0x00000000, // dev: (0x00, 0x00), // inode: 0x0, @@ -170,7 +168,7 @@ fn check_maps_entry_parsing_64bit() { .unwrap(), MapsEntry { address: (0x7f5985f46000, 0x7f5985f48000), - perms: ['r', 'w', '-', 'p'], + // perms: *b"rw-p", // offset: 0x00039000, // dev: (0x103, 0x06), // inode: 0x76021795, @@ -183,7 +181,7 @@ fn check_maps_entry_parsing_64bit() { .unwrap(), MapsEntry { address: (0x35b1a21000, 0x35b1a22000), - perms: ['r', 'w', '-', 'p'], + // perms: *b"rw-p", // offset: 0x00000000, // dev: (0x00, 0x00), // inode: 0x0, @@ -207,7 +205,7 @@ fn check_maps_entry_parsing_32bit() { .unwrap(), MapsEntry { address: (0x08056000, 0x08077000), - perms: ['r', 'w', '-', 'p'], + // perms: *b"rw-p", // offset: 0x00000000, // dev: (0x00, 0x00), // inode: 0x0, @@ -222,7 +220,7 @@ fn check_maps_entry_parsing_32bit() { .unwrap(), MapsEntry { address: (0xb7c79000, 0xb7e02000), - perms: ['r', '-', '-', 'p'], + // perms: *b"r--p", // offset: 0x00000000, // dev: (0x08, 0x01), // inode: 0x60662705, @@ -235,7 +233,7 @@ fn check_maps_entry_parsing_32bit() { .unwrap(), MapsEntry { address: (0xb7e02000, 0xb7e03000), - perms: ['r', 'w', '-', 'p'], + // perms: *b"rw-p", // offset: 0x00000000, // dev: (0x00, 0x00), // inode: 0x0, From 44c746116ec8bdbe623ea435d00251dbc9773e82 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 17:58:10 -0700 Subject: [PATCH 14/19] only one err for io fail --- src/symbolize/gimli/parse_running_mmaps_unix.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index b10fdb48..40963e82 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -56,13 +56,14 @@ pub(super) struct MapsEntry { } pub(super) fn parse_maps() -> Result, &'static str> { + let failed_io_err = "couldn't read /proc/self/maps"; let mut v = Vec::new(); let mut proc_self_maps = - File::open("/proc/self/maps").map_err(|_| "Couldn't open /proc/self/maps")?; + File::open("/proc/self/maps").map_err(|_| failed_io_err)?; let mut buf = String::new(); let _bytes_read = proc_self_maps .read_to_string(&mut buf) - .map_err(|_| "Couldn't read /proc/self/maps")?; + .map_err(|_| failed_io_err)?; for line in buf.lines() { v.push(line.parse()?); } From 491ea5bf3a8053bc188560c3003167330031ec3f Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 17:58:51 -0700 Subject: [PATCH 15/19] Use a superpub field in MapsEntry This may eliminate some of the overhead of a getter. Also use a slightly more modern code style. --- src/symbolize/gimli/libs_dl_iterate_phdr.rs | 7 +++---- src/symbolize/gimli/parse_running_mmaps_unix.rs | 7 +------ 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/symbolize/gimli/libs_dl_iterate_phdr.rs b/src/symbolize/gimli/libs_dl_iterate_phdr.rs index d52819f6..ef995f5c 100644 --- a/src/symbolize/gimli/libs_dl_iterate_phdr.rs +++ b/src/symbolize/gimli/libs_dl_iterate_phdr.rs @@ -36,10 +36,9 @@ fn infer_current_exe( #[cfg(not(target_os = "hurd"))] if let Some(entries) = maps { let opt_path = entries - .iter() - .find(|e| e.ip_matches(base_addr) && e.pathname().len() > 0) - .map(|e| e.pathname()) - .cloned(); + .into_iter() + .find(|e| e.ip_matches(base_addr) && !e.pathname.is_empty()) + .map(|e| e.pathname); if let Some(path) = opt_path { return path; } diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index 40963e82..26bbda0a 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -52,7 +52,7 @@ pub(super) struct MapsEntry { /// in general the pathname may be ambiguous. (I.e. you cannot tell if the /// denoted filename actually ended with the text "(deleted)", or if that /// was added by the maps rendering. - pathname: OsString, + pub(super) pathname: OsString, } pub(super) fn parse_maps() -> Result, &'static str> { @@ -72,11 +72,6 @@ pub(super) fn parse_maps() -> Result, &'static str> { } impl MapsEntry { - #[inline] - pub(super) fn pathname(&self) -> &OsString { - &self.pathname - } - #[inline] pub(super) fn ip_matches(&self, ip: usize) -> bool { self.address.0 <= ip && ip < self.address.1 From 39ad27673b221c063e352ed974ee87e57c237f78 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 18:13:53 -0700 Subject: [PATCH 16/19] Revert "Use a superpub field in MapsEntry" This reverts commit 06ccd70da9f3367fce2842cab5cfa3212d0cbea5. --- src/symbolize/gimli/libs_dl_iterate_phdr.rs | 7 ++++--- src/symbolize/gimli/parse_running_mmaps_unix.rs | 7 ++++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/symbolize/gimli/libs_dl_iterate_phdr.rs b/src/symbolize/gimli/libs_dl_iterate_phdr.rs index ef995f5c..d52819f6 100644 --- a/src/symbolize/gimli/libs_dl_iterate_phdr.rs +++ b/src/symbolize/gimli/libs_dl_iterate_phdr.rs @@ -36,9 +36,10 @@ fn infer_current_exe( #[cfg(not(target_os = "hurd"))] if let Some(entries) = maps { let opt_path = entries - .into_iter() - .find(|e| e.ip_matches(base_addr) && !e.pathname.is_empty()) - .map(|e| e.pathname); + .iter() + .find(|e| e.ip_matches(base_addr) && e.pathname().len() > 0) + .map(|e| e.pathname()) + .cloned(); if let Some(path) = opt_path { return path; } diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index 26bbda0a..40963e82 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -52,7 +52,7 @@ pub(super) struct MapsEntry { /// in general the pathname may be ambiguous. (I.e. you cannot tell if the /// denoted filename actually ended with the text "(deleted)", or if that /// was added by the maps rendering. - pub(super) pathname: OsString, + pathname: OsString, } pub(super) fn parse_maps() -> Result, &'static str> { @@ -72,6 +72,11 @@ pub(super) fn parse_maps() -> Result, &'static str> { } impl MapsEntry { + #[inline] + pub(super) fn pathname(&self) -> &OsString { + &self.pathname + } + #[inline] pub(super) fn ip_matches(&self, ip: usize) -> bool { self.address.0 <= ip && ip < self.address.1 From e3cab69817bcffd55a5525271df45c402826b20c Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 18:38:27 -0700 Subject: [PATCH 17/19] Try turning && to & --- src/symbolize/gimli/elf.rs | 8 ++++---- src/symbolize/gimli/parse_running_mmaps_unix.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/symbolize/gimli/elf.rs b/src/symbolize/gimli/elf.rs index a3ddc9df..64e0c83b 100644 --- a/src/symbolize/gimli/elf.rs +++ b/src/symbolize/gimli/elf.rs @@ -257,7 +257,7 @@ impl<'a> Object<'a> { .iter() .filter_map(|header| { let name = self.sections.section_name(self.endian, header).ok()?; - if name.starts_with(b".zdebug_") && &name[8..] == debug_name { + if name.starts_with(b".zdebug_") & (&name[8..] == debug_name) { Some(header) } else { None @@ -287,7 +287,7 @@ impl<'a> Object<'a> { Err(i) => i.checked_sub(1)?, }; let sym = self.syms.get(i)?; - if sym.address <= addr && addr <= sym.address + sym.size { + if (sym.address <= addr) & (addr <= sym.address + sym.size) { self.strings.get(sym.name).ok() } else { None @@ -302,7 +302,7 @@ impl<'a> Object<'a> { for section in self.sections.iter() { if let Ok(Some(mut notes)) = section.notes(self.endian, self.data) { while let Ok(Some(note)) = notes.next() { - if note.name() == ELF_NOTE_GNU && note.n_type(self.endian) == NT_GNU_BUILD_ID { + if (note.name() == ELF_NOTE_GNU) & (note.n_type(self.endian) == NT_GNU_BUILD_ID) { return Some(note.desc()); } } @@ -353,7 +353,7 @@ fn decompress_zlib(input: &[u8], output: &mut [u8]) -> Option<()> { 0, TINFL_FLAG_USING_NON_WRAPPING_OUTPUT_BUF | TINFL_FLAG_PARSE_ZLIB_HEADER, ); - if status == TINFLStatus::Done && in_read == input.len() && out_read == output.len() { + if (status == TINFLStatus::Done) & (in_read == input.len()) & (out_read == output.len()) { Some(()) } else { None diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index 40963e82..bdde1755 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -79,7 +79,7 @@ impl MapsEntry { #[inline] pub(super) fn ip_matches(&self, ip: usize) -> bool { - self.address.0 <= ip && ip < self.address.1 + (self.address.0 <= ip) & (ip < self.address.1) } #[cfg(target_os = "android")] From 9240bd0125e874bb186b9e76340a3d906c347620 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 23:10:28 -0700 Subject: [PATCH 18/19] Revert "Try turning && to &" This reverts commit 3a4e3087ea0abb7df0781a5a2c0e49bf3234d610. --- src/symbolize/gimli/elf.rs | 8 ++++---- src/symbolize/gimli/parse_running_mmaps_unix.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/symbolize/gimli/elf.rs b/src/symbolize/gimli/elf.rs index 64e0c83b..a3ddc9df 100644 --- a/src/symbolize/gimli/elf.rs +++ b/src/symbolize/gimli/elf.rs @@ -257,7 +257,7 @@ impl<'a> Object<'a> { .iter() .filter_map(|header| { let name = self.sections.section_name(self.endian, header).ok()?; - if name.starts_with(b".zdebug_") & (&name[8..] == debug_name) { + if name.starts_with(b".zdebug_") && &name[8..] == debug_name { Some(header) } else { None @@ -287,7 +287,7 @@ impl<'a> Object<'a> { Err(i) => i.checked_sub(1)?, }; let sym = self.syms.get(i)?; - if (sym.address <= addr) & (addr <= sym.address + sym.size) { + if sym.address <= addr && addr <= sym.address + sym.size { self.strings.get(sym.name).ok() } else { None @@ -302,7 +302,7 @@ impl<'a> Object<'a> { for section in self.sections.iter() { if let Ok(Some(mut notes)) = section.notes(self.endian, self.data) { while let Ok(Some(note)) = notes.next() { - if (note.name() == ELF_NOTE_GNU) & (note.n_type(self.endian) == NT_GNU_BUILD_ID) { + if note.name() == ELF_NOTE_GNU && note.n_type(self.endian) == NT_GNU_BUILD_ID { return Some(note.desc()); } } @@ -353,7 +353,7 @@ fn decompress_zlib(input: &[u8], output: &mut [u8]) -> Option<()> { 0, TINFL_FLAG_USING_NON_WRAPPING_OUTPUT_BUF | TINFL_FLAG_PARSE_ZLIB_HEADER, ); - if (status == TINFLStatus::Done) & (in_read == input.len()) & (out_read == output.len()) { + if status == TINFLStatus::Done && in_read == input.len() && out_read == output.len() { Some(()) } else { None diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index bdde1755..40963e82 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -79,7 +79,7 @@ impl MapsEntry { #[inline] pub(super) fn ip_matches(&self, ip: usize) -> bool { - (self.address.0 <= ip) & (ip < self.address.1) + self.address.0 <= ip && ip < self.address.1 } #[cfg(target_os = "android")] From cd4254ae0a7ca4585941b1aa85cd6646b4afa662 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 23:27:55 -0700 Subject: [PATCH 19/19] fmt --- src/symbolize/gimli/parse_running_mmaps_unix.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index 40963e82..eecc1a6e 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -58,8 +58,7 @@ pub(super) struct MapsEntry { pub(super) fn parse_maps() -> Result, &'static str> { let failed_io_err = "couldn't read /proc/self/maps"; let mut v = Vec::new(); - let mut proc_self_maps = - File::open("/proc/self/maps").map_err(|_| failed_io_err)?; + let mut proc_self_maps = File::open("/proc/self/maps").map_err(|_| failed_io_err)?; let mut buf = String::new(); let _bytes_read = proc_self_maps .read_to_string(&mut buf) @@ -101,8 +100,7 @@ impl FromStr for MapsEntry { fn from_str(s: &str) -> Result { let missing_field = "failed to find all map fields"; let parse_err = "failed to parse all map fields"; - let mut parts = s - .split_ascii_whitespace(); + let mut parts = s.split_ascii_whitespace(); let range_str = parts.next().ok_or(missing_field)?; let perms_str = parts.next().ok_or(missing_field)?; let offset_str = parts.next().ok_or(missing_field)?;