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

Fixes for VS preview #45902

Merged
merged 5 commits into from
Dec 11, 2020
Merged

Fixes for VS preview #45902

merged 5 commits into from
Dec 11, 2020

Conversation

sdmaclea
Copy link
Contributor

@sdmaclea sdmaclea commented Dec 10, 2020

Compile crossdac w/o c++latest
Disable warning about overriding /TP with /TC

Compile crossdac w/o c++latest
Disable warning about overriding /TP with /TC
@sdmaclea
Copy link
Contributor Author

I don't have access to a Windows machine at the moment. So this is a draft change for #45763.
I can create and use a VM, but I was hoping someone who has been testing fixes for #45763, could verify this fixes that issue.

@sdmaclea sdmaclea marked this pull request as draft December 10, 2020 19:08
Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@sdmaclea sdmaclea marked this pull request as ready for review December 10, 2020 20:09
@sdmaclea
Copy link
Contributor Author

FYI removing constexpr due to

2020-12-10T20:12:08.2485132Z /__w/1/s/src/coreclr/pal/src/exception/remote-unwind.cpp:1980:21: error: variable declaration in a constexpr function is a C++14 extension [-Werror,-Wc++14-extensions]
2020-12-10T20:12:08.2486446Z     unw_accessors_t a = {0};
2020-12-10T20:12:08.2486919Z                     ^
2020-12-10T20:12:08.2488130Z /__w/1/s/src/coreclr/pal/src/exception/remote-unwind.cpp:1982:5: error: use of this statement in a constexpr function is a C++14 extension [-Werror,-Wc++14-extensions]
2020-12-10T20:12:08.2489007Z     a.find_proc_info = find_proc_info;
2020-12-10T20:12:08.2489845Z     ^
2020-12-10T20:12:08.2490995Z /__w/1/s/src/coreclr/pal/src/exception/remote-unwind.cpp:1978:34: error: constexpr function never produces a constant expression [-Winvalid-constexpr]
2020-12-10T20:12:08.2491892Z static constexpr unw_accessors_t init_unwind_accessors()
2020-12-10T20:12:08.2492490Z                                  ^
2020-12-10T20:12:08.2493656Z /__w/1/s/src/coreclr/pal/src/exception/remote-unwind.cpp:1982:22: note: subexpression not valid in a constant expression
2020-12-10T20:12:08.2494457Z     a.find_proc_info = find_proc_info;
2020-12-10T20:12:08.2494930Z                      ^
2020-12-10T20:12:08.2544095Z 3 errors generated.

@janvorli
Copy link
Member

Hmm, ok, it seems that the function pointers are not considered consts. Never mind.

Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@vitek-karas
Copy link
Member

I tried building the first version of this and it still fails:

CMake Error at pal/src/libunwind/src/CMakeLists.txt:345 (set_source_files_properties):
    set_source_files_properties called with incorrect number of arguments.

@vitek-karas
Copy link
Member

I fetched the latest and now it fails with this really weird:

  FAILED: dlls/mscoree/coreclr/inject_debug_resources.timestamp 
  dlls\mscoree\coreclr\CMakeFiles\inject_debug_resources.timestamp-20cf953.bat 247c0b8ffd5ac623
  Microsoft (R) C/C++ Optimizing Compiler Version 19.28.29617 for x64
  Copyright (C) Microsoft Corporation.  All rights reserved.
  
  daccess.cpp
  C:\Program Files\dotnet
  The application to execute does not exist: 'F:/dotnet/runtime2/artifacts/bin/coreclr/Linux.x64.Debug/DacTableGen/DacTableGen.dll'

I don't see anything else in the log which would indicate failure.
There are lot of warnings though - all look like this:

  [151/1680] Building CXX object jit\static\CMakeFiles\clrjit_obj.dir\__\disasm.cpp.obj
  cl : Command line warning D9025 : overriding '/W3' with '/W4'

@sdmaclea
Copy link
Contributor Author

@vitek-karas I wonder if the weird errors are -ninja related. Can you try to build w/o -ninja? Maybe rm -rf artifacts to be sure... We shouldn't be injecting resources into Linux.x64.Debug/DacTableGen/DacTableGen.dll as far I know.

@sdmaclea
Copy link
Contributor Author

/cc @jkoritzinsky

@sdmaclea
Copy link
Contributor Author

We may want to disable 9025 more globally if it is causing issues until we can fix it properly.

@vitek-karas
Copy link
Member

Non-ninja build works just fine

@agocke
Copy link
Member

agocke commented Dec 11, 2020

Confirmed, this fixes the dogfood problems. I say we merge this and handle the Ninja DAC failures separately.

@am11
Copy link
Member

am11 commented Dec 11, 2020

@vitek-karas, is it possible that error is logged from

trace::error(_X("The application to execute does not exist: '%s'"), app_candidate.c_str());
Not sure what role -ninja might be playing; but building without -ninja first and then with -ninja without -clean / deleting artifacts directory could be leaving the artifacts\obj directory is this weird state. If you haven't already tried, maybe delete the artifacts directory again and try building with -ninja to see if it succeeds.

@vitek-karas
Copy link
Member

I ran git clean -xdf before each try, so there should be no stuff left lying around.

@vitek-karas
Copy link
Member

Tried once more - still fails as mentioned above.

@janvorli
Copy link
Member

I can also see that ninja generates obj files into artifacts\nmakeobj\windows.x64.Debug instead of artifacts\obj\coreclr\windows.x64.Checked\src. I think that should be changed to match the previous way.

@sdmaclea
Copy link
Contributor Author

sdmaclea commented Dec 11, 2020

@hoyosjs Mentioned that it was a known and deferred issue that ninja couldn't be used to build the crossdac.

@jkoritzinsky Is the command @vitek-karas shows above correct for anually building?

@sdmaclea sdmaclea merged commit 574e625 into dotnet:master Dec 11, 2020
@jkoritzinsky
Copy link
Member

Yes that command should work. But due to the Dac issue he may have to specify specific subsets.

@ghost ghost locked as resolved and limited conversation to collaborators Jan 10, 2021
@AntonLapounov
Copy link
Member

Disable warning about overriding /TP with /TC

It is not possible to disable D* warnings. I see 10 identical warnings for libunwind_xdac build:

cl : command line warning D9014: invalid value '9025' for '/wd'; assuming '5999' [artifacts\obj\coreclr\Linux.x64.Release\crossgen\pal\src\libunwind\src\libunwind_xdac.vcxproj]

@sdmaclea sdmaclea deleted the vsDogFood branch June 10, 2021 00:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants