2021 - Error reporting/exception handling bugs

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[rainer]

I found and fixed a few bugs in the error reporting/exception handling code of the JavaScript engine. Symptoms were uninitialized (garbage) fields in JSErrorReport structs, with malloc-related crashes as a consequence. I also fixed the currently commented out code in jsexn.c.

The missing initializations are in jscntxt.c (functions js_ReportErrorVA and js_ReportErrorNumberVA). Both functions allocate a JSErrorReport struct on the stack and then pass it's address to js_ReportErrorAgain. But not all struct fields are correctly initialized to 0, causing crashes later on, for code like

if (report->ucmessage)
	JS_free(report->ucmessage);

To fix it, put in a memset(&report, 0, sizeof report) at the beginning of each function.

In jsexn.c, function exn_initPrivate, the code should read:

#if 1 /* was 0, but works now */
    if (report->uclinebuf != NULL) {
	jsint len = js_strlen(report->uclinebuf) + 1;
	newReport->uclinebuf =
	    (const jschar *)JS_malloc(cx, len * sizeof(jschar));
	js_strncpy((jschar *)newReport->uclinebuf, report->uclinebuf, len);
	newReport->uctokenptr = newReport->uclinebuf + (report->uctokenptr -
							report->uclinebuf);
    } else
#endif
	newReport->uclinebuf = newReport->uctokenptr = NULL;

    /* Executing the code below makes a seg fault where JS_malloc fails!? */
#if 1 /* had several bugs, but works now */
    if (report->ucmessage != NULL) {
        jsint len = js_strlen(report->ucmessage) + 1;
        newReport->ucmessage =
	    (const jschar *)JS_malloc(cx, len * sizeof(jschar));
        js_strncpy((jschar *)newReport->ucmessage, report->ucmessage, len);

        if (report->messageArgs) {
            int i;

            for (i = 0; report->messageArgs[i] != NULL; i++)
                ;
            JS_ASSERT(i);
            newReport->messageArgs =
                (const jschar **)JS_malloc(cx, (i + 1) * sizeof(jschar *));
            for (i = 0; report->messageArgs[i] != NULL; i++) {
                len = js_strlen(report->messageArgs[i]) + 1;
                newReport->messageArgs[i] =
                    (const jschar *)JS_malloc(cx, len * sizeof(jschar));
                js_strncpy((jschar *)(newReport->messageArgs[i]),
                           report->messageArgs[i], len);
            }
            newReport->messageArgs[i] = NULL;
        } else {
            newReport->messageArgs = NULL;
        }
    } else {
        newReport->ucmessage = NULL;
        newReport->messageArgs = NULL;
    }
#else
    newReport->ucmessage = NULL;
    newReport->messageArgs = NULL;
#endif

    /* this didn't get copied correctly */
    newReport->errorNumber = report->errorNumber;

    /* Note that this is before it gets flagged with JSREPORT_EXCEPTION */
    newReport->flags = report->flags;

In function exn_destroyPrivate the code

if (report->ucmessage)
    JS_free(cx, (void *)report->ucmessage);

appears twice. Delete one. Further down there is code:

        while(*args++ != NULL)
            JS_free(cx, (void *)*args);

This leaks the first arg, because args has already been incremented before the JS_free. I replaced it with

	while (*args != NULL)
	    JS_free(cx, (void *)*args++);

That's all.

Another exception handling problem concerns LiveConnect: LiveConnect isn't exception-aware yet, meaning you can't catch Java exceptions from JavaScript. Are you going to implement that functionality any time soon, or should I have a go at it myself? (Just trying to avoid duplicated effort.)

Best regards,

Rainer Staringer
AAA+ Software

Comment 1

[Mike McCabe]

Thanks for the many excellent fixes.  I'll close the bug when I get them checked
in.

Some of the fixes are to functions in jsexn.c that are part of a mechanism for
reflecting runtime errors as exceptions.  This feature is currently turned off
through the JS_HAS_ERROR_EXCEPTIONS switch in jsconfig.h.  Reflecting javascript
errors as catchable exceptions is something that we want for the next release,
but the design wasn't completed by the last release, and it's not clear that
we'll go with them exactly as implemented in the code bracketed by
JS_HAS_ERROR_EXCEPTIONS.  (They're my creation, and I hope we do...)

Liveconnect (javascript to java connection) is finished, but the current work is
still in the SpiderMonkey140_BRANCH, and has not been committed to the tip.  It
includes a full mapping of Java exceptions to javascript-catchable values.

Comment 2

[Mike McCabe]

Fixed in the SpiderMonkeyDev_BRANCH.  This should get merged into the tip before
the next client release.

Thanks again for the great fixes.  Anything else you noticed?

Comment 3

[glynn]

qa contact set to [e-mail]

Comment 4

[leger]

Javacsript component begin retired.  Moving this bug to Javascript Engine.