Closed Bug 701239 Opened 8 years ago Closed 8 years ago

"Assertion failure: strcmp(lval + strlen(lval) - 9, " = <next>") == 0," or "Assertion failure: top != 0,"

Categories

(Core :: JavaScript Engine, defect, critical)

x86
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla11
Tracking Status
firefox9 --- unaffected
firefox10 - verified
firefox11 --- verified
firefox12 --- verified
firefox-esr10 --- unaffected
status1.9.2 --- unaffected

People

(Reporter: gkw, Assigned: luke)

References

(Blocks 2 open bugs)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [sg:critical]js-triage-done [qa!])

Attachments

(2 files)

Attached file stacks
uneval(
  Function("\
    function y(x,w) {\
      for([w]in 1) {}\
    } (6)\
  ")
)

asserts js debug shell on m-c changeset 4fb61ebbf8ff with patch v1 from bug 697279 without any CLI arguments at Assertion failure: strcmp(lval + strlen(lval) - 9, " = <next>") == 0,

A variant,

uneval(
    Function("\
        function rzewfy(x,w) {\
            for(var [,w] = false in 2097151) {\
                \"\\u6F9F16\";\
            }\
        }\
        print(\u3056);\
    ")
)

asserts at Assertion failure: top != 0, instead
Attached patch fixSplinter Review
It turns out the code isn't actually dead (except in the test suite of course...).  The patch reverts the code removal (here is the original removal: http://hg.mozilla.org/mozilla-central/rev/a6c390aa3bf2#l1.56).
Assignee: general → luke
Status: NEW → ASSIGNED
Attachment #573410 - Flags: review?(jwalden+bmo)
Whiteboard: js-triage-needed → js-triage-done
Comment on attachment 573410 [details] [diff] [review]
fix

Partially reverts original patch that accidentally landed right before branch, so I think low risk.  Fixes safe crash (although unlikely due to non-standard JS).
Attachment #573410 - Flags: approval-mozilla-aurora?
Attachment #573410 - Flags: review?(jwalden+bmo) → review+
https://hg.mozilla.org/mozilla-central/rev/525552b0a840
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
The second testcase in comment 0 crashes on aurora opt (64 bit), tested revision dcfa8d61f3c5. LangFuzz found a similar test that crashed only on aurora and a bisect for the fix led me to this bug.

Backtrace for the test in comment 0:

==19866== Invalid read of size 1
==19866==    at 0x4A9BBE: Decompile(SprintStack*, unsigned char*, int) (jsopcode.cpp:2731)
==19866==    by 0x4AFC65: DecompileCode(JSPrinter*, JSScript*, unsigned char*, unsigned int, unsigned int) (jsopcode.cpp:4818)
==19866==    by 0x4B17EE: js_DecompileFunction (jsopcode.cpp:5010)
==19866==    by 0x4ABC0A: Decompile(SprintStack*, unsigned char*, int) (jsopcode.cpp:2427)
==19866==    by 0x4AFC65: DecompileCode(JSPrinter*, JSScript*, unsigned char*, unsigned int, unsigned int) (jsopcode.cpp:4818)
==19866==    by 0x4B17EE: js_DecompileFunction (jsopcode.cpp:5010)
==19866==    by 0x4A90E9: js_DecompileToString (jsopcode.cpp:4876)
==19866==    by 0x417EC7: JS_DecompileFunction (jsapi.cpp:5037)
==19866==    by 0x4533D5: fun_toStringHelper(JSContext*, JSObject*, unsigned int) (jsfun.cpp:1670)
==19866==    by 0x453600: fun_toSource(JSContext*, unsigned int, JS::Value*) (jsfun.cpp:1721)
==19866==    by 0x47F437: js::InvokeKernel(JSContext*, js::CallArgs, js::MaybeConstruct) (jscntxtinlines.h:297)
==19866==    by 0x47FB9A: js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::Value*) (jsinterp.h:148)
==19866==  Address 0x105e111cf is not stack'd, malloc'd or (recently) free'd


The LangFuzz test backtrace looks similar.

Luke, is this crash also save? And are you aware this is on aurora?
Group: core-security
Depends on: 676763
(In reply to Christian Holler (:decoder) from comment #5)
> Luke, is this crash also save? And are you aware this is on aurora?

The patch already has aurora-approval requested, so yes :)  I wasn't thinking about release timing when I landed the patches.  It is safe, though, due to the LOCAL_ASSERT.
(In reply to Luke Wagner [:luke] from comment #6)
> The patch already has aurora-approval requested, so yes :)  I wasn't
> thinking about release timing when I landed the patches.  It is safe,
> though, due to the LOCAL_ASSERT.

Oh, I totally did not see the aurora-approval ^_^ all fine then.
Group: core-security
Attachment #573410 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Once more, the Sith will rule the galaxy...

This test finally crashes unsafe makes this s-g (aurora rev dcfa8d61f3c5, opt 64 bit, options -m -n):


AddRegExpCases( /\u0041/, "/\\u0041/",   "A", "A", 1, 0, ["A"] );
function AddRegExpCases(regexp, str_regexp, pattern, str_pattern, length, index, matches_array ) {
  let [ matches_array   ]  = ''   ? this : this;     
  for ( var matches = 0; matches < (AddRegExpCases); matches++ ) {}
}


Backtrace:

==26313==    at 0x4C29746: strlen (mc_replace_strmem.c:282)
==26313==    by 0x4B0A85: DecompileDestructuringLHS(SprintStack*, unsigned char*, unsigned char*, int*) (jsopcode.cpp:762)
==26313==    by 0x4B0FCD: DecompileDestructuring(SprintStack*, unsigned char*, unsigned char*) (jsopcode.cpp:1760)
==26313==    by 0x4A9C87: Decompile(SprintStack*, unsigned char*, int) (jsopcode.cpp:3544)
==26313==    by 0x4AFC65: DecompileCode(JSPrinter*, JSScript*, unsigned char*, unsigned int, unsigned int) (jsopcode.cpp:4818)
==26313==    by 0x4B17EE: js_DecompileFunction (jsopcode.cpp:5010)
==26313==    by 0x4A90E9: js_DecompileToString (jsopcode.cpp:4876)
==26313==    by 0x417EC7: JS_DecompileFunction (jsapi.cpp:5037)
==26313==  Address 0x9f38beb is not stack'd, malloc'd or (recently) free'd
Group: core-security
Blocks: 676763
No longer depends on: 676763
Whiteboard: js-triage-done → js-triage-done [qa+]
Does this patch apply to the 1.9.2 branch, or is it fixing a regression since then?
blocking1.9.2: --- → ?
status1.9.2: --- → ?
Whiteboard: js-triage-done [qa+] → [sg:critical]js-triage-done [qa+]
comment 1 is saying this is a regression from bug 696813 so it does not affect the 1.9.2 branch.
Blocks: 696813
blocking1.9.2: ? → ---
Keywords: regression
Verified on latest Fx10, 11, and 12 on Mac OS 10.7.x.

On builds prior to the fix, using the test cases in comment #0 the browser would crash and an assertion would appear in the shell. Using the latest Fx10, Fx11 and Fx12 debug and opt builds, I get no crash and instead of an assertion I get a Javascript strict warning.
Whiteboard: [sg:critical]js-triage-done [qa+] → [sg:critical]js-triage-done [qa!]
(In reply to juan becerra [:juanb] from comment #12)
> Verified on latest Fx10, 11, and 12 on Mac OS 10.7.x.

-> VERIFIED based on this.
Status: RESOLVED → VERIFIED
Group: core-security
A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/testBug701239.js.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.