Repository: uutils/procps
Pull request #263 (2024-11-04 08:02:41)

ci: use `-Cinstrument-coverage` instead of `-Zprofile`
Trying to fix the broken code coverage in the CI.


Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 4b19ee7..53ada8b 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -69,21 +69,16 @@ jobs:
 
     - name: rust toolchain ~ install
       uses: dtolnay/rust-toolchain@nightly
-    - name: set RUSTFLAGS
-      shell: bash
-      run: |
-        if [ "$RUNNER_OS" == "Windows" ]; then
-          # -Clink-dead-code conflicts with sysinfo crate and leads to STATUS_DLL_NOT_FOUND error, so we don't use it on Windows
-          echo "RUSTFLAGS=-Zprofile -Ccodegen-units=1 -Copt-level=0 -Coverflow-checks=off -Zpanic_abort_tests -Cpanic=abort" >> $GITHUB_ENV
-        else
-          echo "RUSTFLAGS=-Zprofile -Ccodegen-units=1 -Copt-level=0 -Clink-dead-code -Coverflow-checks=off -Zpanic_abort_tests -Cpanic=abort" >> $GITHUB_ENV
-        fi
+      with:
+        components: llvm-tools-preview
     - name: Test
       run: cargo test --no-fail-fast
       env:
         CARGO_INCREMENTAL: "0"
         RUSTC_WRAPPER: ""
+        RUSTFLAGS: "-Cinstrument-coverage -Zcoverage-options=branch -Ccodegen-units=1 -Copt-level=0 -Coverflow-checks=off -Zpanic_abort_tests -Cpanic=abort"
         RUSTDOCFLAGS: "-Cpanic=abort"
+        LLVM_PROFILE_FILE: "procps-%p-%m.profraw"
     - name: "`grcov` ~ install"
       id: build_grcov
       shell: bash
@@ -111,9 +106,9 @@ jobs:
         COVERAGE_REPORT_FILE="${COVERAGE_REPORT_DIR}/lcov.info"
         mkdir -p "${COVERAGE_REPORT_DIR}"
         # display coverage files
-        grcov . --output-type files --ignore build.rs --ignore "vendor/*" --ignore "/*" --ignore "[a-zA-Z]:/*" --excl-br-line "^\s*((debug_)?assert(_eq|_ne)?!|#\[derive\()" | sort --unique
+        grcov . --binary-path="${COVERAGE_REPORT_DIR}" --output-type files --ignore build.rs --ignore "vendor/*" --ignore "/*" --ignore "[a-zA-Z]:/*" --excl-br-line "^\s*((debug_)?assert(_eq|_ne)?!|#\[derive\()" | sort --unique
         # generate coverage report
-        grcov . --output-type lcov --output-path "${COVERAGE_REPORT_FILE}" --branch --ignore build.rs --ignore "vendor/*" --ignore "/*" --ignore "[a-zA-Z]:/*" --excl-br-line "^\s*((debug_)?assert(_eq|_ne)?!|#\[derive\()"
+        grcov . --binary-path="${COVERAGE_REPORT_DIR}" --output-type lcov --output-path "${COVERAGE_REPORT_FILE}" --branch --ignore build.rs --ignore "vendor/*" --ignore "/*" --ignore "[a-zA-Z]:/*" --excl-br-line "^\s*((debug_)?assert(_eq|_ne)?!|#\[derive\()"
         echo "report=${COVERAGE_REPORT_FILE}" >> $GITHUB_OUTPUT
     - name: Upload coverage results (to Codecov.io)
       uses: codecov/codecov-action@v4
diff --git a/src/uu/pmap/src/maps_format_parser.rs b/src/uu/pmap/src/maps_format_parser.rs
index aee8d64..faaa3cc 100644
--- a/src/uu/pmap/src/maps_format_parser.rs
+++ b/src/uu/pmap/src/maps_format_parser.rs
@@ -3,59 +3,17 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
-use std::fmt;
 use std::io::{Error, ErrorKind};
 
-// Represents a parsed single line from /proc/<PID>/maps for the default and device formats. It
-// omits the inode information because it's not used by those formats.
+// Represents a parsed single line from /proc/<PID>/maps.
 #[derive(Debug, PartialEq)]
 pub struct MapLine {
     pub address: String,
     pub size_in_kb: u64,
-    pub perms: Perms,
-    pub offset: String,
-    pub device: String,
+    pub perms: String,
     pub mapping: String,
 }
 
-// Represents a set of permissions from the "perms" column of /proc/<PID>/maps.
-#[derive(Clone, Copy, Debug, PartialEq)]
-pub struct Perms {
-    pub readable: bool,
-    pub writable: bool,
-    pub executable: bool,
-    pub shared: bool,
-}
-
-impl From<&str> for Perms {
-    fn from(s: &str) -> Self {
-        let mut chars = s.chars();
-
-        Self {
-            readable: chars.next() == Some('r'),
-            writable: chars.next() == Some('w'),
-            executable: chars.next() == Some('x'),
-            shared: chars.next() == Some('s'),
-        }
-    }
-}
-
-// Please note: While `Perms` has four boolean fields, it's string representation has five
-// characters because pmap's default and device formats use five characters for the perms,
-// with the last character always being '-'.
-impl fmt::Display for Perms {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(
-            f,
-            "{}{}{}{}-",
-            if self.readable { 'r' } else { '-' },
-            if self.writable { 'w' } else { '-' },
-            if self.executable { 'x' } else { '-' },
-            if self.shared { 's' } else { '-' },
-        )
-    }
-}
-
 // Parses a single line from /proc/<PID>/maps. See
 // https://www.kernel.org/doc/html/latest/filesystems/proc.html for details about the expected
 // format.
@@ -72,20 +30,9 @@ pub fn parse_map_line(line: &str) -> Result<MapLine, Error> {
     let (perms, rest) = rest
         .split_once(' ')
         .ok_or_else(|| Error::from(ErrorKind::InvalidData))?;
-    let perms = Perms::from(perms);
-
-    let (offset, rest) = rest
-        .split_once(' ')
-        .ok_or_else(|| Error::from(ErrorKind::InvalidData))?;
-    let offset = format!("{offset:0>16}");
-
-    let (device, rest) = rest
-        .split_once(' ')
-        .ok_or_else(|| Error::from(ErrorKind::InvalidData))?;
-    let device = parse_device(device)?;
+    let perms = parse_perms(perms);
 
-    // skip the "inode" column
-    let mapping: String = rest.splitn(2, ' ').skip(1).collect();
+    let mapping: String = rest.splitn(4, ' ').skip(3).collect();
     let mapping = mapping.trim_ascii_start();
     let mapping = parse_mapping(mapping);
 
@@ -93,8 +40,6 @@ pub fn parse_map_line(line: &str) -> Result<MapLine, Error> {
         address,
         size_in_kb,
         perms,
-        offset,
-        device,
         mapping,
     })
 }
@@ -113,12 +58,13 @@ fn parse_address(memory_range: &str) -> Result<(String, u64), Error> {
     Ok((format!("{start:0>16}"), size_in_kb))
 }
 
-// Pads the device info from /proc/<PID>/maps with zeros and turns AB:CD into 0AB:000CD.
-fn parse_device(device: &str) -> Result<String, Error> {
-    let (major, minor) = device
-        .split_once(':')
-        .ok_or_else(|| Error::from(ErrorKind::InvalidData))?;
-    Ok(format!("{major:0>3}:{minor:0>5}"))
+// Turns a 4-char perms string from /proc/<PID>/maps into a 5-char perms string. The first three
+// chars are left untouched.
+fn parse_perms(perms: &str) -> String {
+    let perms = perms.replace("p", "-");
+
+    // the fifth char seems to be always '-' in the original pmap
+    format!("{perms}-")
 }
 
 fn parse_mapping(mapping: &str) -> String {
@@ -140,60 +86,44 @@ fn parse_mapping(mapping: &str) -> String {
 mod test {
     use super::*;
 
-    fn create_map_line(
-        address: &str,
-        size_in_kb: u64,
-        perms: Perms,
-        offset: &str,
-        device: &str,
-        mapping: &str,
-    ) -> MapLine {
+    fn create_map_line(address: &str, size_in_kb: u64, perms: &str, mapping: &str) -> MapLine {
         MapLine {
             address: address.to_string(),
             size_in_kb,
-            perms,
-            offset: offset.to_string(),
-            device: device.to_string(),
+            perms: perms.to_string(),
             mapping: mapping.to_string(),
         }
     }
 
-    #[test]
-    fn test_perms_to_string() {
-        assert_eq!("-----", Perms::from("---p").to_string());
-        assert_eq!("---s-", Perms::from("---s").to_string());
-        assert_eq!("rwx--", Perms::from("rwxp").to_string());
-    }
-
     #[test]
     fn test_parse_map_line() {
         let data = [
             (
-                create_map_line("000062442eb9e000", 16, Perms::from("r--p"), "0000000000000000", "008:00008", "konsole"),
+                create_map_line("000062442eb9e000", 16, "r----", "konsole"),
                 "62442eb9e000-62442eba2000 r--p 00000000 08:08 10813151                   /usr/bin/konsole"
             ),
             (
-                create_map_line("000071af50000000", 132, Perms::from("rw-p"), "0000000000000000", "000:00000", "  [ anon ]"),
+                create_map_line("000071af50000000", 132, "rw---", "  [ anon ]"),
                 "71af50000000-71af50021000 rw-p 00000000 00:00 0 "
             ),
             (
-                create_map_line("00007ffc3f8df000", 132, Perms::from("rw-p"), "0000000000000000", "000:00000", "  [ stack ]"),
+                create_map_line("00007ffc3f8df000", 132, "rw---", "  [ stack ]"),
                 "7ffc3f8df000-7ffc3f900000 rw-p 00000000 00:00 0                          [stack]"
             ),
             (
-                create_map_line("000071af8c9e6000", 16, Perms::from("rw-s"), "0000000105830000", "000:00010", "  [ anon ]"),
+                create_map_line("000071af8c9e6000", 16, "rw-s-", "  [ anon ]"),
                 "71af8c9e6000-71af8c9ea000 rw-s 105830000 00:10 1075                      anon_inode:i915.gem"
             ),
             (
-                create_map_line("000071af6cf0c000", 3560, Perms::from("rw-s"), "0000000000000000", "000:00001", "memfd:wayland-shm (deleted)"),
+                create_map_line("000071af6cf0c000", 3560, "rw-s-", "memfd:wayland-shm (deleted)"),
                 "71af6cf0c000-71af6d286000 rw-s 00000000 00:01 256481                     /memfd:wayland-shm (deleted)"
             ),
             (
-                create_map_line("ffffffffff600000", 4, Perms::from("--xp"), "0000000000000000", "000:00000", "  [ anon ]"),
+                create_map_line("ffffffffff600000", 4, "--x--", "  [ anon ]"),
                 "ffffffffff600000-ffffffffff601000 --xp 00000000 00:00 0                  [vsyscall]"
             ),
             (
-                create_map_line("00005e8187da8000", 24, Perms::from("r--p"), "0000000000000000", "008:00008", "hello   world"),
+                create_map_line("00005e8187da8000", 24, "r----", "hello   world"),
                 "5e8187da8000-5e8187dae000 r--p 00000000 08:08 9524160                    /usr/bin/hello   world"
             ),
         ];
@@ -231,14 +161,10 @@ mod test {
     }
 
     #[test]
-    fn test_parse_device() {
-        assert_eq!("012:00034", parse_device("12:34").unwrap());
-        assert_eq!("000:00000", parse_device("00:00").unwrap());
-    }
-
-    #[test]
-    fn test_parse_device_without_colon() {
-        assert!(parse_device("1234").is_err());
+    fn test_parse_perms() {
+        assert_eq!("-----", parse_perms("---p"));
+        assert_eq!("---s-", parse_perms("---s"));
+        assert_eq!("rwx--", parse_perms("rwxp"));
     }
 
     #[test]
diff --git a/src/uu/pmap/src/pmap.rs b/src/uu/pmap/src/pmap.rs
index baa24fc..04b24ff 100644
--- a/src/uu/pmap/src/pmap.rs
+++ b/src/uu/pmap/src/pmap.rs
@@ -4,7 +4,7 @@
 // file that was distributed with this source code.
 
 use clap::{crate_version, Arg, ArgAction, Command};
-use maps_format_parser::{parse_map_line, MapLine};
+use maps_format_parser::parse_map_line;
 use std::env;
 use std::fs;
 use std::io::Error;
@@ -49,12 +49,11 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
             }
         }
 
-        if matches.get_flag(options::DEVICE) {
-            output_device_format(pid).map_err(|_| set_exit_code(1)).ok();
-        } else {
-            output_default_format(pid)
-                .map_err(|_| set_exit_code(1))
-                .ok();
+        match parse_maps(pid) {
+            Ok(total) => println!(" total {total:>16}K"),
+            Err(_) => {
+                set_exit_code(1);
+            }
         }
     }
 
@@ -75,70 +74,21 @@ fn parse_cmdline(pid: &str) -> Result<String, Error> {
     Ok(cmdline.into())
 }
 
-fn process_maps<F>(pid: &str, mut process_line: F) -> Result<(), Error>
-where
-    F: FnMut(&MapLine),
-{
+fn parse_maps(pid: &str) -> Result<u64, Error> {
     let path = format!("/proc/{pid}/maps");
     let contents = fs::read_to_string(path)?;
+    let mut total = 0;
 
     for line in contents.lines() {
         let map_line = parse_map_line(line)?;
-        process_line(&map_line);
-    }
-
-    Ok(())
-}
-
-fn output_default_format(pid: &str) -> Result<(), Error> {
-    let mut total = 0;
-
-    process_maps(pid, |map_line| {
         println!(
             "{} {:>6}K {} {}",
             map_line.address, map_line.size_in_kb, map_line.perms, map_line.mapping
         );
         total += map_line.size_in_kb;
-    })?;
-
-    println!(" total {total:>16}K");
-
-    Ok(())
-}
-
-fn output_device_format(pid: &str) -> Result<(), Error> {
-    let mut total_mapped = 0;
-    let mut total_writeable_private = 0;
-    let mut total_shared = 0;
-
-    println!("Address           Kbytes Mode  Offset           Device    Mapping");
-
-    process_maps(pid, |map_line| {
-        println!(
-            "{} {:>7} {} {} {} {}",
-            map_line.address,
-            map_line.size_in_kb,
-            map_line.perms,
-            map_line.offset,
-            map_line.device,
-            map_line.mapping
-        );
-        total_mapped += map_line.size_in_kb;
-
-        if map_line.perms.writable && !map_line.perms.shared {
-            total_writeable_private += map_line.size_in_kb;
-        }
-
-        if map_line.perms.shared {
-            total_shared += map_line.size_in_kb;
-        }
-    })?;
-
-    println!(
-        "mapped: {total_mapped}K    writeable/private: {total_writeable_private}K    shared: {total_shared}K"
-    );
+    }
 
-    Ok(())
+    Ok(total)
 }
 
 pub fn uu_app() -> Command {


Test patch:
diff --git a/tests/by-util/test_pmap.rs b/tests/by-util/test_pmap.rs
index 7d0ec60..fb7004b 100644
--- a/tests/by-util/test_pmap.rs
+++ b/tests/by-util/test_pmap.rs
@@ -73,22 +73,6 @@ fn test_non_existing_pid() {
         .no_output();
 }
 
-#[test]
-#[cfg(target_os = "linux")]
-fn test_device() {
-    let pid = process::id();
-
-    for arg in ["-d", "--device"] {
-        let result = new_ucmd!()
-            .arg(arg)
-            .arg(pid.to_string())
-            .succeeds()
-            .stdout_move_str();
-
-        assert_device_format(pid, &result);
-    }
-}
-
 #[test]
 fn test_invalid_arg() {
     new_ucmd!().arg("--definitely-invalid").fails().code_is(1);
@@ -110,46 +94,9 @@ fn assert_format(pid: u32, s: &str) {
 
     let rest = rest.trim_end();
     let (memory_map, last_line) = rest.rsplit_once('\n').unwrap();
-    let re = Regex::new(r"(?m)^[0-9a-f]{16} +[1-9][0-9]*K (-|r)(-|w)(-|x)(-|s)- (  \[ (anon|stack) \]|[a-zA-Z0-9._-]+)$").unwrap();
+    let re = Regex::new("(?m)^[0-9a-f]{16} +[1-9][0-9]*K (-|r)(-|w)(-|x)(-|s)- (  $$[ (anon|stack) $$]|[a-zA-Z0-9._-]+)$").unwrap();
     assert!(re.is_match(memory_map));
 
     let re = Regex::new("^ total +[1-9][0-9]*K$").unwrap();
     assert!(re.is_match(last_line));
 }
-
-// Ensure `s` has the following device format (--device):
-//
-// 1234:   /some/path
-// Address           Kbytes Mode  Offset           Device    Mapping
-// 000073eb5f4c7000       8 rw--- 0000000000036000 008:00008 ld-linux-x86-64.so.2
-// 00007ffd588fc000     132 rw--- 0000000000000000 000:00000   [ stack ]
-// ffffffffff600000       4 --x-- 0000000000000000 000:00000   [ anon ]
-// ...
-// mapped: 3060K    writeable/private: 348K    shared: 0K
-#[cfg(target_os = "linux")]
-fn assert_device_format(pid: u32, s: &str) {
-    let lines: Vec<_> = s.lines().collect();
-    let line_count = lines.len();
-
-    let re = Regex::new(&format!("^{pid}:   .+[^ ]$")).unwrap();
-    assert!(re.is_match(lines[0]));
-
-    let expected_header = "Address           Kbytes Mode  Offset           Device    Mapping";
-    assert_eq!(expected_header, lines[1]);
-
-    let re = Regex::new(
-        r"^[0-9a-f]{16} +[1-9][0-9]* (-|r)(-|w)(-|x)(-|s)- [0-9a-f]{16} [0-9a-f]{3}:[0-9a-f]{5} (  \[ (anon|stack) \]|[a-zA-Z0-9._-]+)$",
-    )
-    .unwrap();
-
-    for line in lines.iter().take(line_count - 1).skip(2) {
-        assert!(re.is_match(line), "failing line: {line}");
-    }
-
-    let re = Regex::new(r"^mapped: \d+K\s{4}writeable/private: \d+K\s{4}shared: \d+K$").unwrap();
-    assert!(
-        re.is_match(lines[line_count - 1]),
-        "failing line: {}",
-        lines[line_count - 1]
-    );
-}