Bug 200033

Summary: [WPE] Unable to use jsc_weak_xxxx APIs in WPE web-extensions
Product: WebKit Reporter: munezbn.dev <munezbn.dev>
Component: WPE WebKitAssignee: Nobody <webkit-unassigned>
Status: RESOLVED DUPLICATE    
Severity: Critical CC: agomez, aperez, berto, bugs-noreply, don.olmstead, dpino, mcatanzaro, mrobinson
Priority: P2    
Version: Other   
Hardware: Other   
OS: Linux   
URL: https://lists.webkit.org/pipermail/webkit-wpe/2019-July/000211.html
See Also: https://bugs.webkit.org/show_bug.cgi?id=210366

Description munezbn.dev@gmail.com 2019-07-23 08:58:47 PDT
Component: Javascriptcore

OS:  Cross Compiling WPEWebkit-2.24.x for Raspberry Pi3 on Fedora 28 using Wpe-buildroot

Overview: 

        jsc_weak_xxx symbols are not exported in ibwpewebkit.so . So if we try to build a web-extension which uses any jsc_weak_xxx APIs then build will fail with error *undefined reference to `jsc_weak_value_new'*

Steps to Reproduce: 

        1) Create any simple web-extension : https://github.com/munezbn/sample_webkit_extension

        2) Add a few code which will use any jsc_weak_xxx API. Try to build the code.
        *  JSCValue             *js_function;
           JSCWeakValue *weak_value;
           weak_value = jsc_weak_value_new (js_function);*

Actual Results: Compilation failed with error *undefined reference to `jsc_weak_value_new'*

Expected Results: Compile without any linking error


Additional Information: Output of readelf can be found at https://lists.webkit.org/pipermail/webkit-wpe/2019-July/000211.html
Comment 1 munezbn.dev@gmail.com 2019-07-23 09:01:42 PDT
---------------------------- readelf -------------------------------
readelf -a wpebuild/build/wpewebkit-2.24.1/lib/libJavaScriptCore.a | grep
jsc_value_function_call
   425: 0000158d   268 FUNC    GLOBAL DEFAULT   22 jsc_value_function_callv
   462: 00002355   144 FUNC    GLOBAL DEFAULT   22 jsc_value_function_call
readelf -a wpebuild/build/wpewebkit-2.24.1/lib/libWPEWebKit-1.0.so.2.4.2 |
grep jsc_value_function_call
   620: 00c38bad   268 FUNC    GLOBAL DEFAULT   10 jsc_value_function_callv
   759: 00c39975   144 FUNC    GLOBAL DEFAULT   10 jsc_value_function_call
315311: 00c38bad   268 FUNC    GLOBAL DEFAULT   10 jsc_value_function_callv
315450: 00c39975   144 FUNC    GLOBAL DEFAULT   10 jsc_value_function_call

readelf -a wpebuild/build/wpewebkit-2.24.1/lib/libJavaScriptCore.a | grep
jsc_weak_value_new
    54: 00000000    44 OBJECT  LOCAL  DEFAULT   25 _ZZ18jsc_weak_value_newE1
    99: 00000157    94 FUNC    GLOBAL DEFAULT    7 jsc_weak_value_new
readelf -a wpebuild/build/wpewebkit-2.24.1/lib/libWPEWebKit-1.0.so.2.4.2 |
grep jsc_weak_value_new
*<NothinG>*
-------------------------- grep ---------------------------
grep *jsc_value_function_call* -r wpebuild/build/wpewebkit-2.24.1/lib/
Binary file wpebuild/build/wpewebkit-2.24.1/lib/libJavaScriptCore.a matches
Binary file wpebuild/build/wpewebkit-2.24.1/lib/libWPEWebKit-1.0.so.2.4.2
matches

grep *jsc_weak* -r wpebuild/build/wpewebkit-2.24.1/lib/
Binary file wpebuild/build/wpewebkit-2.24.1/lib/libJavaScriptCore.a matches
Comment 2 munezbn.dev@gmail.com 2019-07-27 00:45:05 PDT
I figured out the issue but not sure how to fix it.

