Stream: t-compiler/meetings

Topic: [planning meeting] 2020-09-25


nikomatsakis (Sep 25 2020 at 13:42, on Zulip):

Hey @T-compiler/meeting -- we have a planning meeting in about 20 minutes.

nikomatsakis (Sep 25 2020 at 13:45, on Zulip):

Actually, @T-compiler/meeting, scratch that, we have no proposed meetings.

simulacrum (Sep 25 2020 at 13:45, on Zulip):

we were going to talk about llvm?

nikomatsakis (Sep 25 2020 at 13:46, on Zulip):

What we do have is a lot of meetings that need minutes =) including one I said I would do

nikomatsakis (Sep 25 2020 at 13:46, on Zulip):

Oh, sorry, I didn't know about that or forgot about it

nikomatsakis (Sep 25 2020 at 13:46, on Zulip):

Was that discussed in yesterday's triage meeting?

Wesley Wiser (Sep 25 2020 at 13:46, on Zulip):

https://rust-lang.zulipchat.com/#narrow/stream/238009-t-compiler.2Fmeetings/topic/.5Bweekly.20meeting.5D.202020-09-24.20.2354818/near/211136244

Wesley Wiser (Sep 25 2020 at 13:47, on Zulip):

https://rust-lang.zulipchat.com/#narrow/stream/238009-t-compiler.2Fmeetings/topic/.5Bweekly.20meeting.5D.202020-09-24.20.2354818/near/211137649

nikomatsakis (Sep 25 2020 at 13:47, on Zulip):

I'm skimming now :)

nikomatsakis (Sep 25 2020 at 13:48, on Zulip):

OK, @T-compiler/meeting, apologies, last ping, but in ~10 minutes we will replace planning meeting with a discussion about LLVM 11. I'd appreciate if someone was able to collect a few notes -- e.g., the issues we've had or other data.

nagisa (Sep 25 2020 at 13:49, on Zulip):

I've a conflicting meeting, sadly, so won't be able to be there.

nikomatsakis (Sep 25 2020 at 13:57, on Zulip):

OK, @nagisa, too bad. Any quick thoughts to share? ("Revert!" "Don't revert!")

nikomatsakis (Sep 25 2020 at 13:57, on Zulip):

cc @pnkfelix are you around today?

nikomatsakis (Sep 25 2020 at 13:57, on Zulip):

also @Nikita Popov

nikomatsakis (Sep 25 2020 at 14:02, on Zulip):

OK, @T-compiler/meeting, "planning meeting" starting now, but the plan is to discuss LLVM, as there are no proposed meetings this go around.

nikomatsakis (Sep 25 2020 at 14:02, on Zulip):

@Santiago Pastorino do you have a convenient list of LLVM-related issues?

eddyb (Sep 25 2020 at 14:03, on Zulip):

it's a meeting about planning the LLVM 11 plan

Wesley Wiser (Sep 25 2020 at 14:03, on Zulip):

Here's what I found:

I'm still looking for other stuff.

nikomatsakis (Sep 25 2020 at 14:03, on Zulip):

Performance impact?

nikomatsakis (Sep 25 2020 at 14:03, on Zulip):

I guess that's probably summarized in #73526

nikomatsakis (Sep 25 2020 at 14:04, on Zulip):

Final perf results are in and show a significant and sustained improvement in wall time for optimized builds on most benchmarks. Thanks all!

pnkfelix (Sep 25 2020 at 14:05, on Zulip):

(if we're you're gathering a list of LLVM related issues, be sure to catgorize whether the issue is fixed by the LLVM 11 upgrade or broken by it)

pnkfelix (Sep 25 2020 at 14:05, on Zulip):

