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

RESOLVED FIXED in Firefox 46

Status

()

Core
Graphics
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: keeler, Assigned: lsalzman)

Tracking

Trunk
mozilla46
Points:
---

Firefox Tracking Flags

(firefox46 fixed)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 attachment)

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....
(Assignee)

Comment 1

2 years ago
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?
(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?
Flags: needinfo?(lsalzman)
Whiteboard: [gfx-noted]
(Assignee)

Comment 3

2 years ago
Created attachment 8702689 [details] [diff] [review]
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
Assignee: nobody → lsalzman
Status: NEW → ASSIGNED
Flags: needinfo?(lsalzman)
Attachment #8702689 - Flags: review?(jmuizelaar)
Comment on attachment 8702689 [details] [diff] [review]
remove unnecessary -fkeep-inline-functions from Skia moz.build

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

\o/
Attachment #8702689 - Flags: review?(jmuizelaar) → review+

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6ae2e882c357
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox46: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.