Bug 147816 – Enable optimization causes undefined reference error for debug build

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Ting-Wei Lan]

We usually disable optimization for debug build, but I hope it doesn't fail when optimization is enabled. I got these messages when my CXXFLAGS contains -O2:

lib/libjavascriptcoregtk-4.0.so.18.2.4: undefined reference to 'JSC::JSCell::inherits(JSC::ClassInfo const*) const'
lib/libjavascriptcoregtk-4.0.so.18.2.4: undefined reference to 'JSC::JSCell::structure() const'
../../Source/WebCore/editing/InsertNodeBeforeCommand.cpp:47: error: undefined reference to 'WebCore::ContainerNode::renderer() const'

I will attach a patch that add 3 lines of include to fix this problem.

[Ting-Wei Lan]

Created attachment 258592 [details]
Fix debug build when optimization is enabled

[Alexey Proskuryakov]

Comment on attachment 258592 [details]
Fix debug build when optimization is enabled

View in context: https://bugs.webkit.org/attachment.cgi?id=258592&action=review

> Source/JavaScriptCore/ChangeLog:3
> +        Fix debug build when optimization is enabled

This patch seems OK to land, however I'm not sure what problem you are solving here. My understanding is that debug build is by definition -O0, and I don't see why to change that.

If you are trying to enable assertions, that should be done by fixing release build with ASSERT_DISABLED set to 0.

[Ting-Wei Lan]

Yes, debug build should be -O0, but I think it should not fail just because optimization is accidentally enabled. I found this problem when I forgot to check my CXXFLAGS before starting a build.

[Alexey Proskuryakov]

I can imagine how much time can be wasted on debugging an issue like this, and how annoying that would be.

If this is not a configuration that needs to be built, I think that it's better to catch that with an explicit compile error, so that it's easy to detect and fix. What do you think?

[Ting-Wei Lan]

I think we still need to fix undefined reference problems because they are all caused by assert. Should I change the commit message to 'Fix undefined reference errors when assertion is enabled'?

I think we can add some warning messages in a cmake file or commonly-used header, so users can see a lot of warnings. They know they are probably doing wrong things because it is hard to ignore all messages.

I think we can add an error message to stop the build in build-webkit script or other scripts that are not used in building distribution packages. It is possible for users of source-based package managers (ports, pkgsrc, portage) to set CXXFLAGS globally for all packages, and causing build failure is very annoying because it stops building of all packages depending on WebKit. Packages usually use release build, but it is still possible for users to set an option to change it to debug build. (This option is called FULLDEBUG in FreeBSD ports although it seldom works)

[Alexey Proskuryakov]

Comment on attachment 258592 [details]
Fix debug build when optimization is enabled

Are you saying that exactly the same issue happens in a release build with ASSERT_DISABLED set to 0? Then yes, I agree that this should be done.

[Ting-Wei Lan]

I think it should happen in release build with ASSERT_DISABLED=0 because all these problems are caused by function calls done in assert, but I cannot test it because it doesn't build:

../../Source/WTF/wtf/dtoa/bignum.cc:772:30: error: use of undeclared identifier 'a'
        ASSERT(Bignum::Equal(a, *this));
                             ^
../../Source/WTF/wtf/Assertions.h:235:8: note: expanded from macro 'ASSERT'
    (!(assertion) ? \
       ^
1 error generated.

[WebKit Commit Bot]

Comment on attachment 258592 [details]
Fix debug build when optimization is enabled

Clearing flags on attachment: 258592

Committed r188256: <http://trac.webkit.org/changeset/188256>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.