Bug 11336 – HashTable.h not zeroing pointers in constructor

• Status: RESOLVED
• Resolution: WONTFIX
• Component: JavaScriptCore

[Michael Emmel]

In debug mode the new m_next and m_previous pointers are net set to zero
leadin to crashes.

[Michael Emmel]

Created attachment 11126 [details]
zero pointers in constructor HashTable.h

[Michael Emmel]

Created attachment 11127 [details]
Missed the second constructor

[Maciej Stachowiak]

I don't think this is necessary because the constructors all call addIterator, which initializes both m_next and m_previous. Will ask for Darin's input.

[Michael Emmel]

Okay the problem is  in the assert

assert(table->m_iterators != it);
it->m_next = table->m_iterators;

If their is a problem it triggers before we initalize the m_next so its still garbage and makes it hard to debug.

Been a while since I looked at this.
The assert could be moved below and still work.

[Darin Adler]

(In reply to comment #3)
> I don't think this is necessary because the constructors all call addIterator,
> which initializes both m_next and m_previous. Will ask for Darin's input.

Maciej's correct about my original intent.

[Michael Emmel]

Can you move the assert below the setting of the value then ?
So the pointer is not garbage when the assert triggers.

From this

assert(table->m_iterators != it);
it->m_next = table->m_iterators;

To this
//set pointer so its not garbage
it->m_next = table->m_iterators;
//assert now if its incorrect.
assert(table->m_iterators != it);

Its not completely obvious the next pointer is garbage
if you trigger this in a debugger.

[Darin Adler]

Comment on attachment 11127 [details]
Missed the second constructor 

This patch won't compile (missing colon in the second constructor).

And I'm not convinced the change is helpful. I understand that this confused you once while debugging, but I'd prefer to just leave this as-is unless you present a more compelling argument to change it.

[Michael Emmel]

I don't think the patch is the right answer in the first place.
If you do hit the assert your dealing with some intresting info in a debugger.
And it does its job below.
I've got no strong feelings either way it was just confusing at the time.

The reason I hit it the first time is actually a bit more involved I had
compiled JavaScriptCore and WebKit with different debug flags but this is more a build/linking issue. From my experience this does not seem to work.