Skip to content

update miri - #74146

Merged
bors merged 1 commit into
rust-lang:masterfrom
RalfJung:miri
Jul 10, 2020
Merged

update miri#74146
bors merged 1 commit into
rust-lang:masterfrom
RalfJung:miri

Conversation

@RalfJung

Copy link
Copy Markdown
Member

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

@RalfJung

Copy link
Copy Markdown
MemberAuthor

@bors r+

@bors

bors commented Jul 8, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit eb268380f3326ed33e34605b5fd80d74d806d20e has been approved by RalfJung

@bors

bors commented Jul 8, 2020

Copy link
Copy Markdown
Collaborator

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

@borsbors 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
Copy Markdown
MemberAuthor

@bors r-
local CI failure

@borsbors 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
Copy Markdown
MemberAuthor

@bors r+

@bors

bors commented Jul 8, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 512f9e9 has been approved by RalfJung

@borsbors 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
@ManishearthManishearth mentioned this pull request Jul 8, 2020
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 8, 2020
@ManishearthManishearth mentioned this pull request Jul 8, 2020
@Manishearth

Copy link
Copy Markdown
Member

@bors r-

#74155 (comment)

@borsbors 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
Copy Markdown
MemberAuthor

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
Copy Markdown
MemberAuthor

@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.

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

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
Contributor

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

RalfJung commented Jul 8, 2020

Copy link
Copy Markdown
MemberAuthor

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
Copy Markdown
Contributor

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

Copy link
Copy Markdown
MemberAuthor

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.

@RalfJungRalfJung removed the O-windows Operating system: Windows label Jul 9, 2020
@RalfJung

Copy link
Copy Markdown
MemberAuthor

@bors r+

@bors

bors commented Jul 9, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 81e4805df28013cddb633167a9556e1301daf6c3 has been approved by RalfJung

@borsbors 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
Copy Markdown
MemberAuthor

@bors r+

@bors

bors commented Jul 9, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 35fae73 has been approved by RalfJung

Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 9, 2020
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 9, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 9, 2020
…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:masterJul 10, 2020
@RalfJung
RalfJung deleted the miri branch July 11, 2020 09:53
@RalfJung

Copy link
Copy Markdown
MemberAuthor

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
Copy Markdown
Member

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

@RalfJung

Copy link
Copy Markdown
MemberAuthor

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
Copy Markdown
Member

Yeah using the command you have provided.

@RalfJung

Copy link
Copy Markdown
MemberAuthor

That's great, thanks a lot :)

@RalfJungRalfJung mentioned this pull request Jul 11, 2020
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 11, 2020
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
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

retep998 commented Jul 14, 2020

Copy link
Copy Markdown
Contributor

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

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

Labels

S-waiting-on-borsStatus: 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.

miri no longer builds after rust-lang/rust#74059

7 participants

@RalfJung@bors@Manishearth@rustbot@retep998@mati865@cuviper