Bug 34866 – REGRESSION(before r54642?): Leopard Debug Bot crashed on fast/forms/old-names.html

• Component: Forms
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

Leopard Debug Bot crashed on fast/forms/old-names.html

ASSERTION FAILED: malloc_size(p)
(/Volumes/Big/WebKit-BuildSlave/leopard-intel-debug/build/WebKitBuild/Debug/JavaScriptCore.framework/PrivateHeaders/ValueCheck.h:52
static void WTF::ValueCheck<P*>::checkConsistency(const P*) [with P =
WebCore::AtomicStringImpl])

http://build.webkit.org/results/Leopard%20Intel%20Debug%20(Tests)/r54686%20(10385)/fast/forms/old-names-stderr.txt

The checkins look unrelated, but I doubt this is a flake given that
it's an ASSERT.

Dave Levin notes he also reproduced this on his machine.

[David Levin]

Created attachment 48599 [details]
A crash dump. Repros on leopard with debug build and "run-webkit-tests  LayoutTests/fast/"

[David Levin]

One more note, I adjusted the rev in the title because it looks like the crash was introduced some time before 2-9-2010 

(My first occurrence happened the night of 2-8 just after 10pm but I may not have sync'ed for a while before that so I don't know when it started).

[David Levin]

Adding ap because it looks like he added the checks that are failing (in r53957), so maybe he has some insights about why it might fire.

[Alexey Proskuryakov]

This assertion failure means that there are stale AtomicStringImpl* keys in the HashMap being checked. It may or may not be an exploitable security issue, let's track it as such for now.

[Alexey Proskuryakov]

Actually, some cursory analysis suggests that this is probably not a security issue. Even more than that, it's likely just a misplaced consistency check - it should be fine for an inconsistent cache to be used when creating an HTMLFormCollection.

Going to think about this again in the morning...

[David Levin]

I tried to narrow down the revision range (using git bisect) but unfortunately, there were enough other crashes and failures in layout tests during this time to trip it up.

When I did sync to ap's original change, this assert didn't seem to be firing.

[Alexey Proskuryakov]

Created attachment 48646 [details]
proposed fix

Yes, clearly just an incorrect check. Dave, I'm surprised it doesn't happen for you with the revision it was added - did you try running just this one test with --repeat 100?

[David Levin]

(In reply to comment #7)
> Created an attachment (id=48646) [details]
> proposed fix
> 
> Yes, clearly just an incorrect check. Dave, I'm surprised it doesn't happen for
> you with the revision it was added - did you try running just this one test
> with --repeat 100?

No. In recent builds it would happen after two runs when I do run-webkit-test LayoutTests/fast but when it was checked in, this didn't seem to be the case, so it appears to happen more frequently now but since it is a bad check, any number of things could happen to make an assert fire more often.

[Alexey Proskuryakov]

Committed revision 54727.