-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
v6.5.0 fails to build using clang-3.4. #8323
Comments
Can you post or link to the build log? |
What I've seen building 6.5.0 this morning is that the builds fail during the V8 compilation on both Ubuntu Precise and Debian Wheezy. Both were trying to use The builds failed in the same way for Wheezy and Precise, on both 32bit and 64bit architectures. The error message in all cases from
I have attached those Newer versions of Debian and Ubuntu (which do have |
Working on getting a full build log @bnoordhuis, please stay tuned. |
Full build log attached @bnoordhuis. This was attempting to build on Ubuntu Precise, amd64, with |
Stops with:
|
@chrislea Thanks. It's hitting an ICE, an internal compiler error. You're using clang 3.4.1 now. Is testing with 3.4.2 an option for you? If that doesn't fix the issue, we have three options:
|
I think 3. is out of the question - that's effectively a breaking change.
Less breaking than 3. at this point. |
@bnoordhuis going to a newer version of clang is not immediately possible, as in, I can't do it in the next 30min. But I can look into it and see how much work will be involved. |
Why is 3 a breaking change? |
We're now failing to compile on mainstream linux distros we previously compiled on...? |
Yes, but that is not a breaking change for users. People who build packages are impacted, yes, but if we can build a package successfully using either 3.5 that is compatible on that distro, it is not a breaking change. |
More details from clang:
|
Commenting out the lambda line makes the ICE go away. I think we should be able work around this issue by converting the lambda to function. I would still be interested in reports on whether clang 3.4.2 has already fixed this problem. |
Quick and dirty hacked workaround: diff --git a/deps/v8/src/heap/mark-compact.cc b/deps/v8/src/heap/mark-compact.cc
index de40fa0..511fc5a 100644
--- a/deps/v8/src/heap/mark-compact.cc
+++ b/deps/v8/src/heap/mark-compact.cc
@@ -3494,12 +3494,24 @@ int NumberOfPointerUpdateTasks(int pages) {
return Min(kMaxTasks, (pages + kPagesPerTask - 1) / kPagesPerTask);
}
+// Work-around bug in clang-3.4
+// https://github.com/nodejs/node/issues/8323
+template <PointerDirection direction>
+struct Foo {
+ PageParallelJob<PointerUpdateJobTraits<direction> >& job_;
+ Foo(PageParallelJob<PointerUpdateJobTraits<direction> >& job) : job_(job) {}
+ void operator()(MemoryChunk* chunk) {
+ job_.AddPage(chunk, 0);
+ }
+};
+
template <PointerDirection direction>
void UpdatePointersInParallel(Heap* heap, base::Semaphore* semaphore) {
PageParallelJob<PointerUpdateJobTraits<direction> > job(
heap, heap->isolate()->cancelable_task_manager(), semaphore);
- RememberedSet<direction>::IterateMemoryChunks(
- heap, [&job](MemoryChunk* chunk) { job.AddPage(chunk, 0); });
+ //RememberedSet<direction>::IterateMemoryChunks(
+ // heap, [&job](MemoryChunk* chunk) { job.AddPage(chunk, 0); });
+ RememberedSet<direction>::IterateMemoryChunks(heap, Foo<direction>(job));
PointersUpdatingVisitor visitor(heap);
int num_pages = job.NumberOfPages();
int num_tasks = NumberOfPointerUpdateTasks(num_pages); |
I just tried building with clang-3.4.2 without the above workaround, and I no longer see the internal compiler error. I think switching to building w/ 3.4.2 if at all possible is going to be a better solution. |
@ofrobots doesn't look like it: http://packages.ubuntu.com/precise/clang-3.4 |
Is the argument that we can't bump the requirement to 3.4.2 because precise only ships 3.4.1? I don't have a problem with that. It only affects the intersection of users that build from source on precise. They can still use the binaries from https://nodejs.org/. |
@bnoordhuis I wouldn't call it that, just stating that it doesn't seem to be available and I have no real insight into precise packaging. Can only assume its.. careful. |
Actually, there are other options for Precise. Ubuntu Wheezy is the real problem. I tried all day yesterday to get a sane compiler new enough made for Wheezy that would work and failed in a variety of different ways. I don't think we can do it without a) a lot more work or b) forcing people running Wheezy to upgrade their |
That won't be so great to do for Independently, a head's up that Node |
Okay. That may be wise @ofrobots since as far as I can tell, there's a similar situation for Ubuntu Precise (which I know people are actually really running out in the wild). I don't think we can ship this "as is" unless we update |
Do you have more details about this? I think this would be worth opening a separate issue, unless it's been done already. |
PR-URL: #8343 Fixes: #8323 Reviewed-By: rvagg - Rod Vagg <[email protected]>
Ref: nodejs#8343 Fixes: nodejs#8323 PR-URL: nodejs#8317 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Ref: #8343 Fixes: #8323 PR-URL: #8317 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Ref: nodejs#8343 Fixes: nodejs#8323 PR-URL: nodejs#8317 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Fixes: nodejs#8323 Refs: nodejs#8343 Refs: nodejs#8317 PR-URL: nodejs#11029 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Myles Borins <[email protected]>
Fixes: nodejs#8323 Refs: nodejs#8343 Refs: nodejs#8317 PR-URL: nodejs#11029 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Myles Borins <[email protected]>
Our package builds for Debian Wheezy, Ubuntu Precise, and perhaps others, are failing on v6.5.0 using
clang-3.4
.This seems to be a regression possibly introduced by V8 5.1.
See also: #8054 (comment)
cc @nodejs/v8 @nodejs/ctc
The text was updated successfully, but these errors were encountered: