This sets up and action that runs on:

• Pushes
• Pull requests
• A weekly schedule (to proactively point out any new warnings from new stable/beta releases)

This enforces:

• cargo fmt, cargo clippy, cargo test, and cargo doc warnings and hard errors on stable and beta
• cargo check and cargo test hard errors on an MSRV version
• cargo miri test on nightly

I think it would make sense to expose the inline string size limit and then have the public API be something along the lines of

ecow::{
    eco_vec,
    format_eco, // Maybe rename to eco_format to keep name ordering consistent?
    vec::{EcoVec, IntoIter},
    str::{EcoString, INLINE_SIZE_LIMIT},
}

The existence of IntoIter to me is enough to have a separate vec module at least (and there is the chance that other iterators could be added like Drain)

I would be up for re-exporting or exclusively exposing EcoVec and EcoString in the root of the API though since they are likely going to be the most commonly use types

Resolves #18

This follows the described restructuring, renaming, and switches the unit tests to integration tests. The only change that the latter involved was switching a single call from .extend_from_byte_slice() to .extend_from_slice() since .extend_from_byte_slice() isn't included in the public API (and should get tested well by the EcoString tests anyways)

Wanted to check before I go ahead and open a PR, but would you be fine with a CI job to publish code coverage information to codecov? I'm planning on adding property testing and fuzzing, and having code coverage information published would be helpful (and you can get a cool little badge for it)

This just sets up the scaffolding for writing future tests using loom

The atomics usage currently mirrors std, so I doubt there will be any issues, but it certainly still seems good to have

I was doing some testing (got a lot planned for that btw), and I noticed that the test_mem_size test fails

#[test]
fn test_mem_size() {
    let word = mem::size_of::<usize>();
    assert_eq!(mem::size_of::<EcoVec<u8>>(), 2 * word);
    assert_eq!(mem::size_of::<Option<EcoVec<u8>>>(), 2 * word);

    if cfg!(target_endian = "little") {
        assert_eq!(mem::size_of::<EcoString>(), 2 * word);    // <-- Fails here. left: 16, right: 8
        assert_eq!(mem::size_of::<Option<EcoString>>(), 3 * word);
    }
}

It looks like the inline limit on 32-bit LE systems is still 15. I'd love to submit a PR to fix things, but I wasn't sure if the right approach was to use a smaller limit on 32-bit systems or to keep a larger limit even though the word size is smaller

Last PR I can do for the day (got a lot of IRL stuff today), but it's another soundness fix

This fixes EcoVec's Send and Sync bounds to match the standard library's Arc

Arc-like constructs need both Send and Sync on the inner type to allow for Send or Sync. Without this it's trivial to violate the inner type's bound

E.g. a type can be Send + !Sync, but that would allow you to create two Arcs on the same thread and then pass one of the Arcs to another thread. From there you have a reference to the type from multiple threads even though it's !Sync. A simple example program that makes miri angry

use std::cell::Cell;

use ecow::eco_vec;

fn main() {
    let send_not_sync = Cell::new("foo");
    let one = eco_vec![send_not_sync];
    let another = one.clone();

    std::thread::spawn(move || loop {
        another[0].set("bar");
        another[0].set("foo");
    });

    // 😱
    loop {
        println!("{}", one[0].get());
    }
}

$ cargo +nightly miri run
// -- 8< -- build output -- 8< --
error: Undefined Behavior: Data race detected between (1) Read on thread `main` and (2) Write on thread `<unnamed>` at alloc1739+0x18. (2) just happened here
   --> /.../rustlib/src/rust/library/core/src/cell.rs:413:31
    |
413 |         mem::replace(unsafe { &mut *self.value.get() }, val)
    |                               ^^^^^^^^^^^^^^^^^^^^^^ Data race detected between (1) Read on thread `main` and (2) Write on thread `<unnamed>` at alloc1739+0x18. (2) just happened here
    |
help: and (1) occurred earlier here
   --> src/main.rs:17:24
    |
17  |         println!("{}", one[0].get());
    |                        ^^^^^^^^^^^^
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
    = note: BACKTRACE (of the first span):
    = note: inside `std::cell::Cell::<&str>::replace` at /.../rustlib/src/rust/library/core/src/cell.rs:413:31: 413:53
    = note: inside `std::cell::Cell::<&str>::set` at /.../rustlib/src/rust/library/core/src/cell.rs:363:19: 363:36
note: inside closure
   --> src/main.rs:11:9
    |
11  |         another[0].set("bar");
    |         ^^^^^^^^^^^^^^^^^^^^^

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to previous error

For even more fun you can change one of the strings that get swapped to something really big and then you can get fun runtime issues.

