Skip to content

Commit

Permalink
Prevent TimeStamps from being modified (#821)
Browse files Browse the repository at this point in the history
Our `TimeStamp` type is internally an `Instant`. The representation of
`Instant` depends on the platform, but for many platforms it internally
consists of unsigned numbers. For WASI its worse, because the moment the
WASI VM starts, the time starts at 0. This means any subtraction from
the time results in a numeric underflow and might panic. To prevent this
from happening, `TimeStamp`s can no longer be manually modified, they
always represent valid points in time.
  • Loading branch information
CryZe authored Jun 22, 2024
1 parent 922cc84 commit ccc781e
Show file tree
Hide file tree
Showing 9 changed files with 26 additions and 85 deletions.
8 changes: 0 additions & 8 deletions src/platform/no_std/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,6 @@ impl Instant {
}
}

impl Sub<Duration> for Instant {
type Output = Instant;

fn sub(self, rhs: Duration) -> Instant {
Self(self.0 - rhs)
}
}

impl Sub for Instant {
type Output = Duration;

Expand Down
27 changes: 0 additions & 27 deletions src/platform/normal/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,15 +127,6 @@ cfg_if::cfg_if! {
}
}

impl Sub<Duration> for Instant {
type Output = Instant;

#[inline]
fn sub(self, rhs: Duration) -> Instant {
Self(self.0 - rhs)
}
}

impl Sub for Instant {
type Output = Duration;

Expand Down Expand Up @@ -171,15 +162,6 @@ cfg_if::cfg_if! {
}
}

impl Sub<Duration> for Instant {
type Output = Instant;

#[inline]
fn sub(self, rhs: Duration) -> Instant {
Self((self.0 as i64 - i64::try_from(rhs.whole_nanoseconds()).unwrap()) as u64)
}
}

impl Sub for Instant {
type Output = Duration;

Expand All @@ -203,15 +185,6 @@ cfg_if::cfg_if! {
}
}

impl Sub<Duration> for Instant {
type Output = Instant;

#[inline]
fn sub(self, rhs: Duration) -> Instant {
Self(time::ext::InstantExt::sub_signed(self.0, rhs))
}
}

impl Sub for Instant {
type Output = Duration;

Expand Down
8 changes: 0 additions & 8 deletions src/platform/wasm/unknown/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,6 @@ impl Instant {
}
}

impl Sub<Duration> for Instant {
type Output = Instant;

fn sub(self, rhs: Duration) -> Instant {
Self(self.0 - rhs)
}
}

impl Sub for Instant {
type Output = Duration;

Expand Down
8 changes: 0 additions & 8 deletions src/platform/wasm/web/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,6 @@ impl Instant {
}
}

impl Sub<Duration> for Instant {
type Output = Instant;

fn sub(self, rhs: Duration) -> Instant {
Self(self.0 - rhs)
}
}

impl Sub for Instant {
type Output = Duration;

Expand Down
14 changes: 1 addition & 13 deletions src/timing/time_stamp.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
use crate::{
platform::{Duration, Instant},
TimeSpan,
};
use crate::{platform::Instant, TimeSpan};
use core::ops::Sub;

/// A `TimeStamp` stores a point in time that can be used to calculate a
Expand All @@ -26,12 +23,3 @@ impl Sub for TimeStamp {
TimeSpan::from(self.0 - rhs.0)
}
}

impl Sub<TimeSpan> for TimeStamp {
type Output = TimeStamp;

#[inline]
fn sub(self, rhs: TimeSpan) -> TimeStamp {
TimeStamp(self.0 - Duration::from(rhs))
}
}
22 changes: 12 additions & 10 deletions src/timing/timer/active_attempt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@ use crate::{
#[derive(Debug, Clone)]
pub struct ActiveAttempt {
pub state: State,
/// The date time when the attempt started.
pub attempt_started: AtomicDateTime,
/// The time stamp when the attempt started.
pub start_time: TimeStamp,
pub start_time_with_offset: TimeStamp,
// This gets adjusted after resuming
pub adjusted_start_time: TimeStamp,
/// The original offset gets kept around to undo the pauses.
pub original_offset: TimeSpan,
/// The adjusted offset gets modified as pauses get accumulated.
pub adjusted_offset: TimeSpan,
pub game_time_paused_at: Option<TimeSpan>,
pub loading_times: Option<TimeSpan>,
}
Expand Down Expand Up @@ -54,9 +57,8 @@ impl ActiveAttempt {
game_time,
};
}
State::NotEnded { time_paused_at, .. } => {
time_paused_at.unwrap_or_else(|| TimeStamp::now() - self.adjusted_start_time)
}
State::NotEnded { time_paused_at, .. } => time_paused_at
.unwrap_or_else(|| TimeStamp::now() - self.start_time + self.adjusted_offset),
};

