Closed Bug 329751 Opened 14 years ago Closed 14 years ago

Coverity doesn't think obj_eval should null check caller->pc

Categories

(Core :: JavaScript Engine, defect, trivial)

PowerPC
macOS
defect
Not set
trivial

Tracking

()

RESOLVED FIXED

People

(Reporter: timeless, Assigned: timeless)

References

(Blocks 1 open bug, )

Details

(Keywords: coverity)

Attachments

(1 obsolete file)

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/js/src&command=DIFF_FRAMESET&file=jsobj.c&rev2=3.152&rev1=3.151
Changed the behavior of the code /slightly/ 

The old code checked caller->script before having fun.
I think that there's an assertion somewhere in spidermonkey that if you have a script on the stack and it's calling another script, then that calling script's pc has to be non 0. which means that coverity is really complaining that we're not trusting our own assertion.

If I'm wrong, then this code could theoretically crash in some really messed up scenario. I'd rather not be wrong :).
Attached patch replace check with assert (obsolete) — Splinter Review
Attachment #214412 - Flags: review?(mrbkap)
Comment on attachment 214412 [details] [diff] [review]
replace check with assert

I buy it -- thanks again.

Where are these Coverity results you're reading?

/be
Attachment #214412 - Flags: review?(mrbkap) → review+
the report that the government commisioned. i don't actually have my own account, so i'm borrowing someone else's. given the number of things i've already done w/ this report and how i was willing to use coverity before, i will get my own account.

http://scan.coverity.com:7454/
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 214412 [details] [diff] [review]
replace check with assert

mozilla/js/src/jsobj.c 	3.245
Attachment #214412 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.