RESOLVED INVALID 72862
[Mac] check-for-weak-vtables-and-externals should ignore symbols from system headers. 
https://bugs.webkit.org/show_bug.cgi?id=72862
Summary [Mac] check-for-weak-vtables-and-externals should ignore symbols from system ...
Hajime Morrita
Reported 2011-11-20 23:41:32 PST
heck-for-weak-vtables-and-externals should ignore symbols from system headers because the system header forces them to export and we cannot control it when we use WTF_EXPORT-based symbol visibility control. For example, /usr/include/c++/4.2.1/new contains following line: #pragma GCC visibility push(default) Fortunately JSC depends on very few such C++ symbols so I think this relaxation doesn't harm.
Attachments
Patch (1.64 KB, patch)
2011-11-20 23:55 PST, Hajime Morrita 		mrowe: review-
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Hajime Morrita
Comment 1 2011-11-20 23:55:16 PST
Created attachment 116038 [details] Patch
Mark Rowe (bdash)
Comment 2 2011-11-21 02:38:09 PST
I don’t think silently allowing these symbols to be exported this is the right solution. Our frameworks should only be exporting symbols that they provide, not random symbols from system headers. Take a look at the definition of OTHER_LDFLAGS in WebKit2/Configurations/Base.xcconfig for the approach that we use to address this in the one framework we currently have that does not use an explicit symbol export list.
Mark Rowe (bdash)
Comment 3 2011-11-21 02:38:46 PST
Comment on attachment 116038 [details] Patch Marking as r- per above comment.
Hajime Morrita
Comment 4 2011-11-21 03:20:22 PST
(In reply to comment #2) > I don’t think silently allowing these symbols to be exported this is the right solution. Our frameworks should only be exporting symbols that they provide, not random symbols from system headers. Take a look at the definition of OTHER_LDFLAGS in WebKit2/Configurations/Base.xcconfig for the approach that we use to address this in the one framework we currently have that does not use an explicit symbol export list. Wow, this is exactly what I want. Thanks much for your advice! Closing the bug since we don't need this change.
Note You need to log in before you can comment on or make changes to this bug.