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.

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

fix Miri offset_from #66083

Merged
merged 5 commits into from
Nov 5, 2019
Merged

fix Miri offset_from #66083

merged 5 commits into from
Nov 5, 2019

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Nov 4, 2019

This is needed to make rust-lang/miri#1032 pass.

@rust-highfive
Copy link
Collaborator

r? @varkor

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 4, 2019
@RalfJung
Copy link
Member Author

RalfJung commented Nov 4, 2019

This PR fixes https://github.com/RalfJung/miri-test-libstd, which currently fails.

@RalfJung
Copy link
Member Author

RalfJung commented Nov 5, 2019

r? @oli-obk

@rust-highfive rust-highfive assigned oli-obk and unassigned varkor Nov 5, 2019
let b = self.read_immediate(args[1])?.to_scalar()?;

// Special case: if both scalars are *equal integers*
// and not NULL, their offset is 0.
Copy link
Contributor

Choose a reason for hiding this comment

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

why is the null case important? The docs state nothing of the kind. Or is the "valid object" thing the problem?

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, since wrapping_offset_from is safe, and you can supply null pointers to it... I guess this should be allowed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed this is about the "valid object" part. There's never a valid object at 0, not even one of size 0, so 0 is never a valid input here.

wrapping_offset_from does not have the "valid object" requirement.

@RalfJung
Copy link
Member Author

RalfJung commented Nov 5, 2019

@bors r=oli-obk p=1
(priority because this breaks Miri's test suite.)

@bors
Copy link
Contributor

bors commented Nov 5, 2019

📌 Commit a593b54 has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 5, 2019
@bors
Copy link
Contributor

bors commented Nov 5, 2019

⌛ Testing commit a593b54 with merge 3a1b3b3...

bors added a commit that referenced this pull request Nov 5, 2019
fix Miri offset_from

This is needed to make rust-lang/miri#1032 pass.
@bors
Copy link
Contributor

bors commented Nov 5, 2019

☀️ Test successful - checks-azure
Approved by: oli-obk
Pushing 3a1b3b3 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 5, 2019
@bors bors merged commit a593b54 into rust-lang:master Nov 5, 2019
bors added a commit to rust-lang/miri that referenced this pull request Nov 5, 2019
test offset_from

This currently fails and needs a rustc fix: rust-lang/rust#66083
@RalfJung RalfJung deleted the miri-offset-from branch November 6, 2019 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants