Bug 217150 - [JSC] We should not tag C function with JIT code related ptr tag
Summary: [JSC] We should not tag C function with JIT code related ptr tag
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-09-30 16:04 PDT by Yusuke Suzuki
Modified: 2020-09-30 23:02 PDT (History)
7 users (show)

See Also:

Attachments
Patch (36.65 KB, patch)
2020-09-30 16:07 PDT, Yusuke Suzuki 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (36.65 KB, patch)
2020-09-30 16:14 PDT, Yusuke Suzuki 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (36.73 KB, patch)
2020-09-30 16:28 PDT, Yusuke Suzuki 		mark.lam: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (36.72 KB, patch)
2020-09-30 17:12 PDT, Yusuke Suzuki 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (37.68 KB, patch)
2020-09-30 17:26 PDT, Yusuke Suzuki 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (37.78 KB, patch)
2020-09-30 17:29 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>