Skip to content

libprofiler_builtins: Set compilation flags more correctly for C code. #60402

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

Merged
merged 2 commits into from
May 8, 2019

Conversation

michaelwoerister
Copy link
Member

In particular, set COMPILER_RT_HAS_FCNTL_LCK and COMPILER_RT_HAS_ATOMICS as appropriate. This should get rid of the various runtime warnings when executing instrumented binaries.

The build script is using a heuristic here that hopefully is sufficient for the time being.

r? @alexcrichton

Fixes #59531.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 30, 2019
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Collaborator

bors commented Apr 30, 2019

📌 Commit d52fde2 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 Apr 30, 2019
Centril added a commit to Centril/rust that referenced this pull request Apr 30, 2019
…-build, r=alexcrichton

libprofiler_builtins: Set compilation flags more correctly for C code.

In particular, set `COMPILER_RT_HAS_FCNTL_LCK` and `COMPILER_RT_HAS_ATOMICS` as appropriate. This should get rid of the various runtime warnings when executing instrumented binaries.

The build script is using a heuristic here that hopefully is sufficient for the time being.

r? @alexcrichton

Fixes rust-lang#59531.
@tesuji
Copy link
Contributor

tesuji commented Apr 30, 2019

Failed in #60420 (comment) .
@rustbot modify labels to -S-waiting-on-bors and +S-waiting-on-author

@rustbot rustbot 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 Apr 30, 2019
@Centril
Copy link
Contributor

Centril commented May 1, 2019

@bors r- ^

@michaelwoerister michaelwoerister force-pushed the update-profiler-rt-build branch from d52fde2 to d4dfbeb Compare May 2, 2019 09:51
@michaelwoerister
Copy link
Member Author

Typo fixed.

@bors r=alexcrichton

@bors
Copy link
Collaborator

bors commented May 2, 2019

📌 Commit d4dfbeb 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 May 2, 2019
kennytm added a commit to kennytm/rust that referenced this pull request May 2, 2019
…-build, r=alexcrichton

libprofiler_builtins: Set compilation flags more correctly for C code.

In particular, set `COMPILER_RT_HAS_FCNTL_LCK` and `COMPILER_RT_HAS_ATOMICS` as appropriate. This should get rid of the various runtime warnings when executing instrumented binaries.

The build script is using a heuristic here that hopefully is sufficient for the time being.

r? @alexcrichton

Fixes rust-lang#59531.
@bors
Copy link
Collaborator

bors commented May 4, 2019

⌛ Testing commit d4dfbeb with merge c3b08da044476a942cf78b3b4fc67d943a5890be...

@bors
Copy link
Collaborator

bors commented May 4, 2019

💔 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 May 4, 2019
@michaelwoerister
Copy link
Member Author

I was able reproduce this locally. It seems that Clang 7 can't handle some of the C instrinsics for Arm64 Windows yet. Switching to Clang 8 fixes the problem.

@rust-lang/infra, how would I update the clang version defined here?

rust/appveyor.yml

Lines 152 to 164 in 55c48b4

# If we're compiling for MSVC then we, like most other distribution builders,
# switch to clang as the compiler. This'll allow us eventually to enable LTO
# amongst LLVM and rustc. Note that we only do this on MSVC as I don't think
# clang has an output mode compatible with MinGW that we need. If it does we
# should switch to clang for MinGW as well!
#
# Note that the LLVM installer is an NSIS installer
#
# Original downloaded here came from
# http://releases.llvm.org/7.0.0/LLVM-7.0.0-win64.exe
- if NOT defined MINGW_URL appveyor-retry appveyor DownloadFile https://s3-us-west-1.amazonaws.com/rust-lang-ci2/rust-ci-mirror/LLVM-7.0.0-win64.exe
- if NOT defined MINGW_URL .\LLVM-7.0.0-win64.exe /S /NCRC /D=C:\clang-rust
- if NOT defined MINGW_URL set RUST_CONFIGURE_ARGS=%RUST_CONFIGURE_ARGS% --set llvm.clang-cl=C:\clang-rust\bin\clang-cl.exe

@alexcrichton
Copy link
Member

Ok I've uploaded the 8.0.0 binary to our CI bucket, if you update s/7/8/g here I think it should work

@michaelwoerister
Copy link
Member Author

❤️

In particular, set COMPILER_RT_HAS_FCNTL_LCK and COMPILER_RT_HAS_ATOMICS
as appropriate.
@michaelwoerister michaelwoerister force-pushed the update-profiler-rt-build branch from d4dfbeb to 0ffc573 Compare May 8, 2019 15:05
@michaelwoerister
Copy link
Member Author

Clang version updated.

@bors r=alexcrichton

@bors
Copy link
Collaborator

bors commented May 8, 2019

📌 Commit 0ffc573 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 May 8, 2019
@bors
Copy link
Collaborator

bors commented May 8, 2019

⌛ Testing commit 0ffc573 with merge 9bee31424656ffbdc627226a394857a8aeb80ff9...

@Centril
Copy link
Contributor

Centril commented May 8, 2019

@bors retry

@bors
Copy link
Collaborator

bors commented May 8, 2019

⌛ Testing commit 0ffc573 with merge 3f5152e...

bors added a commit that referenced this pull request May 8, 2019
…lexcrichton

libprofiler_builtins: Set compilation flags more correctly for C code.

In particular, set `COMPILER_RT_HAS_FCNTL_LCK` and `COMPILER_RT_HAS_ATOMICS` as appropriate. This should get rid of the various runtime warnings when executing instrumented binaries.

The build script is using a heuristic here that hopefully is sufficient for the time being.

r? @alexcrichton

Fixes #59531.
@bors
Copy link
Collaborator

bors commented May 8, 2019

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 8, 2019
@bors bors merged commit 0ffc573 into rust-lang:master May 8, 2019
@rust-highfive
Copy link
Contributor

📣 Toolstate changed by #60402!

Tested on commit 3f5152e.
Direct link to PR: #60402

💔 rls on windows: test-pass → test-fail (cc @Xanewok, @rust-lang/infra).
🎉 rls on linux: test-fail → test-pass (cc @Xanewok, @rust-lang/infra).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request May 8, 2019
Tested on commit rust-lang/rust@3f5152e.
Direct link to PR: <rust-lang/rust#60402>

💔 rls on windows: test-pass → test-fail (cc @Xanewok, @rust-lang/infra).
🎉 rls on linux: test-fail → test-pass (cc @Xanewok, @rust-lang/infra).
@michaelwoerister
Copy link
Member Author

It landed :) Thanks for your help, @alexcrichton!

I'm wondering how RLS could be affected by these changes...

@kennytm
Copy link
Member

kennytm commented May 9, 2019

There's a spurious test in RLS 😓

@michaelwoerister
Copy link
Member Author

Ah, OK. Thanks for the info, @kennytm!

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.

libprofiler_builtins is missing compile-time flags
8 participants