Source: rust-tracing-core
Version: 0.1.36-1
Severity: normal
Tags: patch upstream
Dear rust-tracing-core maintainers,
Currently, rust-tracing-core assumes that Rust niche optimization[1]
remains the
same, even though Rust makes no such guarantee. As of Rust 1.97, this
hardcoded
assumption is no longer true, meaning that the
metadata::tests::level_filter_reprs
test will break[2]:
167s ---- metadata::tests::level_filter_reprs stdout ----
167s
167s thread 'metadata::tests::level_filter_reprs' (5315) panicked at
src/metadata.rs:1119:13:
167s assertion `left == right` failed: repr changed for LevelFilter::OFF
167s left: 5
167s right: 18446744073709551615
167s stack backtrace:
167s 0: __rustc::rust_begin_unwind
167s at /usr/src/rustc-1.97.1/library/std/src/panicking.rs:689:5
167s 1: core::panicking::panic_fmt
167s at /usr/src/rustc-1.97.1/library/core/src/panicking.rs:80:14
167s 2: core::panicking::assert_failed_inner
167s at /usr/src/rustc-1.97.1/library/core/src/panicking.rs:434:23
167s 3: core::panicking::assert_failed::<usize, usize>
167s at /usr/src/rustc-1.97.1/library/core/src/panicking.rs:394:5
167s 4: tracing_core::metadata::tests::level_filter_reprs
167s at ./src/metadata.rs:1119:13
167s 5: tracing_core::metadata::tests::level_filter_reprs::{closure#0}
167s at ./src/metadata.rs:1102:28
167s 6: <tracing_core::metadata::tests::level_filter_reprs::{closure#0} as
core::ops::function::FnOnce<()>>::call_once
167s at /usr/src/rustc-1.97.1/library/core/src/ops/function.rs:250:5
167s 7: <fn() -> core::result::Result<(), alloc::string::String> as
core::ops::function::FnOnce<()>>::call_once
167s at /usr/src/rustc-1.97.1/library/core/src/ops/function.rs:250:5
167s note: Some details are omitted, run with `RUST_BACKTRACE=full` for a
verbose backtrace.
Naturally, this bug will not impact Debian until Rust is updated to 1.97.
However, it is affecting Ubuntu and being tracked as LP: #2167368.
I have attached a patch which cherry-picks the upstream fix[3]. The fix
makes it so the crate works with all Rust versions past the MSRV -- I
successfully ran all tests with both 1.93.1 and 1.97.1, and I verified that
the package builds against Debian unstable.
Please let me know if you need anything else.
Best wishes,
Max
[1]: https://www.0xatticus.com/posts/understanding_rust_niche/
[2]:
https://autopkgtest.ubuntu.com/results/autopkgtest-stonking/stonking/amd64/r/rust-tracing-core/20260912_123714_a8bc3@/log.gz
[3]: https://github.com/tokio-rs/tracing/pull/3537
Description: Get LevelFilter::OFF niche representation dynamically
Determining the value for `Option::None` at compile time via a union
transmute ensures that we always match the exact representation
selected by the compiler's niche optimization for `LevelFilter` on any
Rust version. The niche optimization changed in Rust 1.97.
.
This patch has been cherry-picked from upstream to unblock the Rust
1.97 transition. It can be dropped once we get an upstream tracing-core
version which contains the fix.
Author: Augie Fackler <[email protected]>
Origin: upstream, https://github.com/tokio-rs/tracing/pull/3537.diff
Bug: https://github.com/tokio-rs/tracing/pull/3537
Bug-Ubuntu: https://bugs.launchpad.net/bugs/2167368
---
--- a/src/metadata.rs
+++ b/src/metadata.rs
@@ -677,12 +677,13 @@
const INFO_USIZE: usize = LevelInner::Info as usize;
const DEBUG_USIZE: usize = LevelInner::Debug as usize;
const TRACE_USIZE: usize = LevelInner::Trace as usize;
- // Using the value of the last variant + 1 ensures that we match the value
- // for `Option::None` as selected by the niche optimization for
- // `LevelFilter`. If this is the case, converting a `usize` value into a
- // `LevelFilter` (in `LevelFilter::current`) will be an identity conversion,
- // rather than generating a lookup table.
- const OFF_USIZE: usize = LevelInner::Error as usize + 1;
+ // Dynamically determining the value for `Option::None` at compile time
+ // via `core::mem::transmute` ensures that we always match the exact representation
+ // selected by the compiler's niche optimization for `LevelFilter` on any
+ // Rust version. This guarantees that converting a `usize` value into a
+ // `LevelFilter` (in `LevelFilter::current`) is always a zero-cost identity
+ // conversion, rather than generating a lookup table.
+ const OFF_USIZE: usize = unsafe { core::mem::transmute(LevelFilter::OFF) };
/// Returns a `LevelFilter` that matches the most verbose [`Level`] that any
/// currently active [`Subscriber`] will enable.
@@ -886,11 +887,10 @@
// `Option<Level>`) compiles down to a single integer value. This is
// necessary for storing the global max in an `AtomicUsize`, and for ensuring
// that we use fast integer-integer comparisons, as mentioned previously. In
-// order to ensure this, we exploit the niche optimization. The niche
-// optimization for `Option<{enum with a numeric repr}>` will choose
-// `(HIGHEST_DISCRIMINANT_VALUE + 1)` as the representation for `None`.
-// Therefore, the integer representation of `LevelFilter::OFF` (which is
-// `None`) will be the number 5. `OFF` must compare higher than every other
+// order to ensure this, we exploit the niche optimization. The exact raw
+// representation selected by `rustc` for `Option::None` may vary across
+// compiler versions, so `LevelFilter::OFF_USIZE` is determined dynamically
+// via `core::mem::transmute`. `OFF` must compare higher than every other
// level in order for it to filter as expected. Since we want to use a single
// `cmp` instruction, we can't special-case the integer value of `OFF` to
// compare higher, as that will generate more code. Instead, we need it to be
@@ -911,7 +911,7 @@
impl PartialEq<LevelFilter> for Level {
#[inline(always)]
fn eq(&self, other: &LevelFilter) -> bool {
- self.0 as usize == filter_as_usize(&other.0)
+ self.0 as usize == filter_as_usize_sort_key(other.0)
}
}
@@ -952,42 +952,46 @@
impl PartialOrd<LevelFilter> for Level {
#[inline(always)]
fn partial_cmp(&self, other: &LevelFilter) -> Option<cmp::Ordering> {
- Some(filter_as_usize(&other.0).cmp(&(self.0 as usize)))
+ Some(filter_as_usize_sort_key(other.0).cmp(&(self.0 as usize)))
}
#[inline(always)]
fn lt(&self, other: &LevelFilter) -> bool {
- filter_as_usize(&other.0) < (self.0 as usize)
+ filter_as_usize_sort_key(other.0) < (self.0 as usize)
}
#[inline(always)]
fn le(&self, other: &LevelFilter) -> bool {
- filter_as_usize(&other.0) <= (self.0 as usize)
+ filter_as_usize_sort_key(other.0) <= (self.0 as usize)
}
#[inline(always)]
fn gt(&self, other: &LevelFilter) -> bool {
- filter_as_usize(&other.0) > (self.0 as usize)
+ filter_as_usize_sort_key(other.0) > (self.0 as usize)
}
#[inline(always)]
fn ge(&self, other: &LevelFilter) -> bool {
- filter_as_usize(&other.0) >= (self.0 as usize)
+ filter_as_usize_sort_key(other.0) >= (self.0 as usize)
}
}
+const USE_NATIVE_SORT: bool = LevelFilter::OFF_USIZE > LevelInner::Error as usize;
+
#[inline(always)]
-fn filter_as_usize(x: &Option<Level>) -> usize {
+fn filter_as_usize_sort_key(x: Option<Level>) -> usize {
match x {
- Some(Level(f)) => *f as usize,
- None => LevelFilter::OFF_USIZE,
+ Some(Level(f)) => f as usize,
+ // The niche optimization for LevelFilter::OFF isn't guaranteed
+ // to be the last variant + 1, so we explicitly return that for sorting here.
+ None => if USE_NATIVE_SORT { LevelFilter::OFF_USIZE } else { LevelInner::Error as usize + 1 },
}
}
impl PartialEq<Level> for LevelFilter {
#[inline(always)]
fn eq(&self, other: &Level) -> bool {
- filter_as_usize(&self.0) == other.0 as usize
+ filter_as_usize_sort_key(self.0) == other.0 as usize
}
}
@@ -999,56 +1003,56 @@
#[inline(always)]
fn lt(&self, other: &LevelFilter) -> bool {
- filter_as_usize(&other.0) < filter_as_usize(&self.0)
+ filter_as_usize_sort_key(other.0) < filter_as_usize_sort_key(self.0)
}
#[inline(always)]
fn le(&self, other: &LevelFilter) -> bool {
- filter_as_usize(&other.0) <= filter_as_usize(&self.0)
+ filter_as_usize_sort_key(other.0) <= filter_as_usize_sort_key(self.0)
}
#[inline(always)]
fn gt(&self, other: &LevelFilter) -> bool {
- filter_as_usize(&other.0) > filter_as_usize(&self.0)
+ filter_as_usize_sort_key(other.0) > filter_as_usize_sort_key(self.0)
}
#[inline(always)]
fn ge(&self, other: &LevelFilter) -> bool {
- filter_as_usize(&other.0) >= filter_as_usize(&self.0)
+ filter_as_usize_sort_key(other.0) >= filter_as_usize_sort_key(self.0)
}
}
impl Ord for LevelFilter {
#[inline(always)]
fn cmp(&self, other: &Self) -> cmp::Ordering {
- filter_as_usize(&other.0).cmp(&filter_as_usize(&self.0))
+ filter_as_usize_sort_key(other.0).cmp(&filter_as_usize_sort_key(self.0))
}
}
impl PartialOrd<Level> for LevelFilter {
#[inline(always)]
fn partial_cmp(&self, other: &Level) -> Option<cmp::Ordering> {
- Some((other.0 as usize).cmp(&filter_as_usize(&self.0)))
+ Some((other.0 as usize).cmp(&filter_as_usize_sort_key(self.0)))
}
#[inline(always)]
fn lt(&self, other: &Level) -> bool {
- (other.0 as usize) < filter_as_usize(&self.0)
+ (other.0 as usize) < filter_as_usize_sort_key(self.0)
}
#[inline(always)]
fn le(&self, other: &Level) -> bool {
- (other.0 as usize) <= filter_as_usize(&self.0)
+ (other.0 as usize) <= filter_as_usize_sort_key(self.0)
}
#[inline(always)]
fn gt(&self, other: &Level) -> bool {
- (other.0 as usize) > filter_as_usize(&self.0)
+ (other.0 as usize) > filter_as_usize_sort_key(self.0)
}
#[inline(always)]
fn ge(&self, other: &Level) -> bool {
- (other.0 as usize) >= filter_as_usize(&self.0)
+ (other.0 as usize) >= filter_as_usize_sort_key(self.0)
}
}
@@ -1101,7 +1105,7 @@
#[test]
fn level_filter_reprs() {
let mapping = [
- (LevelFilter::OFF, LevelInner::Error as usize + 1),
+ (LevelFilter::OFF, LevelFilter::OFF_USIZE),
(LevelFilter::ERROR, LevelInner::Error as usize),
(LevelFilter::WARN, LevelInner::Warn as usize),
(LevelFilter::INFO, LevelInner::Info as usize),
@@ -1119,4 +1123,41 @@
assert_eq!(expected, repr, "repr changed for {:?}", filter)
}
}
+
+ #[test]
+ fn level_filter_ordering() {
+ assert!(LevelFilter::OFF < LevelFilter::ERROR);
+ assert!(LevelFilter::OFF < LevelFilter::WARN);
+ assert!(LevelFilter::OFF < LevelFilter::INFO);
+ assert!(LevelFilter::OFF < LevelFilter::DEBUG);
+ assert!(LevelFilter::OFF < LevelFilter::TRACE);
+
+ assert!(LevelFilter::OFF < Level::ERROR);
+ assert!(LevelFilter::OFF < Level::WARN);
+ assert!(LevelFilter::OFF < Level::INFO);
+ assert!(LevelFilter::OFF < Level::DEBUG);
+ assert!(LevelFilter::OFF < Level::TRACE);
+
+ assert!(LevelFilter::ERROR < LevelFilter::WARN);
+ assert!(LevelFilter::WARN < LevelFilter::INFO);
+ assert!(LevelFilter::INFO < LevelFilter::DEBUG);
+ assert!(LevelFilter::DEBUG < LevelFilter::TRACE);
+
+ assert!(Level::ERROR < LevelFilter::WARN);
+ assert!(Level::WARN < LevelFilter::INFO);
+ assert!(Level::INFO < LevelFilter::DEBUG);
+ assert!(Level::DEBUG < LevelFilter::TRACE);
+
+ assert!(LevelFilter::ERROR > LevelFilter::OFF);
+ assert!(LevelFilter::WARN > LevelFilter::OFF);
+ assert!(LevelFilter::INFO > LevelFilter::OFF);
+ assert!(LevelFilter::DEBUG > LevelFilter::OFF);
+ assert!(LevelFilter::TRACE > LevelFilter::OFF);
+
+ assert!(Level::ERROR > LevelFilter::OFF);
+ assert!(Level::WARN > LevelFilter::OFF);
+ assert!(Level::INFO > LevelFilter::OFF);
+ assert!(Level::DEBUG > LevelFilter::OFF);
+ assert!(Level::TRACE > LevelFilter::OFF);
+ }
}