let game_time = self
Expand All @@ -75,11 +77,11 @@ impl ActiveAttempt {
..
} = self.state
{
return Some(TimeStamp::now() - self.start_time_with_offset - pause_time);
return Some(TimeStamp::now() - self.start_time + self.original_offset - pause_time);
}

if self.start_time_with_offset != self.adjusted_start_time {
Some(self.start_time_with_offset - self.adjusted_start_time)
if self.original_offset != self.adjusted_offset {
Some(self.original_offset - self.adjusted_offset)
} else {
None
}
Expand All @@ -105,7 +107,7 @@ impl ActiveAttempt {
return Err(Error::TimerPaused);
}

let real_time = TimeStamp::now() - self.adjusted_start_time;
let real_time = TimeStamp::now() - self.start_time + self.adjusted_offset;

if real_time < TimeSpan::zero() {
return Err(Error::NegativeTime);
Expand Down
14 changes: 8 additions & 6 deletions src/timing/timer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ impl Timer {
if self.active_attempt.is_none() {
let attempt_started = AtomicDateTime::now();
let start_time = TimeStamp::now();
let start_time_with_offset = start_time - self.run.offset();
let offset = self.run.offset();

self.active_attempt = Some(ActiveAttempt {
state: State::NotEnded {
Expand All @@ -291,8 +291,8 @@ impl Timer {
},
attempt_started,
start_time,
start_time_with_offset,
adjusted_start_time: start_time_with_offset,
original_offset: offset,
adjusted_offset: offset,
game_time_paused_at: None,
loading_times: None,
});
Expand Down Expand Up @@ -497,7 +497,8 @@ impl Timer {
};

if time_paused_at.is_none() {
*time_paused_at = Some(TimeStamp::now() - active_attempt.adjusted_start_time);
*time_paused_at =
Some(TimeStamp::now() - active_attempt.start_time + active_attempt.adjusted_offset);
Ok(Event::Paused)
} else {
Err(Error::AlreadyPaused)
Expand All @@ -513,7 +514,8 @@ impl Timer {
};

if let Some(pause_time) = *time_paused_at {
active_attempt.adjusted_start_time = TimeStamp::now() - pause_time;
active_attempt.adjusted_offset =
pause_time - (TimeStamp::now() - active_attempt.start_time);
*time_paused_at = None;
Ok(Event::Resumed)
} else {
Expand Down Expand Up @@ -579,7 +581,7 @@ impl Timer {
};

if let Some(active_attempt) = &mut self.active_attempt {
active_attempt.adjusted_start_time = active_attempt.start_time_with_offset;
active_attempt.adjusted_offset = active_attempt.original_offset;
Ok(event)
} else {
Err(Error::NoRunInProgress)
Expand Down
8 changes: 4 additions & 4 deletions tests/rendering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,8 @@ fn actual_split_file() {
check(
&layout.state(&mut image_cache, &timer.snapshot()),
&image_cache,
"42b2292f5c884c17",
"1e4c66359bdc32e8",
"c1e9e757c15a35ae",
"4fe65a630b531c54",
"actual_split_file",
);
}
Expand Down Expand Up @@ -228,8 +228,8 @@ fn timer_delta_background() {
&layout.state(&mut image_cache, &timer.snapshot()),
&image_cache,
[250, 300],
"e96525c84aa77f66",
"bd8139d0f625af38",
"748fa26a41a8d5a3",
"8bae1351d0dd52d7",
"timer_delta_background_stopped",
);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/run_files/livesplit1.0.lss
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<GameIcon />
<GameName>Pac-Man World 3</GameName>
<CategoryName>Any%</CategoryName>
<Offset>00:00:00</Offset>
<Offset>12:34:56</Offset>
<AttemptCount>3</AttemptCount>
<RunHistory>
<Time id="1">
Expand Down

0 comments on commit ccc781e

Please sign in to comment.