Closed
Bug 558541
Opened 15 years ago
Closed 15 years ago
js_SetPropertyHelper doesn't notify tracer when setting a non-writable property with strict option on or "Assertion failure: s0->isQuad(), at ../jstracer.cpp" or "Assertion failure: v_ins->isF64(), at ../jstracer.cpp"
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
People
(Reporter: Waldo, Assigned: Waldo)
References
Details
(Keywords: assertion, regression, testcase, Whiteboard: [sg:critical?])
Attachments
(4 files, 1 obsolete file)
|
2.09 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
|
1.86 KB,
patch
|
Details | Diff | Splinter Review | |
|
1.86 KB,
patch
|
Details | Diff | Splinter Review | |
|
1.88 KB,
patch
|
Details | Diff | Splinter Review |
Shell testcase, crashes with gnarly low-level assertion:
options("strict");
for (var i = 0; i < 5; i++)
Boolean.prototype = 42;
Unlike bug 558249 (fallout from bug 550402), this goes back a ways -- I can reproduce the problem with a 3.6 tree. Strangely, I have trouble causing a failure inside the browser with javascript.options.strict set to true; not sure what's up there, but the shell behavior is more than enough to make me worry about the browser as well.
I noticed this while working on the fix for bug 558249, but since it's applicable a ways back it seems better to separate it out into this bug -- let that one take care of the shorter-term regression.
Comment 1•15 years ago
|
||
autoBisect shows this is probably related to bug 490666:
The first bad revision is:
changeset: 27505:3519ddacbe2e
user: Andreas Gal
date: Thu Apr 30 15:52:13 2009 -0700
summary: We don't cache access to shared properties in the property cache (490666, r=igor,brendan).
I had to follow this method: (cat | js -j -i or by direct typing/copy+paste) to get the testcase to crash as per bug 558249 comment 3. This might be a reason why jsfunfuzz didn't find it beforehand, though I might be wrong.
Blocks: 490666
Comment 2•15 years ago
|
||
This used to assert at:
Assertion failure: s0->isQuad(), at ../jstracer.cpp
but now seems to assert at:
Assertion failure: v_ins->isF64(), at ../jstracer.cpp:9347
Summary: js_SetPropertyHelper doesn't notify tracer when setting a non-writable property with strict option on → js_SetPropertyHelper doesn't notify tracer when setting a non-writable property with strict option on or "Assertion failure: s0->isQuad(), at ../jstracer.cpp" or "Assertion failure: v_ins->isF64(), at ../jstracer.cpp"
Comment 3•15 years ago
|
||
can we get an ETA?
Comment 4•15 years ago
|
||
I saw the patch fly by recently... bug 558249 has it. Is this a dup?
/be
| Assignee | ||
Comment 5•15 years ago
|
||
The test has the same problem as the test in the bug Brendan just mentioned (this isn't a dup, btw).
It's basically hopeless to attempt to obfuscate the intent of this patch. Maybe a diversionary bug of some sort might work, conceivably. Not sure what would be a good diversion that doesn't spill the beans completely -- ideas welcome.
Attachment #438835 -
Flags: review?(jorendorff)
Updated•15 years ago
|
Attachment #438835 -
Flags: review?(jorendorff) → review+
| Assignee | ||
Comment 6•15 years ago
|
||
Patch landed, backed out for much orange -- to be investigated.
Updated•15 years ago
|
blocking2.0: --- → beta1+
Comment 7•15 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
| Assignee | ||
Comment 8•15 years ago
|
||
This made its way into TM successfully, then got ported to m-c in the changeset in comment 7.
Still needs backporting to 192 and 191... :-\
status1.9.1:
--- → ?
status1.9.2:
--- → ?
Updated•14 years ago
|
Updated•14 years ago
|
blocking1.9.1: ? → .17+
blocking1.9.2: ? → .14+
| Assignee | ||
Comment 9•14 years ago
|
||
| Assignee | ||
Comment 10•14 years ago
|
||
So, I tweaked the patch to apply to branches -- they're what I just uploaded. Porting was fairly easy, I think, although the code through there had changed somewhat (but not too much, if my transformation was correct -- see below).
I tried to rerun the original testcase and verify failure before, pass after. Neither patch fails before and passes after. I don't know how to explain this. And, because I've been awake since 5:20 EST and it's now 15:55 PST (18:55 EST) due to a BOS->SFO flight today (and I didn't get a full night's sleep last night), I am not at my most lucid right now, nor do I believe I could really do this full justice if I were to investigate. And being confident enough to push? It just ain't gonna happen today, at least not for me personally. But the freeze is today, so something needs to be done. I'll see if I can hunt anyone down who can help me out here.
| Assignee | ||
Comment 11•14 years ago
|
||
Okay, I had dmandelin look at this, and on *his* system the test was spewing the wrong number of warnings pre-patch and the right number post, so on that basis he thinks it's good. So I think this is good to go.
Attachment #504884 -
Attachment is obsolete: true
| Assignee | ||
Comment 12•14 years ago
|
||
...and the 192 patch.
Comment 13•14 years ago
|
||
| Assignee | ||
Comment 14•14 years ago
|
||
Tests landed in TM, since this is fixed in branches now:
http://hg.mozilla.org/tracemonkey/rev/2d196b5a4224
Flags: in-testsuite+
Updated•14 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•