Closed Bug 679112 Opened 10 years ago Closed 9 years ago

crash: SpiderMonkey typed array APIs seem to lose type safety

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: ddahl, Assigned: sfink)

Details

(Whiteboard: js-triage-needed)

Attachments

(1 file, 2 obsolete files)

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."
Whiteboard: js-triage-needed
Nikhil, could you take this one? Cc'ing helpers.

/be
Split this bit that brendan wrote out from the crypto.gerRandomValues patch in bug 440046
Blocks: 440046
Attachment #553779 - Flags: review?(luke)
Attachment #553779 - Flags: review?(luke) → review+
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
Attached patch rebased against m-c (obsolete) — Splinter Review
Attachment #557855 - Flags: review?
Attachment #557855 - Flags: review? → review?(luke)
Why are the JSUint32 types in the header being turned into uint32 types in the definitions?
(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
(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"...
ah. nothing to see here. Looks like my bad typing skillz are the problem... sorry.
Comment on attachment 557855 [details] [diff] [review]
rebased against m-c

Ah.  Well, IIUC, r+ on this patch minus the s/JSUint32/uint32/ changes.
Attachment #557855 - Flags: review?(luke) → review+
Attachment #553779 - Attachment is obsolete: true
Keywords: checkin-needed
Assignee: general → ddahl
changed owner to brendan as I did not write this patch
Assignee: ddahl → brendan
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! :-)
Status: NEW → ASSIGNED
Keywords: checkin-needed
Updated patch's user and message
Attachment #557855 - Attachment is obsolete: true
Attachment #567290 - Flags: review+
Still a number of jsuint->uint changes left...
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));
No longer blocks: 440046
Not mine at this point, found it rotting on my list. Steve, please close if obsolete. Thanks,

/be
Assignee: brendan → sphink
Yep, all of those (or their replacements) now assert either directly or indirectly via asArrayBufferObject.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.