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]
: 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)
: Jason Orendorff [:jorendorff]
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: ---

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 User image 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 ]) + '() {}');
} catch(exc1) {}
function eval() { eval(  ) ; }
function DoWhile_3()
try {
  DoWhile_3(  ) ;
} catch(e) { }
function f()
function test() {
  try {
    for (var i in f());
  } catch ( foopy  ) {  }  

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 User image Christian Holler (:decoder) 2012-05-14 04:50:32 PDT
Just testing some new JSBugMon functionality here.
Comment 2 User image Christian Holler (:decoder) 2012-05-14 05:00:48 PDT
Should probably learn my own syntax...
Comment 3 User image 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 User image David Mandelin [:dmandelin] 2012-05-22 17:48:34 PDT
Moving this back a cycle--IGC takes priority.
Comment 5 User image Bill McCloskey (:billm) 2012-06-13 17:06:30 PDT
Created attachment 632965 [details] [diff] [review]

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 User image Ed Morley [:emorley] 2012-06-19 01:25:45 PDT
Comment 8 User image 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 User image Bill McCloskey (:billm) 2012-06-22 11:04:53 PDT
Comment on attachment 632965 [details] [diff] [review]

[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 User image Gary Kwong [:gkw] [:nth10sd] 2012-06-22 11:20:32 PDT
(removing keywords because bug 723313 has been identified as the regressor)
Comment 11 User image Lukas Blakk [:lsblakk] use ?needinfo 2012-06-22 15:31:16 PDT
Comment on attachment 632965 [details] [diff] [review]

low risk, sec critical - approving.
Comment 13 User image Christian Holler (:decoder) 2012-06-28 14:06:45 PDT
JSBugMon: This bug has been automatically verified fixed.
Comment 14 User image 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.