1288457 - GetPrototypeFromConstructor should throw TypeError from GetFunctionRealm if proxy is revoked

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[André Bargull [:anba]]

Test case:
---
for (var {constructor:c, args} of [
  {constructor: Array, args: []},
  {constructor: Boolean, args: []},
  {constructor: DataView, args: [new ArrayBuffer(0)]},
  {constructor: Number, args: []},
  {constructor: String, args: []},
  {constructor: ArrayBuffer, args: [0]},
  {constructor: Int8Array, args: [new ArrayBuffer(0)]},
  {constructor: Int8Array, args: [[]]},
  {constructor: Int8Array, args: [new Int8Array(0)]},
  {constructor: RegExp, args: []},
  {constructor: Function, args: []},
  {constructor: function*(){}.constructor, args: []},
  {constructor: Error, args: []},
  {constructor: RangeError, args: []},
  {constructor: Map, args: []},
  {constructor: Set, args: []},
  {constructor: WeakMap, args: []},
  {constructor: WeakSet, args: []},
  {constructor: Promise, args: [function(){}]},
  {constructor: Intl.Collator, args: []},
  {constructor: Intl.DateTimeFormat, args: []},
  {constructor: Intl.NumberFormat, args: []},
  {constructor: Date, args: []},
  {constructor: Object, args: []},
  {constructor: function(){}, args: []},
]) {
    var rp = Proxy.revocable(function(){}, {
        get(t, pk, r) {
            if (pk === "prototype") {
                print("REVOKE");
                rp.revoke();
                return undefined;
            }
            return Reflect.get(t, pk, r);
        }
    });
    try {
        Reflect.construct(c, args, rp.proxy);
        print("no error:", c.name);
    } catch (e) {
        print("error:", c.name, e.name, e.message);
    }
}
---

Expected: Each constructor invocation prints "REVOKE" and a TypeError message.
Actual: "REVOKE" not printed for some constructors, all constructors don't throw a TypeError.

