58508 2011-04-13 20:05:37 -0700 Fix linking with Sun Studio 12: function pointers for extern "C" are treated differently 2011-04-15 00:56:09 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) PC Other RESOLVED FIXED P2 Normal --- 0 bentaylor.solx86 webkit-unassigned ap commit-queue oldest_to_newest 385467 0 bentaylor.solx86 2011-04-13 20:05:37 -0700 The Sun CC compiler treats C functions and C++ functions differently, as if they had a different calling sequence (they don't, but they could). So if you declare a function in C++ having a function pointer as a parameter, it's understood to be C++ even if it had previously been declared as extern "C". This could be a compiler error, though. In any case, the end result is that WebKit fails to link because of an undefined reference to NPN_PluginThreadAsyncCall. Problem originally identified by Thiago Macieria in bug https://bugs.webkit.org/show_bug.cgi?id=24932 and fixed by patch webkit17-17.diff in his suite. 385469 1 bentaylor.solx86 2011-04-13 20:14:00 -0700 Here's the current error message without the patch. pkgbuild: Undefined first referenced pkgbuild: symbol in file pkgbuild: NPN_PluginThreadAsyncCall /export/home/bent/packages/BUILD/qt-4.7.2/i386/qt-everywhere-opensource-src-4.7.2/lib/libQtWebKit.so pkgbuild: ld: fatal: Symbol referencing errors. No output written to ../../../../bin/assistant pkgbuild: make[4]: *** [../../../../bin/assistant] Error 1 pkgbuild: make[4]: Leaving directory `/export/home/bent/packages/BUILD/qt-4.7.2/i386/qt-everywhere-opensource-src-4.7.2/tools/assistant/tools/assistant' 385504 2 89530 bentaylor.solx86 2011-04-13 21:46:51 -0700 Created attachment 89530 Fix linking issue on Solaris 10 with Sun Studio 12, Fix a linking problem for a plugin, as function pointers for extern "C" are handled different Patch to fix the linking issue on Solaris with Sun Studio 12 385541 3 89530 ap 2011-04-13 23:16:30 -0700 Comment on attachment 89530 Fix linking issue on Solaris 10 with Sun Studio 12, Fix a linking problem for a plugin, as function pointers for extern "C" are handled different View in context: https://bugs.webkit.org/attachment.cgi?id=89530&action=review > Source/JavaScriptCore/wtf/MainThread.h:40 > +extern "C" { > + typedef void MainThreadFunction(void*); > +} This doesn't look good to me. We use callOnMainThread in many places, and every time, we supply a C++ function to it (see e.g. Document::postTask()). > Source/WebCore/ChangeLog:10 > + No new tests. (OOPS!) The patch cannot be landed with this line. You can just remove it, since it's obvious why there are no tests. > Source/WebCore/plugins/npapi.cpp:179 > +extern "C" { > + void NPN_PluginThreadAsyncCall(NPP instance, void(*func) (void *), void *userData) > + { > + PluginMainThreadScheduler::scheduler().scheduleCall(instance, func, userData); > + } > } I don't understand why you had to wrap function body in extern "C". Its declaration already has extern "C", so the compiler knows that it has C linkage. Note that you didn't face a problem with NPN_ScheduleTimer, which also takes a function pointer. Would the following change alone do the trick? PluginMainThreadScheduler::scheduler().scheduleCall(instance, (MainThreadFunction)func, userData); And if that works, would a C++ style cast (probably a reinterpret_cast) work, too? 385638 4 bentaylor.solx86 2011-04-14 04:29:30 -0700 (In reply to comment #3) > I don't understand why you had to wrap function body in extern "C". Its declaration already has extern "C", so the compiler knows that it has C linkage. Note that you didn't face a problem with NPN_ScheduleTimer, which also takes a function pointer. > > Would the following change alone do the trick? > > PluginMainThreadScheduler::scheduler().scheduleCall(instance, (MainThreadFunction)func, userData); > > And if that works, would a C++ style cast (probably a reinterpret_cast) work, too? Here is the output from the compiler when we try your suggestion. "plugins/npapi.cpp", line 175: Warning: function void(_NPP*,void(*)(void*),void*) overloads extern "C" void(_NPP*,extern "C" void(*)(void*),void*) because of different language linkages. "plugins/npapi.cpp", line 176: Error: MainThreadFunction is not defined. "plugins/npapi.cpp", line 176: Error: Badly formed expression. Thoughts? 385770 5 ap 2011-04-14 08:13:58 -0700 WTF::MainThreadFunction 385960 6 89627 bentaylor.solx86 2011-04-14 12:35:15 -0700 Created attachment 89627 Updated patch which fixes the linking issue for the plugin, as function pointers for extern "C" are handled differently Updated patch, per reviewer comments and suggestions, to fix this linking error in Solaris with Sun Studio 12. 385964 7 89627 ap 2011-04-14 12:46:54 -0700 Comment on attachment 89627 Updated patch which fixes the linking issue for the plugin, as function pointers for extern "C" are handled differently Ideally, we should make PluginMainThreadScheduler::MainThreadFunction extern "C" instead of doing reinterpret_cast. But this is an improvement already. 386459 8 commit-queue 2011-04-15 00:53:34 -0700 The commit-queue encountered the following flaky tests while processing attachment 89627: http/tests/xmlhttprequest/basic-auth.html bug 51613 (author: ap@webkit.org) The commit-queue is continuing to process your patch. 386462 9 89627 commit-queue 2011-04-15 00:56:04 -0700 Comment on attachment 89627 Updated patch which fixes the linking issue for the plugin, as function pointers for extern "C" are handled differently Clearing flags on attachment: 89627 Committed r83957: <http://trac.webkit.org/changeset/83957> 386463 10 commit-queue 2011-04-15 00:56:09 -0700 All reviewed patches have been landed. Closing bug. 89530 2011-04-13 21:46:51 -0700 2011-04-14 12:35:15 -0700 Fix linking issue on Solaris 10 with Sun Studio 12, Fix a linking problem for a plugin, as function pointers for extern "C" are handled different webkit-bug-58508-function_ptr_for_extern_C_treated_differently.diff text/plain 2600 bentaylor.solx86 SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gODM4MTApCisrKyBTb3VyY2Uv SmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTQgQEAK KzIwMTEtMDQtMTMgIEJlbiBUYXlsb3IgIDxiZW50YXlsb3Iuc29seDg2QGdtYWlsLmNvbT4KKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTg1MDgKKworICAgICAgICBGaXggYSBsaW5r aW5nIHByb2JsZW0gZm9yIGEgcGx1Z2luIG9uIFNvbGFyaXMgd2l0aCB0aGUgU3VuCisgICAgICAg IFN0dWRpbyBjb21waWxlciwgYXMgZnVuY3Rpb24gcG9pbnRlcnMgZm9yIGV4dGVybiAiQyIgYXJl IGRpZmZlcmVudAorCisgICAgICAgICogd3RmL01haW5UaHJlYWQuaDoKKwogMjAxMS0wNC0xMyAg T2xpdmVyIEh1bnQgIDxvbGl2ZXJAYXBwbGUuY29tPgogCiAgICAgICAgIEZpeCAzMmJpdCB0ZXN0 cy4KSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS93dGYvTWFpblRocmVhZC5oCj09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS93dGYvTWFpblRocmVhZC5oCShyZXZpc2lvbiA4 MzgwNykKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS93dGYvTWFpblRocmVhZC5oCSh3b3JraW5n IGNvcHkpCkBAIC0zNSw3ICszNSw5IEBACiBuYW1lc3BhY2UgV1RGIHsKIAogdHlwZWRlZiB1aW50 MzJfdCBUaHJlYWRJZGVudGlmaWVyOwotdHlwZWRlZiB2b2lkIE1haW5UaHJlYWRGdW5jdGlvbih2 b2lkKik7CitleHRlcm4gIkMiIHsKKyAgICB0eXBlZGVmIHZvaWQgTWFpblRocmVhZEZ1bmN0aW9u KHZvaWQqKTsKK30KIAogLy8gTXVzdCBiZSBjYWxsZWQgZnJvbSB0aGUgbWFpbiB0aHJlYWQuCiB2 b2lkIGluaXRpYWxpemVNYWluVGhyZWFkKCk7CkluZGV4OiBTb3VyY2UvV2ViQ29yZS9DaGFuZ2VM b2cKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCShyZXZpc2lvbiA4Mzgx MCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsx LDE2IEBACisyMDExLTA0LTEzICBCZW4gVGF5bG9yICA8YmVudGF5bG9yLnNvbHg4NkBnbWFpbC5j b20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTU4NTA4CisKKyAgICAgICAgRml4 IGEgbGlua2luZyBwcm9ibGVtIGZvciBhIHBsdWdpbiBvbiBTb2xhcmlzIHdpdGggdGhlIFN1bgor ICAgICAgICBTdHVkaW8gY29tcGlsZXIsIGFzIGZ1bmN0aW9uIHBvaW50ZXJzIGZvciBleHRlcm4g IkMiIGFyZSBkaWZmZXJlbnQKKworICAgICAgICBObyBuZXcgdGVzdHMuIChPT1BTISkKKworICAg ICAgICAqIHBsdWdpbnMvbnBhcGkuY3BwOgorCiAyMDExLTA0LTEzICBPbGl2ZXIgSHVudCAgPG9s aXZlckBhcHBsZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgR2VvZmYgR2FyZW4uCkluZGV4 OiBTb3VyY2UvV2ViQ29yZS9wbHVnaW5zL25wYXBpLmNwcAo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2Uv V2ViQ29yZS9wbHVnaW5zL25wYXBpLmNwcAkocmV2aXNpb24gODM4MDcpCisrKyBTb3VyY2UvV2Vi Q29yZS9wbHVnaW5zL25wYXBpLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTcxLDkgKzE3MSwxMSBA QCB2b2lkIE5QTl9Qb3BQb3B1cHNFbmFibGVkU3RhdGUoTlBQIGluc3RhCiAgICAgcGx1Z2luVmll d0Zvckluc3RhbmNlKGluc3RhbmNlKS0+cG9wUG9wdXBzRW5hYmxlZFN0YXRlKCk7CiB9CiAKLXZv aWQgTlBOX1BsdWdpblRocmVhZEFzeW5jQ2FsbChOUFAgaW5zdGFuY2UsIHZvaWQgKCpmdW5jKSAo dm9pZCAqKSwgdm9pZCAqdXNlckRhdGEpCi17Ci0gICAgUGx1Z2luTWFpblRocmVhZFNjaGVkdWxl cjo6c2NoZWR1bGVyKCkuc2NoZWR1bGVDYWxsKGluc3RhbmNlLCBmdW5jLCB1c2VyRGF0YSk7Citl eHRlcm4gIkMiIHsKKyAgICB2b2lkIE5QTl9QbHVnaW5UaHJlYWRBc3luY0NhbGwoTlBQIGluc3Rh bmNlLCB2b2lkKCpmdW5jKSAodm9pZCAqKSwgdm9pZCAqdXNlckRhdGEpCisgICAgeworICAgICAg ICBQbHVnaW5NYWluVGhyZWFkU2NoZWR1bGVyOjpzY2hlZHVsZXIoKS5zY2hlZHVsZUNhbGwoaW5z dGFuY2UsIGZ1bmMsIHVzZXJEYXRhKTsKKyAgICB9CiB9CiAKIE5QRXJyb3IgTlBOX0dldFZhbHVl Rm9yVVJMKE5QUCBpbnN0YW5jZSwgTlBOVVJMVmFyaWFibGUgdmFyaWFibGUsIGNvbnN0IGNoYXIq IHVybCwgY2hhcioqIHZhbHVlLCB1aW50MzJfdCogbGVuKQo= 89627 2011-04-14 12:35:15 -0700 2011-04-15 00:56:04 -0700 Updated patch which fixes the linking issue for the plugin, as function pointers for extern "C" are handled differently webkit-bug-58508-function-pointers-for-extern_C-treated-differently.diff text/plain 1878 bentaylor.solx86 SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDgzODgxKQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTYgQEAKKzIwMTEtMDQtMTQgIEJlbiBUYXls b3IgIDxiZW50YXlsb3Iuc29seDg2QGdtYWlsLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICBGaXggYnVpbGRpbmcgd2l0aCBTdW4gU3R1ZGlvIDEy OiBmdW5jdGlvbiBwb2ludGVycyBmb3IgZXh0ZXJuICJDIiBhcmUgdHJlYXRlZCBkaWZmZXJlbnRs eQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTg1MDgK KworICAgICAgICBTaW5jZSBleHRlcm4gIkMiIG1ha2VzIGEgZGlmZmVyZW50IHR5cGUgKGFsdGhv dWdoIG1vc3QgY29tcGlsZXJzIGlnbm9yZSB0aGF0KSwKKyAgICAgICAgd2Ugc2hvdWxkIGJlIG1v cmUgY2FyZWZ1bCB3aGVuIHBhc3NpbmcgTlBBUEkgY2FsbGJhY2sgZnVuY3Rpb25zLgorCisgICAg ICAgICogcGx1Z2lucy9ucGFwaS5jcHA6CisgICAgICAgIChOUE5fUGx1Z2luVGhyZWFkQXN5bmND YWxsKToKKwogMjAxMS0wNC0xNCAgS2VubmV0aCBSdXNzZWxsICA8a2JyQGdvb2dsZS5jb20+CiAK ICAgICAgICAgUmV2aWV3ZWQgYnkgRGltaXRyaSBHbGF6a292LgpJbmRleDogU291cmNlL1dlYkNv cmUvcGx1Z2lucy9ucGFwaS5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGx1Z2lu cy9ucGFwaS5jcHAJKHJldmlzaW9uIDgzODU0KQorKysgU291cmNlL1dlYkNvcmUvcGx1Z2lucy9u cGFwaS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTE3MSw5ICsxNzEsMTEgQEAgdm9pZCBOUE5fUG9w UG9wdXBzRW5hYmxlZFN0YXRlKE5QUCBpbnN0YQogICAgIHBsdWdpblZpZXdGb3JJbnN0YW5jZShp bnN0YW5jZSktPnBvcFBvcHVwc0VuYWJsZWRTdGF0ZSgpOwogfQogCi12b2lkIE5QTl9QbHVnaW5U aHJlYWRBc3luY0NhbGwoTlBQIGluc3RhbmNlLCB2b2lkICgqZnVuYykgKHZvaWQgKiksIHZvaWQg KnVzZXJEYXRhKQorZXh0ZXJuICJDIiB0eXBlZGVmIHZvaWQgUGx1Z2luVGhyZWFkQXN5bmNDYWxs RnVuY3Rpb24odm9pZCopOwordm9pZCBOUE5fUGx1Z2luVGhyZWFkQXN5bmNDYWxsKE5QUCBpbnN0 YW5jZSwgUGx1Z2luVGhyZWFkQXN5bmNDYWxsRnVuY3Rpb24gZnVuYywgdm9pZCogdXNlckRhdGEp CiB7Ci0gICAgUGx1Z2luTWFpblRocmVhZFNjaGVkdWxlcjo6c2NoZWR1bGVyKCkuc2NoZWR1bGVD YWxsKGluc3RhbmNlLCBmdW5jLCB1c2VyRGF0YSk7CisgICAgLy8gQ2FsbGJhY2sgZnVuY3Rpb24g dHlwZSBvbmx5IGRpZmZlcnMgZnJvbSBNYWluVGhyZWFkRnVuY3Rpb24gYnkgYmVpbmcgZXh0ZXJu ICJDIiwgd2hpY2ggZG9lc24ndCBhZmZlY3QgY2FsbGluZyBjb252ZW50aW9uIG9uIGFueSBjb21w aWxlcnMgd2UgdXNlLgorICAgIFBsdWdpbk1haW5UaHJlYWRTY2hlZHVsZXI6OnNjaGVkdWxlcigp LnNjaGVkdWxlQ2FsbChpbnN0YW5jZSwgcmVpbnRlcnByZXRfY2FzdDxQbHVnaW5NYWluVGhyZWFk U2NoZWR1bGVyOjpNYWluVGhyZWFkRnVuY3Rpb24qPihmdW5jKSwgdXNlckRhdGEpOwogfQogCiBO UEVycm9yIE5QTl9HZXRWYWx1ZUZvclVSTChOUFAgaW5zdGFuY2UsIE5QTlVSTFZhcmlhYmxlIHZh cmlhYmxlLCBjb25zdCBjaGFyKiB1cmwsIGNoYXIqKiB2YWx1ZSwgdWludDMyX3QqIGxlbikK