687768 - Crash [@ JSString::length]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following test crashes on mozilla-central revision 06445f55f009 (options -m -n -a), tested on 32 bit:


expected = '1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,';
function slice(a, b) {
  return expected;
}
function f() {
  var length = 20;
  var index = 0;
  function get3() {
    if (length - index < 3)
      return null;
    return slice(index, ++index);
  }
  var bytes = null;
  while (bytes = get3()) {  }
}
f();


GDB Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x0805a2d8 in JSString::length (this=0x0) at ../../vm/String.h:288
288             return d.lengthAndFlags >> LENGTH_SHIFT;
(gdb) bt
#0  0x0805a2d8 in JSString::length (this=0x0) at ../../vm/String.h:288
#1  0x080a8e29 in js_ValueToBoolean (v=...) at /srv/repos/mozilla-central/js/src/jsbool.cpp:182
#2  0x083fcf4a in js::mjit::stubs::ValueToBoolean (f=...) at /srv/repos/mozilla-central/js/src/methodjit/StubCalls.cpp:924
#3  0xf73f04af in ?? ()
Backtrace stopped: previous frame inner to this frame (corrupt stack?)


Filing s-s because of the corrupt stack warning. Not sure if this is a regular null-deref, or some form of corruption.

Comment 1

[Jeff Walden [:Waldo]]

I don't crash on latest mozilla-inbound with a 64-bit shell with this, for what it's worth.

Comment 2

[David Mandelin [:dmandelin]]

(In reply to Christian Holler (:decoder) from comment #0)
> #3  0xf73f04af in ?? ()
> Backtrace stopped: previous frame inner to this frame (corrupt stack?)
> 
> 
> Filing s-s because of the corrupt stack warning. Not sure if this is a
> regular null-deref, or some form of corruption.

TI uses ebp as a general purpose register, which prevents gdb from walking the stack normally. So this is probably not corruption. I'd like Brian to confirm that, though.

Comment 3

[Daniel Veditz [:dveditz]]

Brian: if this is in fact just a null pointer deref and not corruption please remove the sg:critical status whiteboard marking and clear the sensitive flag.

Comment 4

[Brian Hackett [Laid off!]]

Attachment: patch

TI FrameState bug, only affects Firefox 9.  The stack is fine, gdb just can't unwind it properly.  I can't remove the s-s.

Before branching, the frame state discards entries for locals which are dead, pretending they are synced.  It would do this even if there were copies of those entries, and subsequent uses of that copy could try to use the invalid memory for the local (e.g. if registers for the local were evicted before branching).  In this case a ValueToBoolean being passed null would write that payload with a string type tag read from the invalid slot.

Comment 5

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b412c0760572

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/b412c0760572

Comment 8

[u279076]

qa- as this is not verifiable unless you have a debug build. If someone has a debug build wants to verify this fix, please do so.

Comment 9

[Christian Holler (:decoder)]

A testcase for this bug was automatically identified at js/src/jit-test/tests/jaeger/bug687768.js.