$ # --release optimizes stuff away
$ # --show-all prevents arbitrary bytes from messing with my terminal session
$ cargo run | bat --show-all
// -- 8< -- searching for "home" -- 8< --
9962   │ foo#/rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/io/mod.rsfailed·to·write·whole·bufferfor
       │ matter·errorfatal·runtime·error:·␊
9963   │ thread·result·panicked·on·dropassertion·failed:·self.is_allocated()/.../ecow/s
       │ rc/vec.rsassertion·failed:·self.is_unique()assertion·failed:·target·>·
self.capacity()␀␀␀␀␀␀␀␀␀attempt·to·add
       │ ·with·overflow␀␀␀␀attempt·to·subtract·with·overflow␀␀␀␀␀%                                                   26;46m␀␀␀␀␀␀␀␀␀␀attempt·to·multiply·with·overflowreference
       │ ·count·overflow␀␀␀␀␀␀␀,O\xFC\xFFhO\xFC\xFFjO\xFC\xFFlO\xFC\xFFnO\xFC\xFFthere·is·no·such·thing·as·a·relaxed·
       │ fence␀␀␀/rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/core/src/sync/atomic.rs␀>P\xFC\xFFSP\xFC\xFF
       │ hP\xFC\xFF}P\xFC\xFF\x92P\xFC\xFFfailed·to·spawn·thread/rustc/fc594f15669680fa70d255faec3ca3fb507c3405/libra
       │ ry/std/src/thread/mod.rsthread·name·may·not·contain·interior·null·bytes␀␀\xA2e\xFC\xFF\xDAe\xFC\xFFie\xFC\xF
       │ F\x89e\xFC\xFF\xE2g\xFC\xFF\u{1a}h\xFC\xFF\xA9g\xFC\xFF\xC9g\xFC\xFFinternal·error:·entered·unreachable·code
       │ /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/io/error/repr_bitpacked.rs␀␀\xFEx\xFC\xFF\u{
       │ 12}y\xFC\xFF·y\xFC\xFF3y\xFC\xFF/.../Pro␊

(Being totally honest I'm just having fun seeing what wacky stuff can happen with Rust's UB. Sometimes I miss how cursed C programming can be)

The last PR in today's barrage is an extra spooky one :ghost::ghost::ghost:

I believe this should be enough to fix exception safety from overflowing the ref count. This prevents a possible use-after-free that can occur in incredibly degenerate programs. For example

use ecow::EcoString;

fn main() {
    let one = EcoString::from("This is so large that it's a `Repr::Large`");
    let two = one.clone();

    for _ in 0..(usize::MAX - 1) {
        let _ = std::panic::catch_unwind(|| {
            std::mem::forget(two.clone());
        });
    }

    // The ref count is now at 2 + usize::MAX - 1 aka we overflowed to 1 :D

    drop(two);

    // Now that the ref count is 0 the backing allocation gets dropped, buuuuut we still have a
    // reference to it which lets us UAF

    println!("Boom {one}");
}

Considering this involves overflowing a usize it takes a considerable amount of time (I tried optimizing things, but even using cross to run on a 32-bit platform gets really slow when we start catching panics)

If you wind up adding an std feature you could change ref_count_overflow() to abort when the feature is enabled which matches the standard library's handling

I figured it would be worthwhile to start digging through issues reported in the advisory-db. So far it hasn't found anything interesting, but this includes areas that could have issues from future optimizations (primarily avoiding extra bounds/capacity checking)

This is just the first set since there's more I would like to add (namely around panicking in either clone() or drop(), but that seems fine so far). Common sources of issues seem to be

• Wrong Send or Sync bounds: already fixed
• Forgetting to bounds check, or off by one error when bounds checking: already seems decently tested, but could be worth some property testing
• Panicking in unexpected places: everything seems to update the bounds correctly to avoid dropping things multiple times, but could use some more testing since it's something that could get some error-prone performance improvements in the future
• Using incorrect alignment: seems fine to me at a glance, and has some existing tests
• Multiple mutable references to the same data: seems to correctly check uniqueness before allowing for mutation, although some more debug_assert!s wouldn't hurt
• Use after free: Only issue I found was that extremely degenerate clone() issue. All the lifetimes seem right, so far
• Making assumptions about #[repr(Rust)] layout: EcoVec seems appropriately #[repr(C)] and running with RUSTFLAGS="-Zrandomize-layout" didn't find any issues

Still have more auditing to do, but so far things seem very solid :+1:

Added Counter trait

Don't think you wanna merge as is because I had to make the Vec ptr field into a regular, nullable pointer as I couldn't figure out how to create a static Header<Rc> value for any Rc: Counter.

Also fixed some clippy lints.

It removed some unnecessary references and changed a str comparison to a char comparison, it also removed a clone in a test case, but I kept it because it seemed intentional.