-
Notifications
You must be signed in to change notification settings - Fork 54
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
Fix Windows link errors #10
Comments
I suspect it's related to this file, https://github.com/bnoordhuis/v8-cmake/blob/master/v8/src/heap/third-party/heap-api.h. But I have not find proper way to fix this. |
Looks like windows enable |
I kind of figure out why the problem. It's visual studio compiler aggressively evalate expression never got executed (See L203 for example). v8-cmake/v8/src/heap/heap-inl.h Lines 202 to 207 in d4eb596
I am not sure visual studio compiler has flag to disable this. |
Also after I resolved related code (comment related code), windows build still failed with LINK : fatal error LNK1181: ????????"CMakeFiles\v8_snapshot.dir\embedded.S.obj" |
Provide a stub `third_party_heap::Heap` implementation to work around linker erors with Visual Studio. Refs: #10
Ah, that sounds plausible! I arrive at the opposite conclusion though: VS doesn't eliminate dead code as aggressively as clang and gcc. :-) I updated #3 with a stub implementation of |
I also try to figure it the flags used by v8 official team. But turned out they are using clang on windows, so I guess we have to fix msvc on our own. |
This is brilliant idea :). Much better than my original plan. |
Provide a stub `third_party_heap::Heap` implementation to work around linker erors with Visual Studio. Refs: #10
I've also had 3 days of hell trying to get this to build in a conan recipe... I came across your cmake effort during my journey as coming up against the same problem.. It does indeed appear to be the perplexing decision to use: My release builds were successfully linking mksnapshot.exe -- but debug build was failing for the same reason as yours. My solution in the end was to just optimize the debug build by specifying v8_optimized_debug=true to v8gen.py. Its already slow as hell anyway due to having to specify enable_iterator_debugging=true for the debug build... Otherwise any projects linking to debug lib MUST also use SECURE_SCL=0. And all static library dependencies. Otherwise it will randomly crash if it does manage to link. I'm still waiting on the full build to finish to be sure that resolves it and I can still link to v8_monolith! Hopefully that will occur today. I reported to v8 team: |
Fixed by #18! |
Provide a stub `third_party_heap::Heap` implementation to work around linker erors with Visual Studio. Refs: #10
Fixes: #10 PR-URL: #18 Reviewed-By: Ben Noordhuis <[email protected]>
I can see why they did that though: it means the code gets checked by the compiler, regardless of the platform. You don't get that when it's fenced off by #ifdefs. I'll try to upstream the patch we're currently carrying, that should hopefully also resolve your issue. |
Provide a stub `third_party_heap::Heap` implementation to work around linker erors with Visual Studio. Refs: #10
Fixes: #10 PR-URL: #18 Reviewed-By: Ben Noordhuis <[email protected]>
Provide a stub `third_party_heap::Heap` implementation to work around linker erors with Visual Studio. Refs: #10
Fixes: #10 PR-URL: #18 Reviewed-By: Ben Noordhuis <[email protected]>
Provide a stub `third_party_heap::Heap` implementation to work around linker erors with Visual Studio. cl.exe in debug mode seems to eliminate dead code not as aggressively as clang or gcc, resulting in references to `third_party_heap::Heap` remaining in unreachable code paths. Refs: bnoordhuis/v8-cmake#10 Bug: v8:10427 Change-Id: I61fde11697adc663b182f60c132eda435a7f11bd Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2159490 Commit-Queue: Ben Noordhuis <[email protected]> Commit-Queue: Ulan Degenbaev <[email protected]> Auto-Submit: Ben Noordhuis <[email protected]> Reviewed-by: Ulan Degenbaev <[email protected]> Cr-Commit-Position: refs/heads/master@{#67293}
Honorable mention for whoever fixes that and gets the CI green!
The text was updated successfully, but these errors were encountered: