Loading http://www.debka.com/headline.php?hid=5579 hits the following ASSERT:
ASSERTION FAILED: m_refCount >= 0
Thread 0 Crashed:
Created attachment 23442 [details]
This doesn't crash in the latest nightly (r36403).
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:
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]
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]
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
Landed in r36528.