471716 - jsobj.cpp DEBUG / [@ js_DumpObject] crash on JSFunction with null proto

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Andrew Paprocki]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.5) Gecko/2008120122 Firefox/3.0.5
Build Identifier: mozilla-central

When calling JS_DumpObject() on a JSFunction with a null proto, it crashes in fprintf() because proto is passed as an argument. Attaching a patch which simply checks for proto prior to printing and also fixes an "#if DEBUG" to be "#ifdef DEBUG" to be compatible with some other uses of -DDEBUG (and to be consistent with the rest of the file).

Reproducible: Always

Comment 1

[Andrew Paprocki]

Attachment: Proposed patch

Comment 2

[Nochum Sossonko [:Natch]]

Did you want to ask for review on the attached patch?

Comment 3

[Nochum Sossonko [:Natch]]

Comment on attachment 354975 [details] [diff] [review]
Proposed patch

You have to ask for a module owner/peer to review this.

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 354975 [details] [diff] [review]
Proposed patch

Overbrace multiline if/then/else parts. With that nit, r=me but I'd rather jorendorff deal with this (on vacation, js_DumpObject was his baby -- it's very helpful, btw).

/be

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

This isn't a trivial bug: things like this have hit me in the middle of a long, complex, debugging session and have cost me lots of lost time.

One nit in the description of the bug: the problem isn't passing NULL to a '%p' printf directive; the problem is that we also attempt to pass the name of the proto's class (via STOBJ_GET_CLASS(proto)->name) and attempting to get the class out of proto is what crashes.

Comment 6

[Andrew Paprocki]

I meant in general that proto was being used as an argument while null :) IIRC glibc printf handles null values, but I'm not sitting on top of glibc, so either one crashes for me.

Comment 7

[Andrew Paprocki]

Attachment: Update for Brendan's nit

Comment 8

[Jason Orendorff [:jorendorff]]

Comment on attachment 354981 [details] [diff] [review]
Update for Brendan's nit

Thanks!

Comment 9

[Brendan Eich [:brendan]]

NPOTB, important to avoid unexpected loss of debugging state.

/be

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/tracemonkey/rev/139ce7cb0fc1

Comment 11

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/139ce7cb0fc1

Comment 12

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/0a04b1a5f992