Bug 181683

Summary: PAL should link to JavaScriptCore rather than WTF
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: PlatformAssignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, bugs-noreply, don.olmstead, mcatanzaro, mmaxfield, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch
none
Patch annulen: review+

Michael Catanzaro
Reported 2018-01-16 08:03:36 PST
Currently we are accidentally linking WTF into both libjavascriptcoregtk and libwebkit2gtk, which could in theory cause problems similar to bug #179914. There are only two possible solutions: * Ensure WTF always built as a shared library if JavaScriptCore is built as a shared library * Change PAL to link to JavaScriptCore instead of WTF The problem here is caused by layer hopping. It can never be a problem if each library only links to the next lowest-layered library. The downside is this allows PAL to use JavaScriptCore symbols, which is not super desirable. I came up with a hack yesterday for PAL to link to WTF usually, but link to JavaScriptCore instead if ${JavaScriptCore_LIBRARY_TYPE} MATCHES "SHARED" AND ${WTF_LIBRARY_TYPE} MATCHES "STATIC". But that is getting too complex and introduces more possibility for platform-dependent build failures. Better to just accept that PAL will depend on JavaScriptCore. Note: I'm not brave enough to update the XCode build. We can use that to enforce that no JavaScriptCore symbols actually get used, I guess.
Attachments
Patch (2.21 KB, patch)
2018-01-16 09:29 PST, Michael Catanzaro
no flags
Patch (3.27 KB, patch)
2018-01-16 10:06 PST, Michael Catanzaro
annulen: review+
Michael Catanzaro
Comment 1 2018-01-16 08:04:38 PST
(In reply to Michael Catanzaro from comment #0) > The problem here is caused by layer hopping. It can never be a problem if > each library only links to the next lowest-layered library. Note that, to my knowledge, PAL is the only lib we have that violates this rule.
Konstantin Tokarev
Comment 2 2018-01-16 08:19:00 PST
>Note that, to my knowledge, PAL is the only lib we have that violates this rule. Not at all. WebKit links to WebCore and JSC, and there are more examples in Tools
Michael Catanzaro
Comment 3 2018-01-16 08:20:59 PST
(In reply to Konstantin Tokarev from comment #2) > >Note that, to my knowledge, PAL is the only lib we have that violates this rule. > > Not at all. WebKit links to WebCore and JSC, and Ah yes, but it shouldn't need to do that. > there are more examples in Tools No doubt :)
Michael Catanzaro
Comment 4 2018-01-16 09:29:43 PST
Michael Catanzaro
Comment 5 2018-01-16 09:34:17 PST
Comment on attachment 331394 [details] Patch It's not right, I should change WebCore to not link directly to JSC now.
Michael Catanzaro
Comment 6 2018-01-16 10:06:55 PST
Konstantin Tokarev
Comment 7 2018-01-16 10:20:44 PST
Comment on attachment 331396 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=331396&action=review > Source/WebCore/PAL/pal/CMakeLists.txt:26 > + JavaScriptCore${DEBUG_SUFFIX} In the perfect world it should be PUBLIC if JSC is SHARED and PRIVATE otherwise. Or, it could be specified as PRIVATE here and ports who build JSC as shared can add it as INTERFACE in a separate command. But it's ok in this form for now too.
Michael Catanzaro
Comment 8 2018-01-16 11:41:12 PST
(In reply to Konstantin Tokarev from comment #7) > In the perfect world it should be PUBLIC if JSC is SHARED and PRIVATE > otherwise. This would be easiest... currently we make almost no use of library dependency visibility, though. Let's explore that in future patches.
Michael Catanzaro
Comment 9 2018-01-16 11:41:53 PST
Radar WebKit Bug Importer
Comment 10 2018-01-16 11:44:57 PST
Note You need to log in before you can comment on or make changes to this bug.