Skip to content

Fix some sanity checks for the behavior of utimesat and stat - #27896

Open
hoodmane wants to merge 3 commits into
emscripten-core:mainfrom
hoodmane:fix-time-precision
Open

hoodmane wants to merge 3 commits into
emscripten-core:mainfrom
hoodmane:fix-time-precision

Conversation

@hoodmane

@hoodmane hoodmane commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator
  1. Creating a file then updating its mtime with utimensat UTIME_NOW should never move the mtime backwards.
  2. Creating a file, stat-ing it, then using utimesat with the mtime that stat returned, then stating it again should return identical timestamp.

Both failing due to some problems with rounding.

This fixes the regressions in the CPython test suite on Emscripten 6.0.11:
python/cpython#158627

1. Creating a file then updating its mtime with utimensat UTIME_NOW should never
   move the mtime backwards.
2. Creating a file, stat-ing it, then using utimesat with the mtime that stat
   returned, then stating it again should return identical timestamp.

Both failing due to some problems with rounding.
@hoodmane
hoodmane force-pushed the fix-time-precision branch from 68ec8d3 to 86ecc6a Compare October 6, 2026 20:42
Comment thread src/lib/libsyscall.js
// Use a sub-millisecond clock for "now" if possible. This is needed to
// ensure that a later UTIME_NOW doesn't set the time earlier than an
// earlier UTIME_NOW, see test_utime_now.c
var now = globalThis.performance?.timeOrigin ? performance.timeOrigin + performance.now() : Date.now();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think can just write this as emscripten_get_now().

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

emscripten_get_now() doesn't add performance.timeOrigin

Comment thread src/lib/libsyscall.js
{{{ makeSetValue('buf', C_STRUCTS.stat.st_ctim.tv_nsec, 'Math.floor((ctime % 1000) * 1_000_000)', SIZE_TYPE) }}};
// Round to the nearest 10 microseconds. This matches the granularity
// that __syscall_utimensat rounds down to when setting timestamps, so
// that values read via stat() round-trip through utimensat().

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if I should revert #27794 instead. Perhaps my attempt if fixing the rounding issues just made things worse?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for reverting #27794.

@hoodmane

hoodmane commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

But it would be nice to also add these checks.

@sbc100

sbc100 commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

Sure, would you have time to create a new PR (or update this one) such that the round parts of #27794 and new tests added?

@sbc100

sbc100 commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

When I say revert #27794, just mean the parts that were doing the extra rounding.

I do think we might need to come up with and alternative fix for the flakiness in #25280 if we do that though (maybe just make the test less brittle).

@hoodmane

hoodmane commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

Okay will do.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants