Bug 80282 – undefined reference to JSC::IdentifierTable::~IdentifierTable() on EFL port

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[ChangSeok Oh]

I faced following linking error...
> /usr/bin/ld: ../../lib/libwtf_efl.a(WTFThreadData.cpp.o): undefined reference to symbol 'JSC::IdentifierTable::~IdentifierTable()'
with this command...
> ./Tools/Scripts/build-webkit --efl --debug --no-svg --makeargs="-j 4" --cmakearg="-DSHARED_CORE=ON" --prefix=/usr/local --mutation-observers

[ChangSeok Oh]

Created attachment 130129 [details]
Patch

[ChangSeok Oh]

It seems that splitting declaration and definition of IdentifierTable::~IndentifierTable() and IdentifierTable::add() causes this problem. 
I think it's better to move those definitions into WTFThreadData.h where their declaration is located in.

[ChangSeok Oh]

(In reply to comment #2)
> It seems that splitting declaration and definition of IdentifierTable::~IndentifierTable() and IdentifierTable::add() causes this problem. 
> I think it's better to move those definitions into WTFThreadData.h where their declaration is located in.

Of course, If NO specific reasons. :)

[Alexey Proskuryakov]

Changing cross-platform code in this way may be OK, but needs a better explanation than being a build fix. In what way is the EFL port different from other ports using JSC or V8?

[ChangSeok Oh]