The compiler by default uses "--no-undefined" linker flag,
Even though extension is not linked to any program during build time ( but loaded in web process at run-time) , it is expected that extension library throwing this error due to "--no-undefined"
But the main problem is that libwebkit doesnt have jsc_weak_xxx API. I suspect that this is because linker doesn't link JSCWeakValue.o to libWPEwebkit which is because there is no code in webkit which uses jsc_weak_xxx API internally.
This usually happens when we use "--as-needed" linker option. But this is not the case for jsc_value_xxxx API which is used internally in webkit ( webview, jsccontext, jscexception etc).

Just to confirm above theory I added a jsc_weak_xxxx call in JSCContext.cpp and recompiled libWPEwebkit, and indeed now libWPEWebkit has all jsc_weak_xxxx symbols and was able to compile my web extension.

can someone suggest how to fix this. may be I can create a patch and submit.
Comment 3 Michael Catanzaro 2019-07-27 08:21:05 PDT
I agree we should support --no-undefined.

I recall talking to Don a few weeks ago about CMake object libraries, which I don't fully understand. But I guess the proper solution is to turn JSC into an object library rather than a static library. That only needs to be done for WPE port because only WPE port exports public API from JSC built as a static library:

https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html#object-libraries

But I don't know how to actually make all the CMake changes required to do that. There is an ADD_WHOLE_ARCHIVE_TO_LIBRARIES macro in OptionsGTK.cmake that we could use here as a really big hammer for a quick fix. We could copy that into Source/WebKit/PlatformWPE.cmake and then use for the WebKit target. (That actually adds --whole-archive to all libraries, but we only need it for JavaScriptCore, so it could be refined a bit.) Either way, this wouldn't be great as it will bloat the installed library with useless code, but I think it should work.
Comment 4 Andres Gomez Garcia 2019-12-12 07:15:34 PST
The ADD_WHOLE_ARCHIVE_TO_LIBRARIES macro was added by Martin, so maybe he can come with valuable input.
Comment 5 Adrian Perez 2020-12-08 01:57:04 PST

*** This bug has been marked as a duplicate of bug 219457 ***
Comment 6 Adrian Perez 2020-12-08 02:29:03 PST
(In reply to Michael Catanzaro from comment #3)
> I agree we should support --no-undefined.
> 
> I recall talking to Don a few weeks ago about CMake object libraries, which
> I don't fully understand. But I guess the proper solution is to turn JSC
> into an object library rather than a static library. That only needs to be
> done for WPE port because only WPE port exports public API from JSC built as
> a static library:
> 
> https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html#object-
> libraries
> 
> But I don't know how to actually make all the CMake changes required to do
> that. There is an ADD_WHOLE_ARCHIVE_TO_LIBRARIES macro in OptionsGTK.cmake
> that we could use here as a really big hammer for a quick fix. We could copy
> that into Source/WebKit/PlatformWPE.cmake and then use for the WebKit
> target. (That actually adds --whole-archive to all libraries, but we only
> need it for JavaScriptCore, so it could be refined a bit.) Either way, this
> wouldn't be great as it will bloat the installed library with useless code,
> but I think it should work.

The ADD_WHOLE_ARCHIVE_TO_LIBRARIES() macro is flawed because it won't
resolve target names to their file names, for example printing a before
and after using it on “TestWebCore_LIBRARIES” (the only use at the moment)
results in:

 -- BEFORE: WebKit::WebCore;gtest;GTK::GTK
 -- AFTER: -Wl,--whole-archive;WebKit::WebCore;-Wl,--no-whole-archive;-Wl,--whole-archive;gtest;-Wl,--no-whole-archive;-Wl,--whole-archive;GTK::GTK;-Wl,--no-whole-archive

…which somewhat wrong because the list contains CMake target names
(like WebKit::WebCore), while the linker expects either “-lWebCore” or
“path/to/libWebCore.a” as arguments in between the “-Wl,--whole-archive”
and the “-Wl,--no-whole-archive” flags.

I *think_ that it somewhat works because CMake realizes that “-Wl,…” is not
a valid target name nor an actual file and it passes the flags as-is to the
linker command line. This looks very finicky to me.
Comment 7 Michael Catanzaro 2020-12-08 06:23:27 PST
Wow. Everything about this bug... wow.