From 25bddbf8dd06db638e91391248127ebbdf130b78 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sun, 2 Jul 2023 00:14:45 -0700 Subject: [PATCH 01/27] 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 deeeb2971..38405a4aa 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -18,7 +18,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: usize, /// device (major, minor) @@ -102,14 +102,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 = hex(offset_str)?; let dev = if let Some((major, minor)) = dev_str.split_once(':') { @@ -142,7 +140,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, @@ -157,7 +155,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, @@ -170,7 +168,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, @@ -194,7 +192,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, @@ -209,7 +207,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, @@ -222,7 +220,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 8b391224770b6aa1a00b5be9e7e230263b4203f3 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 15:34:53 -0700 Subject: [PATCH 02/27] 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 38405a4aa..252478814 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -18,13 +18,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: usize, + // offset: usize, /// 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 @@ -102,28 +102,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 = hex(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, }) } @@ -140,10 +140,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(), } ); @@ -155,10 +155,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(), } ); @@ -168,10 +168,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(), } ); @@ -192,10 +192,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(), } ); @@ -207,10 +207,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(), } ); @@ -220,10 +220,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 d8103177abd805f7300251138b66c0a62326ebe8 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 15:47:54 -0700 Subject: [PATCH 03/27] Use opt-level=3 --- .github/workflows/check-binary-size.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check-binary-size.yml b/.github/workflows/check-binary-size.yml index 5104b6d88..32763912f 100644 --- a/.github/workflows/check-binary-size.yml +++ b/.github/workflows/check-binary-size.yml @@ -41,7 +41,7 @@ jobs: python3 x.py build library --stage 0 cp -r ./build/x86_64-unknown-linux-gnu/stage0/bin ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin cp -r ./build/x86_64-unknown-linux-gnu/stage0/lib/*.so ./build/x86_64-unknown-linux-gnu/stage0-sysroot/lib - ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin/rustc -O foo.rs -o binary-reference + ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin/rustc -Copt-level=3 foo.rs -o binary-reference - name: Build binary with PR version of backtrace run: | cd library/backtrace @@ -52,7 +52,7 @@ jobs: python3 x.py build library --stage 0 cp -r ./build/x86_64-unknown-linux-gnu/stage0/bin ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin cp -r ./build/x86_64-unknown-linux-gnu/stage0/lib/*.so ./build/x86_64-unknown-linux-gnu/stage0-sysroot/lib - ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin/rustc -O foo.rs -o binary-updated + ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin/rustc -Copt-level=3 foo.rs -o binary-updated - name: Display binary size run: | ls -la binary-* From ca8c99d16b06cb52f7b3d6365f15dc8a3ca3bf80 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 15:55:38 -0700 Subject: [PATCH 04/27] Omit needless error strings --- .../gimli/parse_running_mmaps_unix.rs | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index 252478814..10f7f61c5 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -86,34 +86,36 @@ impl FromStr for MapsEntry { // e.g.: "7f5985f46000-7f5985f48000 rw-p 00039000 103:06 76021795 /usr/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2" // e.g.: "35b1a21000-35b1a22000 rw-p 00000000 00:00 0" 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(' ') // 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("Couldn't find address")?; - let perms_str = parts.next().ok_or("Couldn't find permissions")?; - let offset_str = parts.next().ok_or("Couldn't find offset")?; - let dev_str = parts.next().ok_or("Couldn't find dev")?; - let inode_str = parts.next().ok_or("Couldn't find inode")?; + 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(|_| "Couldn't parse hex number"); + 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 6ff19b3c2ee37429d23847f3c6214138821a5543 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 16:01:51 -0700 Subject: [PATCH 05/27] Try adding inlining --- src/symbolize/gimli/parse_running_mmaps_unix.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index 10f7f61c5..4c526c535 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -69,10 +69,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 } @@ -85,6 +87,7 @@ impl FromStr for MapsEntry { // e.g.: "ffffffffff600000-ffffffffff601000 --xp 00000000 00:00 0 [vsyscall]" // e.g.: "7f5985f46000-7f5985f48000 rw-p 00039000 103:06 76021795 /usr/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2" // e.g.: "35b1a21000-35b1a22000 rw-p 00000000 00:00 0" + #[inline] fn from_str(s: &str) -> Result { let missing_field = "failed to find all map fields"; let parse_err = "failed to parse all map fields"; From 572bffa6f5d31aadcee8b95b9cf0250ac4837425 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 16:04:02 -0700 Subject: [PATCH 06/27] Temporarily disable most of CI to be nice --- .github/workflows/main.yml | 206 ++++++++++++++++++------------------- 1 file changed, 103 insertions(+), 103 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 29fff2795..00a195e7f 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -22,18 +22,18 @@ jobs: rust: beta - os: ubuntu-20.04 rust: nightly - - os: macos-latest - rust: stable - - os: macos-latest - rust: nightly + # - os: macos-latest + # rust: stable + # - os: macos-latest + # rust: nightly # Note that these are on nightly due to rust-lang/rust#63700 not being # on stable yet - - os: windows-latest - rust: stable-x86_64-msvc - - os: windows-latest - rust: stable-i686-msvc - - os: windows-latest - rust: stable-x86_64-gnu + # - os: windows-latest + # rust: stable-x86_64-msvc + # - os: windows-latest + # rust: stable-i686-msvc + # - os: windows-latest + # rust: stable-x86_64-gnu steps: - uses: actions/checkout@v3 with: @@ -115,77 +115,77 @@ jobs: - run: cargo build --manifest-path crates/as-if-std/Cargo.toml - run: cargo build --manifest-path crates/as-if-std/Cargo.toml --no-default-features - windows_arm64: - name: Windows AArch64 - runs-on: windows-latest - steps: - - uses: actions/checkout@v3 - with: - submodules: true - - name: Install Rust - run: rustup update stable --no-self-update && rustup default stable - shell: bash - - run: echo RUSTFLAGS=-Dwarnings >> $GITHUB_ENV - shell: bash - - run: rustup target add aarch64-pc-windows-msvc - - run: cargo test --no-run --target aarch64-pc-windows-msvc - - run: cargo test --no-run --target aarch64-pc-windows-msvc --features verify-winapi - - ios: - name: iOS - runs-on: macos-latest - strategy: - matrix: - include: - - target: aarch64-apple-ios - sdk: iphoneos - - target: x86_64-apple-ios - sdk: iphonesimulator - steps: - - uses: actions/checkout@v3 - with: - submodules: true - - run: rustup target add ${{ matrix.target }} - - run: | - export RUSTFLAGS=-Dwarnings - export SDK_PATH=`xcrun --show-sdk-path --sdk ${{ matrix.sdk }}` - export RUSTFLAGS="-C link-arg=-isysroot -C link-arg=$SDK_PATH" - cargo test --no-run --target ${{ matrix.target }} - name: Build tests - - docker: - name: Docker - runs-on: ubuntu-20.04 - strategy: - fail-fast: false - matrix: - target: - - aarch64-unknown-linux-gnu - - arm-unknown-linux-gnueabihf - - armv7-unknown-linux-gnueabihf - - i586-unknown-linux-gnu - - i686-unknown-linux-gnu - - powerpc64-unknown-linux-gnu - - s390x-unknown-linux-gnu - - x86_64-pc-windows-gnu - - x86_64-unknown-linux-gnu - - x86_64-unknown-linux-musl - - arm-linux-androideabi - - armv7-linux-androideabi - - aarch64-linux-android - - i686-linux-android - - x86_64-linux-android - steps: - - uses: actions/checkout@v3 - with: - submodules: true - - name: Install Rust - run: rustup update stable && rustup default stable - - run: rustup target add ${{ matrix.target }} - - run: cargo generate-lockfile - - run: echo RUSTFLAGS=-Dwarnings >> $GITHUB_ENV - shell: bash - - run: ./ci/run-docker.sh ${{ matrix.target }} + # windows_arm64: + # name: Windows AArch64 + # runs-on: windows-latest + # steps: + # - uses: actions/checkout@v3 + # with: + # submodules: true + # - name: Install Rust + # run: rustup update stable --no-self-update && rustup default stable + # shell: bash + # - run: echo RUSTFLAGS=-Dwarnings >> $GITHUB_ENV + # shell: bash + # - run: rustup target add aarch64-pc-windows-msvc + # - run: cargo test --no-run --target aarch64-pc-windows-msvc + # - run: cargo test --no-run --target aarch64-pc-windows-msvc --features verify-winapi + + # ios: + # name: iOS + # runs-on: macos-latest + # strategy: + # matrix: + # include: + # - target: aarch64-apple-ios + # sdk: iphoneos + # - target: x86_64-apple-ios + # sdk: iphonesimulator + # steps: + # - uses: actions/checkout@v3 + # with: + # submodules: true + # - run: rustup target add ${{ matrix.target }} + # - run: | + # export RUSTFLAGS=-Dwarnings + # export SDK_PATH=`xcrun --show-sdk-path --sdk ${{ matrix.sdk }}` + # export RUSTFLAGS="-C link-arg=-isysroot -C link-arg=$SDK_PATH" + # cargo test --no-run --target ${{ matrix.target }} + # name: Build tests + + # docker: + # name: Docker + # runs-on: ubuntu-20.04 + # strategy: + # fail-fast: false + # matrix: + # target: + # - aarch64-unknown-linux-gnu + # - arm-unknown-linux-gnueabihf + # - armv7-unknown-linux-gnueabihf + # - i586-unknown-linux-gnu + # - i686-unknown-linux-gnu + # - powerpc64-unknown-linux-gnu + # - s390x-unknown-linux-gnu + # - x86_64-pc-windows-gnu + # - x86_64-unknown-linux-gnu + # - x86_64-unknown-linux-musl + # - arm-linux-androideabi + # - armv7-linux-androideabi + # - aarch64-linux-android + # - i686-linux-android + # - x86_64-linux-android + # steps: + # - uses: actions/checkout@v3 + # with: + # submodules: true + # - name: Install Rust + # run: rustup update stable && rustup default stable + # - run: rustup target add ${{ matrix.target }} + # - run: cargo generate-lockfile + # - run: echo RUSTFLAGS=-Dwarnings >> $GITHUB_ENV + # shell: bash + # - run: ./ci/run-docker.sh ${{ matrix.target }} rustfmt: name: Rustfmt @@ -198,28 +198,28 @@ jobs: run: rustup update stable && rustup default stable && rustup component add rustfmt - run: cargo fmt --all -- --check - build: - name: Build Targets - runs-on: ubuntu-20.04 - strategy: - matrix: - target: - - wasm32-unknown-unknown - - wasm32-wasi - - x86_64-unknown-fuchsia - - x86_64-fortanix-unknown-sgx - - x86_64-unknown-illumos - steps: - - uses: actions/checkout@v3 - with: - submodules: true - - name: Install Rust - run: rustup update nightly && rustup default nightly - - run: rustup target add ${{ matrix.target }} - - run: echo RUSTFLAGS=-Dwarnings >> $GITHUB_ENV - shell: bash - - run: cargo build --target ${{ matrix.target }} - - run: cargo build --manifest-path crates/as-if-std/Cargo.toml --target ${{ matrix.target }} + # build: + # name: Build Targets + # runs-on: ubuntu-20.04 + # strategy: + # matrix: + # target: + # - wasm32-unknown-unknown + # - wasm32-wasi + # - x86_64-unknown-fuchsia + # - x86_64-fortanix-unknown-sgx + # - x86_64-unknown-illumos + # steps: + # - uses: actions/checkout@v3 + # with: + # submodules: true + # - name: Install Rust + # run: rustup update nightly && rustup default nightly + # - run: rustup target add ${{ matrix.target }} + # - run: echo RUSTFLAGS=-Dwarnings >> $GITHUB_ENV + # shell: bash + # - run: cargo build --target ${{ matrix.target }} + # - run: cargo build --manifest-path crates/as-if-std/Cargo.toml --target ${{ matrix.target }} msrv: name: MSRV From 9df8ebbd84601fc81932ad47877bc7ca40235ffa Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 16:09:14 -0700 Subject: [PATCH 07/27] Revert "Try adding inlining" This reverts commit 6ff19b3c2ee37429d23847f3c6214138821a5543. --- src/symbolize/gimli/parse_running_mmaps_unix.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/symbolize/gimli/parse_running_mmaps_unix.rs b/src/symbolize/gimli/parse_running_mmaps_unix.rs index 4c526c535..10f7f61c5 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -69,12 +69,10 @@ 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 } @@ -87,7 +85,6 @@ impl FromStr for MapsEntry { // e.g.: "ffffffffff600000-ffffffffff601000 --xp 00000000 00:00 0 [vsyscall]" // e.g.: "7f5985f46000-7f5985f48000 rw-p 00039000 103:06 76021795 /usr/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2" // e.g.: "35b1a21000-35b1a22000 rw-p 00000000 00:00 0" - #[inline] fn from_str(s: &str) -> Result { let missing_field = "failed to find all map fields"; let parse_err = "failed to parse all map fields"; From cb775f119cbb04a9929921d9f61ec69e54e7050a Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 16:27:09 -0700 Subject: [PATCH 08/27] 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 10f7f61c5..0756214ed 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -89,8 +89,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 1166cc8b4927450dae10310bd9a65fd6adad4d53 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 16:36:33 -0700 Subject: [PATCH 09/27] 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 0756214ed..d7a1c6888 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -103,20 +103,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 923e09095920b4df43947b75b4a28eb9054c1c8d Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 16:56:54 -0700 Subject: [PATCH 10/27] 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 d7a1c6888..0756214ed 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -103,20 +103,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 cc6caa0396c0bbab997f0379da95a8eb43cd2fc6 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 16:57:22 -0700 Subject: [PATCH 11/27] Revert "Use opt-level=3" This reverts commit d8103177abd805f7300251138b66c0a62326ebe8. --- .github/workflows/check-binary-size.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check-binary-size.yml b/.github/workflows/check-binary-size.yml index 32763912f..5104b6d88 100644 --- a/.github/workflows/check-binary-size.yml +++ b/.github/workflows/check-binary-size.yml @@ -41,7 +41,7 @@ jobs: python3 x.py build library --stage 0 cp -r ./build/x86_64-unknown-linux-gnu/stage0/bin ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin cp -r ./build/x86_64-unknown-linux-gnu/stage0/lib/*.so ./build/x86_64-unknown-linux-gnu/stage0-sysroot/lib - ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin/rustc -Copt-level=3 foo.rs -o binary-reference + ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin/rustc -O foo.rs -o binary-reference - name: Build binary with PR version of backtrace run: | cd library/backtrace @@ -52,7 +52,7 @@ jobs: python3 x.py build library --stage 0 cp -r ./build/x86_64-unknown-linux-gnu/stage0/bin ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin cp -r ./build/x86_64-unknown-linux-gnu/stage0/lib/*.so ./build/x86_64-unknown-linux-gnu/stage0-sysroot/lib - ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin/rustc -Copt-level=3 foo.rs -o binary-updated + ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin/rustc -O foo.rs -o binary-updated - name: Display binary size run: | ls -la binary-* From 6a05e64a88648e274e3ba432ad28d46fcf337b1a Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 17:01:38 -0700 Subject: [PATCH 12/27] 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 0756214ed..07c6c4446 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -69,10 +69,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 9bc29a0e9a35ff2b26e59caaba27ff990fe5f9d1 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 17:13:51 -0700 Subject: [PATCH 13/27] 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 07c6c4446..c848536b6 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -18,7 +18,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: usize, /// device (major, minor) @@ -105,12 +105,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(':') { @@ -123,7 +125,7 @@ impl FromStr for MapsEntry { Ok(MapsEntry { address, - // perms, + perms, // offset, // dev, // inode, @@ -143,7 +145,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, @@ -158,7 +160,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, @@ -171,7 +173,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, @@ -195,7 +197,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, @@ -210,7 +212,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, @@ -223,7 +225,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 9c00af84ee412eeb0f36a95b3eaff233ede04db4 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 17:24:58 -0700 Subject: [PATCH 14/27] 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 c848536b6..538cb1850 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -18,7 +18,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: usize, /// device (major, minor) @@ -105,7 +105,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()?]; @@ -125,7 +125,7 @@ impl FromStr for MapsEntry { Ok(MapsEntry { address, - perms, + // perms, // offset, // dev, // inode, From 1d63a5c5066c6dbcaf77f540799c96cdaeb5bf2f Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 17:26:37 -0700 Subject: [PATCH 15/27] 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 538cb1850..7f26f7018 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -145,7 +145,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, @@ -160,7 +160,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, @@ -173,7 +173,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, @@ -197,7 +197,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, @@ -212,7 +212,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, @@ -225,7 +225,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 65100f1ffc611b22ced6fcaf7d41d49794547402 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 17:31:37 -0700 Subject: [PATCH 16/27] Use 1 codegen unit to eliminate variance --- .github/workflows/check-binary-size.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check-binary-size.yml b/.github/workflows/check-binary-size.yml index 5104b6d88..dfd64fe48 100644 --- a/.github/workflows/check-binary-size.yml +++ b/.github/workflows/check-binary-size.yml @@ -41,7 +41,7 @@ jobs: python3 x.py build library --stage 0 cp -r ./build/x86_64-unknown-linux-gnu/stage0/bin ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin cp -r ./build/x86_64-unknown-linux-gnu/stage0/lib/*.so ./build/x86_64-unknown-linux-gnu/stage0-sysroot/lib - ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin/rustc -O foo.rs -o binary-reference + ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin/rustc -O -Ccodegen-units=1 foo.rs -o binary-reference - name: Build binary with PR version of backtrace run: | cd library/backtrace @@ -52,7 +52,7 @@ jobs: python3 x.py build library --stage 0 cp -r ./build/x86_64-unknown-linux-gnu/stage0/bin ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin cp -r ./build/x86_64-unknown-linux-gnu/stage0/lib/*.so ./build/x86_64-unknown-linux-gnu/stage0-sysroot/lib - ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin/rustc -O foo.rs -o binary-updated + ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin/rustc -O -Ccodegen-units=1 foo.rs -o binary-updated - name: Display binary size run: | ls -la binary-* From 24c081177fa858331eb6e5f5185df348cf397571 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 17:36:57 -0700 Subject: [PATCH 17/27] Revert "Use 1 codegen unit to eliminate variance" This reverts commit 65100f1ffc611b22ced6fcaf7d41d49794547402. It did nothing, lol. --- .github/workflows/check-binary-size.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check-binary-size.yml b/.github/workflows/check-binary-size.yml index dfd64fe48..5104b6d88 100644 --- a/.github/workflows/check-binary-size.yml +++ b/.github/workflows/check-binary-size.yml @@ -41,7 +41,7 @@ jobs: python3 x.py build library --stage 0 cp -r ./build/x86_64-unknown-linux-gnu/stage0/bin ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin cp -r ./build/x86_64-unknown-linux-gnu/stage0/lib/*.so ./build/x86_64-unknown-linux-gnu/stage0-sysroot/lib - ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin/rustc -O -Ccodegen-units=1 foo.rs -o binary-reference + ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin/rustc -O foo.rs -o binary-reference - name: Build binary with PR version of backtrace run: | cd library/backtrace @@ -52,7 +52,7 @@ jobs: python3 x.py build library --stage 0 cp -r ./build/x86_64-unknown-linux-gnu/stage0/bin ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin cp -r ./build/x86_64-unknown-linux-gnu/stage0/lib/*.so ./build/x86_64-unknown-linux-gnu/stage0-sysroot/lib - ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin/rustc -O -Ccodegen-units=1 foo.rs -o binary-updated + ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin/rustc -O foo.rs -o binary-updated - name: Display binary size run: | ls -la binary-* From 16b5f6eef7403fb2970817c8e11d7d2a99dadc51 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 17:38:21 -0700 Subject: [PATCH 18/27] 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 7f26f7018..e1446d717 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -18,7 +18,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: usize, /// device (major, minor) @@ -105,7 +105,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()?]; @@ -125,7 +125,7 @@ impl FromStr for MapsEntry { Ok(MapsEntry { address, - // perms, + perms, // offset, // dev, // inode, From 268a18da6dddedec47d2bcca2442993bd8d95f2b Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 17:38:30 -0700 Subject: [PATCH 19/27] 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 e1446d717..c848536b6 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -145,7 +145,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, @@ -160,7 +160,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, @@ -173,7 +173,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, @@ -197,7 +197,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, @@ -212,7 +212,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, @@ -225,7 +225,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 dca80ea72431f4950ec7325ea0648109dc07cc8b Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 17:38:38 -0700 Subject: [PATCH 20/27] 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 c848536b6..07c6c4446 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -18,7 +18,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: usize, /// device (major, minor) @@ -105,14 +105,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(':') { @@ -125,7 +123,7 @@ impl FromStr for MapsEntry { Ok(MapsEntry { address, - perms, + // perms, // offset, // dev, // inode, @@ -145,7 +143,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, @@ -160,7 +158,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, @@ -173,7 +171,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, @@ -197,7 +195,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, @@ -212,7 +210,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, @@ -225,7 +223,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 e48b001d4efe5491981e5614caa2f94cc91028ff Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 17:58:10 -0700 Subject: [PATCH 21/27] 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 07c6c4446..b51476df9 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -54,13 +54,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 06ccd70da9f3367fce2842cab5cfa3212d0cbea5 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 17:58:51 -0700 Subject: [PATCH 22/27] 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 9f0304ce8..432e2f163 100644 --- a/src/symbolize/gimli/libs_dl_iterate_phdr.rs +++ b/src/symbolize/gimli/libs_dl_iterate_phdr.rs @@ -20,10 +20,9 @@ pub(super) fn native_libraries() -> Vec { fn infer_current_exe(base_addr: usize) -> OsString { if let Ok(entries) = super::parse_running_mmaps::parse_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 b51476df9..da7f3a9b2 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -50,7 +50,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> { @@ -70,11 +70,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 6c7e3ace279941ddd668c496be8d9add42387c7c Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 18:13:53 -0700 Subject: [PATCH 23/27] 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 432e2f163..9f0304ce8 100644 --- a/src/symbolize/gimli/libs_dl_iterate_phdr.rs +++ b/src/symbolize/gimli/libs_dl_iterate_phdr.rs @@ -20,9 +20,10 @@ pub(super) fn native_libraries() -> Vec { fn infer_current_exe(base_addr: usize) -> OsString { if let Ok(entries) = super::parse_running_mmaps::parse_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 da7f3a9b2..b51476df9 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -50,7 +50,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> { @@ -70,6 +70,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 3a4e3087ea0abb7df0781a5a2c0e49bf3234d610 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 18:38:27 -0700 Subject: [PATCH 24/27] 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 b0eec0762..ad855373c 100644 --- a/src/symbolize/gimli/elf.rs +++ b/src/symbolize/gimli/elf.rs @@ -201,7 +201,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 @@ -231,7 +231,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 @@ -246,7 +246,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()); } } @@ -297,7 +297,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 b51476df9..0a95833eb 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -77,7 +77,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) } } From 921ea5ad92a43b357ec40f56d75db4f658ee34a5 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 23:10:28 -0700 Subject: [PATCH 25/27] 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 ad855373c..b0eec0762 100644 --- a/src/symbolize/gimli/elf.rs +++ b/src/symbolize/gimli/elf.rs @@ -201,7 +201,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 @@ -231,7 +231,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 @@ -246,7 +246,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()); } } @@ -297,7 +297,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 0a95833eb..b51476df9 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -77,7 +77,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 } } From f90ee25ac3757e47c6169898efb151e21ee4ddbb Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 23:27:55 -0700 Subject: [PATCH 26/27] 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 b51476df9..9805d0030 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -56,8 +56,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) @@ -91,8 +90,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)?; From b92869e1195ebf2503a5e393b871139e7ff2f87f Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 6 Jul 2023 23:30:27 -0700 Subject: [PATCH 27/27] Experiment with a bufreader --- src/symbolize/gimli/libs_dl_iterate_phdr.rs | 7 ++-- .../gimli/parse_running_mmaps_unix.rs | 34 +++++++++---------- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/symbolize/gimli/libs_dl_iterate_phdr.rs b/src/symbolize/gimli/libs_dl_iterate_phdr.rs index 9f0304ce8..a5f6b92c5 100644 --- a/src/symbolize/gimli/libs_dl_iterate_phdr.rs +++ b/src/symbolize/gimli/libs_dl_iterate_phdr.rs @@ -18,12 +18,11 @@ pub(super) fn native_libraries() -> Vec { } fn infer_current_exe(base_addr: usize) -> OsString { - if let Ok(entries) = super::parse_running_mmaps::parse_maps() { + if let Some(entries) = super::parse_running_mmaps::parse_maps() { let opt_path = entries - .iter() + .filter_map(|e| e.ok()) .find(|e| e.ip_matches(base_addr) && e.pathname().len() > 0) - .map(|e| e.pathname()) - .cloned(); + .map(|e| e.pathname().clone()); 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 9805d0030..87ebf1c91 100644 --- a/src/symbolize/gimli/parse_running_mmaps_unix.rs +++ b/src/symbolize/gimli/parse_running_mmaps_unix.rs @@ -3,9 +3,9 @@ // general purpose, but it hasn't been tested elsewhere. use super::mystd::fs::File; -use super::mystd::io::Read; +use super::mystd::io::{self, BufRead, BufReader, ErrorKind}; use super::mystd::str::FromStr; -use super::{OsString, String, Vec}; +use super::OsString; #[derive(PartialEq, Eq, Debug)] pub(super) struct MapsEntry { @@ -53,19 +53,17 @@ pub(super) struct MapsEntry { pathname: OsString, } -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 buf = String::new(); - let _bytes_read = proc_self_maps - .read_to_string(&mut buf) - .map_err(|_| failed_io_err)?; - for line in buf.lines() { - v.push(line.parse()?); - } - - Ok(v) +pub(super) fn parse_maps() -> Option>> { + let proc_self_maps = match File::open("/proc/self/maps") { + Ok(f) => f, + Err(_) => return None, + }; + let buf_read = BufReader::new(proc_self_maps); + Some( + buf_read + .lines() + .map(|res| res.and_then(|s| s.parse().map_err(|e| io::Error::from(e)))), + ) } impl MapsEntry { @@ -81,15 +79,15 @@ impl MapsEntry { } impl FromStr for MapsEntry { - type Err = &'static str; + type Err = io::ErrorKind; // Format: address perms offset dev inode pathname // e.g.: "ffffffffff600000-ffffffffff601000 --xp 00000000 00:00 0 [vsyscall]" // e.g.: "7f5985f46000-7f5985f48000 rw-p 00039000 103:06 76021795 /usr/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2" // e.g.: "35b1a21000-35b1a22000 rw-p 00000000 00:00 0" 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 missing_field = ErrorKind::NotFound; + let parse_err = ErrorKind::InvalidData; 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)?;