Bug 217150

Summary: [JSC] We should not tag C function with JIT code related ptr tag
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
mark.lam: review+, ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch none

Description Yusuke Suzuki 2020-09-30 16:04:39 PDT
[JSC] We should not tag C function with JIT code related ptr tag
Comment 1 Yusuke Suzuki 2020-09-30 16:07:47 PDT
Created attachment 410162 [details]
Patch
Comment 2 Yusuke Suzuki 2020-09-30 16:14:42 PDT
Created attachment 410165 [details]
Patch
Comment 3 Yusuke Suzuki 2020-09-30 16:28:14 PDT
Created attachment 410170 [details]
Patch
Comment 4 Mark Lam 2020-09-30 16:47:56 PDT
Comment on attachment 410170 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=410170&action=review

r=me with fixes.

> Source/JavaScriptCore/jit/JITOperations.cpp:1227
> +            DisallowGC disallowGC;

Why disallowGC here?  I don't think we should be disallowing GC for the entire duration of the time we're executing the native function.  Please remove this.

> Source/JavaScriptCore/jit/JITOperations.cpp:1255
> +        DisallowGC disallowGC;

Ditto.  Please remove.

> Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:1673
> +            DisallowGC disallowGC;

Ditto.  Please remove.

> Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:1692
> +        DisallowGC disallowGC;

Ditto.  Please remove.

> Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:1963
> +    DisallowGC disallowGC;

Ditto.  Please remove.

> Source/JavaScriptCore/llint/LLIntThunks.cpp:190
>  #endif

nit: can you add `// ENABLE(JIT)` after this #endif to make it clearer what section it terminates?
Comment 5 Yusuke Suzuki 2020-09-30 17:12:33 PDT
Created attachment 410174 [details]
Patch
Comment 6 Yusuke Suzuki 2020-09-30 17:26:23 PDT
Created attachment 410180 [details]
Patch
Comment 7 Yusuke Suzuki 2020-09-30 17:29:02 PDT
Created attachment 410181 [details]
Patch
Comment 8 Yusuke Suzuki 2020-09-30 18:37:23 PDT
Comment on attachment 410170 [details]
Patch

Discussed with Mark at Slack and the latest patch resolved comments.
Comment 9 EWS 2020-09-30 22:15:19 PDT
Committed r267820: <https://trac.webkit.org/changeset/267820>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 410181 [details].
Comment 10 Radar WebKit Bug Importer 2020-09-30 22:16:21 PDT
<rdar://problem/69822573>