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.

Sign up for GitHub

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

Jump to bottom

Debug build fails on Windows: Assertion failed: vector iterators incompatible #26694

Closed
billti opened this issue Mar 15, 2019 · 3 comments · Fixed by #26702
Closed

Debug build fails on Windows: Assertion failed: vector iterators incompatible #26694

billti opened this issue Mar 15, 2019 · 3 comments · Fixed by #26702
Labels
build Issues and PRs related to build files or the CI. regression Issues related to regressions. windows Issues and PRs related to the Windows platform.

Comments

@billti
Copy link
Contributor

billti commented Mar 15, 2019

I made a fix for this, then realized it's already been fixed upstream in V8. See https://chromium.googlesource.com/v8/v8.git/+/385aa80aff32210d098498d1cd44d42bc70ee1d4 . Logging here for tracking and awareness.

Building on Windows via: ".\vcbuild debug x86" fails with:

 generating: "Debug\obj\v8_snapshot\/embedded.S" "Debug\obj\v8_snapshot\/snapshot.cc"
  c:\program files (x86)\microsoft visual studio\2017\enterprise\vc\tools\msvc\14.16.27023\include\vector(199) : Assertion failed: vector iterators incompatible

This issue is in the V8 currently in Node.js master, and is fixed in the upstream commit listed above.
@addaleax
Copy link
Member

Are you talking about a released version of Node.js or the master branch? The fix should be included in #26685.

@billti
Copy link
Contributor Author

billti commented Mar 15, 2019

I just did a sync/build of master this morning and hit this issue, so a new drop should fix in master. (A git tag --contains 385aa80aff32210 shows that fix is in the V8 7.4.288.8 being targeted.

Thanks!

@billti billti closed this as completed Mar 15, 2019
@refack
Copy link
Contributor

refack commented Mar 16, 2019 •
edited
Loading

It's still there in 269103a

DEV D:\code\node>git rev-parse HEAD
269103a0e5e30cc217bde1660087e87dfc722b8a

DEV D:\code\node>msbuild /p:Configuration=Debug ...
...
CustomBuild:
  generating: "Debug\obj\v8_snapshot\/embedded.S" "Debug\obj\v8_snapshot\/snapshot.cc"
  D:\bin\dev\vs\2019\CommunityPreview\VC\Tools\MSVC\14.20.27404\include\vector(170) : Assertion failed: vector iterators incompatible
D:\bin\dev\vs\2019\CommunityPreview\MSBuild\Microsoft\VC\v160\Microsoft.CppCommon.targets(209,5): error MSB6006: "cmd.exe" exited with code -1073740791. [D:\code\node\deps\v8\gypfiles\v8_snapshot.vcxproj]
Done Building Project "D:\code\node\deps\v8\gypfiles\v8_snapshot.vcxproj" (Build target(s)) -- FAILED.

Reopening as #26685 is planned to land after V8 7.4 goes stable at 4/23.

@refack refack reopened this Mar 16, 2019
@refack refack mentioned this issue Mar 16, 2019
Merged
@refack refack added windows Issues and PRs related to the Windows platform. build Issues and PRs related to build files or the CI. regression Issues related to regressions. labels Mar 16, 2019
@refack refack mentioned this issue Mar 16, 2019
Merged
2 tasks
@refack refack self-assigned this Mar 16, 2019
refack added a commit to refack/node that referenced this issue Mar 17, 2019
@refack
deps,v8: cherry-pick 385aa80
8e308e8 
Original commit message:

    Correct removal of redundant moves

    The logic for removing while iterating is non-standard and
    a left over from a previous index based loop. This patch
    replaces it with a standard erase based version.

    This fixes a runtime crash with MSVC that invalidates the
    iterator and then asserts. This also makes the code safe
    in case the last move can be redundant.

    Change-Id: Ie6990e0d65a3b83a4b7da3e2e89ed4e60a6cd215
    Reviewed-on: https://chromium-review.googlesource.com/c/1488762
    Reviewed-by: Ben Titzer <[email protected]>
    Commit-Queue: Ben Titzer <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#59868}

Refs: v8/v8@385aa80

PR-URL: nodejs#26702
Fixes: nodejs#26694
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@refack refack removed their assignment Mar 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. regression Issues related to regressions. windows Issues and PRs related to the Windows platform.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

deps: V8: cherry-pick 385aa80 refack/node
3 participants
@refack @addaleax @billti