|Summary:||REGRESSION (r36427): ASSERTION FAILED: m_refCount >= 0 in RegisterID::deref()|
|Product:||WebKit||Reporter:||Matt Lilek <dev+webkit>|
|Severity:||Normal||CC:||ggaren, mjs, zwarich|
|Version:||528+ (Nightly build)|
|OS:||OS X 10.5|
Description Matt Lilek 2008-09-15 08:40:37 PDT
Comment 3 Geoffrey Garen 2008-09-15 12:18:48 PDT
Matt, should we close this bug?
Comment 4 Matt Lilek 2008-09-15 13:49:50 PDT
(In reply to comment #3) > Matt, should we close this bug? > Thats up to you and whether you think this is worth investigating. I'll bump this down to a P2 since it only happens in a debug build.
Comment 5 Geoffrey Garen 2008-09-15 13:52:00 PDT
Sorry! I forgot that nightly == release build. Back up to P1. We definitely want to fix this.
Comment 6 Cameron Zwarich (cpst) 2008-09-16 15:10:46 PDT
I'll assign this to myself. The problem is that in the CodeGenerator::emitConstruct() function, the local RefPtr<RegisterID> funcProto has a negative count after it is deref'd. I don't quite understand why just yet, but hopefully it won't be too hard to fix.
Comment 7 Cameron Zwarich (cpst) 2008-09-16 15:22:04 PDT
Sorry, I meant func, which is protected by a RefPtr<RegisterID> called protectFunc, not protoFunc.
Comment 8 Cameron Zwarich (cpst) 2008-09-16 16:14:56 PDT
This is sort of strange. On the offending call, we have the following RegisterIDs: func: 27 protoFunc: 26 callFrame: 27 When it is allocating the new temporary for protoFunc, it somehow thinks that the register with index 26 is the last one in m_temporaries, which has no ref count, so it removes it and appends a new one. The next time, it still thinks that the register with index 26 is the last one, but it has a ref count, so it makes a new one with index 27. I think the bug here is likely not in the CodeGenerator::emitConstruct() logic, but rather in the fact that the last temporary in m_temporaries isn't the one with index 26, but I'll try to make a reduction and see. Thankfully, debka.com seems fairly old school.
Comment 9 Cameron Zwarich (cpst) 2008-09-16 17:48:07 PDT
Created attachment 23494 [details] Reduction Here's a reduction. It comes from code of the form o = new Constructor[m](m);
Comment 10 Cameron Zwarich (cpst) 2008-09-16 18:09:31 PDT
This is caused by the same incorrect behaviour of CodeGenerator::finalDestination() as in bug 20340. An updated version of that patch fixes this bug as well. I'll post it here for review.
Comment 11 Cameron Zwarich (cpst) 2008-09-16 19:27:34 PDT
Created attachment 23495 [details] Proposed patch Here is a patch that fixes this bug, as well as bug 20340. It passes all tests and all changes it makes to SunSpider codegen are positive. If this is reviewed, I will put the reduction here as a test case in fast/js/codegen-temporaries. Just doing this for NewExprNode::emitCode() caused a lot of tests to fail, so it seems that if we are going to fix this, we need to fix it everywhere.
Comment 12 Cameron Zwarich (cpst) 2008-09-16 19:51:22 PDT
Created attachment 23496 [details] Different proposed patch Maciej and I agreed that the usage of finalDestination() after my patch is now unclear. Here is an easier way to fix this particular bug, but not bug 20340.
Comment 13 Maciej Stachowiak 2008-09-16 19:55:09 PDT
Comment on attachment 23496 [details] Different proposed patch r=me