Bug 217150

Summary:

[JSC] We should not tag C function with JIT code related ptr tag

Product:

WebKit

Reporter:

Yusuke Suzuki <ysuzuki>

Component:

New Bugs

Assignee:

Yusuke Suzuki <ysuzuki>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer

Priority:

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

Yusuke Suzuki

Reported 2020-09-30 16:04:39 PDT

[JSC] We should not tag C function with JIT code related ptr tag

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 attachment proposed patch, testcase, etc.

Yusuke Suzuki

Comment 1 2020-09-30 16:07:47 PDT

Created attachment 410162 [details] Patch

Yusuke Suzuki

Comment 2 2020-09-30 16:14:42 PDT

Created attachment 410165 [details] Patch

Yusuke Suzuki

Comment 3 2020-09-30 16:28:14 PDT

Created attachment 410170 [details] Patch

Mark Lam

Comment 4 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?

Yusuke Suzuki

Comment 5 2020-09-30 17:12:33 PDT

Created attachment 410174 [details] Patch

Yusuke Suzuki

Comment 6 2020-09-30 17:26:23 PDT

Created attachment 410180 [details] Patch

Yusuke Suzuki

Comment 7 2020-09-30 17:29:02 PDT

Created attachment 410181 [details] Patch

Yusuke Suzuki

Comment 8 2020-09-30 18:37:23 PDT

Comment on attachment 410170 [details] Patch Discussed with Mark at Slack and the latest patch resolved comments.