archives

« Bugzilla Issues Index

#1851 — 8.3.16.10: Invalid assertion in step 1


Follow-up of bug 1613.

The assertion was added to fix bug 1613, but it doesn't actually solve the underlying issue.

It is possible to obtain a reference to an uninitialised function object by calling Function[@@create] directly. On that uninitialised function object "prototype" or "constructor" properties can be added. If it is later initialised through the Function constructor, MakeConstructor is being called (15.3.1.1 step 19).

js> var f = Function[getSym("@@create")]()
js> Object.defineProperty(f, "prototype", {value: 0})
js> Function.call(f, "return 1")


The obvious solution to move the property assignments in MakeConstructor() to the top and use DefinePropertyOrThrow() instead isn't actually correct. This approach works for FunctionInitialise() (cf. define-or-throw for "length"), but MakeConstructor() is called after FunctionInitialise(). So if the DefinePropertyOrThrow() invocation in MakeConstructor() fails, but FunctionInitialise() has already being called, we end up with an initialised function object which ought to be a Constructor, but actually isn't.

What about adding an additional check in 15.3.1.1 before step 18 which tests for "prototype" and "constructor" not being present and F being extensible. And if that tests fails, a TypeError exception is thrown. That way the assertion in MakeConstructor is actually correct.


fixed in rev21 editor's draft

I ended up using DefinePropertyOrThrow in MakeConstructor

I'm ok with producing a supposed constructor that hasn't been initialized as long as an exception was thrown somewhere during the creation/initializaiton process. It seems fine if somebody tries to 'new' such a function and they get another exception because it wasn't proper initialized.

I did it this way because I'm more concerned (at least this morning) about initialization reentrancy issue and all the possible ways that could happen including for things that aren't yet specified. I think it is better to explicitly throw the exception at the first possible sign of such issues rather than depending upon assertions.


fixed in rev21 draft