Closed Bug 576716 Opened 10 years ago Closed 9 years ago

Crash [@ TypedArrayTemplate<int>::init] or [@ TypedArrayTemplate<int>::create]

Categories

(Core :: JavaScript Engine, defect, critical)

x86
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: jruderman, Assigned: vlad)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression, testcase, Whiteboard: [ccbr])

Crash Data

Attachments

(2 files, 2 obsolete files)

Int32Array(ArrayBuffer.prototype)


Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
Exception Codes: KERN_INVALID_ADDRESS at 0x0000000000000008
Crashed Thread:  0  Dispatch queue: com.apple.main-thread

Thread 0 Crashed:  Dispatch queue: com.apple.main-thread
0   TypedArrayTemplate<int>::init(JSContext*, JSObject*, int, int) + 372
1   TypedArrayTemplate<int>::create(JSContext*, JSObject*, unsigned int, long*, long*) + 704
2   TypedArrayTemplate<int>::class_constructor(JSContext*, JSObject*, unsigned int, long*, long*) + 144
3   js_Invoke + 2771
4   js_Interpret + 93623
5   js_Execute + 1664
6   JS_ExecuteScript + 59
7   Process(JSContext*, JSObject*, char*, int) + 1360 (js.cpp:515)
8   ProcessArgs(JSContext*, JSObject*, char**, int) + 2026 (js.cpp:837)
9   shell(JSContext*, int, char**, char**) + 223 (js.cpp:5025)
10  main + 220 (js.cpp:5114)
11  _start + 224
12  start + 33
Seems to be a null deref:

(gdb) x/i $eip
0x14cc9e <_ZN18TypedArrayTemplateIiE4initEP9JSContextP8JSObjectii+416>: mov    0x4(%eax),%eax
(gdb) x/b $eax
0x0:    Cannot access memory at address 0x0
Keywords: regression
Opt output from gdb:

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x00000004
0x000ffbe4 in TypedArrayTemplate<int>::create ()
(gdb) bt
#0  0x000ffbe4 in TypedArrayTemplate<int>::create ()
#1  0x000fff6d in TypedArrayTemplate<int>::class_constructor ()
#2  0x00066cac in Invoke<int (*)(JSContext*, JSObject*, unsigned int, long*, long*)> ()
#3  0x000676b8 in js_Invoke ()
#4  0x0005783e in js_Interpret ()
#5  0x000672f6 in js_Execute ()
#6  0x00013648 in JS_ExecuteScript ()
#7  0x00004ef6 in Process ()
#8  0x00008436 in shell ()
#9  0x00008947 in main ()
(gdb) x/i $eip
0xffbe4 <_ZN18TypedArrayTemplateIiE6createEP9JSContextP8JSObjectjPlS5_+3156>:   mov    0x4(%edx),%ecx
(gdb) x/b $edx
0x0:    Cannot access memory at address 0x0
blocking2.0: --- → ?
Summary: Crash [@ TypedArrayTemplate::init] → Crash [@ TypedArrayTemplate<int>::init] or [@ TypedArrayTemplate<int>::create]
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   36982:165a48c9ea89
user:        Vladimir Vukicevic
date:        Sat Jan 09 13:01:37 2010 -0800
summary:     b=532774; add native typed arrays to JavaScript; r=brendan,jorendorff
Blocks: 532774
Whiteboard: [ccbr]
Assignee: general → vladimir
blocking2.0: ? → betaN+
Attached patch fix (obsolete) — Splinter Review
Missing null check, this should fix it.
Attachment #465813 - Flags: review?
Attachment #465813 - Flags: review? → review?(jorendorff)
Comment on attachment 465813 [details] [diff] [review]
fix

jorendorff out, freeze looming -- Waldo has helped here before.

/be
Attachment #465813 - Flags: review?(jorendorff) → review?(jwalden+bmo)
Comment on attachment 465813 [details] [diff] [review]
fix

>diff --git a/js/src/jstypedarray.cpp b/js/src/jstypedarray.cpp

>@@ -1031,6 +1031,13 @@ class TypedArrayTemplate
>         } else if (js_IsTypedArray(other)) {
>             TypedArray *tarray = TypedArray::fromJSObject(other);
> 
>+            if (!tarray) {
>+                // the arg isn't a real typedarray
>+                JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
>+                                     JSMSG_TYPED_ARRAY_BAD_ARGS);
>+                return false;
>+            }
>+
>             //printf ("SizeAndCount: %d %d\n", sizeof(NativeType), tarray->length);
> 
>             if (!createBufferWithSizeAndCount(cx, sizeof(NativeType), tarray->length))

Is this necessary?  js_IsTypedArray means it's a fast array.  If it's fast, doesn't that mean it has a TypedArray* private?  None of the non-buffer testcases fail.  The slow arrays fall through to the else-bad-args case.  (Maybe they shouldn't, in which case the null-check here would make sense, but that's not what we have now.)

A full audit of callers (I hope/think exhaustive, although I might have missed something) reveals another place that needs to be fixed:

http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentUtils.cpp#5790

This testcase (must be accessed online due to vagaries of worker URL-loading requirements) demonstrates the problem:

http://web.mit.edu/jwalden/www/worker.html [CRASHES BROWSER]

r- until the necessity question is answered and the other case is fixed, with a testcase in the bug.
Attachment #465813 - Flags: review?(jwalden+bmo) → review-
Attached patch fix, v2Splinter Review
Ok, updated; the first part I added as a precaution, but it should be an assert.  I fixed the content case, but I'm cc'ing bent to figure out a test case for it.  I'm not sure where the tests are for this stuff in works.
Attachment #465813 - Attachment is obsolete: true
Attachment #465947 - Flags: review?
Ben, can you turn Waldo's test into a testcase?  Not sure where the arraybuffer workers tests are.
Attachment #465947 - Flags: review? → review?(jwalden+bmo)
Comment on attachment 465947 [details] [diff] [review]
fix, v2

Assert rather than check sounds good to me.  I'm not worried about having a test immediately, so long as one does happen shortly.
Attachment #465947 - Flags: review?(jwalden+bmo) → review+
http://hg.mozilla.org/mozilla-central/rev/7b9c03096aa1
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Attachment #466379 - Flags: review?(vladimir) → review+
Backed out because of mochitest-3 orange:

http://hg.mozilla.org/mozilla-central/rev/7f27cd7d8e59
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
uhh.. I think you have the wrong bug?
(nvm, it went in as a ridealong and got taken out as a ridealong -- but this bug should stay closed since the main issue is fixed)
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
(In reply to comment #15)
> (nvm, it went in as a ridealong and got taken out as a ridealong -- but this
> bug should stay closed since the main issue is fixed)

Even though the patch is no longer on mozilla-central?
(In reply to comment #16)
> Even though the patch is no longer on mozilla-central?

Only the test got backed out.
Now with hg header.
Attachment #466379 - Attachment is obsolete: true
Attachment #467520 - Flags: review+
Attachment #467520 - Attachment description: Test for Workers → Test for Workers [checked in]
Crash Signature: [@ TypedArrayTemplate<int>::init] [@ TypedArrayTemplate<int>::create]
You need to log in before you can comment on or make changes to this bug.