Spec:
9.1.14 GetPrototypeFromConstructor, step 4.a [https://tc39.github.io/ecma262/#sec-getprototypefromconstructor]
7.3.22 GetFunctionRealm, step 4.a [https://tc39.github.io/ecma262/#sec-getfunctionrealm]

Comment 1

[Jan de Mooij [:jandem]]

Bug 1529757 fixes this for most of the constructors but not all of them (GeneratorFunction, Collator, DateTimeFormat, NumberFormat). At least GeneratorFunction is a known issue: CreateDynamicFunction doesn't pass a sane JSProtoKey.

Comment 2

[André Bargull [:anba]]

Attachment: Bug 1288457 - Part 1: Change Intl.Collator to use ClassSpec. r=mgaudet!

• JSProto_Collator has to be added to the end until part 9.
• js::CreateCollatorPrototype was renamed to js::CreateCollator because it
  no longer returns the prototype object. Part 7 will change this again.
• CollatorObject::protoClass_ uses PlainObject::class_, which works because
  CollatorObject isn't xrayable.

Comment 3

[André Bargull [:anba]]

Attachment: Bug 1288457 - Part 2: Change Intl.DateTimeFormat to use ClassSpec. r=mgaudet!

Depends on D42870

Comment 4

[André Bargull [:anba]]

Attachment: Bug 1288457 - Part 3: Change Intl.Locale to use ClassSpec. r=mgaudet!

Depends on D42871

Comment 5

[André Bargull [:anba]]

Attachment: Bug 1288457 - Part 4: Change Intl.NumberFormat to use ClassSpec. r=mgaudet!

Depends on D42872

Comment 6

[André Bargull [:anba]]

Attachment: Bug 1288457 - Part 5: Change Intl.PluralRules to use ClassSpec. r=mgaudet!

Depends on D42873

Comment 7

[André Bargull [:anba]]

Attachment: Bug 1288457 - Part 6: Change Intl.RelativeTimeFormat to use ClassSpec. r=mgaudet!

Depends on D42874

Comment 8

[André Bargull [:anba]]

Attachment: Bug 1288457 - Part 7: Remove boilerplate to create Intl constructors. r=mgaudet!

• Remove the various js::CreateXYZ functions be directly iterating over the
  JSProtoKeys in js::InitIntlClass.
• Also remove the unused GlobalObject::getOrCreateIntlObject method and then
  merge GlobalObject::initIntlObject with js::InitIntlClass.

Depends on D42875

Comment 9

[André Bargull [:anba]]

Attachment: Bug 1288457 - Part 8: Use constexpr instead of #defines in js/Class. r=mgaudet!

Depends on D42876

Comment 10

[André Bargull [:anba]]

Attachment: Bug 1288457 - Part 9: Assign an additional bit for CACHED_PROTO_KEY. r=mgaudet!

Increase the CACHED_PROTO_KEY limit from 2⁶ to 2⁷ to allow that all current
JSProto classes can use JSCLASS_HAS_CACHED_PROTO.

Depends on D42877

Comment 11

[André Bargull [:anba]]

Attachment: Bug 1288457 - Part 10: Use cached proto mechanism for Intl classes. r=mgaudet!

This allows to replace NewObjectWithGivenProto with NewObjectWithClassProto, which
aligns Intl code with the rest of the non-Intl built-ins.

Depends on D42878

Comment 12

[André Bargull [:anba]]

Attachment: Bug 1288457 - Part 11: Replace Get{DateTime,Number}FormatConstructor with GetBuiltinConstructor. r=mgaudet!

Now that Intl.DateTimeFormat and Intl.NumberFormat both use JSProtoKey, we can
use the normal GetBuiltinConstructor self-hosting function to retrieve the
built-in constructor function.

Depends on D42880

Comment 13

[André Bargull [:anba]]

Attachment: Bug 1288457 - Part 12: Remove no longer used getOrCreate functions for Intl object from GlobalObject. r=mgaudet!

These methods are no longer needed after part 10 and part 11.

Depends on D42881

Comment 14

[André Bargull [:anba]]

Attachment: Bug 1288457 - Part 13: Add JSProtoKey for AsyncFunction. r=mgaudet!

In a follow-up bug this will be changed to use ClassSpec, along with the rest
of the JSProtoKey classes still using non-ClassSpec initialisation.

Depends on D42882

Comment 15

[André Bargull [:anba]]

Attachment: Bug 1288457 - Part 14: Add JSProtoKey for GeneratorFunction. r=mgaudet!

Depends on D42883

Comment 16

[André Bargull [:anba]]

Attachment: Bug 1288457 - Part 15: Add JSProtoKey for AsyncGeneratorFunction. r=mgaudet!

Depends on D42884

Comment 17

[André Bargull [:anba]]

Attachment: Bug 1288457 - Part 16: Decouple Async(FromSync)IteratorPrototype initialisation from AsyncGeneneratorFunction. r=mgaudet!

AsyncGenerator initialisation isn't required to also always initialise AsyncFromSyncIteratorPrototype,
so split AsyncFromSyncIteratorPrototype, and its prototype, AsyncFromIteratorPrototype, from the async
generator initialisation.

Depends on D42885

Comment 18

[André Bargull [:anba]]

Attachment: Bug 1288457 - Part 17: Remove unnecessary NativeObject downcasts in GlobalObject methods. r=mgaudet!

Many GlobalObject::getOrCreateXYZ() methods explicitly perform casts to NativeObject*
even though the callers directly assign to JSObject*. So remove all explicit casts to
NativeObject which aren't needed in any callers to reduce unnecessary code and to make
the code a bit leaner.

Depends on D42886

Comment 19

[André Bargull [:anba]]

Attachment: Bug 1288457 - Part 18: Remove unnecessary JSFunction downcasts in GlobalObject methods. r=mgaudet!

Similar to part 17, we can also remove unused casts to JSFunction*.

Depends on D42887

Comment 20

[André Bargull [:anba]]

Attachment: Bug 1288457 - Part 19: Add test case. r=mgaudet!

Depends on D42888

Comment 21

[André Bargull [:anba]]

Monthly review reminder ping. :-)

(I'd like to land this bug before bug 1433306, so I can directly use ClassSpec for Intl.ListFormat.)

Comment 22

[Matthew Gaudet (he/him) [:mgaudet]]

One question about the whole stack: So far I don't think there would be any performance impact, but do you expect any?

Comment 23

[André Bargull [:anba]]

(In reply to Matthew Gaudet (he/him) [:mgaudet] from comment #22)

One question about the whole stack: So far I don't think there would be any performance impact, but do you expect any?

No, I don't expect any performance changes due to these changes.

Comment 24

[André Bargull [:anba]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7587b464237e6f98aa1fd161d37ad90003446456

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cb587dae4080
Part 1: Change Intl.Collator to use ClassSpec. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/d8b0d1db18b8
Part 2: Change Intl.DateTimeFormat to use ClassSpec. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/97551047921f
Part 3: Change Intl.Locale to use ClassSpec. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/4ab734bad124
Part 4: Change Intl.NumberFormat to use ClassSpec. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/310181e1596b
Part 5: Change Intl.PluralRules to use ClassSpec. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/79af8c3fa894
Part 6: Change Intl.RelativeTimeFormat to use ClassSpec. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/fe1512265c07
Part 7: Remove boilerplate to create Intl constructors. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/e1a4db22bcb9
Part 8: Use constexpr instead of #defines in js/Class. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/13369226eaca
Part 9: Assign an additional bit for CACHED_PROTO_KEY. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/c743e9d10ae5
Part 10: Use cached proto mechanism for Intl classes. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/19625261f55a
Part 11: Replace Get{DateTime,Number}FormatConstructor with GetBuiltinConstructor. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/226c1017cdbd
Part 12: Remove no longer used getOrCreate functions for Intl object from GlobalObject. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/82e03558dc9f
Part 13: Add JSProtoKey for AsyncFunction. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/c13ce7a93ce9
Part 14: Add JSProtoKey for GeneratorFunction. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/8f4cb4f8495c
Part 15: Add JSProtoKey for AsyncGeneratorFunction. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/eafdeb4aaa02
Part 16: Decouple Async(FromSync)IteratorPrototype initialisation from AsyncGeneneratorFunction. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/d8176a4b1984
Part 17: Remove unnecessary NativeObject downcasts in GlobalObject methods. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/ee723359cd0a
Part 18: Remove unnecessary JSFunction downcasts in GlobalObject methods. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/e350706464ea
Part 19: Add test case. r=mgaudet

Comment 26

[Cosmin Sabou [:CosminS]]

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&fromchange=e350706464ea616e32b6308559f90cc744223b75&tochange=0762557d4a9974cf325329a3064a325eab663e12&selectedJob=271477761

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=271477761&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/0762557d4a9974cf325329a3064a325eab663e12

task 2019-10-16T10:27:12.177Z] /builds/worker/fetches/gcc/bin/g++ -o Unified_cpp_js_src16.o -c -I/builds/worker/workspace/obj-haz-shell/dist/system_wrappers -include /builds/worker/checkouts/gecko/config/gcc_hidden.h -DNDEBUG=1 -DTRIMMED=1 -DWASM_SUPPORTS_HUGE_MEMORY -DJS_CACHEIR_SPEW -DJS_STRUCTURED_SPEW -DJS_HAS_CTYPES -DFFI_BUILDING -DEXPORT_JS_API -DMOZ_HAS_MOZGLUE -I/builds/worker/checkouts/gecko/js/src -I/builds/worker/workspace/obj-haz-shell/js/src -I/builds/worker/workspace/obj-haz-shell/js/src/ctypes/libffi/include -I/builds/worker/checkouts/gecko/js/src/ctypes/libffi/src/x86 -I/builds/worker/workspace/obj-haz-shell/dist/include -I/builds/worker/workspace/obj-haz-shell/dist/include/nspr -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/obj-haz-shell/js/src/js-confdefs.h -Wno-attributes -Wno-ignored-attributes -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wc++1z-compat -Wduplicated-cond -Wimplicit-fallthrough -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=coverage-mismatch -Wno-error=free-nonheap-object -Wno-error=multistatement-macros -Wno-error=class-memaccess -Wformat -Wformat-overflow=2 -Wno-noexcept-type -fno-sized-deallocation -fno-aligned-new -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -freorder-blocks -O3 -fno-omit-frame-pointer -funwind-tables -Werror -fno-strict-aliasing -Werror=format -Wno-shadow -MD -MP -MF .deps/Unified_cpp_js_src16.o.pp Unified_cpp_js_src16.cpp
[task 2019-10-16T10:27:12.177Z] js/src/Unified_cpp_js_src17.o
[task 2019-10-16T10:27:12.610Z] In file included from /builds/worker/checkouts/gecko/js/src/builtin/intl/DateTimeFormat.h:12,
[task 2019-10-16T10:27:12.610Z] from /builds/worker/checkouts/gecko/js/src/vm/GlobalObject.cpp:18,
[task 2019-10-16T10:27:12.610Z] from Unified_cpp_js_src16.cpp:38:
[task 2019-10-16T10:27:12.610Z] /builds/worker/checkouts/gecko/js/src/builtin/intl/CommonFunctions.h:19:10: fatal error: unicode/utypes.h: No such file or directory
[task 2019-10-16T10:27:12.611Z] #include "unicode/utypes.h"
[task 2019-10-16T10:27:12.611Z] ^~~~~~~~~~~~~~~~~~
[task 2019-10-16T10:27:12.611Z] compilation terminated.
[task 2019-10-16T10:27:12.615Z] /builds/worker/checkouts/gecko/config/rules.mk:787: recipe for target 'Unified_cpp_js_src16.o' failed
[task 2019-10-16T10:27:12.615Z] make[3]: *** [Unified_cpp_js_src16.o] Error 1
[task 2019-10-16T10:27:12.615Z] make[3]: *** Waiting for unfinished jobs....
[task 2019-10-16T10:27:12.615Z] Running /builds/worker/workspace/obj-haz-shell/release/build/bitflags-507b701a00584e1d/build-script-build
[task 2019-10-16T10:27:12.638Z] [bitflags 1.2.0] cargo:rustc-cfg=bitflags_const_fn
[task 2019-10-16T10:27:12.638Z] Compiling thread_local v0.3.6
[task 2019-10-16T10:27:12.647Z] Running CARGO_PKG_HOMEPAGE= CARGO_PKG_VERSION_MINOR=3 CARGO_PKG_VERSION_PRE= CARGO_PKG_AUTHORS='Amanieu d'\''Antras <[e-mail]>' CARGO_PKG_VERSION=0.3.6 CARGO_PKG_NAME=thread_local CARGO_PKG_DESCRIPTION='Per-object thread-local storage' CARGO_PKG_VERSION_MAJOR=0 CARGO_PKG_REPOSITORY='https://github.com/Amanieu/thread_local-rs' CARGO_PKG_VERSION_PATCH=6 LD_LIBRARY_PATH='/builds/worker/workspace/obj-haz-shell/release/deps:/builds/worker/fetches/rustc/lib:/builds/worker/fetches/gcc/lib64' CARGO_MANIFEST_DIR=/builds/worker/checkouts/gecko/third_party/rust/thread_local CARGO=/builds/worker/fetches/rustc/bin/cargo /builds/worker/fetches/rustc/bin/rustc --crate-name thread_local /builds/worker/checkouts/gecko/third_party/rust/thread_local/src/lib.rs --color never --crate-type lib --emit=dep-info,metadata,link -C opt-level=2 -C codegen-units=1 -C metadata=b7201616b1d26234 -C extra-filename=-b7201616b1d26234 --out-dir /builds/worker/workspace/obj-haz-shell/release/deps -C linker=/builds/worker/checkouts/gecko/build/cargo-linker -L dependency=/builds/worker/workspace/obj-haz-shell/release/deps --extern lazy_static=/builds/worker/workspace/obj-haz-shell/release/deps/liblazy_static-3af0568294175eab.rlib --cap-lints warn
[task 2019-10-16T10:27:13.281Z] Running /builds/worker/workspace/obj-haz-shell/release/build/regex-a58973d801ddcee5/build-script-build
[task 2019-10-16T10:27:13.311Z] [regex 1.1.9] cargo:rustc-cfg=regex_runtime_teddy_ssse3

Comment 27

[André Bargull [:anba]]

Ugh, sorry about that. At least it should be easy to fix: When I wrote these patches initially, bug 1402379 wasn't yet applied, so the various js/src/intl/*.h files could be included unconditionally. But with bug 1402379 landed, they now need to be guarded with #ifdef ENABLE_INTL_API.

Comment 28

[André Bargull [:anba]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b475ab6fa59bca4f2119148897cf8fb6b74f98fd

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0e2bd4be3e42
Part 1: Change Intl.Collator to use ClassSpec. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/aebd85991cc4
Part 2: Change Intl.DateTimeFormat to use ClassSpec. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/89d7388251d1
Part 3: Change Intl.Locale to use ClassSpec. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/f464ebe632fd
Part 4: Change Intl.NumberFormat to use ClassSpec. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/6d262e7942be
Part 5: Change Intl.PluralRules to use ClassSpec. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/4c8e2c07fa63
Part 6: Change Intl.RelativeTimeFormat to use ClassSpec. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/e3e22cb993b3
Part 7: Remove boilerplate to create Intl constructors. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/673dac1119b6
Part 8: Use constexpr instead of #defines in js/Class. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/779a0df28f3d
Part 9: Assign an additional bit for CACHED_PROTO_KEY. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/8e5db7a22bc7
Part 10: Use cached proto mechanism for Intl classes. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/cf4491c7b279
Part 11: Replace Get{DateTime,Number}FormatConstructor with GetBuiltinConstructor. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/db8c74ffdbf0
Part 12: Remove no longer used getOrCreate functions for Intl object from GlobalObject. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/2b3fe913292a
Part 13: Add JSProtoKey for AsyncFunction. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/e6325a323279
Part 14: Add JSProtoKey for GeneratorFunction. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/000d2a8cc400
Part 15: Add JSProtoKey for AsyncGeneratorFunction. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/fb1e8f3ad7cc
Part 16: Decouple Async(FromSync)IteratorPrototype initialisation from AsyncGeneneratorFunction. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/819e1f01708f
Part 17: Remove unnecessary NativeObject downcasts in GlobalObject methods. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/0d841449ae0b
Part 18: Remove unnecessary JSFunction downcasts in GlobalObject methods. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/1501701f252e
Part 19: Add test case. r=mgaudet

Comment 30

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/0e2bd4be3e42
https://hg.mozilla.org/mozilla-central/rev/aebd85991cc4
https://hg.mozilla.org/mozilla-central/rev/89d7388251d1
https://hg.mozilla.org/mozilla-central/rev/f464ebe632fd
https://hg.mozilla.org/mozilla-central/rev/6d262e7942be
https://hg.mozilla.org/mozilla-central/rev/4c8e2c07fa63
https://hg.mozilla.org/mozilla-central/rev/e3e22cb993b3
https://hg.mozilla.org/mozilla-central/rev/673dac1119b6
https://hg.mozilla.org/mozilla-central/rev/779a0df28f3d
https://hg.mozilla.org/mozilla-central/rev/8e5db7a22bc7
https://hg.mozilla.org/mozilla-central/rev/cf4491c7b279
https://hg.mozilla.org/mozilla-central/rev/db8c74ffdbf0
https://hg.mozilla.org/mozilla-central/rev/2b3fe913292a
https://hg.mozilla.org/mozilla-central/rev/e6325a323279
https://hg.mozilla.org/mozilla-central/rev/000d2a8cc400
https://hg.mozilla.org/mozilla-central/rev/fb1e8f3ad7cc
https://hg.mozilla.org/mozilla-central/rev/819e1f01708f
https://hg.mozilla.org/mozilla-central/rev/0d841449ae0b
https://hg.mozilla.org/mozilla-central/rev/1501701f252e