1441098 - Support serializing and deserializing BigInt

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P3)

Description

[Anne (:annevk)]

Standard change: https://github.com/whatwg/html/pull/3480

New tests: https://github.com/w3c/web-platform-tests/pull/9565

Comment 1

[Anne (:annevk)]

It'd be good to ship this together with BigInt.

Comment 2

[Robin Templeton [:terpri]]

Attachment: structured clone support for BigInt

WIP patch for BigInt serialization. This version serializes BigInt values as ASCII strings; the final version will use a more compact binary format.

Comment 3

[Robin Templeton [:terpri]]

Attachment: Part 1: Add BigInt support in BufferGrayRootsTracer.

This class is used for tracing DOM objects.

Comment 4

[Robin Templeton [:terpri]]

Attachment: Part 2: Define BigInt serialization methods.

writeBytes serializes a BigInt value as a little-endian byte sequence,
ignoring its sign. createFromBytes can be used to deserialize the output
of writeBytes.

Comment 5

[Robin Templeton [:terpri]]

Attachment: Part 3: Add structured clone support for BigInt.

New data types are defined for primitive BigInt values and BigInt
objects. BigInts are serialized starting with 32 bits of sign and length
information, followed by a little-endian byte sequence representing an
unsigned integer.

Comment 6

[Robin Templeton [:terpri]]

Attachment: Part 4: Enable BigInt wrapping from DOM bindings.

Comment 7

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8983288 [details] [diff] [review]
Part 1: Add BigInt support in BufferGrayRootsTracer.

Review of attachment 8983288 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/gc/RootMarking.cpp
@@ +528,2 @@
>      MOZ_ASSERT(thing->getTraceKind() <= JS::TraceKind::Null);
> +#endif

This is fine, but you could simplify this to something like

    MOZ_ASSERT(thing->getTraceKind() != JS::TraceKind(0xff));

to eliminate the #ifdef. We only need to make sure that this dereferences |thing|; it doesn't matter what it does.

Comment 8

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8983289 [details] [diff] [review]
Part 2: Define BigInt serialization methods.

Review of attachment 8983289 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/vm/BigIntType.cpp
@@ +113,5 @@
> +
> +    if (nbytes == 0)
> +        return x;
> +
> +    mpz_import(x->num_, nbytes, -1, 1, 0, 0, bytes);

This is too cryptic for my taste. Can you either do

    mpz_import(x->num_, nbytes, -1 /* order */, 1 /* size */, 0 /* endian */, 0 /* nails */, bytes);

