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]
All layout tests are crashing for the same assert in the Debug bot.
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.
(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?
I wonder if GCC is emitting a 32-bit store instead of a 64-bit store?
Maybe the bug is not changing the "1u" to "1ull" inside SpeculatedType.h?
(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.
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.
(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.
// 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.
(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/
(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/
Fixed in https://trac.webkit.org/changeset/205254.