From b1516c3941ecc544b92fae23e11e59ea07932eb0 Mon Sep 17 00:00:00 2001 From: Pierre Chevalier Date: Tue, 16 Jan 2024 00:38:02 -0800 Subject: [PATCH] Update bitflags to 2.4 Summary: ## Motivation Since the latest compiler update, we are getting `clippy::bad_bit_mask` errors at the callsites of `bitflags!` macros where one of the variant is zero. [Upstream won't address it in the `1.x` branch](https://github.com/bitflags/bitflags/pull/373) and recommends upgrading to the `2.x` branch. We are very close to reaching **zero clippy lints** in [Mononoke and other servers](https://fburl.com/code/pd76yn5e), which would feel nice. ## Specific categories of changes (in case it helps with the code review) The change from `1.x` to `2.x` introduces a number of backward compatibility breakages which I had to workaround in our codebase. See [the release notes for 2.0](https://github.com/bitflags/bitflags/releases/tag/2.0.0) for the explanation for the manual fixes I had to perform at each call site. --- **Adding traits to derive:** ``` #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)] ``` > Generated flags types now derive fewer traits. If you need to maintain backwards compatibility, you can derive the following yourself: --- **Replacing read uses of `.bits` with `.bits()`** > You can now use the .bits() method instead of the old .bits. > The representation of generated flags types has changed from a struct with the single field bits to a newtype. --- **Replacing raw setting of `.bits` with `.from_bits_retain()`** Due to the point above, the representation of the type is not exposed anymore. From [the documentation](https://docs.rs/bitflags/latest/bitflags/example_generated/struct.Flags.html#method.from_bits_retain), `from_bits_retain` "convert from a bits value exactly", which matches the old behaviour --- **Replacing the unsafe `from_bits_unchecked` method with `from_bits_retain`** > The unsafe from_bits_unchecked method is now a safe from_bits_retain method. --- **Extracting some structs outside of the `bitflags!` macro** Apart from the derives that `bitflags` knows about, such as `serde`, `bitflags` now can't deal with custom derives in macros with the previous syntax. I followed the recommendation from [their documentation](https://docs.rs/bitflags/latest/bitflags/index.html#custom-derives) to declare the `struct` ahead of of the macro call and only declare the `impl` block inside the macro. --- **Changes to test output** This does not stand out in the release notes, but as of [this upstream PR](https://github.com/bitflags/bitflags/pull/297), the `Debug` output of generated bitflags has changed. This means any tests that rely on this (and of course, there are a few) needed updating. In particular, the `vespa` tests rely on that output in a non-obvious way. You might have to trust me (and CI) on these ones... Reviewed By: dtolnay Differential Revision: D49742979 fbshipit-source-id: c818c37af45f0964e8fdb7ec6173ad66bb982c00 --- reverie-process/Cargo.toml | 2 +- reverie-process/src/namespace.rs | 2 +- reverie-process/src/seccomp/bpf.rs | 2 +- reverie-syscalls/Cargo.toml | 2 +- reverie-syscalls/src/args/clone.rs | 6 +++--- reverie-syscalls/src/args/mod.rs | 2 +- reverie-syscalls/src/args/poll.rs | 6 +++--- reverie/Cargo.toml | 2 +- reverie/src/subscription.rs | 2 +- safeptrace/Cargo.toml | 2 +- safeptrace/src/lib.rs | 1 + 11 files changed, 15 insertions(+), 14 deletions(-) diff --git a/reverie-process/Cargo.toml b/reverie-process/Cargo.toml index 3bfdd7f..a53be55 100644 --- a/reverie-process/Cargo.toml +++ b/reverie-process/Cargo.toml @@ -10,7 +10,7 @@ license = "BSD-2-Clause" [dependencies] bincode = "1.3.3" -bitflags = "1.3" +bitflags = "2.4" colored = "2.1.0" futures = { version = "0.3.28", features = ["async-await", "compat"] } libc = "0.2.139" diff --git a/reverie-process/src/namespace.rs b/reverie-process/src/namespace.rs index 3ee0f3b..3ba2f8b 100644 --- a/reverie-process/src/namespace.rs +++ b/reverie-process/src/namespace.rs @@ -15,7 +15,7 @@ bitflags::bitflags! { /// A namespace that may be unshared with [`Command::unshare`]. /// /// [`Command::unshare`]: super::Command::unshare - #[derive(Deserialize, Serialize)] + #[derive(Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)] pub struct Namespace: i32 { /// Cgroup namespace. const CGROUP = libc::CLONE_NEWCGROUP; diff --git a/reverie-process/src/seccomp/bpf.rs b/reverie-process/src/seccomp/bpf.rs index c257859..1c6ede4 100644 --- a/reverie-process/src/seccomp/bpf.rs +++ b/reverie-process/src/seccomp/bpf.rs @@ -85,7 +85,7 @@ pub const AUDIT_ARCH_PPC: u32 = EM_PPC; pub const AUDIT_ARCH_PPC64: u32 = EM_PPC64 | __AUDIT_ARCH_64BIT; bitflags::bitflags! { - #[derive(Default)] + #[derive(Default, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)] struct FilterFlags: u32 { const TSYNC = 1 << 0; const LOG = 1 << 1; diff --git a/reverie-syscalls/Cargo.toml b/reverie-syscalls/Cargo.toml index 65d7de7..daa6944 100644 --- a/reverie-syscalls/Cargo.toml +++ b/reverie-syscalls/Cargo.toml @@ -9,7 +9,7 @@ repository = "https://github.com/facebookexperimental/reverie" license = "BSD-2-Clause" [dependencies] -bitflags = "1.3" +bitflags = "2.4" derive_more = "0.99.17" libc = "0.2.139" nix = "0.25" diff --git a/reverie-syscalls/src/args/clone.rs b/reverie-syscalls/src/args/clone.rs index 0f05f9e..2026a07 100644 --- a/reverie-syscalls/src/args/clone.rs +++ b/reverie-syscalls/src/args/clone.rs @@ -17,7 +17,7 @@ use crate::MemoryAccess; bitflags::bitflags! { /// Flags used with the `clone`, `clone3`, or `unshare` syscalls. - #[derive(Serialize, Deserialize)] + #[derive(Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)] pub struct CloneFlags: u64 { /// set if VM shared between processes const CLONE_VM = libc::CLONE_VM as u64; @@ -91,7 +91,7 @@ impl Displayable for CloneFlags { impl FromToRaw for CloneFlags { fn from_raw(raw: usize) -> Self { - unsafe { Self::from_bits_unchecked(raw as u64) } + Self::from_bits_retain(raw as u64) } fn into_raw(self) -> usize { @@ -101,7 +101,7 @@ impl FromToRaw for CloneFlags { impl From for CloneFlags { fn from(flags: nix::sched::CloneFlags) -> Self { - unsafe { CloneFlags::from_bits_unchecked(flags.bits() as u64) } + CloneFlags::from_bits_retain(flags.bits() as u64) } } diff --git a/reverie-syscalls/src/args/mod.rs b/reverie-syscalls/src/args/mod.rs index d6a69c4..3c4eb56 100644 --- a/reverie-syscalls/src/args/mod.rs +++ b/reverie-syscalls/src/args/mod.rs @@ -409,7 +409,7 @@ impl<'a> Displayable for Option> { bitflags::bitflags! { /// stx_mask from statx, see linux/stat.h - #[derive(Serialize, Deserialize)] + #[derive(Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)] pub struct StatxMask: u32 { /// has stx_type const STATX_TYPE = 0x1; diff --git a/reverie-syscalls/src/args/poll.rs b/reverie-syscalls/src/args/poll.rs index 54300c2..dff8ace 100644 --- a/reverie-syscalls/src/args/poll.rs +++ b/reverie-syscalls/src/args/poll.rs @@ -41,8 +41,8 @@ impl From for PollFd { fn from(pollfd: libc::pollfd) -> Self { Self { fd: pollfd.fd, - events: unsafe { PollFlags::from_bits_unchecked(pollfd.events) }, - revents: unsafe { PollFlags::from_bits_unchecked(pollfd.revents) }, + events: PollFlags::from_bits_retain(pollfd.events), + revents: PollFlags::from_bits_retain(pollfd.revents), } } } @@ -60,7 +60,7 @@ impl Displayable for PollFd { bitflags::bitflags! { /// Flags for [`PollFd`]. - #[derive(Default, Serialize, Deserialize)] + #[derive(Default, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)] pub struct PollFlags: libc::c_short { /// There is data to read. const POLLIN = libc::POLLIN; diff --git a/reverie/Cargo.toml b/reverie/Cargo.toml index f12e014..a366e9b 100644 --- a/reverie/Cargo.toml +++ b/reverie/Cargo.toml @@ -12,7 +12,7 @@ license = "BSD-2-Clause" addr2line = "0.21" anyhow = "1.0.75" async-trait = "0.1.71" -bitflags = "1.3" +bitflags = "2.4" byteorder = "1.3" lazy_static = "1.4" libc = "0.2.139" diff --git a/reverie/src/subscription.rs b/reverie/src/subscription.rs index e06cb9f..9e6abc5 100644 --- a/reverie/src/subscription.rs +++ b/reverie/src/subscription.rs @@ -11,7 +11,7 @@ use reverie_syscalls::Sysno; use syscalls::SysnoSet; bitflags! { - #[derive(Default)] + #[derive(Default, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)] struct Instructions: u32 { const CPUID = 1; const RDTSC = 2; diff --git a/safeptrace/Cargo.toml b/safeptrace/Cargo.toml index 0370656..8c1b0c1 100644 --- a/safeptrace/Cargo.toml +++ b/safeptrace/Cargo.toml @@ -9,7 +9,7 @@ repository = "https://github.com/facebookexperimental/reverie" license = "BSD-2-Clause" [dependencies] -bitflags = "1.3" +bitflags = "2.4" futures = { version = "0.3.28", features = ["async-await", "compat"] } lazy_static = "1.4" libc = "0.2.139" diff --git a/safeptrace/src/lib.rs b/safeptrace/src/lib.rs index d304085..04f8e9a 100644 --- a/safeptrace/src/lib.rs +++ b/safeptrace/src/lib.rs @@ -429,6 +429,7 @@ struct ptrace_peeksiginfo_args { bitflags::bitflags! { /// Flags for ptrace peeksiginfo + #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)] pub struct PeekSigInfoFlags: u32 { /// dumping signals from the process-wide signal queue. signals are /// read from the per-thread queue of the specified thread if this