TM: (32-bit) Crash [@ JSString::hasFlag] or (32-bit) [@ js_ConcatStrings] or (64-bit) "Assertion failure: !JSVAL_IS_PRIMITIVE(v), at ../jsnum.cpp" or (64-bit) [@ js_ValueToNumber]

RESOLVED FIXED

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
8 years ago
5 years ago

People

(Reporter: gkw, Assigned: dvander)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
assertion, crash, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ccbr] fixed-in-tracemonkey, crash signature)

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
(Function("\
  for each(let x in [\
  true,\
  (1),\
  (1),\
  (1),\
  (1),\
  true,\
  true,\
  true,\
  (1),\
  true,\
  true,\
  (1),\
  true,\
  true,\
  (1),\
  (1),\
  true,\
  true,\
  true,\
  true\
  ]) { \
    ((function f(aaaaaa) {\
       return aaaaaa.length == 0 ? 0 : aaaaaa[0] + f(aaaaaa.slice(1))\
     })([\
      x,\
      Math.I,\
      '',\
      null,\
      Math.I,\
      null,\
      new String(),\
      new String()\
      ]))\
}"))()

crashes debug JS shell at JSString::hasFlag near null on TM tip with -j and crashes opt shell on TM tip with -j at js_ConcatStrings near null. (Both 32-bit shells)

For 64 bits on both Mac 10.6.2 and Ubuntu, it asserts debug at Assertion failure: !JSVAL_IS_PRIMITIVE(v), at ../jsnum.cpp:985 and crashes opt at js_ValueToNumber near null.

Security-sensitive due to memory address being on stack - I don't know if the patch that caused this has landed on m-c or not.

===

Opt shell stack:

Exception Type:  EXC_BAD_ACCESS (SIGBUS)
Exception Codes: KERN_PROTECTION_FAILURE at 0x000000000000000a
Crashed Thread:  0  Dispatch queue: com.apple.main-thread

Thread 0 Crashed:  Dispatch queue: com.apple.main-thread
0   js-opt-32-tm-darwin           	0x000ce052 js_ConcatStrings + 18
1   ???                           	0x001fb90b 0 + 2078987
2   js-opt-32-tm-darwin           	0x0010f79c js::ExecuteTree(JSContext*, js::TreeFragment*, unsigned int&, js::VMSideExit**) + 732
3   js-opt-32-tm-darwin           	0x0012b838 js::MonitorLoopEdge(JSContext*, unsigned int&, js::RecordReason) + 1048
4   js-opt-32-tm-darwin           	0x0005b1bf js_Interpret + 43039
5   js-opt-32-tm-darwin           	0x00060b91 js_Execute + 625
6   js-opt-32-tm-darwin           	0x0000e0fc JS_ExecuteScript + 60
7   js-opt-32-tm-darwin           	0x00004e58 Process(JSContext*, JSObject*, char*, int) + 1336
8   js-opt-32-tm-darwin           	0x00008f16 main + 1734
9   js-opt-32-tm-darwin           	0x000029ed _start + 208
10  js-opt-32-tm-darwin           	0x0000291c start + 40
(Reporter)

Comment 1

8 years ago
autoBisect shows this is probably related to bug 530900:

The first bad revision is:
changeset:   38563:12827fc411c1
user:        David Anderson
date:        Mon Mar 08 10:28:08 2010 -0800
summary:     Trace recursion when the return keyword is omitted (bug 530900, r=gal).
Blocks: 530900
Whiteboard: [ccbr]
Assignee: general → dvander
Created attachment 432021 [details] [diff] [review]
fix

Changing this code in the linked bug was not necessary, and in fact wrong. It was trying to make a very tiny, irrelevant optimization - if we just popped a frame that had a JSOP_STOP instead of a JSOP_RETURN, just write back a constant. But by this point we've lost the return pc, and reading from the current pc just gets us a JSOP_TRACE that's after the JSOP_CALL.

So the check fails and it takes the wrong branch. This optimization is pointless so I've just reverted the code.
Attachment #432021 - Flags: review?(gal)

Comment 3

8 years ago
Comment on attachment 432021 [details] [diff] [review]
fix

offset = sp_adj - sizeof(double) maybe?
Attachment #432021 - Flags: review?(gal) → review+
bug 530900 was backed out on the branch so this issue is tracemonkey branch only.
Group: core-security
http://hg.mozilla.org/tracemonkey/rev/1f812d89de66
Whiteboard: [ccbr] → [ccbr] fixed-in-tracemonkey
(Reporter)

Updated

8 years ago
Depends on: 552196

Comment 6

8 years ago
http://hg.mozilla.org/mozilla-central/rev/1f812d89de66
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Crash Signature: [@ JSString::hasFlag] [@ js_ConcatStrings] [@ js_ValueToNumber]
A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/bug551705.js.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.