601247 - "Assertion failure: JSVAL_IS_DOUBLE(v)" with localStorage, __proto__ [@ nsStorage2SH::NewEnumerate]

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P1)

Description

[Jesse Ruderman]

Attachment: testcase (fatal assertion failure when loaded)

Assertion failure: JSVAL_IS_DOUBLE(v), at jsapi.h:284

Security-sensitive because I think this means JSVAL_TO_PRIVATE is about to treat a non-pointer as a pointer.  But an opt build does not crash on this testcase.

Comment 1

[Jesse Ruderman]

Attachment: stack trace+

Comment 2

[Boris Zbarsky [:bzbarsky]]

This is technically a bug in this function, but a benign one, I think.  These cases:

10743     case JSENUMERATE_INIT:
10744     case JSENUMERATE_INIT_ALL:

don't read keys; they only write it.  And it happens that in those cases we do set keys to JSVAL_TO_PRIVATE(*statep) and *statep is not a private jsval.... but since we don't use the resulting value in those cases it's not really a problem, except for not making any sense.  ;)

Rewriting with if/then instead of a switch and only setting keys to *statep after the first if block would fix this....

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: . Don't JSVAL_TO_PRIVATE a random jsval.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: diff -w for ease of review

I don't think this needs to stay sec-sensitive, btw.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 480360 [details] [diff] [review]
diff -w for ease of review

Requesting approval for 2.0.

Comment 6

[Justin Lebar (not reading bugmail)]

Backed out in rev 0de6603ae6cb due to orange.

TEST-UNEXPECTED-FAIL | /tests/browser/components/feeds/test/test_bug408328.html | Exited with code 1 during test run
PROCESS-CRASH | /tests/browser/components/feeds/test/test_bug408328.html | application crashed (minidump found)

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1287181873.1287181994.11157.gz

Comment 7

[Boris Zbarsky [:bzbarsky]]

Gah.  Jesse, please do NOT remove my status whiteboard markings, unless you want bugs to get lost....

Especially because it's not obvious that it was this bug that caused the orange, and not the other patch that landed with it.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Yeah, the failure was from the other bug.  I repushed this patch: http://hg.mozilla.org/mozilla-central/rev/7a0f558020ed