Clang ToT builders fail due to some stuff in third_party/angle |
||||
Issue descriptionStarting from https://build.chromium.org/p/chromium.fyi/builders/ClangToTLinux/builds/7340, a lot of Clang ToT builders fail with the following error: FAILED: obj/third_party/angle/angle_util_static/Matrix.o ../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF obj/third_party/angle/angle_util_static/Matrix.o.d -DV8_DEPRECATION_WARNINGS -DUSE_UDEV -DUI_COMPOSITOR_IMAGE_TRANSPORT -DUSE_AURA=1 -DUSE_PANGO=1 -DUSE_CAIRO=1 -DUSE_GLIB=1 -DUSE_NSS_CERTS=1 -DUSE_X11=1 -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=291955 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -DCOMPONENT_BUILD -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DGL_GLEXT_PROTOTYPES -DEGL_EGLEXT_PROTOTYPES -DANGLE_EXPORT= -DEGLAPI= -DGL_APICALL= -I../.. -Igen -I../../third_party/angle/util -I../../third_party/angle/include -I../../third_party/angle/src -I../../third_party/angle/src/common/third_party/numerics -Igen/angle -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -funwind-tables -fPIC -pipe -B../../third_party/binutils/Linux_x64/Release/bin -fcolor-diagnostics -fdebug-prefix-map=/b/c/b/ClangToTLinux/src=. -m64 -march=x86-64 -pthread -Wall -Werror -Wextra -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-deprecated-register -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override -Wno-shift-negative-value -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-address-of-packed-member -O2 -fno-ident -fdata-sections -ffunction-sections -g2 --sysroot=../../build/linux/debian_wheezy_amd64-sysroot -fvisibility=hidden -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -fno-threadsafe-statics -fvisibility-inlines-hidden -std=gnu++11 -fno-rtti -fno-exceptions -c ../../third_party/angle/util/Matrix.cpp -o obj/third_party/angle/angle_util_static/Matrix.o In file included from ../../third_party/angle/util/Matrix.cpp:10: In file included from ../../third_party/angle/util/Matrix.h:15: ../../third_party/angle/src/common/vector_utils.h:445:16: error: no viable conversion from 'const angle::VectorBase<3, float>' to 'const VectorN' (aka 'const Vector<3UL, float>') return dot(*this); ^~~~~ ../../third_party/angle/src/common/vector_utils.h:439:22: note: in instantiation of member function 'angle::VectorBase<3, float>::lengthSquared' requested here return std::sqrt(lengthSquared()); ^ ../../third_party/angle/src/common/vector_utils.h:464:20: note: in instantiation of member function 'angle::VectorBase<3, float>::length' requested here return *this / length(); ^ ../../third_party/angle/util/Matrix.cpp:81:21: note: in instantiation of member function 'angle::VectorBase<3, float>::normalized' requested here Vector3 u = p.normalized(); ^ ../../third_party/angle/src/common/vector_utils.h:19:7: note: candidate constructor (the implicit copy constructor) not viable: cannot bind base class object of type 'const angle::VectorBase<3, float>' to derived class reference 'const angle::Vector<3, float> &' for 1st argument class Vector; ^ ../../third_party/angle/src/common/vector_utils.h:19:7: note: candidate constructor (the implicit move constructor) not viable: no known conversion from 'const angle::VectorBase<3, float>' to 'angle::Vector<3, float> &&' for 1st argument class Vector; ^ ../../third_party/angle/src/common/vector_utils.h:44:5: note: candidate template ignored: inherited constructor cannot be used to copy object VectorBase(const VectorBase<Dimension, Type2> &other); ^ ../../third_party/angle/src/common/vector_utils.h:132:32: note: constructor from base class 'VectorBase<3, float>' inherited here using VectorBase<3, Type>::VectorBase; ^ ../../third_party/angle/src/common/vector_utils.h:88:29: note: passing argument to parameter 'other' here Type dot(const VectorN &other) const; ^ ../../third_party/angle/src/common/vector_utils.h:445:16: error: no viable conversion from 'const angle::VectorBase<4, float>' to 'const VectorN' (aka 'const Vector<4UL, float>') return dot(*this); ^~~~~ ../../third_party/angle/src/common/vector_utils.h:439:22: note: in instantiation of member function 'angle::VectorBase<4, float>::lengthSquared' requested here return std::sqrt(lengthSquared()); ^ ../../third_party/angle/src/common/vector_utils.h:464:20: note: in instantiation of member function 'angle::VectorBase<4, float>::length' requested here return *this / length(); ^ ../../third_party/angle/util/Matrix.cpp:212:53: note: in instantiation of member function 'angle::VectorBase<4, float>::normalized' requested here Vector4 transformed = (mat * Vector4(pt, 1.0f)).normalized(); ^ ../../third_party/angle/src/common/vector_utils.h:19:7: note: candidate constructor (the implicit copy constructor) not viable: cannot bind base class object of type 'const angle::VectorBase<4, float>' to derived class reference 'const angle::Vector<4, float> &' for 1st argument class Vector; ^ ../../third_party/angle/src/common/vector_utils.h:19:7: note: candidate constructor (the implicit move constructor) not viable: no known conversion from 'const angle::VectorBase<4, float>' to 'angle::Vector<4, float> &&' for 1st argument class Vector; ^ ../../third_party/angle/src/common/vector_utils.h:44:5: note: candidate template ignored: inherited constructor cannot be used to copy object VectorBase(const VectorBase<Dimension, Type2> &other); ^ ../../third_party/angle/src/common/vector_utils.h:152:32: note: constructor from base class 'VectorBase<4, float>' inherited here using VectorBase<4, Type>::VectorBase; ^ ../../third_party/angle/src/common/vector_utils.h:88:29: note: passing argument to parameter 'other' here Type dot(const VectorN &other) const; ^ 2 errors generated. The suspect is https://chromium.googlesource.com/chromium/src/+/9b8d527fa720a2b07a154134ec740b69f0622e90, in which a new version of ANGLE was rolled in. I will take a closer look at the failure and report back.
,
Jan 17 2017
After reproducing the issue locally and playing with it around, the culprit is https://chromium-review.googlesource.com/414272: commit 922cbfcb8e824395baa43dca872628298296ab47 Author: Corentin Wallez <cwallez@chromium.org> Date: Fri Nov 25 16:23:18 2016 -0500 common: Add a vector arithmetic helper classes Change-Id: I2f96baedf10d346eaa150bab04f8f6ca3ba573b9 Reviewed-on: https://chromium-review.googlesource.com/414272 Reviewed-by: Corentin Wallez <cwallez@chromium.org> Reviewed-by: Geoff Lang <geofflang@chromium.org> It does a few shady tricks with base/derived classes, which are not accepted by Clang (and will likely not be loved by CFI / UBSan checks). The particular issue that breaks everything is template <size_t Dimension, typename Type> Type VectorBase<Dimension, Type>::lengthSquared() const { return dot(*this); } template <size_t Dimension, typename Type> Type VectorBase<Dimension, Type>::dot(const Vector<Dimension, Type> &other) const { Type sum = Type(); for (size_t i = 0; i < Dimension; ++i) { sum += mData[i] * other.mData[i]; } return sum; } Here we pass a VectorBase reference into dot which accepts VectorN (a derived class). The stable clang seems to silently call a copy constructor (which is probably not the intended behavior, as it slows down everything), and the ToT Clang rightfully refuses to compile such a code. The proposed fix is to change dot() signature to accept VectorBase instead of VectorN. There are also some dangerous / wrong in the general case reinterpret casts in the file, but they are less critical to fix, and I will pass on them for now.
,
Jan 17 2017
The fix is sent for a review: https://chromium-review.googlesource.com/#/c/428903/
,
Jan 17 2017
As a faster measure to overcome the build breakage, I propose to rollback https://chromium.googlesource.com/chromium/src/+/9b8d527fa720a2b07a154134ec740b69f0622e90 that brought the the problematic ANGLE version. Nico, Hans, any objections?
,
Jan 17 2017
+1 for rolling back, this affects the whole clang tot waterfall, not just windows.
,
Jan 17 2017
Revert is in the progress: https://codereview.chromium.org/2640483003/
,
Jan 17 2017
Jamie reports that reverting that roll will break the official builders (sizes check). I have cancelled the rollback CL CQ, and we will try to fast track the fix & ANGLE roll. Objections?
,
Jan 17 2017
Thanks for cancelling the revert- if you executed the rollback as you did initially it would not fix the break and would in fact regress the sizes check on the official builders. Lets land your fix and roll forward, or revert the particular offending CL and roll (might be hard if it's used in a lot of places now, I think that CL went in some time ago).
,
Jan 17 2017
The ANGLE-side fix is in the CQ, sorry for the breakage.
,
Jan 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/angle/angle/+/fd8b469ebc8f63d0579cb62da94fe5d393503a78 commit fd8b469ebc8f63d0579cb62da94fe5d393503a78 Author: Ivan Krasin <krasin@chromium.org> Date: Tue Jan 17 19:20:26 2017 Fix dot() signature to avoid an implicit copy constructor call. That also fixes the build with Clang ToT compiler. BUG= chromium:681912 Change-Id: I1449ee0f1f3f031d64af1a1f64f43808d2a8a1b1 Reviewed-on: https://chromium-review.googlesource.com/428903 Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Corentin Wallez <cwallez@chromium.org> Commit-Queue: Jamie Madill <jmadill@chromium.org> Commit-Queue: Corentin Wallez <cwallez@chromium.org> [modify] https://crrev.com/fd8b469ebc8f63d0579cb62da94fe5d393503a78/src/common/vector_utils.h
,
Jan 17 2017
The ANGLE roll is in the CQ: https://codereview.chromium.org/2638763005
,
Jan 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f757952b5f94005eeb71319010af15af9a1dab7c commit f757952b5f94005eeb71319010af15af9a1dab7c Author: krasin <krasin@chromium.org> Date: Tue Jan 17 22:27:47 2017 Roll ANGLE 47c27e8..fd8b469 https://chromium.googlesource.com/angle/angle.git/+log/47c27e8..fd8b469 BUG= chromium:681912 TBR=jmadill@chromium.org TEST=bots CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2638763005 Cr-Commit-Position: refs/heads/master@{#444165} [modify] https://crrev.com/f757952b5f94005eeb71319010af15af9a1dab7c/DEPS
,
Jan 18 2017
This is now fixed. For instance, ThinLTO Linux ToT bot is green: https://build.chromium.org/p/chromium.fyi/builders/ThinLTO%20Linux%20ToT/builds/1043
,
Jan 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0a8923f06f3915892d131bd7facaeb8cf6e8847f commit 0a8923f06f3915892d131bd7facaeb8cf6e8847f Author: krasin <krasin@chromium.org> Date: Wed Jan 18 19:29:33 2017 Fix unused lambda captures in base/. Clang just got a new warning to warn about unused lambda captures, and that requires us to clean all places with this issue across all the Chromium code base. This CL fixes all such cases in base/. BUG= 681912 Review-Url: https://codereview.chromium.org/2640073002 Cr-Commit-Position: refs/heads/master@{#444436} [modify] https://crrev.com/0a8923f06f3915892d131bd7facaeb8cf6e8847f/base/bind_unittest.cc
,
Jan 18 2017
Why did this break only on the tot bots but not on the bots with a pinned clang?
,
Jan 18 2017
For the record, #14 got into a wrong bug. Answering to #15: I believe Clang now disallows an implicit copy constructor from base to derived class, where a constructor is derived from the base class. Or something. |
||||
►
Sign in to add a comment |
||||
Comment 1 by krasin@chromium.org
, Jan 17 2017