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

update miri #74146

Merged
merged 1 commit into from
Jul 10, 2020
Merged

update miri #74146

merged 1 commit into from
Jul 10, 2020

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Jul 8, 2020

Fixes #74132
Cc @rust-lang/miri r? @ghost

@RalfJung
Copy link
Member Author

RalfJung commented Jul 8, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Jul 8, 2020

📌 Commit eb268380f3326ed33e34605b5fd80d74d806d20e has been approved by RalfJung

@bors
Copy link
Contributor

bors commented Jul 8, 2020

🌲 The tree is currently closed for pull requests below priority 1000, this pull request will be tested once the tree is reopened

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 8, 2020
@RalfJung
Copy link
Member Author

RalfJung commented Jul 8, 2020

@bors r-
local CI failure

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 8, 2020
@RalfJung
Copy link
Member Author

RalfJung commented Jul 8, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Jul 8, 2020

📌 Commit 512f9e9 has been approved by RalfJung

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 8, 2020
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 8, 2020
@Manishearth
Rollup merge of rust-lang#74146 - RalfJung:miri, r=RalfJung
ef081d1 
update miri

Fixes rust-lang#74132
Cc @rust-lang/miri r? @ghost
@Manishearth Manishearth mentioned this pull request Jul 8, 2020
Closed
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 8, 2020
@Manishearth
Rollup merge of rust-lang#74146 - RalfJung:miri, r=RalfJung
be0e874 
update miri

Fixes rust-lang#74132
Cc @rust-lang/miri r? @ghost
@Manishearth Manishearth mentioned this pull request Jul 8, 2020
Closed
@Manishearth
Copy link
Member

@bors r-

#74155 (comment)

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 8, 2020
@RalfJung
Copy link
Member Author

RalfJung commented Jul 8, 2020

Wtf. "The filename, directory name, or volume label syntax is incorrect."? On Windows only? I have no way of debugging that.^^

I assume it is caused by rust-lang/miri#1469 but why would canonicalize not work?

@RalfJung
Copy link
Member Author

RalfJung commented Jul 8, 2020

@rustbot ping windows
Does anybody with Windows experience have an idea what on Earth could be happening here? To reproduce, check out this branch and run ./x.py test src/tools/miri --stage 0. It works fine here on Linux.

@rustbot rustbot added the O-windows Operating system: Windows label Jul 8, 2020
@rustbot
Copy link
Collaborator

rustbot commented Jul 8, 2020

Hey Windows Group! This bug has been identified as a good "Windows candidate".
In case it's useful, here are some instructions for tackling these sorts of
bugs. Maybe take a look?
Thanks! <3

cc @arlosi @danielframpton @gdr-at-ms @kennykerr @luqmana @lzybkr @retep998 @rylev @sivadeilra @spastorino

@retep998
Copy link
Member

retep998 commented Jul 8, 2020

canonicalize is a bad way to turn a relative path into an absolute path (at least on Windows anyway). There are several things to keep in mind:

  • canonicalize only works when the path actually exists on the filesystem, and can be accessed.
  • canonicalize does not work on certain filesystems, like some RAM disks don't support the operation.
  • canonicalize returns a path of the form \\?\C:\foo\bar which can be problematic.
    • Not all programs support that form of path (such as MinGW)
    • If you join anything onto that path you cannot use / or .. or . because \\?\ paths disable the processing of those.

I continue to advocate for the addition of a better API to turn relative paths into absolute paths.

@RalfJung
Copy link
Member Author

RalfJung commented Jul 8, 2020 •
edited
Loading

Any advice for what to do instead? The reason we need this is that Miri changes the working directory before invoking xargo, so to pass relative paths to xargo we have to make them absolute first.

The path having to exist is fine here.

@retep998
Copy link
Member

retep998 commented Jul 8, 2020

The correct way to turn a relative path into an absolute path on Windows is GetFullPathNameW or the pure Rust equivalent. std does not currently provide such a function.

Ways to possibly fix it while still using canonicalize include:

  • Ensure that anything that joins onto that path does not specify / or . or ...
  • Strip the \\?\ prefix.

@RalfJung RalfJung mentioned this pull request Jul 9, 2020
Merged
@RalfJung
Copy link
Member Author

RalfJung commented Jul 9, 2020

I see, thanks. So japaric/xargo#297 should help on the xargo side. But I have no idea if this is sufficient, and I cannot test this, so I think I'll also add a hack on the Miri side to strip the \\?\ prefix.

@RalfJung RalfJung removed the O-windows Operating system: Windows label Jul 9, 2020
@RalfJung
Copy link
Member Author

RalfJung commented Jul 9, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Jul 9, 2020

