Merge #1656

1656: Remove PATH_MAX restriction from with_nix_path and further improve performance r=rtzoeller a=SUPERCILEX

This PR removes the `PATH_MAX` limitation since that's wrong anyway: https://insanecoding.blogspot.com/2007/11/pathmax-simply-isnt.html

A nice side effect is that this lets us further optimize `with_nix_path` by having the compiler not insert probe frames, saving us another fat pile of instructions:

```
2,289,930 ( 1.67%) 305,324 ( 0.99%) 152,662 ( 0.74%) 18 ( 0.15%)  .            .          .          .           .           => ???:__rust_probestack (152,662x)
```

New numbers:

```
19,257,593 (16.43%) 4,415,242 (15.75%) 2,593,635 (12.82%) 41 ( 0.27%) 47 ( 0.12%) 53 ( 0.26%) 7 ( 0.12%) 0          3 ( 0.02%) 2,404,730 (14.52%) 2,721 ( 0.53%) 762,780 (30.96%)    66 ( 0.90%)  => /home/asaveau/Desktop/nix/src/lib.rs:<[u8] as nix::NixPath>::with_nix_path (152,556x)
```

```
Ir                 Dr               Dw               I1mr        D1mr        D1mw       ILmr       DLmr       DLmw       Bc               Bcm        Bi               Bim            

1,067,892 ( 0.91%)       0          457,668 ( 2.26%)  2 ( 0.01%)  0          0          1 ( 0.02%) .          .                .          .                .              .               fn with_nix_path<T, F>(&self, f: F) -> Result<T>
        .                .                .           .           .          .          .          .          .                .          .                .              .               where
        .                .                .           .           .          .          .          .          .                .          .                .              .                   F: FnOnce(&CStr) -> T,
        .                .                .           .           .          .          .          .          .                .          .                .              .               {
        .                .                .           .           .          .          .          .          .                .          .                .              .                   // The real PATH_MAX is 4096, but it's statistically unlikely to have a path longer than
        .                .                .           .           .          .          .          .          .                .          .                .              .                   // ~300 bytes. See the appendix to get stats for your own machine.
        .                .                .           .           .          .          .          .          .                .          .                .              .                   //
        .                .                .           .           .          .          .          .          .                .          .                .              .                   // By being smaller than a memory page, we also avoid the compiler inserting a probe frame:
        .                .                .           .           .          .          .          .          .                .          .                .              .                   // https://docs.rs/compiler_builtins/latest/compiler_builtins/probestack/index.html
        .                .                .           .           .          .          .          .          .                .          .                .              .                   const MAX_STACK_ALLOCATION: usize = 512;
        .                .                .           .           .          .          .          .          .                .          .                .              .           
  305,112 ( 0.26%)       0                0           0           0          0          0          0          0          152,556 ( 0.92%) .                .              .                   if self.len() >= MAX_STACK_ALLOCATION {
        .                .                .           .           .          .          .          .          .                .          .                .              .                       return with_nix_path_allocating(self, f);
        .                .                .           .           .          .          .          .          .                .          .                .              .                   }
        .                .                .           .           .          .          .          .          .                .          .                .              .           
        .                .                .           .           .          .          .          .          .                .          .                .              .                   let mut buf = MaybeUninit::<[u8; MAX_STACK_ALLOCATION]>::uninit();
        .                .                .           .           .          .          .          .          .                .          .                .              .                   let buf_ptr = buf.as_mut_ptr() as *mut u8;
        .                .                .           .           .          .          .          .          .                .          .                .              .           
        .                .                .           .           .          .          .          .          .                .          .                .              .                   unsafe {
        .                .                .           .           .          .          .          .          .                .          .                .              .                       ptr::copy_nonoverlapping(self.as_ptr(), buf_ptr, self.len());
        .                .                .           .           .          .          .          .          .                .          .                .              .                       buf_ptr.add(self.len()).write(0);
        .                .                .           .           .          .          .          .          .                .          .                .              .                   }
        .                .                .           .           .          .          .          .          .                .          .                .              .           
1,067,892 ( 0.91%) 305,112 ( 1.09%) 152,556 ( 0.75%)  2 ( 0.01%)  7 ( 0.02%) 0          1 ( 0.02%) 0          0          305,112 ( 1.84%) 6 ( 0.00%) 152,556 ( 6.19%)     1 ( 0.01%)          match CStr::from_bytes_with_nul(unsafe { slice::from_raw_parts(buf_ptr, self.len() + 1) }) {
8,730,403 ( 7.45%) 1,211,383 ( 4.32%) 1,067,892 ( 5.28%) 18 ( 0.12%)  2 ( 0.00%) 0          2 ( 0.04%) 0          0          1,336,744 ( 8.07%) 609 ( 0.12%) 152,556 ( 6.19%)    62 ( 0.85%)  => /rustc/21b4a9cfdcbb1e76f4b36b5c3cfd64d627285093//library/std/src/ffi/c_str.rs:std::ffi::c_str::CStr::from_bytes_with_nul (152,556x)
  610,224 ( 0.52%) 610,224 ( 2.18%)       .           .           .          .          .          .          .                .          .                .              .                       Ok(s) => Ok(f(s)),
        .                .                .           .           .          .          .          .          .                .          .                .              .                       Err(_) => Err(Errno::EINVAL),
        .                .                .           .           .          .          .          .          .                .          .                .              .                   }
  762,780 ( 0.65%) 610,224 ( 2.18%)       .           .           .          .          .          .          .                .          .                .              .               }
        .                .                .           .           .          .          .          .          .                .          .                .              .           }
```

## Appendix

```rust
use histogram::Histogram;
use std::env;
use std::fs::{canonicalize, read_dir};
use std::path::Path;

fn main() {
    let args = env::args().collect::<Vec<String>>();

    let mut histogram = Histogram::new();
    if args.len() == 2 {
        log(&mut histogram, canonicalize(&args[1]).unwrap());
    } else {
        log(&mut histogram, canonicalize(".").unwrap());
    }

    println!(
        "min={}\nmax={}\nmean={}\np50={}\np90={}\np99={}\np999={}\nstddev={}\nstdvar={}",
        histogram.minimum().unwrap(),
        histogram.maximum().unwrap(),
        histogram.mean().unwrap(),
        histogram.percentile(50.0).unwrap(),
        histogram.percentile(90.0).unwrap(),
        histogram.percentile(99.0).unwrap(),
        histogram.percentile(99.9).unwrap(),
        histogram.stddev().unwrap(),
        histogram.stdvar().unwrap(),
    )
}

fn log(histogram: &mut Histogram, path: impl AsRef<Path>) {
    for dir in read_dir(path.as_ref()).unwrap() {
        let entry = dir.unwrap();
        histogram
            .increment(entry.path().as_os_str().len() as u64)
            .unwrap();
        if entry.file_type().unwrap().is_dir() {
            log(histogram, entry.path());
        }
    }
}
```

Co-authored-by: Alex Saveau <[email protected]>

Author: bors[bot]

CHANGELOG.md

@@ -64,6 +64,9 @@ This project adheres to [Semantic Versioning](https://semver.org/).
   Because of this change, you now need `use std::iter::Extend` to call `extend`
   on a `SigSet`.
   (#[1553](https://github.com/nix-rust/nix/pull/1553))
+- Removed the the `PATH_MAX` restriction from APIs accepting paths. Paths
+  will now be allocated on the heap if they are too long. In addition, large
+  instruction count improvements (~30x) were made to path handling.
 
 ### Fixed
 

src/lib.rs

@@ -156,19 +156,11 @@ feature! {
 #[allow(missing_docs)]
 pub mod unistd;
 
-/*
- *
- * ===== Result / Error =====
- *
- */
-
-use libc::PATH_MAX;
-
-use std::{ptr, result, slice};
-use std::ffi::{CStr, OsStr};
+use std::ffi::{CStr, CString, OsStr};
 use std::mem::MaybeUninit;
 use std::os::unix::ffi::OsStrExt;
 use std::path::{Path, PathBuf};
+use std::{ptr, result, slice};
 
 use errno::Errno;
 
@@ -242,12 +234,9 @@ impl NixPath for CStr {
     }
 
     fn with_nix_path<T, F>(&self, f: F) -> Result<T>
-            where F: FnOnce(&CStr) -> T {
-        // Equivalence with the [u8] impl.
-        if self.len() >= PATH_MAX as usize {
-            return Err(Errno::ENAMETOOLONG)
-        }
-
+    where
+        F: FnOnce(&CStr) -> T,
+    {
         Ok(f(self))
     }
 }
@@ -265,11 +254,19 @@ impl NixPath for [u8] {
     where
         F: FnOnce(&CStr) -> T,
     {
-        if self.len() >= PATH_MAX as usize {
-            return Err(Errno::ENAMETOOLONG);
+        // The real PATH_MAX is typically 4096, but it's statistically unlikely to have a path
+        // longer than ~300 bytes. See the the PR description to get stats for your own machine.
+        // https://github.com/nix-rust/nix/pull/1656
+        //
+        // By being smaller than a memory page, we also avoid the compiler inserting a probe frame:
+        // https://docs.rs/compiler_builtins/latest/compiler_builtins/probestack/index.html
+        const MAX_STACK_ALLOCATION: usize = 1024;
+
+        if self.len() >= MAX_STACK_ALLOCATION {
+            return with_nix_path_allocating(self, f);
         }
 
-        let mut buf = MaybeUninit::<[u8; PATH_MAX as usize]>::uninit();
+        let mut buf = MaybeUninit::<[u8; MAX_STACK_ALLOCATION]>::uninit();
         let buf_ptr = buf.as_mut_ptr() as *mut u8;
 
         unsafe {
@@ -284,6 +281,18 @@ impl NixPath for [u8] {
     }
 }
 
+#[cold]
+#[inline(never)]
+fn with_nix_path_allocating<T, F>(from: &[u8], f: F) -> Result<T>
+where
+    F: FnOnce(&CStr) -> T,
+{
+    match CString::new(from) {
+        Ok(s) => Ok(f(&s)),
+        Err(_) => Err(Errno::EINVAL),
+    }
+}
+
 impl NixPath for Path {
     fn is_empty(&self) -> bool {
         NixPath::is_empty(self.as_os_str())