| Summary: | [Cocoa] Don't soft link DataDetectorsCore | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Darin Adler <darin> | ||||||||||||
| Component: | Platform | Assignee: | Darin Adler <darin> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | ap, conrad_shultz, ddkilzer, mitz, mmaxfield, sam, thorton | ||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Darin Adler
2015-05-09 18:08:17 PDT
Created attachment 252792 [details]
Patch
I noticed the dlopen for this framework in a profile of the web process when launching Safari. The only thing I am unsure about is whether this will cause some kind of dependency cycle in Apple's build process. And yes, I am happy to land the project file fixes separate from the DataDetectorsCore change. Won’t be hard to disentangle them. For this to work, $(SYSTEM_LIBRARY_DIR)/PrivateFrameworks needs to be added to FRAMEWORK_SEARCH_PATHS[sdk=macosx*]. Everything compiles because of the system frameworks include path in OTHER_CFLAGS, but linking is failing. OK. Xcode had actually added that to the project, and I deleted it by hand, but clearly we would need to add it to the .xcconfig file instead as your syntax implies. I currently have this linking for iOS as well as Mac. I guess we don’t want that. I don’t know how to have a conditional framework in the project file. (In reply to comment #5) > I currently have this linking for iOS as well as Mac. I guess we don’t want > that. I don’t know how to have a conditional framework in the project file. Good point! It should go in OTHER_LDFLAGS_PLATFORM[sdk=macosx*]. Created attachment 252797 [details]
Patch
Created attachment 252798 [details]
Patch
Created attachment 252799 [details]
Patch
Created attachment 252816 [details]
Patch
Comment on attachment 252816 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252816&action=review > Source/WebCore/platform/spi/mac/DataDetectorsSPI.h:27 > +#import "DataDetectorsCoreSPI.h" > #import "SoftLinking.h" This being a framework (private) header I would have expected these to be #import <WebCore/DataDetectorsCoreSPI.h> and #import <WebCore/SoftLinking.h>. Committed r184047: <http://trac.webkit.org/changeset/184047> Committed follow-up fix r184052: <http://trac.webkit.org/changeset/184052> Upon compiling with ASAN, the "Check For Weak VTables and Externals" step fails with: ERROR: WebCore has a weak external symbol in it (/Volumes/Data/home/mmaxfield/Build/Debug/WebCore.framework/Versions/A/WebCore) ERROR: A weak external symbol is generated when a symbol is defined in multiple compilation units and is also marked as being exported from the library. ERROR: A common cause of weak external symbols is when an inline function is listed in the linker export file. ERROR: symbol _DDHighlightCreateWithRectsInVisibleRectWithStyleAndDirection ERROR: symbol _DDHighlightPointIsOnHighlight It looks like this patch is relevant. It seems that we need to export the soft-linking symbols to fix that. (In reply to comment #16) > It seems that we need to export the soft-linking symbols to fix that. I don’t think we can simply export those symbols, because they are named the same as their counterparts in DataDetectors. If we did that, when building a target that links against WebKit and DataDetectors, the static might end up record those symbols as coming from WebKit. (In reply to comment #17) > (In reply to comment #16) > > It seems that we need to export the soft-linking symbols to fix that. > > I don’t think we can simply export those symbols, because they are named the > same as their counterparts in DataDetectors. If we did that, when building a > target that links against WebKit and DataDetectors, the static might end up > record those symbols as coming from WebKit. Unless we can exclude them from the blanket reexport of WebCore by WebKit. Is there a downside to not checking for weak vTables and externals when building with ASAN? I do not know if there is anything about ASan that would make a difference, and in fact this doesn't happen on ASan bots, as far as I can tell. Zalan fixed the failure in bug 145070. |