276061 - bogus js_ValueToInt32 calls in jsexn.c, plus recent regressions

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Brendan Eich [:brendan]]

The js_ValueToInt32 calls were old, but cloning one into the fix for bug 243869
made the test URL worse.  Fixing these to use js_ValueToECMAUint32 reveals other
problems:

1. bytes is reassigned badly in the new code in js_ReportUncaughtException,
clobbering the original bytes value that comes from converting the exception to
a string.

2. The recovery of message, filename, and lineno from exnObject in that function
should be predicated on the exnObject being an instance of *Error.  If it's just
a plain object (throw {}), there's no point groveling around in it for
properties that weren't set, and losing the useful "uncaught exception" error.

/be

Comment 1

[Brendan Eich [:brendan]]

Attachment: proposed fix

More work required to fix bug 119719, but that will happen in a patch in that
bug.

This fix does not regress bug 243869.

/be

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

hm...
        report.lineno = (lineno >= 0) ? (uintN) lineno : 0;

gcc tells me:
/home/chb/mozilla/js/src/jsexn.c: In function `js_ReportUncaughtException':
/home/chb/mozilla/js/src/jsexn.c:1097: warning: comparison of unsigned
expression >= 0 is always true

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

cool.
js> throw new Error("foo")
:5: foo
(presumably this is as intended. I'd have expected something like <stdin> as
filename, guess that's not this bug though. hmm, in xpcshell I get:
js> throw new Error("foo")
typein:1: foo
)

anyway, this does seem to fix the problem reported in
<news://news.mozilla.org/cqkq2q$[e-mail]>, both in xpcshell and
jsshell. thanks.

Comment 4

[Brendan Eich [:brendan]]

Attachment: fix js.c to use "typein" as the stdin filename, abstract lack of UTF-8 a bit

The right thing for jsexn.c, not done when errors-as-exceptions and the
filename and lineNumber extensions to ECMA-262 Edition 3 were first
implemented, is to have a callback for UTF-16 to UTF-8, and vice versa, that
embeddings can configure to share their converters.

I would like to deal with that in a separate bug.

/be

Comment 5

[Brendan Eich [:brendan]]

Oh, and SpiderMonkey is used all over the place, has been for 6+ years, so we
should be careful to avoid requiring that a script's filename be UTF-8.  It's a
C string, so it could use a locale-specific (multi-byte, e.g.) character set
form.  Definitely want callbacks for transcoding, with generic names.

/be

Comment 6

[Mike Shaver (:shaver emeritus)]

Comment on attachment 169644 [details] [diff] [review]
fix js.c to use "typein" as the stdin filename, abstract lack of UTF-8 a bit

Yeah, wow, what a mess this code is.  r=shaver

Comment 7

[Asa Dotzler [:asa]]

Comment on attachment 169644 [details] [diff] [review]
fix js.c to use "typein" as the stdin filename, abstract lack of UTF-8 a bit

a=asa for checkin to 1.8a6.

Comment 8

[Brendan Eich [:brendan]]

Fixed.  I'll fix the remaining problems over in bug 232182.

/be