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

ignore {std,fast,vector,this}call on non-x86 windows #54576

Merged
merged 1 commit into from
Sep 29, 2018

Conversation

froydnj
Copy link
Contributor

@froydnj froydnj commented Sep 25, 2018

MSVC ignores these keywords for C/C++ and uses the standard system
calling convention. Rust should do so as well.

Fixes #54569.

@rust-highfive
Copy link
Collaborator

r? @zackmdavis

(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 Sep 25, 2018
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Sep 25, 2018

📌 Commit c0ac5ba has been approved by alexcrichton

@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 Sep 25, 2018
@zackmdavis zackmdavis removed their assignment Sep 26, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Sep 27, 2018
…lexcrichton

ignore {std,fast,vector,this}call on non-x86 windows

MSVC ignores these keywords for C/C++ and uses the standard system
calling convention.  Rust should do so as well.

Fixes rust-lang#54569.
@bors
Copy link
Contributor

bors commented Sep 27, 2018

⌛ Testing commit c0ac5ba with merge 496c2b9d1d134efd8501ad1095637ff6917895f2...

@bors
Copy link
Contributor

bors commented Sep 27, 2018

💔 Test failed - status-appveyor

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 27, 2018
@kennytm kennytm 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-review Status: Awaiting review from the assignee but also interested parties. labels Sep 27, 2018
@kennytm
Copy link
Member

kennytm commented Sep 27, 2018

C:\projects\rust\src/test\codegen\issue-32364.rs:19:11: error: CHECK: expected string not found in input
// CHECK: define internal x86_stdcallcc void @{{.*}}foo{{.*}}()
          ^
C:\projects\rust\build\x86_64-pc-windows-msvc\test\codegen\issue-32364\issue-32364.ll:1:1: note: scanning from here
; ModuleID = 'issue_32364.7rcbfp3g-cgu.0'
^
C:\projects\rust\build\x86_64-pc-windows-msvc\test\codegen\issue-32364\issue-32364.ll:48:1: note: possible intended match here
define internal i32 @_ZN3std3sys7windows7process8ExitCode6as_i3217h8f0526ed8f66b332E(i32* noalias readonly dereferenceable(4) %self) unnamed_addr #1 {
^

@froydnj froydnj force-pushed the non-x86-abi-adjustment branch from c0ac5ba to 32cfa73 Compare September 27, 2018 20:25
@froydnj
Copy link
Contributor Author

froydnj commented Sep 27, 2018

I made the test x86-only. There were other failures, e.g.

thread '[ui] ui\issues\issue-19100.rs' panicked at 'explicit panic', tools\compiletest\src\runtest.rs:3238:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.
failures:
    [ui] ui\issues\issue-19100.rs
test result: FAILED. 6797 passed; 1 failed; 45 ignored; 0 measured; 0 filtered out
thread 'main' panicked at 'Some tests failed', tools\compiletest\src\main.rs:496:22

But that test does not look related to this change. Intermittent?

r? @alexcrichton

@alexcrichton
Copy link
Member

Hm so issue-19100.rs does indeed look intermittent, and to confirm about @kennytm's failure the x86_stdcallcc only makes sense on the i686-* targets we have, not x86_64-*?

MSVC ignores these keywords for C/C++ and uses the standard system
calling convention.  Rust should do so as well.

Fixes rust-lang#54569.
@froydnj froydnj force-pushed the non-x86-abi-adjustment branch from 32cfa73 to 2819def Compare September 27, 2018 20:51
@froydnj
Copy link
Contributor Author

froydnj commented Sep 27, 2018

Hm so issue-19100.rs does indeed look intermittent, and to confirm about @kennytm's failure the x86_stdcallcc only makes sense on the i686-* targets we have, not x86_64-*?

I believe that's correct. x86-64 MSVC ignores stdcall; I suppose LLVM's x86-64 backend must accept it, but ignore it?

Oh dear sorry about this! I think nowadays we support // only-x86 or something like that

Indeed, thanks for pointing that out!

@alexcrichton
Copy link
Member

@bors: r+ delegate=froydnj

@bors
Copy link
Contributor

bors commented Sep 27, 2018

✌️ @froydnj can now approve this pull request

@bors
Copy link
Contributor

bors commented Sep 27, 2018

📌 Commit 2819def has been approved by alexcrichton

@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 Sep 27, 2018
@bors
Copy link
Contributor

bors commented Sep 28, 2018

⌛ Testing commit 2819def with merge 0749928e1ba63da96e1f05cf29352a08ef488a76...

@bors
Copy link
Contributor

bors commented Sep 28, 2018

💔 Test failed - status-travis

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-full-bootstrap of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_fold:end:system_info

Network availability confirmed.
Setting APT mirror in /etc/apt/sources.list: http://us-central1.gce.archive.ubuntu.com/ubuntu/
travis_fold:start:update_heroku
Updating Heroku
$ heroku version
heroku/7.16.0 linux-x64 node-v10.10.0
travis_fold:end:update_heroku
Installing APT Packages
travis_time:start:38a27058
$ travis_apt_get_update
travis_time:end:38a27058:start=1538154169254336296,finish=1538154174590859530,duration=5336523234
---
travis_time:end:365bee28:start=1538154996626967274,finish=1538154996631133690,duration=4166416
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0c1bf7e8
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:2353a6b2
travis_time:start:2353a6b2
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:296aa869
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 28, 2018
@kennytm
Copy link
Member

kennytm commented Sep 28, 2018

@bors retry #40474

@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 Sep 28, 2018
@bors
Copy link
Contributor

bors commented Sep 28, 2018

⌛ Testing commit 2819def with merge c55e7f9ed25671f791000d8a1cbc9f1dd5430e3f...

@bors
Copy link
Contributor

bors commented Sep 28, 2018

💔 Test failed - status-travis

@rust-highfive
Copy link
Collaborator

The job dist-x86_64-musl of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_fold:end:system_info

Network availability confirmed.
Setting APT mirror in /etc/apt/sources.list: http://us-central1.gce.archive.ubuntu.com/ubuntu/
travis_fold:start:update_heroku
Updating Heroku
$ heroku version
heroku/7.16.0 linux-x64 node-v10.10.0
travis_fold:end:update_heroku
Installing APT Packages
travis_time:start:134edae4
$ travis_apt_get_update
travis_time:end:134edae4:start=1538158677928814867,finish=1538158682851748686,duration=4922933819
---
travis_time:end:0341fca0:start=1538159311555061018,finish=1538159311559125877,duration=4064859
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0a0d90d7
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:1d7d88b4
travis_time:start:1d7d88b4
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:1ce01500
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 28, 2018
@alexcrichton
Copy link
Member

@bors: retry

@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 Sep 28, 2018
@bors
Copy link
Contributor

bors commented Sep 29, 2018

⌛ Testing commit 2819def with merge d95fd25...

bors added a commit that referenced this pull request Sep 29, 2018
ignore {std,fast,vector,this}call on non-x86 windows

MSVC ignores these keywords for C/C++ and uses the standard system
calling convention.  Rust should do so as well.

Fixes #54569.
@bors
Copy link
Contributor

bors commented Sep 29, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing d95fd25 to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

6 participants