Bug 250511

Summary:	JSValueGetType() is incorrect for BigInt values
Product:	WebKit	Reporter:	Kasper Isager Dalsgarð <kasperisager>
Component:	JavaScriptCore	Assignee:	Yijia Huang <yijia_huang>
Status:	RESOLVED FIXED
Severity:	Normal	CC:	d_degazio, mark.lam, ticaiolima, webkit-bug-importer, ysuzuki
Priority:	P2	Keywords:	InRadar
Version:	WebKit Local Build
Hardware:	Mac (Apple Silicon)
OS:	macOS 13
See Also:	https://bugs.webkit.org/show_bug.cgi?id=250719
Bug Depends on:	273705
Bug Blocks:

Description Kasper Isager Dalsgarð 2023-01-12 05:47:02 PST

JSValueGetType() currently has no path for values of type BigInt and therefore falls through to the object case, returning kJSTypeObject:

https://github.com/WebKit/WebKit/blob/8196614cb0623111b43d1041469eb3247e5623bb/Source/JavaScriptCore/API/JSValueRef.cpp#L51-L79

I imagine a kJSTypeBigInt value should be introduced and a path for jsValue.isBigInt() added.

Comment 1 Radar WebKit Bug Importer 2023-01-12 14:20:13 PST

<rdar://problem/104194532>

Comment 2 Mark Lam 2023-01-12 14:25:22 PST

Hmmm, JSBigInt directly extends JSCell, and therefore, not is not a JSObject.  We should fix this.

Comment 3 Kasper Isager Dalsgarð 2023-01-13 01:52:59 PST

On a related note, JSValueIsBigInt() would be great as well.

Comment 4 Kasper Isager Dalsgarð 2023-01-13 01:56:11 PST

The same goes for variants of JSValueMakeBigInt*() and JSValueToBigInt*(); those are sorely missed as well.

Comment 5 Mark Lam 2023-01-17 10:48:07 PST

Adding JSValueMakeBigInt*() and JSValueToBigInt*() is going to take a lot more time to make sure the API is correct.

Comment 6 Kasper Isager Dalsgarð 2023-01-17 10:59:35 PST

For converting BigInts to C primitives, there already seem to be private APIs available:

https://github.com/WebKit/WebKit/blob/4abc58df4642909ba616e53f76f7e69b18934b18/Source/JavaScriptCore/runtime/JSCJSValue.h#L312-L313

Comment 7 Mark Lam 2023-01-17 11:19:23 PST

(In reply to Kasper Isager Dalsgarð from comment #6)
> For converting BigInts to C primitives, there already seem to be private
> APIs available:
> 
> https://github.com/WebKit/WebKit/blob/
> 4abc58df4642909ba616e53f76f7e69b18934b18/Source/JavaScriptCore/runtime/
> JSCJSValue.h#L312-L313

The complication isn't with the primitives.  It's with getting the API correct e.g. for interactions with other API, for correctness, for security, for performance, etc.  Designing that and testing it will take time.

Comment 8 Kasper Isager Dalsgarð 2023-01-17 11:22:03 PST

Noted! It also wasn't directly relevant to the initial bug report, so I'll open separate tickets.

Comment 9 Kasper Isager Dalsgarð 2023-05-23 00:18:12 PDT

Is there any news on this?

Comment 10 Kasper Isager Dalsgarð 2023-09-03 03:36:38 PDT

Do let me know if there's anything I can do to help get this fixed. I'd be more than happy to provide a patch as well. Thanks!

Comment 11 Yijia Huang 2023-10-18 15:03:36 PDT

Pull request: https://github.com/WebKit/WebKit/pull/19251

Comment 12 Kasper Isager Dalsgarð 2023-10-18 22:14:36 PDT

That's fantastic, thanks so much! That also deals with most of https://bugs.webkit.org/show_bug.cgi?id=250719, with the exception of conversion back to numbers.

Comment 13 Yijia Huang 2024-01-23 11:29:44 PST

*** Bug 250719 has been marked as a duplicate of this bug. ***

Comment 14 EWS 2024-05-02 13:14:50 PDT

Committed 278275@main (c97a08d9e56f): <https://commits.webkit.org/278275@main>

Reviewed commits have been landed. Closing PR #19251 and removing active labels.

Comment 15 Yijia Huang 2024-05-03 12:18:20 PDT

Reopened Bugzilla.
Failed Xcode build, tracking revert in https://bugs.webkit.org/show_bug.cgi?id=273705.

Comment 16 Yijia Huang 2024-05-03 13:25:55 PDT

Pull request: https://github.com/WebKit/WebKit/pull/28121

Comment 17 EWS 2024-05-09 14:43:19 PDT

Committed 278588@main (e8e344010871): <https://commits.webkit.org/278588@main>

Reviewed commits have been landed. Closing PR #28121 and removing active labels.