Last Comment Bug 679112 - crash: SpiderMonkey typed array APIs seem to lose type safety
: crash: SpiderMonkey typed array APIs seem to lose type safety
Status: RESOLVED FIXED
js-triage-needed
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Steve Fink [:sfink] [:s:]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-15 13:33 PDT by David Dahl :ddahl
Modified: 2012-04-26 11:47 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v 0.1 Brendan's typedarray patch split from bug 440046 (2.42 KB, patch)
2011-08-17 08:21 PDT, David Dahl :ddahl
luke: review+
Details | Diff | Splinter Review
rebased against m-c (2.42 KB, patch)
2011-09-02 09:13 PDT, David Dahl :ddahl
luke: review+
Details | Diff | Splinter Review
For checkin, updated user and checkin comment (2.61 KB, patch)
2011-10-15 09:46 PDT, David Dahl :ddahl
bugzilla: review+
Details | Diff | Splinter Review

Description David Dahl :ddahl 2011-08-15 13:33:42 PDT
Discovered in bug 440046 - it seems this crash is happening in debug builds.

via Brendan: "The SpiderMonkey typed array APIs seem to lose type safety by indirecting via
JSObject*, and they don't assert correct class to make up for that at runtime
in debug builds."
Comment 1 Brendan Eich [:brendan] 2011-08-15 16:17:41 PDT
Nikhil, could you take this one? Cc'ing helpers.

/be
Comment 2 David Dahl :ddahl 2011-08-17 08:21:58 PDT
Created attachment 553779 [details] [diff] [review]
v 0.1 Brendan's typedarray patch split from bug 440046

Split this bit that brendan wrote out from the crypto.gerRandomValues patch in bug 440046
Comment 3 David Dahl :ddahl 2011-09-02 09:12:00 PDT
I should have asked for this to land on the js branch days ago...

in rebasing against the latest m-c i get this error:

In the directory  /home/ddahl/code/moz/obj-x86_64-unknown-linux-gnu-debug/js/src
The following command failed to execute properly:
c++ -o jstypedarray.o -c -I./../../dist/system_wrappers_js -include /home/ddahl/code/moz/mozilla-central/js/src/config/gcc_hidden.h -DOSTYPE="Linux2.6" -DOSARCH=Linux -DEXPORT_JS_API -DIMPL_MFBT -DJS_HAS_CTYPES -DDLL_PREFIX="lib" -DDLL_SUFFIX=".so" -Ictypes/libffi/include -I. -I/home/ddahl/code/moz/mozilla-central/js/src -I. -I./../../dist/include -I./../../dist/include/nsprpub -I/home/ddahl/code/moz/obj-x86_64-unknown-linux-gnu-debug/dist/include/nspr -I/home/ddahl/code/moz/mozilla-central/js/src -I/home/ddahl/code/moz/mozilla-central/js/src/assembler -I/home/ddahl/code/moz/mozilla-central/js/src/yarr -fPIC -fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-variadic-macros -Werror=return-type -pedantic -Wno-long-long -pthread -pipe -DDEBUG -D_DEBUG -DTRACING -g -DUSE_SYSTEM_MALLOC=1 -DENABLE_ASSEMBLER=1 -DENABLE_JIT=1 -DMOZILLA_CLIENT -include ./js-confdefs.h -MD -MF .deps/jstypedarray.pp /home/ddahl/code/moz/mozilla-central/js/src/jstypedarray.cpp
make[5]: *** [jstypedarray.o] Error 1
make[5]: *** Waiting for unfinished jobs....
make[4]: *** [libs_tier_js] Error 2
make[3]: *** [tier_js] Error 2
make[2]: *** [default] Error 2
make[1]: *** [realbuild] Error 2
make: *** [build] Error 2

I'll attach the rebased patch
Comment 4 David Dahl :ddahl 2011-09-02 09:13:06 PDT
Created attachment 557855 [details] [diff] [review]
rebased against m-c
Comment 5 Luke Wagner [:luke] 2011-09-02 10:03:24 PDT
Why are the JSUint32 types in the header being turned into uint32 types in the definitions?
Comment 6 David Dahl :ddahl 2011-09-02 10:09:51 PDT
(In reply to Luke Wagner [:luke] from comment #5)
> Why are the JSUint32 types in the header being turned into uint32 types in
> the definitions?

I just fixed what was in my .rej file:

http://pastebin.mozilla.org/1319624
Comment 7 David Dahl :ddahl 2011-09-02 11:32:48 PDT
(In reply to Luke Wagner [:luke] from comment #5)
> Why are the JSUint32 types in the header being turned into uint32 types in
> the definitions?

Actually, the header changes were mine, I was experimenting. I have reverted them and still this patch is failing with no apparent reason:

"The following command failed to execute properly:
c++ -o jstypedarray.o -c -I"...
Comment 8 David Dahl :ddahl 2011-09-02 11:39:26 PDT
ah. nothing to see here. Looks like my bad typing skillz are the problem... sorry.
Comment 9 Luke Wagner [:luke] 2011-09-02 12:16:15 PDT
Comment on attachment 557855 [details] [diff] [review]
rebased against m-c

Ah.  Well, IIUC, r+ on this patch minus the s/JSUint32/uint32/ changes.
Comment 10 David Dahl :ddahl 2011-10-14 06:14:54 PDT
changed owner to brendan as I did not write this patch
Comment 11 Ed Morley [:emorley] 2011-10-15 07:01:05 PDT
Unless I'm misunderstanding, the r+ was conditional on removing the s/JSUint32/uint32/ changes? Removing checkin-needed until that's resolved either way. Also please could a commit message + author be added.

Thanks! :-)
Comment 12 David Dahl :ddahl 2011-10-15 09:46:38 PDT
Created attachment 567290 [details] [diff] [review]
For checkin, updated user and checkin comment

Updated patch's user and message
Comment 13 :Ms2ger (⌚ UTC+1/+2) 2011-10-15 09:54:38 PDT
Still a number of jsuint->uint changes left...
Comment 14 David Dahl :ddahl 2011-10-17 13:28:33 PDT
After hg pull --update I see all of this patch is rotten and looks kind of like it may be unneeded? 

Does 

JS_ASSERT(atype >= 0 && atype < TypedArray::TYPE_MAX);

superceed

JS_ASSERT(js_IsTypedArray(obj));
Comment 15 Brendan Eich [:brendan] 2012-04-26 11:42:47 PDT
Not mine at this point, found it rotting on my list. Steve, please close if obsolete. Thanks,

/be
Comment 16 Steve Fink [:sfink] [:s:] 2012-04-26 11:47:57 PDT
Yep, all of those (or their replacements) now assert either directly or indirectly via asArrayBufferObject.

Note You need to log in before you can comment on or make changes to this bug.