161308 2016-08-29 03:48:19 -0700 REGRESSION (r205107): ASSERTION FAILED: !(reinterpret_cast<char*>(this)[i]) 2016-08-31 11:22:34 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 161427 161268 1 clopez webkit-unassigned benjamin bugs-noreply cgarcia fpizlo ossy saam ysuzuki zan oldest_to_newest 1224155 0 clopez 2016-08-29 03:48:19 -0700 Revision r205107 <http://trac.webkit.org/r205107> has caused lot of assertions on the Debug build of GTK+: https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug%20%28Tests%29/builds/10835 Backtrace: ASSERTION FAILED: !(reinterpret_cast<char*>(this)[i]) ../../Source/JavaScriptCore/dfg/DFGAbstractValue.h(66) : JSC::DFG::AbstractValue::AbstractValue() 1 0x7f5e53a30411 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(WTFCrash+0x1e) [0x7f5e53a30411] 2 0x7f5e530e5262 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(JSC::DFG::AbstractValue::AbstractValue()+0x96) [0x7f5e530e5262] 3 0x7f5e53115412 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(JSC::DFG::AbstractValue::fullTop()+0x19) [0x7f5e53115412] 4 0x7f5e53114b74 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(JSC::DFG::BasicBlock::BasicBlock(unsigned int, unsigned int, unsigned int, float)+0x12e) [0x7f5e53114b74] 5 0x7f5e5312f9e0 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(JSC::DFG::ByteCodeParser::parseCodeBlock()+0x60e) [0x7f5e5312f9e0] 6 0x7f5e5312ffb6 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(JSC::DFG::ByteCodeParser::parse()+0x1c8) [0x7f5e5312ffb6] 7 0x7f5e531302aa /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(JSC::DFG::parse(JSC::DFG::Graph&)+0x3b) [0x7f5e531302aa] 8 0x7f5e533155c4 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(JSC::DFG::Plan::compileInThreadImpl(JSC::DFG::LongLivedState&)+0xe2) [0x7f5e533155c4] 9 0x7f5e53314fcf /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(JSC::DFG::Plan::compileInThread(JSC::DFG::LongLivedState&, JSC::DFG::ThreadData*)+0x173) [0x7f5e53314fcf] 10 0x7f5e53423754 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(JSC::DFG::Worklist::runThread(JSC::DFG::ThreadData*)+0x316) [0x7f5e53423754] 11 0x7f5e53423a7a /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(JSC::DFG::Worklist::threadFunction(void*)+0x2a) [0x7f5e53423a7a] 12 0x7f5e53a4d6be /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x228c6be) [0x7f5e53a4d6be] 13 0x7f5e53a4d874 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x228c874) [0x7f5e53a4d874] 14 0x7f5e5a04c7ce /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(std::function<void ()>::operator()() const+0x32) [0x7f5e5a04c7ce] 15 0x7f5e53a4d5a0 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x228c5a0) [0x7f5e53a4d5a0] 16 0x7f5e53a887e1 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x22c77e1) [0x7f5e53a887e1] 17 0x7f5e4f88f0a4 /lib/x86_64-linux-gnu/libpthread.so.0(+0x80a4) [0x7f5e4f88f0a4] 18 0x7f5e4afc587d /lib/x86_64-linux-gnu/libc.so.6(clone+0x6d) [0x7f5e4afc587d] 1224158 1 cgarcia 2016-08-29 03:58:47 -0700 All layout tests are crashing for the same assert in the Debug bot. 1224163 2 cgarcia 2016-08-29 06:08:48 -0700 I don't think this is GTK specific, Carlos confirmed he could reproduce the assert with jsconly but only when building with GCC, not with clang. 1224269 3 saam 2016-08-29 12:03:40 -0700 (In reply to comment #2) > I don't think this is GTK specific, Carlos confirmed he could reproduce the > assert with jsconly but only when building with GCC, not with clang. Interesting. Any idea on why this may be? 1224270 4 saam 2016-08-29 12:07:09 -0700 I wonder if GCC is emitting a 32-bit store instead of a 64-bit store? 1224335 5 saam 2016-08-29 14:03:44 -0700 Maybe the bug is not changing the "1u" to "1ull" inside SpeculatedType.h? 1224425 6 clopez 2016-08-29 16:32:02 -0700 (In reply to comment #5) > Maybe the bug is not changing the "1u" to "1ull" inside SpeculatedType.h? I just tested to replace all occurrences of "1u" with "1ull" in SpeculatedType.h, and did a debug build of JSCOnly with GCC 4.9. It is still crashing with the same assert. 1224482 7 saam 2016-08-29 18:40:50 -0700 Ok. Third guess, there is now 4 bytes of padding after the ArrayModes m_arrayModes field, maybe those aren't getting zeroed out? And for some reason, clang zeroes them out. 1224520 8 zan 2016-08-30 00:50:48 -0700 (In reply to comment #7) > Ok. > Third guess, there is now 4 bytes of padding after the ArrayModes > m_arrayModes > field, maybe those aren't getting zeroed out? And for some reason, clang > zeroes > them out. That's the reason. 1224526 9 cgarcia 2016-08-30 01:50:55 -0700 // The WTF Traits for AbstractValue allow the initialization of values with bzero(). // We verify the correctness of this assumption here. So, that's assuming that AbstractValue is always created as a Vector or Hash value and then initialized by them with memset. But AbstractValue::heapTop(), AbstractValue::bytecodeTop() and AbstractValue::fullTop() are creating a stack allocated AbstractValue that is not zero initialized. 1224840 10 clopez 2016-08-30 19:51:20 -0700 (In reply to comment #7) > Ok. > Third guess, there is now 4 bytes of padding after the ArrayModes > m_arrayModes > field, maybe those aren't getting zeroed out? And for some reason, clang > zeroes > them out. It seems that the standard only guarantees that the padding in structures is zero-initialized on some very specific cases [1]. In the rest of cases it is undefined behaviour. And this seems to be one of this cases. That explains why clang is zero-initializing the padding of the structure, but GCC does not. This fixes the issue with GCC: --- a/Source/JavaScriptCore/dfg/DFGAbstractValue.h +++ b/Source/JavaScriptCore/dfg/DFGAbstractValue.h @@ -191,21 +191,21 @@ struct AbstractValue { static AbstractValue heapTop() { - AbstractValue result; + static AbstractValue result; result.makeHeapTop(); return result; } static AbstractValue bytecodeTop() { - AbstractValue result; + static AbstractValue result; result.makeBytecodeTop(); return result; } static AbstractValue fullTop() { - AbstractValue result; + static AbstractValue result; result.makeFullTop(); return result; } Now I wonder if this is the best approach (declaring it as static), or it will be better to directly bzero the structure on the constructor, or perhaps it will be better to modify the assert to check only the values of the members of the structure. [1] https://gustedt.wordpress.com/2012/10/24/c11-defects-initialization-of-padding/ 1224876 11 cgarcia 2016-08-30 22:57:22 -0700 (In reply to comment #10) > (In reply to comment #7) > > Ok. > > Third guess, there is now 4 bytes of padding after the ArrayModes > > m_arrayModes > > field, maybe those aren't getting zeroed out? And for some reason, clang > > zeroes > > them out. > > It seems that the standard only guarantees that the padding in structures is > zero-initialized on some very specific cases [1]. In the rest of cases it is > undefined behaviour. And this seems to be one of this cases. That explains > why clang is zero-initializing the padding of the structure, but GCC does > not. Stack allocated objects are never zero-initialized by the compiler, I think, their members are initialized on construction, so it's expected that the padding is not initialized. AbstractValue adds vector and hash traits to ensure that they are initialized with memset when allocated by a vector or hash objects. In such cases the passing is always 0 because memset fills from the base pointer to the sizeof(). > This fixes the issue with GCC: > > --- a/Source/JavaScriptCore/dfg/DFGAbstractValue.h > +++ b/Source/JavaScriptCore/dfg/DFGAbstractValue.h > @@ -191,21 +191,21 @@ struct AbstractValue { > > static AbstractValue heapTop() > { > - AbstractValue result; > + static AbstractValue result; > result.makeHeapTop(); > return result; > } > > static AbstractValue bytecodeTop() > { > - AbstractValue result; > + static AbstractValue result; > result.makeBytecodeTop(); > return result; > } > > static AbstractValue fullTop() > { > - AbstractValue result; > + static AbstractValue result; > result.makeFullTop(); > return result; > } > > > > Now I wonder if this is the best approach (declaring it as static), or it > will be better to directly bzero the structure on the constructor, or > perhaps it will be better to modify the assert to check only the values of > the members of the structure. I guess we really want a new AbstractValue object every time those methods are called. I think the check is what is wrong, not the callers. Could we remove that check while we find a better solution? The GTK+ Debug bot has exited early for two days > > [1] > https://gustedt.wordpress.com/2012/10/24/c11-defects-initialization-of- > padding/ 1225003 12 ysuzuki 2016-08-31 11:22:34 -0700 Fixed in https://trac.webkit.org/changeset/205254.