or

    mpz_import(x->num_, nbytes,
               -1, // order
               1, // size
    .
    .
    .

or just prefix with the comment

    // mpz_import(rop, count, order, size, endian, nails, op)

Hm. The second one might be best, since you could then expand some of these cryptic parameters a little

    -1,  // order: least significant word first
    1,   // size: 1 byte per "word"
    ...

(I don't know the gmp API; I'm just parroting the doc I found.)

I'd also be fine with

    mpz_import(x->num_, nbytes
               , -1 // order: ...
               ,  1 // size: ...
    .
    .
    .

if you prefer that.

@@ +304,5 @@
> +BigInt::byteLength(BigInt* x)
> +{
> +    if (mpz_sgn(x->num_) == 0)
> +        return 0;
> +    return JS_ROUNDUP(mpz_sizeinbase(x->num_, 2), 8) / 8;

I think this multiplies by 8 then immediately divides by 8. I think this is equivalent to the more straightforward

  return JS_HOWMANY(mpz_sizeinbase(x->num_, 2), 8);

@@ +308,5 @@
> +    return JS_ROUNDUP(mpz_sizeinbase(x->num_, 2), 8) / 8;
> +}
> +
> +void
> +BigInt::writeBytes(BigInt* x, void* buffer)

This scares me -- an API that takes a pointer without a length. Can you either pass in a maxLength that you assert on, or alternatively accept a RangedPtr? (mfbt/RangedPtr.h) I haven't actually used RangedPtr yet, but it seems like maybe it's the right thing for something like this.

@@ +311,5 @@
> +void
> +BigInt::writeBytes(BigInt* x, void* buffer)
> +{
> +    size_t countp;
> +    mpz_export(buffer, &countp, -1, 1, 0, 0, x->num_);

The signature is close enough that you can just say something like

    // c.f. parameters in createFromBytes, above.

and if that bothers the pedant in you, I *want* the "compare to" meaning here, not the incorrectly assumed meaning of "see also", because it's mpz_export vs mpz_import. ;-) Perhaps plain English would be better.

Comment 9

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8983290 [details] [diff] [review]
Part 3: Add structured clone support for BigInt.

Review of attachment 8983290 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/vm/StructuredClone.cpp
@@ +108,5 @@
>  
>      SCTAG_SHARED_ARRAY_BUFFER_OBJECT,
>      SCTAG_SHARED_WASM_MEMORY_OBJECT,
>  
> +#ifdef ENABLE_BIGINT

No #ifdef here. We need these numbers to be backwards-compatible, so you need to reserve these two whether you use them or not. The next person will have to go after these.

@@ +1181,5 @@
> +{
> +    bool signBit = bi->sign() < 1;
> +    size_t length = BigInt::byteLength(bi);
> +    // The length must fit in 31 bits to leave room for a sign bit.
> +    if (length > INT32_MAX)

do you need length > size_t(INT32_MAX) to avoid a signedness comparison warning?

@@ +1185,5 @@
> +    if (length > INT32_MAX)
> +        return false;
> +    uint32_t lengthAndSign = length | (static_cast<uint32_t>(signBit) << 31);
> +
> +    mozilla::UniquePtr<uint8_t, JS::FreePolicy> buf(static_cast<uint8_t*>(js_malloc(length)));

Use JS::UniquePtr so you don't have to give the FreePolicy. Actually, use

    JS::UniquePtr<uint8_t[]> buf(static_cast<uint8_t*>(js_malloc(length));

You could even use JS::UniqueChars, but then you'd have a uint8_t vs char confusion.

@@ +1189,5 @@
> +    mozilla::UniquePtr<uint8_t, JS::FreePolicy> buf(static_cast<uint8_t*>(js_malloc(length)));
> +    if (!buf)
> +        return nullptr;
> +
> +    BigInt::writeBytes(bi, buf.get());

I think this would feel safer as writeBytes(bi, buf.get(), length) or writeBytes(bi, RangedPtr(buf.get(), length));

@@ +1190,5 @@
> +    if (!buf)
> +        return nullptr;
> +
> +    BigInt::writeBytes(bi, buf.get());
> +    if (!out.writePair(tag, static_cast<uint32_t>(lengthAndSign)))

lengthAndSign is already a uint32_t

@@ +1964,5 @@
> +
> +    if (nbytes == 0)
> +        return BigInt::create(context());
> +
> +    UniquePtr<uint8_t, JS::FreePolicy> buf(static_cast<uint8_t*>(js_malloc(nbytes)));

JS::UniquePtr<uint8_t[]>

@@ +2303,5 @@
> +        if (tag == SCTAG_BIGINT_OBJECT && !PrimitiveToObject(context(), vp))
> +            return false;
> +        break;
> +      }
> +#endif

You could choose to report a specific error here #ifndef ENABLE_BIGINT ("BigInt unsupported" perhaps) instead of falling through, but it doesn't matter too much.

Comment 10

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8983291 [details] [diff] [review]
Part 4: Enable BigInt wrapping from DOM bindings.

Review of attachment 8983291 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bindings/BindingUtils.h
@@ +978,5 @@
>        return MaybeWrapObjectValue(cx, rval);
>      }
> +#ifdef ENABLE_BIGINT
> +    if (rval.isBigInt())
> +      return JS_WrapValue(cx, rval);

Put curly brackets around the consequent in Gecko.

I don't know why strings and objects have the MaybeWrap* functions for avoiding JS_WrapValue (maybe just for performance?), but I guess that just underscores the fact that I shouldn't really be reviewing this file. Try peterv, bz, or smaug, I guess?

Comment 11

[Robin Templeton [:terpri]]

Attachment: Part 1: Add BigInt support in BufferGrayRootsTracer.

This class is used for tracing DOM objects.

Comment 12

[Robin Templeton [:terpri]]

Attachment: Part 2: Define BigInt serialization methods.

writeBytes serializes a BigInt value as a little-endian byte sequence,
ignoring its sign. createFromBytes can be used to deserialize the output
of writeBytes.

Comment 13

[Robin Templeton [:terpri]]

Attachment: Part 3: Add structured clone support for BigInt.

New data types are defined for primitive BigInt values and BigInt
objects. BigInts are serialized starting with 32 bits of sign and length
information, followed by a little-endian byte sequence representing an
unsigned integer.

Comment 14

[Robin Templeton [:terpri]]

(In reply to Steve Fink [:sfink] [:s:] (PTO June 31) from comment #8)
> Comment on attachment 8983289 [details] [diff] [review]
> Part 2: Define BigInt serialization methods.
> 
> Review of attachment 8983289 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/vm/BigIntType.cpp
> @@ +113,5 @@
> > +
> > +    if (nbytes == 0)
> > +        return x;
> > +
> > +    mpz_import(x->num_, nbytes, -1, 1, 0, 0, bytes);
> 
> This is too cryptic for my taste. Can you either do
> 
>     mpz_import(x->num_, nbytes, -1 /* order */, 1 /* size */, 0 /* endian
> */, 0 /* nails */, bytes);
> [...]

Fixed (and added a comment for mpz_export)

> @@ +304,5 @@
> > +BigInt::byteLength(BigInt* x)
> > +{
> > +    if (mpz_sgn(x->num_) == 0)
> > +        return 0;
> > +    return JS_ROUNDUP(mpz_sizeinbase(x->num_, 2), 8) / 8;
> 
> I think this multiplies by 8 then immediately divides by 8. I think this is
> equivalent to the more straightforward
> 
>   return JS_HOWMANY(mpz_sizeinbase(x->num_, 2), 8);

JS_HOWMANY looks like the right macro here. It should expand to (n+7)/8, which matches the required size from the GMP manual.

> @@ +308,5 @@
> > +    return JS_ROUNDUP(mpz_sizeinbase(x->num_, 2), 8) / 8;
> > +}
> > +
> > +void
> > +BigInt::writeBytes(BigInt* x, void* buffer)
> 
> This scares me -- an API that takes a pointer without a length. Can you
> either pass in a maxLength that you assert on, or alternatively accept a
> RangedPtr? (mfbt/RangedPtr.h) I haven't actually used RangedPtr yet, but it
> seems like maybe it's the right thing for something like this.

I changed this to use RangedPtr, using buffer+maxLength for the assertion (based on https://searchfox.org/mozilla-central/source/js/src/vm/JSAtom-inl.h#113 , but without the dereference because that appears to be undefined behavior for uninitialized non-character arrays)

Comment 15

[Robin Templeton [:terpri]]

(In reply to Steve Fink [:sfink] [:s:] (PTO June 31) from comment #9)
> Comment on attachment 8983290 [details] [diff] [review]
> Part 3: Add structured clone support for BigInt.
> 
> Review of attachment 8983290 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/vm/StructuredClone.cpp
> @@ +108,5 @@
> >  
> >      SCTAG_SHARED_ARRAY_BUFFER_OBJECT,
> >      SCTAG_SHARED_WASM_MEMORY_OBJECT,
> >  
> > +#ifdef ENABLE_BIGINT
> 
> No #ifdef here. We need these numbers to be backwards-compatible, so you
> need to reserve these two whether you use them or not. The next person will
> have to go after these.

Fixed

> @@ +1181,5 @@
> > +{
> > +    bool signBit = bi->sign() < 1;
> > +    size_t length = BigInt::byteLength(bi);
> > +    // The length must fit in 31 bits to leave room for a sign bit.
> > +    if (length > INT32_MAX)
> 
> do you need length > size_t(INT32_MAX) to avoid a signedness comparison
> warning?

dunno, but it's clearer with the cast either way :)

> @@ +1185,5 @@
> > +    if (length > INT32_MAX)
> > +        return false;
> > +    uint32_t lengthAndSign = length | (static_cast<uint32_t>(signBit) << 31);
> > +
> > +    mozilla::UniquePtr<uint8_t, JS::FreePolicy> buf(static_cast<uint8_t*>(js_malloc(length)));
> 
> Use JS::UniquePtr so you don't have to give the FreePolicy. Actually, use
> 
>     JS::UniquePtr<uint8_t[]> buf(static_cast<uint8_t*>(js_malloc(length));
> 
> You could even use JS::UniqueChars, but then you'd have a uint8_t vs char
> confusion.

Fixed (along with the other UniquePtr uses)

> @@ +1189,5 @@
> > +    mozilla::UniquePtr<uint8_t, JS::FreePolicy> buf(static_cast<uint8_t*>(js_malloc(length)));
> > +    if (!buf)
> > +        return nullptr;
> > +
> > +    BigInt::writeBytes(bi, buf.get());
> 
> I think this would feel safer as writeBytes(bi, buf.get(), length) or
> writeBytes(bi, RangedPtr(buf.get(), length));

This is now using the RangedPtr-based method from the previous patch.

> @@ +1190,5 @@
> > +    if (!buf)
> > +        return nullptr;
> > +
> > +    BigInt::writeBytes(bi, buf.get());
> > +    if (!out.writePair(tag, static_cast<uint32_t>(lengthAndSign)))
> 
> lengthAndSign is already a uint32_t
> 
> @@ +1964,5 @@
> > +
> > +    if (nbytes == 0)
> > +        return BigInt::create(context());
> > +
> > +    UniquePtr<uint8_t, JS::FreePolicy> buf(static_cast<uint8_t*>(js_malloc(nbytes)));
> 
> JS::UniquePtr<uint8_t[]>
> 
> @@ +2303,5 @@
> > +        if (tag == SCTAG_BIGINT_OBJECT && !PrimitiveToObject(context(), vp))
> > +            return false;
> > +        break;
> > +      }
> > +#endif
> 
> You could choose to report a specific error here #ifndef ENABLE_BIGINT
> ("BigInt unsupported" perhaps) instead of falling through, but it doesn't
> matter too much.

Changed this to use a better error message

Comment 16

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8984922 [details] [diff] [review]
Part 2: Define BigInt serialization methods.

Review of attachment 8984922 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/vm/BigIntType.cpp
@@ +320,5 @@
> +    // Check that the buffer being filled is large enough to hold the
> +    // integer we're writing. The result of the RangedPtr addition is
> +    // restricted to the buffer's range.
> +    size_t maxLength = byteLength(x);
> +    MOZ_ASSERT(buffer + maxLength);

Ugh, I wish RangedPtr had a cleaner-looking way of asserting bounds. Sorry for the trouble, but can you add an explanation to the assert to make it more obvious what's going on? eg

    MOZ_ASSERT(buffer + maxLength, "out of bounds access to buffer");

(It's still a little weird because you'll never actually see that message string; the asserts happen internally.)

But also... why "maxLength"? Shouldn't that be bigIntLength or reprSize or something like that?

Comment 17

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8984923 [details] [diff] [review]
Part 3: Add structured clone support for BigInt.

Review of attachment 8984923 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great! You can carry over my r+ on any patches that have updates from this last round of comments.

::: js/src/vm/StructuredClone.cpp
@@ +1961,5 @@
> +BigInt*
> +JSStructuredCloneReader::readBigInt(uint32_t data)
> +{
> +    size_t nbytes = data & JS_BITMASK(31);
> +    bool isNegative = data  & (1 << 31);

nit: extra space before &

Comment 18

[Robin Templeton [:terpri]]

(In reply to Steve Fink [:sfink] [:s:] (PTO June 31) from comment #16)
> Comment on attachment 8984922 [details] [diff] [review]
> Part 2: Define BigInt serialization methods.
> 
> Review of attachment 8984922 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/vm/BigIntType.cpp
> @@ +320,5 @@
> > +    // Check that the buffer being filled is large enough to hold the
> > +    // integer we're writing. The result of the RangedPtr addition is
> > +    // restricted to the buffer's range.
> > +    size_t maxLength = byteLength(x);
> > +    MOZ_ASSERT(buffer + maxLength);
> 
> Ugh, I wish RangedPtr had a cleaner-looking way of asserting bounds. Sorry
> for the trouble, but can you add an explanation to the assert to make it
> more obvious what's going on? eg
> 
>     MOZ_ASSERT(buffer + maxLength, "out of bounds access to buffer");
> 
> (It's still a little weird because you'll never actually see that message
> string; the asserts happen internally.)
> 
> But also... why "maxLength"? Shouldn't that be bigIntLength or reprSize or
> something like that?

Yeah, reprSize would make more sense here. Added an assert message too

Comment 19

[Robin Templeton [:terpri]]

Attachment: Part 2: Define BigInt serialization methods.

writeBytes serializes a BigInt value as a little-endian byte sequence,
ignoring its sign. createFromBytes can be used to deserialize the output
of writeBytes.

Comment 20

[Robin Templeton [:terpri]]

Attachment: Part 3: Add structured clone support for BigInt.

New data types are defined for primitive BigInt values and BigInt
objects. BigInts are serialized starting with 32 bits of sign and length
information, followed by a little-endian byte sequence representing an
unsigned integer.

Comment 21

[Robin Templeton [:terpri]]

Attachment: Part 4: Enable BigInt wrapping from DOM bindings.

This allows BigInts to be serialized in IndexedDB values and postMessage
messages. Previously, using BigInts in those contexts caused an
assertion failure in MaybeWrapValue.

Comment 22

[Robin Templeton [:terpri]]

I think JS_WrapValue can be called directly from MaybeWrapValue for BigInt values, without having a function to check for same-zone wrapping like the one used for strings. JSCompartment::wrap is a no-op for BigInts if the compartment and value have the same zone, although JS_WrapValue has some assertions and a call to ExposeValueToActiveJS that may not be necessary in that case.

Comment 23

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8985290 [details] [diff] [review]
Part 4: Enable BigInt wrapping from DOM bindings.

JS_WrapValue can always be called directly; that's not the issue.

The issue is that it's slow.  By the time you get to the "no-op" bit, you've already done 3 separate out-of-line function calls and some other work.

For strings, the performance of that was just not acceptable in the context of bindings.  For BigInt, maybe it's fine.  Certainly in the context of structured cloning, where there is tons of overhead of that sort already...

It's probably best to have a comment explaining why we're not bothering with the "check the zone" optimization here.

It's also entirely unclear to me why MaybeWrapValue is involved here at all.  What's the stack to the assertion that was getting hit?

Comment 24

[Robin Templeton [:terpri]]

For WPT's IndexedDB test (https://github.com/web-platform-tests/wpt/blob/master/html/infrastructure/safe-passing-of-structured-data/structured_clone_bigint.html), the stack starts with:

#0  0x00007fffdf386487 in mozilla::dom::MaybeWrapValue(JSContext*, JS::MutableHandle<JS::Value>) (cx=0x7fffbbe24000, rval=$JS::BigIntValue())
    at /home/robin/src/gecko/obj-x86_64-pc-linux-gnu/dist/include/mozilla/dom/BindingUtils.h:979
#1  0x00007fffe065d38d in mozilla::dom::MessageEventBinding::get_data(JSContext*, JS::Handle<JSObject*>, mozilla::dom::MessageEvent*, JSJitGetterCallArgs) (cx=0x7fffbbe24000, obj=
    (JSObject * const) 0x7fffb7c02190 [object MessageEvent], self=0x7fffda5e4ca0, args=$JS::BigIntValue()) at /home/robin/src/gecko/obj-x86_64-pc-linux-gnu/dom/bindings/MessageEventBinding.cpp:676
#2  0x00007fffe135b27f in mozilla::dom::binding_detail::GenericGetter<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) (cx=0x7fffbbe24000, argc=0, vp=0x7fffb54f5350) at /home/robin/src/gecko/dom/bindings/BindingUtils.cpp:3161
#3  0x00007fffe5534f14 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (cx=0x7fffbbe24000, native=0x7fffe135afb0 <mozilla::dom::binding_detail::GenericGetter<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*)>, args=...) at /home/robin/src/gecko/js/src/vm/JSContext-inl.h:274
#4  0x00007fffe5521a2f in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (cx=0x7fffbbe24000, args=..., construct=js::NO_CONSTRUCT) at /home/robin/src/gecko/js/src/vm/Interpreter.cpp:471
#5  0x00007fffe5522000 in InternalCall(JSContext*, js::AnyInvokeArgs const&) (cx=0x7fffbbe24000, args=...) at /home/robin/src/gecko/js/src/vm/Interpreter.cpp:520
#6  0x00007fffe5522076 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) (cx=0x7fffbbe24000, fval=$JS::Value((JSObject *) 0x7fffb7735800 [object Function "get data"]), thisv=$JS::Value((JSObject *) 0x7fffb7c02190 [object MessageEvent]), args=..., rval=$JS::UndefinedValue()) at /home/robin/src/gecko/js/src/vm/Interpreter.cpp:539
#7  0x00007fffe5522a6a in js::CallGetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) (cx=0x7fffbbe24000, thisv=$JS::Value((JSObject *) 0x7fffb7c02190 [object MessageEvent]), getter=$JS::Value((JSObject *) 0x7fffb7735800 [object Function "get data"]), rval=$JS::UndefinedValue()) at /home/robin/src/gecko/js/src/vm/Interpreter.cpp:654

The JS backtrace contains the function accessing the 'data' property. mozilla::dom::MessageEventBinding::get_data, which calls MaybeWrapValue, is autogenerated from WebIDL (https://searchfox.org/mozilla-central/source/dom/webidl/MessageEvent.webidl#17).

Comment 25

[Boris Zbarsky [:bzbarsky]]

Ah, the "any" is a BigInt.  Yes, that makes sense.

Comment 26

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8985290 [details] [diff] [review]
Part 4: Enable BigInt wrapping from DOM bindings.

r=me with a comment about how we can make the wrapping faster as needed and so forth.

Comment 27

[Robin Templeton [:terpri]]

Attachment: Part 4: Enable BigInt wrapping from DOM bindings.

Comment 28

[Robin Templeton [:terpri]]

Attachment: Part 4: Enable BigInt wrapping from DOM bindings.

Thanks for reviewing this, bz. Added a comment on the lack of optimized
wrapping:

// Wrap BigInt values without checking the zone first. At present,
// this is used primarily for structured cloning, so avoiding the
// overhead of unnecessary JS_WrapValue calls is less important than
// for strings and objects.

Comment 29

[Robin Templeton [:terpri]]

or rather:

// This could be optimized by checking the zone first, similar to
// the way strings are handled. At present, this is used primarily
// for structured cloning, so avoiding the overhead of JS_WrapValue
// calls is less important than for other types.

(copied the wrong version into my earlier comment)

Comment 30

[Dorel Luca [:dluca]]

I was unable to apply patch in order to land it. This was the error I was given:

applying bug-1441098---Part-1-Add-BigInt-support-in-BufferG.patch
patching file js/src/gc/RootMarking.cpp
Hunk #2 FAILED at 528
1 out of 2 hunks FAILED -- saving rejects to file js/src/gc/RootMarking.cpp.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh bug-1441098---Part-1-Add-BigInt-support-in-BufferG.patch

Comment 31

[Andreea Pavel [:apavel]]

:terpri i looked at the above error, the following changes need to be made, apparently:


-    MOZ_ASSERT(thing->getTraceKind() <= JS::TraceKind::Null);
+    MOZ_ASSERT(thing->getTraceKind() != JS::TraceKind(0xff));

Comment 32

[Robin Templeton [:terpri]]

Attachment: Part 1: Add BigInt support in BufferGrayRootsTracer.

This class is used for tracing DOM objects.

Comment 33

[Robin Templeton [:terpri]]

Attachment: Part 2: Define BigInt serialization methods.

writeBytes serializes a BigInt value as a little-endian byte sequence,
ignoring its sign. createFromBytes can be used to deserialize the output
of writeBytes.

Comment 34

[Robin Templeton [:terpri]]

Attachment: Part 3: Add structured clone support for BigInt.

New data types are defined for primitive BigInt values and BigInt
objects. BigInts are serialized starting with 32 bits of sign and length
information, followed by a little-endian byte sequence representing an
unsigned integer.

Comment 35

[Robin Templeton [:terpri]]

Attachment: Part 4: Enable BigInt wrapping from DOM bindings.

Comment 36

[Robin Templeton [:terpri]]

Rebased and re-uploaded from git (parts 1 and 2 were outdated wrt central). Build-only try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=af7d6bf37ec72d54a4edff5ef84d405d2a8c9dc7

Comment 37

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f87f53656ed5
Part 1: Add BigInt support in BufferGrayRootsTracer. r=sfink
https://hg.mozilla.org/integration/mozilla-inbound/rev/940e6648c1f2
Part 2: Define BigInt serialization methods. r=sfink
https://hg.mozilla.org/integration/mozilla-inbound/rev/84ae27690d6b
Part 3: Add structured clone support for BigInt. r=sfink
https://hg.mozilla.org/integration/mozilla-inbound/rev/a41bcbc841f9
Part 4: Enable BigInt wrapping from DOM bindings. r=bz

Comment 38

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/f87f53656ed5
https://hg.mozilla.org/mozilla-central/rev/940e6648c1f2
https://hg.mozilla.org/mozilla-central/rev/84ae27690d6b
https://hg.mozilla.org/mozilla-central/rev/a41bcbc841f9

Comment 39

[Robin Templeton [:terpri]]

Attachment: bug 1441098 - Update test262 exclusions for BigInt. r=anba

Disable BigInt variants of tests disabled due to bug 1317416.