20911 2008-09-18 01:24:33 -0700 REGRESSION(r36480?): Reproducible assertion failure below derefStructureIDs 64-bit JavaScriptCore 2008-09-18 17:47:12 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Mac OS X 10.5 RESOLVED FIXED HasReduction, Regression P1 Major --- 1 mrowe webkit-unassigned oliver oldest_to_newest 91595 0 mrowe 2008-09-18 01:24:33 -0700 After building jsc 64-bit: > function f() { a } a; f() ASSERTION FAILED: !m_deletionHasBegun (./wtf/RefCounted.h:47 void WTF::RefCounted<T>::deref() [with T = JSC::StructureID]) Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_INVALID_ADDRESS at address: 0x00000000bbadbeef 0x00000001000c89a6 in WTF::RefCounted<JSC::StructureID>::deref (this=0x100000000) at RefCounted.h:47 47 ASSERT(!m_deletionHasBegun); (gdb) bt #0 0x00000001000c89a6 in WTF::RefCounted<JSC::StructureID>::deref (this=0x100000000) at RefCounted.h:47 #1 0x000000010011b246 in JSC::CodeBlock::derefStructureIDs (this=0x1006175b0, vPC=0x100617850) at JavaScriptCore/VM/CodeBlock.cpp:938 #2 0x000000010011b367 in JSC::CodeBlock::~CodeBlock (this=0x1006175b0) at JavaScriptCore/VM/CodeBlock.cpp:898 #3 0x00000001000b42f5 in JSC::ProgramCodeBlock::~ProgramCodeBlock (this=0x1006175b0) at CodeBlock.h:297 #4 0x00000001000b430d in JSC::ProgramCodeBlock::~ProgramCodeBlock (this=0x1006175b0) at CodeBlock.h:297 #5 0x00000001000c3536 in WTF::deleteOwnedPtr<JSC::ProgramCodeBlock> (ptr=0x1006175b0) at OwnPtr.h:51 #6 0x00000001000c3562 in WTF::OwnPtr<JSC::ProgramCodeBlock>::~OwnPtr (this=0x100830c40) at OwnPtr.h:69 #7 0x00000001000c3583 in WTF::OwnPtr<JSC::ProgramCodeBlock>::~OwnPtr (this=0x100830c40) at OwnPtr.h:69 #8 0x00000001000c3a4e in JSC::ProgramNode::~ProgramNode (this=0x100830a00) at nodes.h:2195 #9 0x000000010003d925 in JSC::ParserRefCounted::deref (this=0x100830a00) at nodes.cpp:107 #10 0x00000001000b9979 in WTF::RefPtr<JSC::ProgramNode>::~RefPtr (this=0x7fff5fbff450) at RefPtr.h:50 #11 0x00000001000b9991 in WTF::RefPtr<JSC::ProgramNode>::~RefPtr (this=0x7fff5fbff450) at RefPtr.h:50 #12 0x0000000100073c86 in JSC::Interpreter::evaluate (exec=0x100608d70, scopeChain=@0x100608c20, sourceURL=@0x10000f650, startingLineNumber=1, source=@0x7fff5fbff570, thisValue=0x0) at interpreter.cpp:90 #13 0x0000000100073d16 in JSC::Interpreter::evaluate (exec=0x100608d70, scopeChain=@0x100608c20, sourceURL=@0x10000f650, startingLineNumber=1, code=@0x7fff5fbff5d0, thisV=0x0) at interpreter.cpp:62 #14 0x0000000100001459 in runInteractive (globalObject=0x100560000) at JavaScriptCore/kjs/Shell.cpp:381 #15 0x0000000100002aa2 in jscmain (argc=1, argv=0x7fff5fbff728, globalData=0x100809400) at JavaScriptCore/kjs/Shell.cpp:479 #16 0x0000000100002b1b in main (argc=1, argv=0x7fff5fbff728) at JavaScriptCore/kjs/Shell.cpp:307 (gdb) This results in a crash in release build that breaks most JavaScript execution and all JSCore tests. 91596 1 mrowe 2008-09-18 01:27:29 -0700 Code inside CodeGenerator::emitResolve is doing: instructions().append(0); This appends an Instruction with "operand" set to zero, which leaves the high bits of the Instruction unset. This causes a comparison with 0 to fail when we access the value via the "structureID" member of the union, as the high bits of the value contain non-zero data. 91597 2 23524 mrowe 2008-09-18 01:29:29 -0700 Created attachment 23524 Make "operand" member pointer-sized The attached patch makes the "operand" member of the union pointer-sized, which ensures that the high bits of the union will also be set when initializing an Instruction with 0. This fixes the assertion failures and crashes in the layout tests. I don't have time to perf test this change tonight. I wouldn't expect it to have any effect in 32-bit, but it may have some effect in 64-bit. 91599 3 mrowe 2008-09-18 01:32:51 -0700 There is other code in CodeGenerator that does instructions().append(0);, so this change may fix other yet-to-be-diagnosed problems too. 91610 4 slewis 2008-09-18 02:52:32 -0700 I tested the perf. The last good build on jsc 64 bit I found was 36479. Here are the 64 bit results TEST COMPARISON FROM TO DETAILS ============================================================================= ** TOTAL **: *1.011x as slow* 986.1ms +/- 0.3% 997.2ms +/- 1.3% significant ============================================================================= 3d: ?? 155.4ms +/- 0.8% 155.8ms +/- 0.8% not conclusive: might be *1.002x as slow* cube: ?? 53.0ms +/- 0.3% 53.2ms +/- 0.8% not conclusive: might be *1.005x as slow* morph: - 59.3ms +/- 1.7% 59.3ms +/- 1.1% raytrace: ?? 43.2ms +/- 0.4% 43.4ms +/- 0.6% not conclusive: might be *1.003x as slow* access: *1.029x as slow* 99.9ms +/- 0.4% 102.8ms +/- 4.8% significant binary-trees: ?? 17.3ms +/- 1.3% 17.6ms +/- 3.6% not conclusive: might be *1.014x as slow* fannkuch: *1.020x as slow* 17.4ms +/- 1.3% 17.7ms +/- 1.2% significant nbody: *1.034x as slow* 56.6ms +/- 0.4% 58.5ms +/- 7.3% significant nsieve: ?? 8.7ms +/- 2.6% 9.0ms +/- 5.3% not conclusive: might be *1.040x as slow* bitops: *1.020x as slow* 63.2ms +/- 0.6% 64.5ms +/- 1.6% significant 3bit-bits-in-byte: ?? 10.7ms +/- 2.1% 10.8ms +/- 2.5% not conclusive: might be *1.019x as slow* bits-in-byte: *1.031x as slow* 12.9ms +/- 1.1% 13.3ms +/- 4.0% significant bitwise-and: ?? 21.6ms +/- 1.3% 21.6ms +/- 1.4% not conclusive: might be *1.005x as slow* nsieve-bits: *1.030x as slow* 18.1ms +/- 0.8% 18.6ms +/- 2.7% significant controlflow: ?? 15.3ms +/- 1.5% 15.6ms +/- 1.8% not conclusive: might be *1.013x as slow* recursive: ?? 15.3ms +/- 1.5% 15.6ms +/- 1.8% not conclusive: might be *1.013x as slow* crypto: *1.021x as slow* 64.0ms +/- 0.4% 65.3ms +/- 2.0% significant aes: *1.027x as slow* 18.6ms +/- 1.3% 19.1ms +/- 3.4% significant md5: ?? 23.1ms +/- 0.7% 23.5ms +/- 4.1% not conclusive: might be *1.015x as slow* sha1: *1.023x as slow* 22.2ms +/- 0.9% 22.7ms +/- 2.4% significant date: *1.008x as slow* 95.3ms +/- 0.4% 96.1ms +/- 1.0% significant format-tofte: ?? 43.0ms +/- 0.5% 43.1ms +/- 0.7% not conclusive: might be *1.002x as slow* format-xparb: *1.012x as slow* 52.3ms +/- 0.4% 53.0ms +/- 1.4% significant math: ?? 127.7ms +/- 1.1% 128.9ms +/- 1.5% not conclusive: might be *1.010x as slow* cordic: *1.013x as slow* 38.8ms +/- 0.7% 39.3ms +/- 1.6% significant partial-sums: ?? 63.2ms +/- 1.4% 64.0ms +/- 1.7% not conclusive: might be *1.013x as slow* spectral-norm: - 25.8ms +/- 4.1% 25.7ms +/- 1.7% regexp: ?? 48.1ms +/- 1.0% 48.5ms +/- 1.5% not conclusive: might be *1.006x as slow* dna: ?? 48.1ms +/- 1.0% 48.5ms +/- 1.5% not conclusive: might be *1.006x as slow* string: *1.009x as slow* 317.1ms +/- 0.3% 319.9ms +/- 1.0% significant base64: - 24.6ms +/- 1.1% 24.6ms +/- 1.4% fasta: ?? 59.0ms +/- 0.9% 59.1ms +/- 1.4% not conclusive: might be *1.003x as slow* tagcloud: *1.014x as slow* 111.8ms +/- 0.6% 113.3ms +/- 1.3% significant unpack-code: ?? 70.3ms +/- 0.8% 70.7ms +/- 0.8% not conclusive: might be *1.006x as slow* validate-input: ?? 51.5ms +/- 1.3% 52.1ms +/- 2.0% not conclusive: might be *1.014x as slow* and the 32 bit results TEST COMPARISON FROM TO DETAILS ============================================================================= ** TOTAL **: *1.019x as slow* 984.5ms +/- 0.3% 1003.2ms +/- 1.3% significant ============================================================================= 3d: *1.014x as slow* 155.8ms +/- 0.4% 157.9ms +/- 1.7% significant cube: *1.016x as slow* 53.1ms +/- 0.8% 54.0ms +/- 2.0% significant morph: *1.028x as slow* 58.5ms +/- 0.4% 60.2ms +/- 2.3% significant raytrace: - 44.0ms +/- 0.7% 43.7ms +/- 1.0% access: *1.010x as slow* 99.6ms +/- 0.7% 100.6ms +/- 0.6% significant binary-trees: - 17.1ms +/- 2.4% 17.0ms +/- 0.9% fannkuch: - 17.8ms +/- 1.4% 17.6ms +/- 1.6% nbody: *1.019x as slow* 56.0ms +/- 0.6% 57.0ms +/- 0.9% significant nsieve: ?? 8.8ms +/- 2.2% 8.9ms +/- 2.1% not conclusive: might be *1.017x as slow* bitops: ?? 63.6ms +/- 0.8% 64.1ms +/- 1.6% not conclusive: might be *1.007x as slow* 3bit-bits-in-byte: 1.103x as fast 11.8ms +/- 1.6% 10.7ms +/- 3.2% significant bits-in-byte: *1.024x as slow* 12.7ms +/- 1.7% 13.0ms +/- 1.6% significant bitwise-and: *1.028x as slow* 21.4ms +/- 1.1% 22.0ms +/- 2.8% significant nsieve-bits: *1.037x as slow* 17.7ms +/- 1.9% 18.4ms +/- 1.5% significant controlflow: ?? 15.6ms +/- 1.8% 15.7ms +/- 1.7% not conclusive: might be *1.010x as slow* recursive: ?? 15.6ms +/- 1.8% 15.7ms +/- 1.7% not conclusive: might be *1.010x as slow* crypto: ?? 64.0ms +/- 0.9% 64.7ms +/- 1.2% not conclusive: might be *1.011x as slow* aes: ?? 18.6ms +/- 1.3% 18.6ms +/- 1.5% not conclusive: might be *1.003x as slow* md5: ?? 23.1ms +/- 1.8% 23.5ms +/- 1.6% not conclusive: might be *1.013x as slow* sha1: ?? 22.2ms +/- 1.1% 22.6ms +/- 1.6% not conclusive: might be *1.016x as slow* date: *1.025x as slow* 95.1ms +/- 0.3% 97.4ms +/- 2.8% significant format-tofte: ?? 43.0ms +/- 0.4% 43.0ms +/- 1.1% not conclusive: might be *1.002x as slow* format-xparb: *1.043x as slow* 52.1ms +/- 0.3% 54.4ms +/- 4.8% significant math: *1.027x as slow* 126.8ms +/- 0.8% 130.3ms +/- 2.4% significant cordic: ?? 40.3ms +/- 0.5% 40.8ms +/- 6.4% not conclusive: might be *1.011x as slow* partial-sums: *1.041x as slow* 61.1ms +/- 0.9% 63.6ms +/- 1.3% significant spectral-norm: ?? 25.4ms +/- 1.7% 25.9ms +/- 1.6% not conclusive: might be *1.018x as slow* regexp: ?? 48.2ms +/- 0.9% 48.9ms +/- 1.6% not conclusive: might be *1.013x as slow* dna: ?? 48.2ms +/- 0.9% 48.9ms +/- 1.6% not conclusive: might be *1.013x as slow* string: *1.025x as slow* 316.1ms +/- 0.7% 323.9ms +/- 1.7% significant base64: ?? 24.5ms +/- 1.2% 24.6ms +/- 2.1% not conclusive: might be *1.006x as slow* fasta: *1.021x as slow* 59.0ms +/- 1.2% 60.3ms +/- 1.9% significant tagcloud: *1.021x as slow* 111.7ms +/- 0.8% 114.0ms +/- 1.5% significant unpack-code: *1.037x as slow* 69.7ms +/- 0.9% 72.3ms +/- 2.2% significant validate-input: *1.029x as slow* 51.1ms +/- 1.3% 52.6ms +/- 3.0% significant 91614 5 slewis 2008-09-18 03:10:49 -0700 ignore the 32bit results they are actually 64bit. 32bit does not have a regression 91666 6 ggaren 2008-09-18 13:56:08 -0700 Surprising that the size of the instruction stream matters to performance with CTI enabled, since CTI doesn't use the instruction stream. To avoid the performance controversy, we could just prohibit initialization of an instruction using an integral value, and force the initializing code to specify a type, in this case, Instruction*. 91667 7 mrowe 2008-09-18 14:02:59 -0700 I don't think this is a regression with CTI enabled based on Stephanie's comment in comment #5. The 64-bit regression will probably also go away once we make CTI work for x86_64. 91708 8 oliver 2008-09-18 17:47:12 -0700 Committing to http://svn.webkit.org/repository/webkit/trunk ... M JavaScriptCore/ChangeLog M JavaScriptCore/VM/Instruction.h Committed r36637 23524 2008-09-18 01:29:29 -0700 2008-09-18 01:29:29 -0700 Make "operand" member pointer-sized bug-20911-v1.patch text/plain 440 mrowe ZGlmZiAtLWdpdCBhL0phdmFTY3JpcHRDb3JlL1ZNL0luc3RydWN0aW9uLmggYi9KYXZhU2NyaXB0 Q29yZS9WTS9JbnN0cnVjdGlvbi5oCmluZGV4IDJjMzE3ZGUuLmRlODdmMzkgMTAwNjQ0Ci0tLSBh L0phdmFTY3JpcHRDb3JlL1ZNL0luc3RydWN0aW9uLmgKKysrIGIvSmF2YVNjcmlwdENvcmUvVk0v SW5zdHJ1Y3Rpb24uaApAQCAtNDYsNyArNDYsNyBAQCBuYW1lc3BhY2UgSlNDIHsKIAogICAgICAg ICB1bmlvbiB7CiAgICAgICAgICAgICBPcGNvZGUgb3Bjb2RlOwotICAgICAgICAgICAgaW50IG9w ZXJhbmQ7CisgICAgICAgICAgICBpbnRwdHJfdCBvcGVyYW5kOwogICAgICAgICAgICAgU3RydWN0 dXJlSUQqIHN0cnVjdHVyZUlEOwogICAgICAgICAgICAgU3RydWN0dXJlSURDaGFpbiogc3RydWN0 dXJlSURDaGFpbjsKICAgICAgICAgICAgIEpTQ2VsbCoganNDZWxsOwo=