(e.g. #73151 is apparently fixed by the upgrade)

Santiago Pastorino (Sep 25 2020 at 14:05, on Zulip):

nikomatsakis said:

Santiago Pastorino do you have a convenient list of LLVM-related issues?

I don't, but let me look on the previous weekly meeting agenda's

Wesley Wiser (Sep 25 2020 at 14:06, on Zulip):

Issues fixed by the upgrade aren't linked to the PR so I'm having a lot of trouble finding them

pnkfelix (Sep 25 2020 at 14:06, on Zulip):

I was honestly expecting to see more issues linking to PR #73526. Maybe the fact that there are so few is a really good sign.

pnkfelix (Sep 25 2020 at 14:07, on Zulip):

Wesley Wiser said:

Issues fixed by the upgrade aren't linked to the PR so I'm having a lot of trouble finding them

I'll skim the closed issues tagged with A-LLVM

nikomatsakis (Sep 25 2020 at 14:07, on Zulip):
Wesley Wiser (Sep 25 2020 at 14:09, on Zulip):

Yeah, that's correct.

nikomatsakis (Sep 25 2020 at 14:09, on Zulip):
pnkfelix (Sep 25 2020 at 14:09, on Zulip):

/me is going to take notes here: https://hackmd.io/j-O85DHLSU-IlOxACHt_Xw

nikomatsakis (Sep 25 2020 at 14:11, on Zulip):

I'm reading the discussion from previous meeting, it seems like we don't think rc3 backport will fix

Wesley Wiser (Sep 25 2020 at 14:11, on Zulip):

At least from our discussion on the initial PR to move to LLVM beta, the motivating factor seemed to be performance. There's no bug fixes called out in the discussion thread. https://zulip-archive.rust-lang.org/238009tcompilermeetings/14860weeklymeeting2020071654818.html#204093867

nikomatsakis (Sep 25 2020 at 14:12, on Zulip):

I guess to me a revert doesn't seem necessary, if #76980 is the only known issue

nikomatsakis (Sep 25 2020 at 14:12, on Zulip):

I think though the concerns were also one of "no doubt more problems will be found"?

Wesley Wiser (Sep 25 2020 at 14:13, on Zulip):

To clarify, you mean reverting back to LLVM 10 on the beta branch?

nikomatsakis (Sep 25 2020 at 14:13, on Zulip):

but maybe I'm jumping the gun :)

nikomatsakis (Sep 25 2020 at 14:13, on Zulip):

Wesley Wiser said:

To clarify, you mean reverting back to LLVM 10 on the beta branch?

correct

Santiago Pastorino (Sep 25 2020 at 14:13, on Zulip):

raw list (sorry that I didn't even check if you've discussed them already and anything) but sharing ...

Santiago Pastorino (Sep 25 2020 at 14:13, on Zulip):

#76042
#76387
#76980
#76466
#76247

#74498 (llvm 9)

Wesley Wiser (Sep 25 2020 at 14:14, on Zulip):

#76387 is reported against LLVM 10

pnkfelix (Sep 25 2020 at 14:14, on Zulip):

(apparently I jumped back so far that I'm looking at bugs that were fixed by the LLVM 10 upgrade...)

pnkfelix (Sep 25 2020 at 14:14, on Zulip):

(such as issue #72176)

Wesley Wiser (Sep 25 2020 at 14:16, on Zulip):

#76247 is reported against LLVM 10 as well, no indication if it's fixed by LLVM 11.

nikomatsakis (Sep 25 2020 at 14:19, on Zulip):

(are you investigating the others? should we divvy them up?)

pnkfelix (Sep 25 2020 at 14:20, on Zulip):

hmm what are we doing about #76387? Are we going to backport @Aaron Hill 's proposed patch to LLVM?

Aaron Hill (Sep 25 2020 at 14:20, on Zulip):

One of the reviewers said they'd take a look at my patch

pnkfelix (Sep 25 2020 at 14:20, on Zulip):

oh did I say we'd just wait another week there? I think I said that yesterday. :)

Aaron Hill (Sep 25 2020 at 14:21, on Zulip):

when does the bacport decision need to be made by?

pnkfelix (Sep 25 2020 at 14:21, on Zulip):

(since it did seem like review was progressing)

Wesley Wiser (Sep 25 2020 at 14:21, on Zulip):

seem to be the known issues specifically introduced by the LLVM 11 upgrade. The others are probably still broken but not caused by the LLVM 11 upgrade.

pnkfelix (Sep 25 2020 at 14:21, on Zulip):

the backport decision for small-ish things like #76387 can wait a while yet. Its this bigger question of whether to back out the whole LLVM11 upgrade that we need to decide sooner, due to all the fallout that would result

Wesley Wiser (Sep 25 2020 at 14:21, on Zulip):

#76466 is a bit concerning to me as it's not clear from the issue if upstream is even aware of the issue.

Santiago Pastorino (Sep 25 2020 at 14:21, on Zulip):

#74498 regressed on llvm 9 upgrade, so too old

nikomatsakis (Sep 25 2020 at 14:23, on Zulip):

Wesley Wiser said:

#76466 is a bit concerning to me as it's not clear from the issue if upstream is even aware of the issue.

yeah it sort of looks like "no"

pnkfelix (Sep 25 2020 at 14:24, on Zulip):

(anyone have a 32-bit windows VM handy to try to replicate the problem? of #76466?)

Santiago Pastorino (Sep 25 2020 at 14:24, on Zulip):

Wesley Wiser said:

#76466 is a bit concerning to me as it's not clear from the issue if upstream is even aware of the issue.

should we ping llvm crew?

pnkfelix (Sep 25 2020 at 14:24, on Zulip):

I'm not certain we've gotten local duplication of the problem as described

pnkfelix (Sep 25 2020 at 14:25, on Zulip):

pnkfelix said:

(anyone have a 32-bit windows VM handy to try to replicate the problem? of #76466?)

(hmm, actually, the original bug reporter said they were on windows 64-bit. so now I'm definitely confused...)

Santiago Pastorino (Sep 25 2020 at 14:27, on Zulip):

it seems like lld is having some issues to split when there are spaces

pnkfelix (Sep 25 2020 at 14:27, on Zulip):

from skimming the comments for #76960, it seems like we haven't reported an upstream bug report there (nor for #76466, as noted above)

LeSeulArtichaut (Sep 25 2020 at 14:28, on Zulip):

should we ping llvm crew?

I guess that would be the right thing to do then, right?
(To debug LLD and file a bug upstream)

pnkfelix (Sep 25 2020 at 14:28, on Zulip):

also regarding #76960: does the vk-shader-macros crate work on stable?

pnkfelix (Sep 25 2020 at 14:30, on Zulip):

(i'm going to infer "yes" based on its use of proc-macro-hack crate)

pnkfelix (Sep 25 2020 at 14:32, on Zulip):

my own investigation found that #73151 and #73396 were both fixed by the LLVM 11 upgrade

pnkfelix (Sep 25 2020 at 14:33, on Zulip):

now, for #73151, someone did file a request for the LLVM fix to be backported to LLVM 10 (and that backport did happen, back in mid June)

pnkfelix (Sep 25 2020 at 14:33, on Zulip):

but our discussion here isn't about reverting to LLVM 10 and then upgrading to latest 10.x, is it?

nikomatsakis (Sep 25 2020 at 14:34, on Zulip):

I don't think so

nikomatsakis (Sep 25 2020 at 14:34, on Zulip):

that seems to inject yet more uncertainty

nikomatsakis (Sep 25 2020 at 14:34, on Zulip):

are there still pending investigations or have we bottomed that out?

nikomatsakis (Sep 25 2020 at 14:34, on Zulip):

(seems like the latter)

Wesley Wiser (Sep 25 2020 at 14:35, on Zulip):

I was trying to find if #76466 had an upstream bug but I couldn't find one (open or closed).

Nikita Popov (Sep 25 2020 at 14:37, on Zulip):

Hi! When the LLVM 11 upgrade landed, I believe the plan was to revert it from the beta branch if there's too many issues, as it landed directly before branching. Now, what constitutes "too many issues"...

nikomatsakis (Sep 25 2020 at 14:37, on Zulip):

It seems like that is the judgement call we have to make now, I guess

nikomatsakis (Sep 25 2020 at 14:38, on Zulip):

(side note that it'd be nice if we could write out some of our reasoning to start to develop a kind of "decision tree" via precedent...)

nikomatsakis (Sep 25 2020 at 14:38, on Zulip):

one question I don't really know is how many folks are hitting these issues

pnkfelix (Sep 25 2020 at 14:39, on Zulip):

can I get a quick confirmation that my hackmd has collected a summary of all the data? https://hackmd.io/j-O85DHLSU-IlOxACHt_Xw

pnkfelix (Sep 25 2020 at 14:40, on Zulip):

Even ignoring the performance wins, my intuition from reading over the bugs that were fixed by the LLVM 11 upgrade leads me to think that we should stick with it.

Wesley Wiser (Sep 25 2020 at 14:40, on Zulip):

That looks correct to me.

nikomatsakis (Sep 25 2020 at 14:42, on Zulip):

when I last compared the hackmd to the zulip stream it looked fairly complete

nikomatsakis (Sep 25 2020 at 14:42, on Zulip):

there are definitely many advantages

nikomatsakis (Sep 25 2020 at 14:42, on Zulip):

I have a general bias for "track LLVM"

pnkfelix (Sep 25 2020 at 14:42, on Zulip):

I do have one slight misgiving that we haven't raised here

pnkfelix (Sep 25 2020 at 14:42, on Zulip):

LLVM 11 still hasn't been released, right?

nikomatsakis (Sep 25 2020 at 14:43, on Zulip):

I think https://github.com/rust-lang/rust/issues/76466 is the one that worries me the most. If it's as simple as "building rust in paths with spaces is broken on windows", that seems potentially far-reaching.

nikomatsakis (Sep 25 2020 at 14:43, on Zulip):

but if so then the reported regression case is way too complex

nikomatsakis (Sep 25 2020 at 14:43, on Zulip):

32-bit Windows specific

nikomatsakis (Sep 25 2020 at 14:43, on Zulip):

that should go in the notes I guess

nikomatsakis (Sep 25 2020 at 14:44, on Zulip):

pnkfelix said:

LLVM 11 still hasn't been released, right?

Not that I know of...

pnkfelix (Sep 25 2020 at 14:44, on Zulip):

pnkfelix said:

LLVM 11 still hasn't been released, right?

the reason I ask, is that I think a significant number of developers opt to use the system LLVM. When we planned out this upgrade, our hope was that LLVM 11 itself would be released by the time of our own update hit stable, right?

pnkfelix (Sep 25 2020 at 14:44, on Zulip):

my memory is that we did not say that an LLVM 11 delay in release would be automatic grounds for backing out the upgrade

nikomatsakis (Sep 25 2020 at 14:45, on Zulip):

I think we probably did hope for that. On the other hand, aren't those developers building LLVM now?

nikomatsakis (Sep 25 2020 at 14:45, on Zulip):

you mean rustc developers, right?

pnkfelix (Sep 25 2020 at 14:45, on Zulip):

but still: I would like to understand if we are going to be causing serious problems for those people using system LLVM?

pnkfelix (Sep 25 2020 at 14:45, on Zulip):

yes, I do mean rustc developers

pnkfelix (Sep 25 2020 at 14:45, on Zulip):

I don't know what their workflow is

nikomatsakis (Sep 25 2020 at 14:45, on Zulip):

because this backport doesn't seem to effect them -- they are building nightly, not older betas?

pnkfelix (Sep 25 2020 at 14:45, on Zulip):

I just know some people, when I mention LLVM builds being an issue for bootstrap times, they respond with "I'm using system LLVM"

nikomatsakis (Sep 25 2020 at 14:45, on Zulip):

I see

pnkfelix (Sep 25 2020 at 14:46, on Zulip):

hmm, you have a good point: What are those people doing today...?

Nikita Popov (Sep 25 2020 at 14:46, on Zulip):

I'm not sure I understand the issue there, as system LLVM (>= 8 I think) is supported.

pnkfelix (Sep 25 2020 at 14:46, on Zulip):

We don't depend on features of particular LLVM versions when we do these upgrades?

pnkfelix (Sep 25 2020 at 14:46, on Zulip):

okay then never mind, I guess.

Nikita Popov (Sep 25 2020 at 14:46, on Zulip):

The concern regarding rustc + system llvm interaction usually with regards to cross-language LTO, as clang / the LTO plugin needs to use the same LLVM versions.

pnkfelix (Sep 25 2020 at 14:47, on Zulip):

concern withdrawn then.

Nikita Popov (Sep 25 2020 at 14:48, on Zulip):

And you are right, LLVM 11 is not released yet, though expected soon(TM) -- how much time is there until the stable release again?

pnkfelix (Sep 25 2020 at 14:48, on Zulip):

nikomatsakis said:

I think https://github.com/rust-lang/rust/issues/76466 is the one that worries me the most. If it's as simple as "building rust in paths with spaces is broken on windows", that seems potentially far-reaching.

this probably deserves some dedicated investigation

simulacrum (Sep 25 2020 at 14:48, on Zulip):

just over 2 weeks till stable release

pnkfelix (Sep 25 2020 at 14:48, on Zulip):

heh. some quick dedicated investigation then

nikomatsakis (Sep 25 2020 at 14:49, on Zulip):

@Nikita Popov

accurate summary?

nikomatsakis (Sep 25 2020 at 14:49, on Zulip):

pnkfelix said:

heh. some quick dedicated investigation then

if someone has access to a 32 bit windows machine I guess it'd be fairly simple. I have no idea how common 32 bit windows is

nikomatsakis (Sep 25 2020 at 14:49, on Zulip):

Maybe @Ryan Levick can help :point_up: :)

Wesley Wiser (Sep 25 2020 at 14:50, on Zulip):

nikomatsakis said:

I think https://github.com/rust-lang/rust/issues/76466 is the one that worries me the most. If it's as simple as "building rust in paths with spaces is broken on windows", that seems potentially far-reaching.

It's _just_ that simple. I just tried beta on Windows 10 64-bit in a path with multiple spaces and --target wasm32-unknown-unknown and it works fine.

nikomatsakis (Sep 25 2020 at 14:50, on Zulip):

(sorry to subscribe you to the stream, @Ryan Levick)

pnkfelix (Sep 25 2020 at 14:50, on Zulip):

nikomatsakis said:

I think https://github.com/rust-lang/rust/issues/76466 is the one that worries me the most. If it's as simple as "building rust in paths with spaces is broken on windows", that seems potentially far-reaching.

it may be broken for all LLD users on windows. Who relies on LLD besides WASM?

nikomatsakis (Sep 25 2020 at 14:50, on Zulip):

@Wesley Wiser it's not just that simple?

Wesley Wiser (Sep 25 2020 at 14:50, on Zulip):

IE, that builds fine.

nikomatsakis (Sep 25 2020 at 14:50, on Zulip):

yeah so it's something more specific, tied to lld I guess, and maybe 32 bit

Wesley Wiser (Sep 25 2020 at 14:50, on Zulip):

But I'm on Win 64-bit so perhaps it's specific to 32-bit.

nikomatsakis (Sep 25 2020 at 14:51, on Zulip):

I figured it must be or we'd have seen more people reporting it

pnkfelix (Sep 25 2020 at 14:51, on Zulip):

but original bug reporter said they were on Windows 64-bit ...

nikomatsakis (Sep 25 2020 at 14:51, on Zulip):

Did you try the exact repo case, @Wesley Wiser ?

Wesley Wiser (Sep 25 2020 at 14:51, on Zulip):

No, I didn't. Just a basic hello world.

nikomatsakis (Sep 25 2020 at 14:51, on Zulip):

If we can't reproduce it, rolling back LLVM 11 seems silly. But the argument around cross-lang inlining is real.

Wesley Wiser (Sep 25 2020 at 14:51, on Zulip):

Let me do that now.

nikomatsakis (Sep 25 2020 at 14:52, on Zulip):

That may be a policy we should settle on (only use released LLVM versions)

Ryan Levick (Sep 25 2020 at 14:52, on Zulip):

I don't have a 32 bit machine but I can try to reproduce on a 64 bit. I can also ask colleagues who likely have a 32 bit machine

nikomatsakis (Sep 25 2020 at 14:52, on Zulip):

@Ryan Levick do you know which bug we're talking about? It's #76466

Ryan Levick (Sep 25 2020 at 14:52, on Zulip):

Yep I can see the chat history :blush:

nikomatsakis (Sep 25 2020 at 14:53, on Zulip):

(I figured it might be kind of complex to follow)

Wesley Wiser (Sep 25 2020 at 14:53, on Zulip):

Oh interesting

Wesley Wiser (Sep 25 2020 at 14:53, on Zulip):

I have a repo of the same test case on Win 10 64-bit in a path with spaces.

Nikita Popov (Sep 25 2020 at 14:55, on Zulip):

@nikomatsakis That looks correct to me. For cross-lang LTO, you'd have to use a clang-11 pre-release build. It's pretty likely that the final version there will be released in the next two weeks, but there's no guarantees.

nikomatsakis (Sep 25 2020 at 14:55, on Zulip):

Wesley Wiser said:

It repros

ok, so it's kind of specific to wasm or something?

pnkfelix (Sep 25 2020 at 14:56, on Zulip):

probably more subtle that just that, because @Wesley Wiser tried to select wasm as target in their Hello World, no?

Wesley Wiser (Sep 25 2020 at 14:57, on Zulip):

Yeah I did

nikomatsakis (Sep 25 2020 at 14:57, on Zulip):

I have to go afk in a few minutes. I feel like y'all can make the call, I think it's borderline. I guess that even if LLVM 11 is not released when the stable release is out, it will be released shortly thereafter, and then cross-lang inlining would work again (right @Nikita Popov?)

Nikita Popov (Sep 25 2020 at 14:57, on Zulip):

That's right.

nikomatsakis (Sep 25 2020 at 14:57, on Zulip):

i.e., we wouldn't need to do any point release

Wesley Wiser (Sep 25 2020 at 14:57, on Zulip):

I think "Hello world" works because the only dependency is the std and that isn't located in a path with spaces

pnkfelix (Sep 25 2020 at 14:57, on Zulip):

I think we can stick with LLVM 11 this cycle, because the risks associated with backing it out seem to outweigh the gains

Wesley Wiser (Sep 25 2020 at 14:58, on Zulip):

Which means things might be totally broken if anyone has installed Rust in a path that has spaces in it.

nikomatsakis (Sep 25 2020 at 14:58, on Zulip):

Ah, so @Wesley Wiser it may be that any crate with dependencies and paths in spaces breaks?

Wesley Wiser (Sep 25 2020 at 14:58, on Zulip):

Yeah

pnkfelix (Sep 25 2020 at 14:58, on Zulip):

@Wesley Wiser maybe try a hello world that link to a different crate then.

nikomatsakis (Sep 25 2020 at 14:58, on Zulip):

Not great.

Wesley Wiser (Sep 25 2020 at 14:58, on Zulip):

If I move the test to a directory that doesn't have spaces, it links fine.

pnkfelix (Sep 25 2020 at 14:58, on Zulip):

oy

pnkfelix (Sep 25 2020 at 14:58, on Zulip):

well, on the plus side

pnkfelix (Sep 25 2020 at 14:59, on Zulip):

that's a workaround

pnkfelix (Sep 25 2020 at 14:59, on Zulip):

not a great one given typical windows conventions

pnkfelix (Sep 25 2020 at 14:59, on Zulip):

but one I've had to deploy in previous lives

Ryan Levick (Sep 25 2020 at 14:59, on Zulip):

I can't seem to reproduce on the latest nightly. Checking to make sure I have the same set up

Wesley Wiser (Sep 25 2020 at 14:59, on Zulip):

I'll pull nightly and test too

pnkfelix (Sep 25 2020 at 15:01, on Zulip):

anyone want to argue for a revert to LLVM 10 ?

Ryan Levick (Sep 25 2020 at 15:01, on Zulip):

I don't see any differences between my workspace and the one from the issue. Going to try on the specific nightly from issue

nikomatsakis (Sep 25 2020 at 15:02, on Zulip):

pnkfelix said:

anyone want to argue for a revert to LLVM 10 ?

not I

nikomatsakis (Sep 25 2020 at 15:02, on Zulip):

but it may merit a point release if we get a fix for #76466 soon

pnkfelix (Sep 25 2020 at 15:04, on Zulip):

@Wesley Wiser your recent replication was on a Windows 64-bit box, right?

Wesley Wiser (Sep 25 2020 at 15:04, on Zulip):

Yeah

Wesley Wiser (Sep 25 2020 at 15:04, on Zulip):

I'm still seeing it repo on nightly

pnkfelix (Sep 25 2020 at 15:05, on Zulip):

/me is not able to replicate on his 64-bit Windows VM

eddyb (Sep 25 2020 at 15:05, on Zulip):

hope this isn't a silly question, but wouldn't a 32-bit toolchain run on a 64-bit windows installation just like it does on linux?

eddyb (Sep 25 2020 at 15:05, on Zulip):

as in, both OSes have support for running 32-bit userspace w/ 64-bit kernel, no?

Wesley Wiser (Sep 25 2020 at 15:05, on Zulip):

Yes

Ryan Levick (Sep 25 2020 at 15:06, on Zulip):

I cannot reproduce with the latest nightly nor with the nightly reported in the issue

pnkfelix (Sep 25 2020 at 15:06, on Zulip):

/me is going to try Rust installed by the msi installer instead of Rustup

eddyb (Sep 25 2020 at 15:07, on Zulip):

oh right rustup paths would be in C:\Users\$USER\.rustup\toolchains so potentially no spaces

nagisa (Sep 25 2020 at 15:07, on Zulip):

I suspect this might be related to locales too.

nagisa (Sep 25 2020 at 15:08, on Zulip):

/me imagines somebody using a nbsp as their space of choice in the username.

Wesley Wiser (Sep 25 2020 at 15:08, on Zulip):

Here's what I'm seeing:

C:\projects\test with spaces\hello_world>cargo build --target wasm32-unknown-unknown
   Compiling proc-macro2 v1.0.21
   Compiling unicode-xid v0.2.1
   Compiling syn v1.0.41
   Compiling wasm-bindgen-shared v0.2.68
   Compiling log v0.4.11
   Compiling cfg-if v0.1.10
   Compiling bumpalo v3.4.0
   Compiling lazy_static v1.4.0
   Compiling wasm-bindgen v0.2.68
   Compiling quote v1.0.7
   Compiling wasm-bindgen-backend v0.2.68
   Compiling wasm-bindgen-macro-support v0.2.68
   Compiling wasm-bindgen-macro v0.2.68
   Compiling js-sys v0.3.45
   Compiling web-sys v0.3.45
   Compiling hello_world v0.1.0 (C:\projects\test with spaces\hello_world)
error: linking with `rust-lld` failed: exit code: 1
  |
  = note: "rust-lld" "-flavor" "wasm" "-z" "stack-size=1048576" "--stack-first" "--allow-undefined" "--fatal-warnings" "--no-demangle" "--export-dynamic" "--no-entry" "-L" "C:\\Users\\wesley.wiser\\.rustup\\toolchains\\nightly-x86_64-pc-windows-msvc\\lib\\rustlib\\wasm32-unknown-unknown\\lib" "-L" "C:\\Users\\wesley.wiser\\.rustup\\toolchains\\nightly-x86_64-pc-windows-msvc\\lib\\rustlib\\wasm32-unknown-unknown\\lib\\self-contained" "C:\\projects\\test with spaces\\hello_world\\target\\wasm32-unknown-unknown\\debug\\deps\\hello_world.10x1umxe2ihpk42f.rcgu.o" "C:\\projects\\test with spaces\\hello_world\\target\\wasm32-unknown-unknown\\debug\\deps\\hello_world.1jrmc1rcj97b1rx7.rcgu.o" "C:\\projects\\test with spaces\\hello_world\\target\\wasm32-unknown-unknown\\debug\\deps\\hello_world.2thlrb4v5nonu3ei.rcgu.o" "C:\\projects\\test with spaces\\hello_world\\target\\wasm32-unknown-unknown\\debug\\deps\\hello_world.2ydc3y7vzq74rkde.rcgu.o" "C:\\projects\\test with spaces\\hello_world\\target\\wasm32-unknown-unknown\\debug\\deps\\hello_world.31n4dyidua42hqrf.rcgu.o" "C:\\projects\\test with spaces\\hello_world\\target\\wasm32-unknown-unknown\\debug\\deps\\hello_world.5rz76vvdq2iid4p.rcgu.o" "C:\\projects\\test with spaces\\hello_world\\target\\wasm32-unknown-unknown\\debug\\deps\\hello_world.zd8q8tu961vm25b.rcgu.o" "-o" "C:\\projects\\test with spaces\\hello_world\\target\\wasm32-unknown-unknown\\debug\\deps\\hello_world.wasm" "--export" "main" "--export" "__wbindgen_describe___wbg_addEventListener_1ee029f42ddd26e4" "--export" "__wbindgen_describe___wbg_addEventListener_9e7b0c3f65ebc0d7"

{snip}

  = note: rust-lld: error: cannot open C:\\projects\\test\: no such file or directory
          rust-lld: error: cannot open with\: no such file or directory
          rust-lld: error: cannot open spaces\\hello_world\\target\\wasm32-unknown-unknown\\debug\\deps\\hello_world.10x1umxe2ihpk42f.rcgu.o: no such file or directory
          rust-lld: error: cannot open C:\\projects\\test\: no such file or directory
          rust-lld: error: cannot open with\: no such file or directory
          rust-lld: error: cannot open spaces\\hello_world\\target\\wasm32-unknown-unknown\\debug\\deps\\hello_world.1jrmc1rcj97b1rx7.rcgu.o: no such file or directory
          rust-lld: error: cannot open C:\\projects\\test\: no such file or directory
          rust-lld: error: cannot open with\: no such file or directory
          rust-lld: error: cannot open spaces\\hello_world\\target\\wasm32-unknown-unknown\\debug\\deps\\hello_world.2thlrb4v5nonu3ei.rcgu.o: no such file or directory
          rust-lld: error: cannot open C:\\projects\\test\: no such file or directory
          rust-lld: error: cannot open with\: no such file or directory
          rust-lld: error: cannot open spaces\\hello_world\\target\\wasm32-unknown-unknown\\debug\\deps\\hello_world.2ydc3y7vzq74rkde.rcgu.o: no such file or directory
          rust-lld: error: cannot open C:\\projects\\test\: no such file or directory
          rust-lld: error: cannot open with\: no such file or directory
          rust-lld: error: cannot open spaces\\hello_world\\target\\wasm32-unknown-unknown\\debug\\deps\\hello_world.31n4dyidua42hqrf.rcgu.o: no such file or directory
          rust-lld: error: cannot open C:\\projects\\test\: no such file or directory
          rust-lld: error: cannot open with\: no such file or directory
          rust-lld: error: cannot open spaces\\hello_world\\target\\wasm32-unknown-unknown\\debug\\deps\\hello_world.5rz76vvdq2iid4p.rcgu.o: no such file or directory
          rust-lld: error: cannot open C:\\projects\\test\: no such file or directory
          rust-lld: error: cannot open with\: no such file or directory
          rust-lld: error: too many errors emitted, stopping now (use -error-limit=0 to see all errors)


error: aborting due to previous error

error: could not compile `hello_world`

To learn more, run the command again with --verbose.
eddyb (Sep 25 2020 at 15:08, on Zulip):

@Ryan Levick was the reproduction attempt with Rust installed in C:\Progam Files?

Ryan Levick (Sep 25 2020 at 15:09, on Zulip):

No, it's my user directory - no spaces

eddyb (Sep 25 2020 at 15:09, on Zulip):

or I guess like @Wesley Wiser's, with spaces in the path of the Cargo package

pnkfelix (Sep 25 2020 at 15:10, on Zulip):

eddyb said:

or I guess like Wesley Wiser's, with spaces in the path of the Cargo package

this is what I tried on my end, but it failed to reproduce for me

eddyb (Sep 25 2020 at 15:10, on Zulip):

ah interesting

pnkfelix (Sep 25 2020 at 15:11, on Zulip):

(also might be interesting to try to reproduce on non-Windows systems.)

Ryan Levick (Sep 25 2020 at 15:11, on Zulip):

Yea I can't reproduce the issue if my path to the cargo project has spaces in it. I can try with the compiler itself

eddyb (Sep 25 2020 at 15:11, on Zulip):

I forgot we even had an MSI installer, tbh, I would've probably moved it manually or tried to install rustup somewhere else

Wesley Wiser (Sep 25 2020 at 15:12, on Zulip):

I'm pretty sure I installed via rustup.

pnkfelix (Sep 25 2020 at 15:12, on Zulip):

pnkfelix said:

/me is going to try Rust installed by the msi installer instead of Rustup

still no luck, even with this method

eddyb (Sep 25 2020 at 15:12, on Zulip):

(since rustc finds all of the relevant files relative to itself, so you can just move/copy an entire .rustup/toolchains/... directory anywhere you want)

Wesley Wiser (Sep 25 2020 at 15:14, on Zulip):

I tried in both cmd.exe and git bash just in case it was something to do with the environment and it fails to build in both.

pnkfelix (Sep 25 2020 at 15:14, on Zulip):

or... oh head-slap: I wasn't passing the --target wasm32-unknown-unknown option. What is wrong with me.

pnkfelix (Sep 25 2020 at 15:15, on Zulip):

okay now I can reproduce

simulacrum (Sep 25 2020 at 15:15, on Zulip):

it's probably an lld bug then

simulacrum (Sep 25 2020 at 15:15, on Zulip):

as someone mentions on the issue

Ryan Levick (Sep 25 2020 at 15:16, on Zulip):

pnkfelix said:

or... oh head-slap: I wasn't passing the --target wasm32-unknown-unknown option. What is wrong with me.

Wondering why I still can't reproduce the issue...

Wesley Wiser (Sep 25 2020 at 15:16, on Zulip):

Yeah, the paths passed to lld seem to be correctly encoded:

"C:\\projects\\test with spaces\\hello_world\\target\\wasm32-unknown-unknown\\debug\\deps\\libweb_sys-4faa97cbfbde07f6.rlib"

eddyb (Sep 25 2020 at 15:17, on Zulip):

is this because windows CLI programs have to parse their own arguments?

pnkfelix (Sep 25 2020 at 15:17, on Zulip):

could it be an issue with our rust-lld wrapper?

eddyb (Sep 25 2020 at 15:17, on Zulip):

and all the caveats around that (cc @Peter Rabbit)

eddyb (Sep 25 2020 at 15:17, on Zulip):

@pnkfelix is that a wrapper? I thought we just renamed the lld we built

pnkfelix (Sep 25 2020 at 15:17, on Zulip):

/me doesn't know

pnkfelix (Sep 25 2020 at 15:20, on Zulip):

where does the installed rust-lld even live? I'm looking in .rustup\toolchains but not finding any sign of it there

eddyb (Sep 25 2020 at 15:21, on Zulip):

it's nested inside rustlib

simulacrum (Sep 25 2020 at 15:21, on Zulip):

it's not a wrapper, renamed binary

pnkfelix (Sep 25 2020 at 15:21, on Zulip):

okay found it, thanks

pnkfelix (Sep 25 2020 at 15:23, on Zulip):

well, we are way over time now

pnkfelix (Sep 25 2020 at 15:23, on Zulip):

It sounds like no one wants to argue for a revert of the LLVM upgrade

eddyb (Sep 25 2020 at 15:24, on Zulip):

huh it relies on just main taking arguments https://github.com/llvm/llvm-project/blob/2a11a197af7e72725fe461ba9917756b1b09661a/lld/tools/lld/lld.cpp#L205

pnkfelix (Sep 25 2020 at 15:24, on Zulip):

so, that's great. I can focus my attention on trying out the fixes to rr that @eddyb has helped discover on my Ryzen box

eddyb (Sep 25 2020 at 15:25, on Zulip):

meanwhile, Rust: https://github.com/rust-lang/rust/blob/b984ef6797ff17faa2b1e0ebb54b78de1491e5fd/library/std/src/sys/windows/args.rs#L21-L30

eddyb (Sep 25 2020 at 15:28, on Zulip):

ah LLVM has something similar https://github.com/llvm/llvm-project/blob/2a11a197af7e72725fe461ba9917756b1b09661a/llvm/lib/Support/InitLLVM.cpp#L24-L54

pnkfelix (Sep 25 2020 at 15:30, on Zulip):

silly Q: is that #ifdef _WIN32 true even on win64?

eddyb (Sep 25 2020 at 15:31, on Zulip):

_WIN32 Defined as 1 when the compilation target is 32-bit ARM, 64-bit ARM, x86, or x64. Otherwise, undefined.

pnkfelix (Sep 25 2020 at 15:31, on Zulip):

yes apparently it is, okay.

eddyb (Sep 25 2020 at 15:31, on Zulip):

from https://docs.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=vs-2019

pnkfelix (Sep 25 2020 at 15:31, on Zulip):

(oh I see, the 32 is meant to mean "not 16-bit")

eddyb (Sep 25 2020 at 15:32, on Zulip):

you love to see it,

eddyb (Sep 25 2020 at 15:34, on Zulip):

potentially relevant commit https://github.com/llvm/llvm-project/commit/270d3faf6e0d09ec00ba51b46241534bc6455256

eddyb (Sep 25 2020 at 15:34, on Zulip):

the actual parser is here https://github.com/llvm/llvm-project/blob/2a11a197af7e72725fe461ba9917756b1b09661a/llvm/lib/Support/CommandLine.cpp#L931-L936

eddyb (Sep 25 2020 at 15:37, on Zulip):

there's also https://github.com/llvm/llvm-project/commit/2d068e534f1671459e1b135852c1b3c10502e929

eddyb (Sep 25 2020 at 15:37, on Zulip):

not really sure how they all interact

eddyb (Sep 25 2020 at 15:38, on Zulip):

@Wesley Wiser you said we pass the correct arguments, but isn't it possible the quotes are printed for the benefit of the user but not actually encoded? wait I just got a very silly idea

Wesley Wiser (Sep 25 2020 at 15:39, on Zulip):

Yeah, that's fair.

eddyb (Sep 25 2020 at 15:39, on Zulip):

@Wesley Wiser @pnkfelix you could replace rust-lld.exe with a simple Rust program that errors after printing its arguments as parsed by Rust

eddyb (Sep 25 2020 at 15:40, on Zulip):

something like this:

fn main() {
    dbg!(std::env::args().collect::<Vec<_>>());
    std::process::exit(1);
}
Wesley Wiser (Sep 25 2020 at 15:42, on Zulip):

Where do I find rust-lld.exe?

eddyb (Sep 25 2020 at 15:42, on Zulip):

it's nested inside rustlib

Wesley Wiser (Sep 25 2020 at 15:42, on Zulip):

In the toolchain folder somewhere?

eddyb (Sep 25 2020 at 15:42, on Zulip):

kind of like our own personal "libexec"

pnkfelix (Sep 25 2020 at 15:42, on Zulip):

yeah I found it recently

pnkfelix (Sep 25 2020 at 15:42, on Zulip):

let me grab that path

eddyb (Sep 25 2020 at 15:43, on Zulip):

should be lib/rustlib/$host/bin/rust-lld.exe or similar

pnkfelix (Sep 25 2020 at 15:44, on Zulip):

.rustup\toolchains\beta-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\bin\for me

pnkfelix (Sep 25 2020 at 15:44, on Zulip):

(which is consistent with what eddy wrote)

Wesley Wiser (Sep 25 2020 at 15:45, on Zulip):
  = note: [src\main.rs:2] std::env::args().collect::<Vec<_>>() = [
              "C:\\Users\\wesley.wiser\\.rustup\\toolchains\\nightly-x86_64-pc-windows-msvc\\lib\\rustlib\\x86_64-pc-windows-msvc\\bin\\rust-lld.exe",
              "-flavor",
              "wasm",
              "@C:\\Users\\WESLEY~1.WIS\\AppData\\Local\\Temp\\rustcE6Ab1l\\linker-arguments",
          ]
eddyb (Sep 25 2020 at 15:45, on Zulip):

:rofl:

eddyb (Sep 25 2020 at 15:45, on Zulip):

glad I asked

pnkfelix (Sep 25 2020 at 15:46, on Zulip):

that @ thing I assume is some sort of directive to inline the contents of a file as arguments I assume?

eddyb (Sep 25 2020 at 15:46, on Zulip):

(time to dump the file - which might need to be done from the fake linker itself)

Wesley Wiser (Sep 25 2020 at 15:46, on Zulip):

will -Zsave-temps or whatever leave that file around?

eddyb (Sep 25 2020 at 15:46, on Zulip):

yeah and the stuff I was looking at doesn't really matter turns out

pnkfelix (Sep 25 2020 at 15:46, on Zulip):

(probably just as easy to have the fake-lld print it though IMO)

eddyb (Sep 25 2020 at 15:47, on Zulip):

@Wesley Wiser I wouldn't bet on it and yeah what @pnkfelix just said

eddyb (Sep 25 2020 at 15:47, on Zulip):

I was gonna say that worst case we look at what GetCommandLineW returns but it's not even relevant

eddyb (Sep 25 2020 at 15:52, on Zulip):

https://github.com/llvm/llvm-project/commit/928e9e172305752583a75a8174ab5a87b4e09d80

eddyb (Sep 25 2020 at 15:52, on Zulip):

now this shouldn't have changed behavior

eddyb (Sep 25 2020 at 15:53, on Zulip):

oh wait it did! it used to hardcode cl::TokenizeGNUCommandLine

eddyb (Sep 25 2020 at 15:54, on Zulip):

@Wesley Wiser if you can make it work, try calling the real linker with an extra -rsp-quoting=posix flag

Wesley Wiser (Sep 25 2020 at 15:54, on Zulip):

I'm not sure what you mean

eddyb (Sep 25 2020 at 15:55, on Zulip):

as in, the fake rust-lld.exe executing the real one with the same arguments plus that one

simulacrum (Sep 25 2020 at 15:55, on Zulip):

rustc --target wasm32-uknown-unknown -Clink-arg=-rsp-quoting=posix perhaps?

eddyb (Sep 25 2020 at 15:55, on Zulip):

@simulacrum will likely end up in the @'d file

simulacrum (Sep 25 2020 at 15:55, on Zulip):

ah hm

pnkfelix (Sep 25 2020 at 15:56, on Zulip):

would you like to see what I got as the contents of the file?

eddyb (Sep 25 2020 at 15:56, on Zulip):

sure

pnkfelix (Sep 25 2020 at 15:56, on Zulip):

/me takes moment to figure out how to get it into a gist

eddyb (Sep 25 2020 at 15:57, on Zulip):

@Wesley Wiser to be clear, I'm imagining just:

fn main() {
    assert!(std::process::Command::new("real-rust-lld")
        .arg("-rsp-quoting=posix")
        .args(std::env::args_os().skip(1))
        .status()
        .unwrap()
        .success());
}
pnkfelix (Sep 25 2020 at 15:58, on Zulip):

https://gist.github.com/pnkfelix/aec7c69439a0c9dbda83210b8978870e

Wesley Wiser (Sep 25 2020 at 15:59, on Zulip):

@eddyb It worked?!

$ cargo build --target wasm32-unknown-unknown
   Compiling hello_world v0.1.0 (C:\projects\test with spaces\hello_world)
    Finished dev [unoptimized + debuginfo] target(s) in 1.61s
Wesley Wiser (Sep 25 2020 at 15:59, on Zulip):

Oh wait

Wesley Wiser (Sep 25 2020 at 16:00, on Zulip):

I guess I need to check status

eddyb (Sep 25 2020 at 16:00, on Zulip):

yeah sorry should've wrote the full example from the start

eddyb (Sep 25 2020 at 16:00, on Zulip):

though I expect it to work because that's what the previous behavior was

Wesley Wiser (Sep 25 2020 at 16:01, on Zulip):

I'm getting this

  = note: lld is a generic driver.
          Invoke ld.lld (Unix), ld64.lld (macOS), lld-link (Windows), wasm-ld (WebAssembly) instead
          thread 'main' panicked at 'assertion failed: std::process::Command::new("rust-lld.exe2").arg("-rsp-quoting=posix").args(std::env::args_os().skip(1)).status().unwrap().success()', src\main.rs:2:5
          note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
eddyb (Sep 25 2020 at 16:02, on Zulip):

aww

eddyb (Sep 25 2020 at 16:02, on Zulip):

btw there's this stuff that you might be able to see if you set RUSTC_LOG=rustc_codegen_ssa::back::link https://github.com/rust-lang/rust/blob/master/compiler/rustc_codegen_ssa/src/back/link.rs#L1068

Wesley Wiser (Sep 25 2020 at 16:03, on Zulip):

I've got to run :wave:

pnkfelix (Sep 25 2020 at 16:03, on Zulip):

we should probably post some of this on the ticket. :wink:

eddyb (Sep 25 2020 at 16:03, on Zulip):

https://github.com/rust-lang/rust/blob/b984ef6797ff17faa2b1e0ebb54b78de1491e5fd/compiler/rustc_codegen_ssa/src/back/link.rs#L1071-L1093

eddyb (Sep 25 2020 at 16:04, on Zulip):

yeah okay we use the target to determine whether we're using windows or GNU encoding

eddyb (Sep 25 2020 at 16:04, on Zulip):

which is why wasm is affected, since it's not MSVC

eddyb (Sep 25 2020 at 16:06, on Zulip):

@pnkfelix do you want to, or should I?

pnkfelix (Sep 25 2020 at 16:06, on Zulip):

just posted a comment but all it is is a link to the conversation we had here after meeting was bascially over

eddyb (Sep 25 2020 at 16:06, on Zulip):

alright I can detail a bit

pnkfelix (Sep 25 2020 at 16:07, on Zulip):

@eddyb this last point you made about rustc_codegen_ssa: does that imply that this is a bug in our code, not LLD?

pnkfelix (Sep 25 2020 at 16:08, on Zulip):

(because its querying "sess.target.target.options.is_like_msvc", and we should be ... asking that about the host?)

eddyb (Sep 25 2020 at 16:08, on Zulip):

oh yeah sorry I didn't go into details: LLD fixed its wasm interface and it now behaves correctly for its platform, but offers a flag to use the "wrong" interface

eddyb (Sep 25 2020 at 16:08, on Zulip):

what we were doing was correct in LLVM 10

pnkfelix (Sep 25 2020 at 16:08, on Zulip):

oh no

pnkfelix (Sep 25 2020 at 16:08, on Zulip):

so our behavior here is going to need to be LLVM version dependent?

eddyb (Sep 25 2020 at 16:11, on Zulip):

I don't think it's a problem because this is rust-lld we build ourselves

eddyb (Sep 25 2020 at 16:12, on Zulip):

@Wesley Wiser ah I think the problem is that the -flavor flag must come first. so maybe put the extra flag at the end?

simulacrum (Sep 25 2020 at 16:12, on Zulip):

we have version-specific behavior for llvm already I think so that shouldn't be a problem

eddyb (Sep 25 2020 at 16:25, on Zulip):

@Wesley Wiser @pnkfelix https://github.com/rust-lang/rust/issues/76466#issuecomment-699024973

eddyb (Sep 25 2020 at 16:28, on Zulip):

@pnkfelix okay yeah looking in your file, you have Dev\\\\issue\\ 76466\\\\issue_76466

eddyb (Sep 25 2020 at 16:28, on Zulip):

which is doubly-escaped because of {:?}

eddyb (Sep 25 2020 at 16:28, on Zulip):

the file contents contain Dev\\issue\ 76466\\issue_76466

eddyb (Sep 25 2020 at 16:29, on Zulip):

the \ is the escaping of a space that doesn't exist on windows. but if we used quotes instead, it should work on both

nagisa (Sep 25 2020 at 17:38, on Zulip):

Can somebody split out this into a different topic within T-compiler?

pnkfelix (Sep 29 2020 at 18:55, on Zulip):

(by the way, the topic from this point on was largely forked into a discussion of the LLD bug. So we ended moving that thread over here: https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/LLD.20breaks.20on.20Windows.20with.20spaces.20in.20filepaths/near/211268555 ; but some of it contains little bits of content as we wrapped up the meeting, like the rough consensus that we would not be rolling back the LLVM upgrade.

Last update: Nov 25 2020 at 02:15UTC