📌 Commit 81e4805df28013cddb633167a9556e1301daf6c3 has been approved by RalfJung

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 9, 2020
@RalfJung
Copy link
Member Author

RalfJung commented Jul 9, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Jul 9, 2020

📌 Commit 35fae73 has been approved by RalfJung

Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 9, 2020
@Manishearth
Rollup merge of rust-lang#74146 - RalfJung:miri, r=RalfJung
b18afd7 
update miri

Fixes rust-lang#74132
Cc @rust-lang/miri r? @ghost
@Manishearth Manishearth mentioned this pull request Jul 9, 2020
Closed
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 9, 2020
@Manishearth
Rollup merge of rust-lang#74146 - RalfJung:miri, r=RalfJung
feaf996 
update miri

Fixes rust-lang#74132
Cc @rust-lang/miri r? @ghost
@Manishearth Manishearth mentioned this pull request Jul 9, 2020
Closed
@Manishearth Manishearth mentioned this pull request Jul 9, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 9, 2020
@bors
Auto merge of rust-lang#74195 - Manishearth:rollup-h3m0sl8, r=Manishe…
e59b08e 
…arth

Rollup of 14 pull requests

Successful merges:

 - rust-lang#73292 (Fixing broken link for the Eq trait)
 - rust-lang#73791 (Allow for parentheses after macro intra-doc-links)
 - rust-lang#74070 ( Use for<'tcx> fn pointers in Providers, instead of having Providers<'tcx>.)
 - rust-lang#74077 (Use relative path for local links to primitives)
 - rust-lang#74079 (Eliminate confusing "globals" terminology.)
 - rust-lang#74107 (Hide `&mut self` methods from Deref in sidebar if there are no `DerefMut` impl for the type.)
 - rust-lang#74136 (Fix broken link in rustdocdoc)
 - rust-lang#74137 (Update cargo)
 - rust-lang#74142 (Liballoc use vec instead of vector)
 - rust-lang#74143 (Try remove unneeded ToString import in liballoc slice)
 - rust-lang#74146 (update miri)
 - rust-lang#74150 (Avoid "blacklist")
 - rust-lang#74184 (Add docs for intra-doc-links)
 - rust-lang#74188 (Tweak `::` -> `:` typo heuristic and reduce verbosity)

Failed merges:

 - rust-lang#74122 (Start-up clean-up)
 - rust-lang#74127 (Avoid "whitelist")

r? @ghost
@bors bors merged commit 089a6e1 into rust-lang:master Jul 10, 2020
@RalfJung RalfJung deleted the miri branch July 11, 2020 09:53
@RalfJung
Copy link
Member Author

I have fixed the cases of adding / or .. to the path in xargo that I found.

It would be great if someone with a way to build rustc on Windows could checkout https://github.com/RalfJung/rust/tree/miri and run x.py test src/tools/miri --stage 0 to see if that works now.

@mati865
Copy link
Contributor

mati865 commented Jul 11, 2020

With miri at eee22ff it's tests pass on native windows-gnu.

@RalfJung
Copy link
Member Author

Did you run them via x.py? That is the key here, as it sets XARGO_RUST_SRC which hits a different code path in cargo-miri.

@mati865
Copy link
Contributor

mati865 commented Jul 11, 2020

Yeah using the command you have provided.

@RalfJung
Copy link
Member Author

That's great, thanks a lot :)

@RalfJung RalfJung mentioned this pull request Jul 11, 2020
Merged
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 11, 2020
@Manishearth
Rollup merge of rust-lang#74241 - RalfJung:miri, r=RalfJung
d6c4c9e 
update miri

This incorporates rust-lang/miri#1474. [Last time](rust-lang#74146) that change caused trouble but I fixed xargo since then and [now it should work](rust-lang#74146 (comment)).

Cc @rust-lang/miri r? @ghost
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 11, 2020
@Manishearth
Rollup merge of rust-lang#74241 - RalfJung:miri, r=RalfJung
95c5fb8 
update miri

This incorporates rust-lang/miri#1474. [Last time](rust-lang#74146) that change caused trouble but I fixed xargo since then and [now it should work](rust-lang#74146 (comment)).

Cc @rust-lang/miri r? @ghost
@retep998
Copy link
Member

retep998 commented Jul 14, 2020 •
edited
Loading

For future reference on making paths absolute vs canonicalize: #59117

@retep998 retep998 mentioned this pull request Jul 14, 2020
Open
@cuviper cuviper added this to the 1.46 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.46
Development

Successfully merging this pull request may close these issues.

miri no longer builds after rust-lang/rust#74059
7 participants
@RalfJung @bors @Manishearth @rustbot @retep998 @mati865 @cuviper