1234419 - The Skia update in bug 1082598 broke clang 3.5 builds on linux

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

0:10.37 clang: error: unknown argument: '-fkeep-inline-functions'
 0:10.38 
 0:10.38 In the directory  /home/keeler/mozilla-central/obj-x86_64-unknown-linux-gnu/gfx/skia
 0:10.38 The following command failed to execute properly:
 0:10.39 /usr/bin/ccache clang++ -o GrResourceCache.o -c -I/home/keeler/mozilla-central/obj-x86_64-unknown-linux-gnu/dist/stl_wrappers -I/home/keeler/mozilla-central/obj-x86_64-unknown-linux-gnu/dist/system_wrappers -include /home/keeler/mozilla-central/config/gcc_hidden.h -DSK_FONTHOST_DOES_NOT_USE_FONTMGR=1 -DSKIA_IMPLEMENTATION=1 -I/home/keeler/mozilla-central/gfx/skia -I. -I/home/keeler/mozilla-central/gfx/skia/skia/include/c -I/home/keeler/mozilla-central/gfx/skia/skia/include/config -I/home/keeler/mozilla-central/gfx/skia/skia/include/core -I/home/keeler/mozilla-central/gfx/skia/skia/include/effects -I/home/keeler/mozilla-central/gfx/skia/skia/include/gpu -I/home/keeler/mozilla-central/gfx/skia/skia/include/images -I/home/keeler/mozilla-central/gfx/skia/skia/include/pathops -I/home/keeler/mozilla-central/gfx/skia/skia/include/pipe -I/home/keeler/mozilla-central/gfx/skia/skia/include/ports -I/home/keeler/mozilla-central/gfx/skia/skia/include/private -I/home/keeler/mozilla-central/gfx/skia/skia/include/utils -I/home/keeler/mozilla-central/gfx/skia/skia/include/utils/mac -I/home/keeler/mozilla-central/gfx/skia/skia/include/utils/win -I/home/keeler/mozilla-central/gfx/skia/skia/include/views -I/home/keeler/mozilla-central/gfx/skia/skia/src/core -I/home/keeler/mozilla-central/gfx/skia/skia/src/gpu -I/home/keeler/mozilla-central/gfx/skia/skia/src/gpu/effects -I/home/keeler/mozilla-central/gfx/skia/skia/src/gpu/gl -I/home/keeler/mozilla-central/gfx/skia/skia/src/image -I/home/keeler/mozilla-central/gfx/skia/skia/src/lazy -I/home/keeler/mozilla-central/gfx/skia/skia/src/opts -I/home/keeler/mozilla-central/gfx/skia/skia/src/sfnt -I/home/keeler/mozilla-central/gfx/skia/skia/src/utils -I/home/keeler/mozilla-central/gfx/skia/skia/src/utils/mac -I/home/keeler/mozilla-central/gfx/skia/skia/src/utils/win -I../../dist/include -I/home/keeler/mozilla-central/obj-x86_64-unknown-linux-gnu/dist/include/nspr -I/home/keeler/mozilla-central/obj-x86_64-unknown-linux-gnu/dist/include/nss -fPIC -DMOZILLA_CLIENT -include ../../mozilla-config.h -MD -MP -MF .deps/GrResourceCache.o.pp -Qunused-arguments -Qunused-arguments -Wall -Wempty-body -Woverloaded-virtual -Wsign-compare -Wwrite-strings -Werror=endif-labels -Werror=int-to-pointer-cast -Werror=missing-braces -Werror=parentheses -Werror=pointer-arith -Werror=return-type -Werror=sequence-point -Werror=switch -Werror=trigraphs -Werror=type-limits -Werror=uninitialized -Werror=unused-label -Werror=non-literal-null-conversion -Werror=sometimes-uninitialized -Wno-invalid-offsetof -Wno-inline-new-delete -Wno-c++0x-extensions -Wno-extended-offsetof -Wno-unknown-warning-option -Wno-return-type-c-linkage -Wno-error=deprecated-declarations -Wno-error=array-bounds -fno-exceptions -fno-strict-aliasing -fno-rtti -fno-exceptions -fno-math-errno -std=gnu++0x -pthread -pipe -DDEBUG -DTRACING -g -fno-omit-frame-pointer -Wno-deprecated-declarations -Wno-overloaded-virtual -Wno-sign-compare -Wno-unused-function -Wno-implicit-fallthrough -Wno-inconsistent-missing-override -Wno-macro-redefined -Wno-unused-private-field -I/home/keeler/mozilla-central/obj-x86_64-unknown-linux-gnu/dist/include/cairo -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/freetype2 -I/usr/include/libpng16 -pthread -I/usr/include/pango-1.0 -I/usr/include/harfbuzz -I/usr/include/pango-1.0 -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/freetype2 -I/usr/include/libdrm -I/usr/include/libpng16 -fkeep-inline-functions /home/keeler/mozilla-central/gfx/skia/skia/src/gpu/GrResourceCache.cpp
 0:10.39 /home/keeler/mozilla-central/config/rules.mk:956: recipe for target 'GrResourceCache.o' failed
 0:10.39 gmake[5]: *** [GrResourceCache.o] Error 1
 0:10.39 gmake[5]: *** Waiting for unfinished jobs....

Comment 1

[Lee Salzman [:lsalzman]]

The problem is that was needed to work around a bug in our own version of clang we use for our clang builds where an inline function is optimized away even though its symbol is necessary for linking... and was fixed in actual release versions, which unfortunately we weren't using. We'd probably need to do some sort of checking to see if we're using our particular clang version and only then use the -fkeep-inline-functions workaround?

Comment 2

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

(In reply to Lee Salzman [:lsalzman] from comment #1)
> We'd probably need
> to do some sort of checking to see if we're using our particular clang
> version and only then use the -fkeep-inline-functions workaround?

That seems reasonable. Who would know the best way to do that?

Comment 3

[Lee Salzman [:lsalzman]]

Attachment: remove unnecessary -fkeep-inline-functions from Skia moz.build

As of clang 3.5+, it seems -fkeep-inline-functions doesn't really do anything anyway (besides potentially cause errors depending on version). So it seems that this was somewhat of a red herring.

Due to luck, I noticed that in implementing the use of that flag, I had moved GrResourceCache.cpp out of the unified sources. This is what really seemed to have fixed the issue.

Just to verify that this indeed yields the desired fix without the need for the command-line flag, I reproduced the original error by moving GrResourceCache.cpp back into unified sources: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bd37394c965b

With this patch, however, we just leave GrResourceCache.cpp out of unified sources, and we get rid of the problematic flag, making everything happy: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0946a20e154e

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8702689 [details] [diff] [review]
remove unnecessary -fkeep-inline-functions from Skia moz.build

Review of attachment 8702689 [details] [diff] [review]:
-----------------------------------------------------------------

\o/

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6ae2e882c357

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6ae2e882c357