Summary: | REGRESSION (r36427): ASSERTION FAILED: m_refCount >= 0 in RegisterID::deref() | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Matt Lilek <dev+webkit> | ||||||||||
Component: | JavaScriptCore | Assignee: | Cameron Zwarich (cpst) <zwarich> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | ggaren, mjs, zwarich | ||||||||||
Priority: | P1 | Keywords: | HasReduction, Regression | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Mac | ||||||||||||
OS: | OS X 10.5 | ||||||||||||
URL: | http://www.debka.com/headline.php?hid=5579 | ||||||||||||
Attachments: |
|
Description
Matt Lilek
2008-09-15 08:40:37 PDT
Created attachment 23442 [details]
Full crashlog
Matt, should we close this bug? (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. Sorry! I forgot that nightly == release build. Back up to P1. We definitely want to fix this. 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. Sorry, I meant func, which is protected by a RefPtr<RegisterID> called protectFunc, not protoFunc. This is sort of strange. On the offending call, we have the following RegisterIDs: func: 27 protoFunc: 26 callFrame[0]: 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. Created attachment 23494 [details]
Reduction
Here's a reduction. It comes from code of the form
o = new Constructor[m](m);
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. 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. 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 on attachment 23496 [details]
Different proposed patch
r=me
Landed in r36528. |