(In reply to comment #4)
> Changing cross-platform code in this way may be OK, but needs a better explanation than being a build fix. In what way is the EFL port different from other ports using JSC or V8?

As I know, current EFL DRT runs just only with the option --cmakearg="-DSHARED_CORE=ON".
http://trac.webkit.org/wiki/WebKitEFLLayoutTest (Of course, I think this is an also bug, we're going to eliminate a this restriction.)
Then WTF functions are wrapped in a libwtf_efl.a which is a 'static' library. It means libwtf_efl.a should have complete definitions to match with declarations, or else we'll face "undefined reference" issue.
So. If this patch doesn't make any problem to other ports, I hope to apply it.

[Alexey Proskuryakov]

It sounds like Identifier.cpp is not built as part of libwtf_efl.a. Can it just be added to the library?

The fact that a shared library need to have complete definitions doesn't mean that all definitions must be in headers.

[ChangSeok Oh]

(In reply to comment #6)
> It sounds like Identifier.cpp is not built as part of libwtf_efl.a. Can it just be added to the library?
> 
> The fact that a shared library need to have complete definitions doesn't mean that all definitions must be in headers.

Yeh, You're generally right.
But in this case, it's not easy including only Identifier.cpp to libwtf_efl.a since it has many dependencies for JavaScriptCore.

[Alexey Proskuryakov]

What does chromium do? I imagine they would also have a problem with JSC dependencies.

[ChangSeok Oh]

(In reply to comment #8)
> What does chromium do? I imagine they would also have a problem with JSC dependencies.

I'm not familiar with chromium port, but I think Chromium port doesn't extract  wtf library from javascriptcore library as static library yet.
I guess this issue is related with these movements bug75673.
I can understand your concern. Then how about moving IdentifierTable::~IdentifierTable() IdentifierTable::add() into a new file like wtf/IdentifierTable.cpp?

[Alexey Proskuryakov]

The Chromium port doesn't use JavaScriptCore, it uses V8. So if they can do it, it should be possible for EFL too.

[Gyuyoung Kim]

EFL build bot is sick because of build error related to wtf. I think EFL port needs to fix it as soon as possible. 

Changseok, could you update patch again according to our internal discussion ?

[ChangSeok Oh]

Created attachment 130303 [details]
Patch

[Raphael Kubo da Costa (:rakuco)]

(In reply to comment #11)
> EFL build bot is sick because of build error related to wtf. I think EFL port needs to fix it as soon as possible.

According to build.webkit.org it's working fine (a few jscore and layout tests don't pass, but there's nothing related to symbols not being found while running DRT).

[Raphael Kubo da Costa (:rakuco)]

OK, I can reproduce the problem if I pass --no-copy-dt-needed-entries and --as-needed to the linker.

Linking CXX executable ../../bin/EWebLauncher
cd /home/rakuco/dev/webkit/WebKit/WebKitBuild/release-shared-core/Release/Tools/EWebLauncher && /usr/bin/cmake -E cmake_link_script CMakeFiles/bin/EWebLauncher.dir/link.txt --verbose=1
/usr/lib/ccache/bin/c++   -O3 -DNDEBUG    -Wl,--no-copy-dt-needed-entries,--as-needed  -L/home/rakuco/dev/e/lib -lecore_x -L/home/rakuco/dev/e/lib -ledje -pthread -L/home/rakuco/dev/e/lib -leina -levas -lecore -lecore_file -lecore_evas -ledje -L/home/rakuco/dev/e/lib -levas -L/home/rakuco/dev/gnome/stow/libsoup/lib -L/home/rakuco/dev/gnome/stow/glib/lib -lsoup-2.4 -lgio-2.0 -lgobject-2.0 -lglib-2.0 CMakeFiles/bin/EWebLauncher.dir/main.c.o  -o ../../bin/EWebLauncher -rdynamic ../../lib/libwtf_efl.a ../../lib/libjavascriptcore_efl.so.0.1.0 ../../lib/libwebcore_efl.so.0.1.0 ../../lib/libewebkit.so.0.1.0 -lcairo -lfreetype -lecore_x -ledje -leina -levas -lecore -lecore_file -lecore_evas -ledje -levas -lxml2 -lxslt -lsqlite3 /home/rakuco/dev/gnome/lib/libglib-2.0.so /home/rakuco/dev/gnome/lib/libgobject-2.0.so -lsoup-2.4 -lgio-2.0 -lgobject-2.0 -lglib-2.0 ../../lib/libwebcore_efl.so.0.1.0 ../../lib/libjavascriptcore_efl.so.0.1.0 ../../lib/libwtf_efl.a -lpthread -licui18n -lz -lxslt -licuuc -lgstapp-0.10 -lgstreamer-0.10 -lgstbase-0.10 -lgstinterfaces-0.10 -lgstpbutils-0.10 -lgstvideo-0.10 -lgstreamer-0.10 -lgstbase-0.10 -lgstinterfaces-0.10 -lgstpbutils-0.10 -lgstvideo-0.10 -lcairo -lfreetype -lecore_x -leina -levas -lecore -lecore_file -ledje -lecore_evas -lxml2 -lsqlite3 -lfontconfig -lpng -ljpeg /home/rakuco/dev/gnome/lib/libglib-2.0.so /home/rakuco/dev/gnome/lib/libgobject-2.0.so -lsoup-2.4 -lgio-2.0 -lgobject-2.0 -lglib-2.0 -ldl -Wl,-rpath,/home/rakuco/dev/webkit/WebKit/WebKitBuild/release-shared-core/Release/lib:/home/rakuco/dev/gnome/lib 
/usr/bin/ld: ../../lib/libwtf_efl.a(WTFThreadData.cpp.o): undefined reference to symbol 'JSC::IdentifierTable::~IdentifierTable()'
/usr/bin/ld: note: 'JSC::IdentifierTable::~IdentifierTable()' is defined in DSO ../../lib/libjavascriptcore_efl.so.0.1.0 so try adding it to the linker command line
../../lib/libjavascriptcore_efl.so.0.1.0: could not read symbols: Invalid operation
collect2: ld returned 1 exit status

[Raphael Kubo da Costa (:rakuco)]

(In reply to comment #10)
> The Chromium port doesn't use JavaScriptCore, it uses V8. So if they can do it, it should be possible for EFL too.

The code in WTFThreadData which uses/declares IdentifierTable stuff is enclosed within #if USE(JSC), so it doesn't really apply to Chromium.

[Raphael Kubo da Costa (:rakuco)]

(In reply to comment #14)
> [...] ../../lib/libwtf_efl.a
> /usr/bin/ld: ../../lib/libwtf_efl.a(WTFThreadData.cpp.o): undefined reference to symbol 'JSC::IdentifierTable::~IdentifierTable()'
> /usr/bin/ld: note: 'JSC::IdentifierTable::~IdentifierTable()' is defined in DSO ../../lib/libjavascriptcore_efl.so.0.1.0 so try adding it to the linker command line
> ../../lib/libjavascriptcore_efl.so.0.1.0: could not read symbols: Invalid operation
> collect2: ld returned 1 exit status

Right, the problem's that these binaries shouldn't be trying to link directly to wtf.a at all. It's coming as a transitive link dependency from the other targets, such as libjavascriptcore_efl.so.

[Alexey Proskuryakov]

Comment on attachment 130303 [details]
Patch

r- per above discussion

[Raphael Kubo da Costa (:rakuco)]

Created attachment 130392 [details]
Remove transitive lib dependencies from targets, check if the EWS bots are happy

[Raphael Kubo da Costa (:rakuco)]

CC'ing the other CMake folks. This change affects all CMake-based ports. I can't test BlackBerry and WinCE myself, so I was only able to blindly make sure they kept working.

[Raphael Kubo da Costa (:rakuco)]

Created attachment 130410 [details]
Set CMAKE_LINK_INTERFACE_LIBRARIES instead of manually changing each target

[Antonio Gomes]

Rob and Ming can be of more help than I do here :/

[Raphael Kubo da Costa (:rakuco)]

Reverting resolution from RESOLVED/FIXED to UNCONFIRMED, I haven't committed anything yet :-)

[Ming Xie]

A quick question, why does our BlackBerry port only need to link PNG and JPEG?

+# Other libraries
+LIST(APPEND WebCore_LIBRARIES
+    ${JPEG_LIBRARY}
+    ${PNG_LIBRARY}
+)
+

[Raphael Kubo da Costa (:rakuco)]

(In reply to comment #23)
> A quick question, why does our BlackBerry port only need to link PNG and JPEG?

I noticed it included the same image decoders the EFL port did, so I added the same library dependencies the EFL port needed. I can't really test if there's any other library missing, though.

[Ming Xie]

(In reply to comment #24)
> (In reply to comment #23)
> > A quick question, why does our BlackBerry port only need to link PNG and JPEG?
> 
> I noticed it included the same image decoders the EFL port did, so I added the same library dependencies the EFL port needed. I can't really test if there's any other library missing, though.

I made this change in my local build:
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -46,6 +46,9 @@ SET(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
 SET(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
 SET(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
 
+# Do not create transitive library dependencies by default
+SET(CMAKE_LINK_INTERFACE_LIBRARIES "")
+

And then I found out our port is able to build with/without specifying:
+# Other libraries
+LIST(APPEND WebCore_LIBRARIES
+    ${JPEG_LIBRARY}
+    ${PNG_LIBRARY}
+)

I'm not sure why. Maybe our BlackBerry port never links these libraries?

[Raphael Kubo da Costa (:rakuco)]

(In reply to comment #25)
> I'm not sure why. Maybe our BlackBerry port never links these libraries?

Source/WebCore/PlatformBlackBerry.cmake does have the JPEG and PNG image decoders in its source list, but it's hard for me to tell whether the dependencies end up being pulled from elsewhere in the platform.

I'll work on a patch that doesn't touch PlatformBlackBerry.cmake.

[Raphael Kubo da Costa (:rakuco)]

Created attachment 130472 [details]
Do not touch PlatformBlackBerry.cmake

[Ming Xie]

(In reply to comment #26)
> (In reply to comment #25)
> > I'm not sure why. Maybe our BlackBerry port never links these libraries?
> 
> Source/WebCore/PlatformBlackBerry.cmake does have the JPEG and PNG image decoders in its source list, but it's hard for me to tell whether the dependencies end up being pulled from elsewhere in the platform.
> 
> I'll work on a patch that doesn't touch PlatformBlackBerry.cmake.

Yeah, let's leave the BlackBerry port alone for now. I will try to find out more info from my end. Thanks Raphael!

[Raphael Kubo da Costa (:rakuco)]

Committed r109983: <http://trac.webkit.org/changeset/109983>

[ChangSeok Oh]

Raphael, are you sure the issue is resolved with your patch?
I'm still facing same issue. :\

>Linking CXX executable ../../bin/EWebLauncher
>[100%] Built target bin/DumpRenderTree
>/usr/bin/ld: ../../lib/libwtf_efl.a(WTFThreadData.cpp.o): undefined reference to symbol 'JSC::IdentifierTable::~IdentifierTable()'
>/usr/bin/ld: note: 'JSC::IdentifierTable::~IdentifierTable()' is defined in DSO ../../lib/libjavascriptcore_efl.so.0.1.0 so try adding it to the linker command line
>../../lib/libjavascriptcore_efl.so.0.1.0: could not read symbols: Invalid operation

What I'm using the command is..
> ./Tools/Scripts/build-webkit --efl --debug --no-svg --makeargs="-j 4" --cmakearg="-DSHARED_CORE=ON" --mutation-observers

[Raphael Kubo da Costa (:rakuco)]

(In reply to comment #30)
> Raphael, are you sure the issue is resolved with your patch?
> I'm still facing same issue. :\

*sigh* CMAKE_LINK_INTERFACE_LIBRARIES was added in CMake 2.8.7, and you're probably using an earlier version.

I've posted a follow-up patch in bug 80469 to fix this issue.