Last Comment Bug 754725 - Assertion failure: (ptrBits & 0x7) == 0, at jsval.h:760 or Crash [@ compartment]
: Assertion failure: (ptrBits & 0x7) == 0, at jsval.h:760 or Crash [@ compartment]
Status: VERIFIED FIXED
[js:p1:fx16][sg:critical][jsbugmon:ve...
: assertion, crash, regression, sec-critical, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86_64 Linux
: -- critical (vote)
: mozilla16
Assigned To: Bill McCloskey (:billm)
:
Mentors:
Depends on:
Blocks: langfuzz
  Show dependency treegraph
 
Reported: 2012-05-13 15:44 PDT by Christian Holler (:decoder)
Modified: 2013-03-11 07:22 PDT (History)
8 users (show)
choller: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
+
fixed
+
fixed
fixed
unaffected


Attachments
patch (2.26 KB, patch)
2012-06-13 17:06 PDT, Bill McCloskey (:billm)
bhackett1024: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Christian Holler (:decoder) 2012-05-13 15:44:54 PDT
The following test asserts/crashes on mozilla-central revision c758cc9b60e5 (options -m -n):


try {
function fun() {
    (new fun (("10"))) ('function pf' + (((true) )[ fun ]) + '() {}');
}
fun();
} catch(exc1) {}
function eval() { eval(  ) ; }
function DoWhile_3()
  eval();
try {
  DoWhile_3(  ) ;
} catch(e) { }
function f()
  test();
function test() {
  try {
    for (var i in f());
  } catch ( foopy  ) {  }  
  gc();
}
test();


Backtrace in Opt build:

==7571== Invalid read of size 8
==7571==    at 0x5781C1: void js::gc::MarkInternal<JSString>(JSTracer*, JSString**) (Heap.h:969)
==7571==    by 0x5799BD: js::gc::MarkValueInternal(JSTracer*, JS::Value*) (Marking.cpp:329)
==7571==    by 0x579BBE: js::gc::MarkValueRootRange(JSTracer*, unsigned long, JS::Value*, char const*) (Marking.cpp:369)
==7571==    by 0x5384B1: js::StackSpace::mark(JSTracer*) (Stack.cpp:527)
==7571==    by 0x4567DD: _ZN2jsL11MarkRuntimeEP8JSTracerb.clone.246 (jsgc.cpp:2325)
==7571==    by 0x456D37: BeginMarkPhase(JSRuntime*) (jsgc.cpp:2975)
==7571==    by 0x45A210: GCCycle(JSRuntime*, bool, long, js::JSGCInvocationKind) (jsgc.cpp:3271)
==7571==    by 0x45B38F: Collect(JSRuntime*, bool, long, js::JSGCInvocationKind, js::gcreason::Reason) (jsgc.cpp:3719)
==7571==    by 0x56C80C: GC(JSContext*, unsigned int, JS::Value*) (TestingFunctions.cpp:52)
==7571==    by 0x47E337: js::InvokeKernel(JSContext*, js::CallArgs, js::MaybeConstruct) (jscntxtinlines.h:426)
==7571==    by 0x47A234: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:2744)
==7571==    by 0x5F9DDD: UncachedInlineCall(js::VMFrame&, js::InitialFrameFlags, void**, bool*, unsigned int) (InvokeHelpers.cpp:376)
==7571==  Address 0xe3000 is not stack'd, malloc'd or (recently) free'd


Assuming s-s and sec-critical due to GC related crash. Furthermore, this crash affects mozilla-aurora and mozilla-beta as well.
Comment 1 Christian Holler (:decoder) 2012-05-14 04:50:32 PDT
Just testing some new JSBugMon functionality here.
Comment 2 Christian Holler (:decoder) 2012-05-14 05:00:48 PDT
Should probably learn my own syntax...
Comment 3 Christian Holler (:decoder) 2012-05-14 05:50:24 PDT
JSBugMon: This bug has been automatically confirmed to be still valid on branch mozilla-aurora  (reproduced on revision 56d137349efc).
JSBugMon: This bug has been automatically confirmed to be still valid on branch mozilla-beta  (reproduced on revision 0540130b5f2f).
JSBugMon: The testcase found in this bug does not reproduce on branch mozilla-release (tried revision 5bcfa0da3be9).
Comment 4 David Mandelin [:dmandelin] 2012-05-22 17:48:34 PDT
Moving this back a cycle--IGC takes priority.
Comment 5 Bill McCloskey (:billm) 2012-06-13 17:06:30 PDT
Created attachment 632965 [details] [diff] [review]
patch

In this test, we have a dead value on the VM stack. It contains garbage--in this case, the tag makes it look like a NullValue, but the low bits are nonzero so that isNull() is false. We do one GC and avoid marking the value because it's dead. However, we don't overwrite it with anything because it's not an object or a string. Later, we do a second GC and try to mark it. isGCThing() returns true because something with a NULL tag counts as a GC thing. But then when we convert it to a Cell and try to mark it, we crash.

The fix is to normalize the low bits of the value while still leaving the tag unaffected.
Comment 7 Ed Morley [:emorley] 2012-06-19 01:25:45 PDT
https://hg.mozilla.org/mozilla-central/rev/84af366e007f
Comment 8 Daniel Veditz [:dveditz] 2012-06-22 10:19:02 PDT
Am I interpreting comment 3 correctly that this is a regression in Firefox 13, and therefore doesn't affect the ESR branch? Although if we don't know the cause of the regression we might still need it if it was due to a security fix taken on that branch.

qawanted: please find when this bug crept in.
Comment 9 Bill McCloskey (:billm) 2012-06-22 11:04:53 PDT
Comment on attachment 632965 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 723313
User impact if declined: Low likelihood of GC crashes, exploits.
Testing completed (on m-c, etc.): On m-c.
Risk to taking this patch (and alternatives if risky): Low. Semantics should be unchanged except when the bug happens.
String or UUID changes made by this patch: None

Regarding ESR, there's no need to take this. It regressed in FF13.
Comment 10 Gary Kwong [:gkw] [:nth10sd] 2012-06-22 11:20:32 PDT
(removing keywords because bug 723313 has been identified as the regressor)
Comment 11 Lukas Blakk [:lsblakk] use ?needinfo 2012-06-22 15:31:16 PDT
Comment on attachment 632965 [details] [diff] [review]
patch

low risk, sec critical - approving.
Comment 13 Christian Holler (:decoder) 2012-06-28 14:06:45 PDT
JSBugMon: This bug has been automatically verified fixed.
Comment 14 Christian Holler (:decoder) 2013-03-11 07:22:35 PDT
Test is too slow to be added and I did not succeed in rewriting, marking in-testsuite-.

Note You need to log in before you can comment on or make changes to this bug.