Last Comment Bug 630939 - Constructor function name retrieved incorrectly
: Constructor function name retrieved incorrectly
Status: RESOLVED FIXED
[fixed-in-tracemonkey]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
: jsd
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-02 11:20 PST by Steve Fink [:sfink] [:s:]
Modified: 2011-07-08 00:24 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.x+


Attachments
Convert JSString to char* for constructor names (1.76 KB, patch)
2011-02-02 11:31 PST, Steve Fink [:sfink] [:s:]
timeless: review+
Details | Diff | Splinter Review

Description Steve Fink [:sfink] [:s:] 2011-02-02 11:20:58 PST
When JSD grabs the name of a constructor to hold onto, it uses incorrect types and interprets a JSString* as a char*. This would produce an invalid string, and a possible crash from reading invalid memory. I haven't observed any negative effects; I just noticed the compile warning.
Comment 1 Steve Fink [:sfink] [:s:] 2011-02-02 11:31:01 PST
Created attachment 509171 [details] [diff] [review]
Convert JSString to char* for constructor names
Comment 2 David Mandelin [:dmandelin] 2011-02-02 17:05:50 PST
Seems not to be a regression, so I think it doesn't block. We'll gladly approve it for landing though if it gets r+.
Comment 3 timeless 2011-02-04 05:21:04 PST
Comment on attachment 509171 [details] [diff] [review]
Convert JSString to char* for constructor names

sorry, this definitely feels like a regression from when someone changed the JS api to hand out Id's instead of Char*s.

please try to match file style until file style is cleaned up.

that means 

+                if ( (ctorName = JS_EncodeString(cx, ctorNameStr)) ) {

should be:

+                if( (ctorName = JS_EncodeString(cx, ctorNameStr)) ) {

Note that I do not happen to like this style, it's merely the style the file uses and as I've been noting elsewhere I have been unable to get it changed because I couldn't get reviews for anything.
Comment 4 Steve Fink [:sfink] [:s:] 2011-03-03 10:53:57 PST
http://hg.mozilla.org/tracemonkey/rev/04b5492cc109
Comment 5 Steve Fink [:sfink] [:s:] 2011-04-28 14:03:16 PDT
http://hg.mozilla.org/mozilla-central/rev/04b5492cc109

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