Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add clock_gettime shim #975

Merged
merged 13 commits into from
Oct 15, 2019
Merged

Add clock_gettime shim #975

merged 13 commits into from
Oct 15, 2019

Conversation

pvdrz
Copy link
Contributor

@pvdrz pvdrz commented Oct 2, 2019 •
edited
Loading

r? @oli-obk

I think there is no way to do proper testing of this other than checking that miri does not crash when calling clock_gettime.

@pvdrz pvdrz force-pushed the clock-shim branch 2 times, most recently from 2ffc663 to 5259ebe Compare October 3, 2019 15:45
@bjorn3
Copy link
Member

bjorn3 commented Oct 3, 2019

Also I'm not sure what to do if the current time is lower than UNIX_EPOCH, which might happen if someone decides to mess with the clock before running miri.

You could calculate the opposite difference (UNIX_EPOCH-now) and then write a negative number, as clock_gettime returns a signed int.

@pvdrz
Copy link
Contributor Author

pvdrz commented Oct 3, 2019

Also I'm not sure what to do if the current time is lower than UNIX_EPOCH, which might happen if someone decides to mess with the clock before running miri.

You could calculate the opposite difference (UNIX_EPOCH-now) and then write a negative number, as clock_gettime returns a signed int.

That sounds like a good idea. I'll try to do that

@pvdrz
Copy link
Contributor Author

pvdrz commented Oct 3, 2019 •
edited
Loading

Also CI is failing because the size of long and time_t is platform dependent. How should I handle that?

Edit: Solved by recovering the types from the target's libc

@bors
Copy link
Contributor

bors commented Oct 8, 2019

☔ The latest upstream changes (presumably #977) made this pull request unmergeable. Please resolve the merge conflicts.

@pvdrz pvdrz force-pushed the clock-shim branch 2 times, most recently from bbef7a0 to e00db1b Compare October 8, 2019 20:43
@pvdrz pvdrz mentioned this pull request Oct 10, 2019
Merged
RalfJung
RalfJung reviewed Oct 10, 2019
View reviewed changes
src/shims/env.rs Outdated Show resolved Hide resolved
RalfJung
RalfJung reviewed Oct 10, 2019
View reviewed changes
src/helpers.rs Outdated Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented Oct 11, 2019

☔ The latest upstream changes (presumably #983) made this pull request unmergeable. Please resolve the merge conflicts.

RalfJung
RalfJung reviewed Oct 11, 2019
View reviewed changes
src/helpers.rs Outdated Show resolved Hide resolved
RalfJung
RalfJung reviewed Oct 11, 2019
View reviewed changes
src/shims/time.rs Outdated Show resolved Hide resolved
RalfJung
RalfJung reviewed Oct 11, 2019
View reviewed changes
src/shims/time.rs Outdated Show resolved Hide resolved
RalfJung
RalfJung reviewed Oct 13, 2019
View reviewed changes
src/helpers.rs Outdated Show resolved Hide resolved
RalfJung
RalfJung reviewed Oct 13, 2019
View reviewed changes
src/helpers.rs Outdated Show resolved Hide resolved
RalfJung
RalfJung reviewed Oct 14, 2019
View reviewed changes
src/shims/time.rs Outdated Show resolved Hide resolved
RalfJung
RalfJung reviewed Oct 14, 2019
View reviewed changes
src/shims/time.rs Outdated Show resolved Hide resolved
RalfJung
RalfJung reviewed Oct 14, 2019
View reviewed changes
src/shims/time.rs Outdated Show resolved Hide resolved
@RalfJung
Copy link
Member

Yeah this looks good, thanks a lot. :)

@bors r+

@bors
Copy link
Contributor

bors commented Oct 14, 2019

📌 Commit f9c7688 has been approved by RalfJung

bors added a commit that referenced this pull request Oct 14, 2019
@bors
Auto merge of #975 - christianpoveda:clock-shim, r=RalfJung
5d66fb4 
Add clock_gettime shim

r? @oli-obk

I think there is no way to do proper testing of this other than checking that miri does not crash when calling `clock_gettime`.
@bors
Copy link
Contributor

bors commented Oct 14, 2019

⌛ Testing commit f9c7688 with merge 5d66fb4...

@RalfJung
Copy link
Member

That push aborted landing the PR I think.

@bors r+ retry

@bors
Copy link
Contributor

bors commented Oct 15, 2019

📌 Commit f9c7688 has been approved by RalfJung

@bors
Copy link
Contributor

bors commented Oct 15, 2019

⌛ Testing commit f9c7688 with merge d902a11...

bors added a commit that referenced this pull request Oct 15, 2019
@bors
Auto merge of #975 - christianpoveda:clock-shim, r=RalfJung
d902a11 
Add clock_gettime shim

r? @oli-obk

I think there is no way to do proper testing of this other than checking that miri does not crash when calling `clock_gettime`.
@RalfJung RalfJung mentioned this pull request Oct 15, 2019
Merged
@bors
Copy link
Contributor

bors commented Oct 15, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: RalfJung
Pushing d902a11 to master...

@bors bors merged commit f9c7688 into rust-lang:master Oct 15, 2019
@bors bors mentioned this pull request Oct 15, 2019
Merged
@pvdrz
Copy link
Contributor Author

pvdrz commented Oct 15, 2019

Thank you, I did a commit to the wrong branch by accident D:

@RalfJung RalfJung mentioned this pull request Oct 18, 2019
Closed
@jonhoo jonhoo mentioned this pull request Jan 18, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RalfJung RalfJung RalfJung left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pvdrz @bjorn3 @bors @RalfJung