Closed Bug 928029 Opened 6 years ago Closed 6 years ago

SelfHosting: check ThrowError argument count


(Core :: JavaScript Engine, defect)

Not set





(Reporter: pnkfelix, Assigned: pnkfelix)




(1 file)

SpiderMonkey does not handle omitted args gracefully in self-hosted calls like ThrowError(JSMSG_PAR_ARRAY_BAD_ARG); the latter needs to be written as ThrowError(JSMSG_PAR_ARRAY_BAD_ARG, "") or you end up with stuff like `strlen(NULL)` because some entries in `errorArgs` were left unfilled.

(Patch to catch this in DEBUG builds will be posted shortly.)
This fix is not as complete as I would like; e.g. it would be nice to
have direct info about which line in the selfhosted code had the
erroneous ThrowError call.

Also, hypothetically we could attempt to catch this problem while
loading the selfhosted code, rather than waiting for the ThrowError
invocation to take place.

(Still, seems like an worthy change and easy to land now.)
Comment on attachment 818620 [details] [diff] [review]
patch A v1: check arg count matches error-template

Review of attachment 818620 [details] [diff] [review]:

This definitely makes sense to land, yes. Can you open a bug about the improvements you mentioned and make it block bug 784288?

::: js/src/vm/SelfHosting.cpp
@@ +84,5 @@
> +    const JSErrorFormatString *efs =
> +        js_GetLocalizedErrorMessage(cx, NULL, NULL, errorNumber);
> +    JS_ASSERT(efs->argCount == args.length() - 1);
> +#endif
> + 

Nit: whitespace
Attachment #818620 - Flags: review?(till) → review+
Assignee: nobody → pnkfelix
Flags: in-testsuite-
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Depends on: 929280
(In reply to Till Schneidereit [:till] from comment #3)
> Can you open a bug about the
> improvements you mentioned and make it block bug 784288?

Filed as Bug 928943.

(I took care of that and forget to link it back here earlier; I've gotten spoiled by github auto-back-refs)
You need to log in before you can comment on or make changes to this bug.