Closed
Bug 1469091
Opened 6 years ago
Closed 6 years ago
--enable-lto is not compatible with --enable-clang-plugin
Categories
(Firefox Build System :: General, defect)
Tracking
(firefox63 fixed)
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: glandium, Assigned: glandium)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
When going past the restrictions described in bug 1469088, the build fails with:
[task 2018-06-16T00:14:30.314Z] 00:14:30 INFO - /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/clang/bin/clang++ -std=gnu++14 -Qunused-arguments -flto=thin -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -I/builds/worker/workspace/build/src/clang/include -fPIC -static-libstdc++ -fvisibility-inlines-hidden -std=c++11 -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wnon-virtual-dtor -fcolor-diagnostics -O3 -DNDEBUG -fno-exceptions -fno-rtti -DLLVM_BUILD_GLOBAL_ISEL -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DHAVE_NEW_ASTMATCHER_NAMES -DHAS_ACCEPTS_IGNORINGPARENIMPCASTS -fno-rtti -fno-exceptions -fno-omit-frame-pointer -Werror -fPIC -shared -o libclang-plugin.dylib ThirdPartyPaths.o Unified_cpp_build_clang-plugin0.o Unified_cpp_build_clang-plugin1.o -L/builds/worker/workspace/build/src/clang/lib -lclangASTMatchers -dynamiclib -install_name @executable_path/libclang-plugin.dylib -compatibility_version 1 -current_version 1 -single_module
[task 2018-06-16T00:14:30.314Z] 00:14:30 INFO - /usr/bin/ld: /builds/worker/workspace/build/src/clang/bin/../lib/LLVMgold.so: error in plugin cleanup (ignored)
[task 2018-06-16T00:14:30.315Z] 00:14:30 INFO - /usr/bin/ld: /builds/worker/workspace/build/src/clang/bin/../lib/LLVMgold.so: error loading plugin
[task 2018-06-16T00:14:30.315Z] 00:14:30 INFO - clang-5.0: [0;1;31merror: [0mlinker command failed with exit code 1 (use -v to see invocation)[0m
[task 2018-06-16T00:14:30.315Z] 00:14:30 INFO - /builds/worker/workspace/build/src/config/rules.mk:679: recipe for target 'libclang-plugin.dylib' failed
[task 2018-06-16T00:14:30.315Z] 00:14:30 INFO - make[4]: *** [libclang-plugin.dylib] Error 1
That's because clang is compiling the clang-plugin sources with -flto, but the linker can't deal with that. The core problem is that we don't have actual host shared libraries, the short term fix might be to strip out the MOZ_LTO_*FLAGS when linking it.
Comment 1•6 years ago
|
||
I feel like I keep saying it, but maybe *this* time we should just make host shared libraries work?
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → mh+mozilla
Comment hidden (mozreview-request) |
Comment 3•6 years ago
|
||
mozreview-review |
Comment on attachment 8990151 [details]
Bug 1469091 - Build the clang plugin as a host shared library.
https://reviewboard.mozilla.org/r/255156/#review262178
Thanks, this is definitely better than the status quo!
::: build/clang-plugin/Makefile.in:7
(Diff revision 1)
> # License, v. 2.0. If a copy of the MPL was not distributed with this
> # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>
> include $(topsrcdir)/config/config.mk
>
> -# In the current moz.build world, we need to override essentially every
> +HOST_LDFLAGS := $(LLVM_LDFLAGS) $(CLANG_LDFLAGS)
Can these get moved to moz.build now?
::: build/clang-plugin/moz.build:11
(Diff revision 1)
>
> -SharedLibrary('clang-plugin')
> +HostSharedLibrary('clang-plugin')
>
> -SOURCES += ['!ThirdPartyPaths.cpp']
> +HOST_SOURCES += ['!ThirdPartyPaths.cpp']
>
> -UNIFIED_SOURCES += [
> +HOST_SOURCES += [
File a followup on `HOST_UNIFIED_SOURCES`? Or maybe we could just try making all `HOST_SOURCES` build as unified and see if anything breaks? There aren't that many of them.
::: build/templates.mozbuild:110
(Diff revision 1)
>
> @template
> +def HostSharedLibrary(name):
> + '''Template for build tools libraries.'''
> + if name != 'clang-plugin':
> + error('Please make sure host shared library support is complete '
Good thinking! This will assuredly save someone headache in the future.
::: config/rules.mk:678
(Diff revision 1)
>
> +$(HOST_SHARED_LIBRARY): $(HOST_OBJS) Makefile
> + $(REPORT_BUILD)
> + $(RM) $@
> +ifdef _MSC_VER
> + # /!\ We assume host and target are using the same compiler
Considering that you only implemented enough of this for the clang-plugin, maybe it's not worthwhile to even have this MSVC block?
::: python/mozbuild/mozbuild/backend/recursivemake.py:1286
(Diff revision 1)
>
> def _process_host_library(self, libdef, backend_file):
> backend_file.write('HOST_LIBRARY_NAME = %s\n' % libdef.basename)
>
> + def _process_host_shared_library(self, libdef, backend_file):
> + backend_file.write('HOST_SHARED_LIBRARY = %s\n' % libdef.lib_name)
nit: this could be :=
::: python/mozbuild/mozbuild/frontend/data.py:770
(Diff revision 1)
>
> +class HostSharedLibrary(HostMixin, Library):
> + """Context derived container object for a host shared library.
> +
> + This class supports less things than SharedLibrary does for target shared
> + libraries."""
Please put a note in the docstring about the fact that this is limited to supporting the clang-plugin currently.
::: python/mozbuild/mozbuild/frontend/emitter.py:357
(Diff revision 1)
> for lib in context.get(variable.replace('USE', 'OS'), []):
> obj.link_system_library(lib)
>
> # We have to wait for all the self._link_library calls above to have
> # happened for obj.cxx_link to be final.
> - if not isinstance(obj, (StaticLibrary, HostLibrary,
> + # FIXME: Theoretically, HostSharedLibrary shouldn't be here.
File a bug for this FIXME and put the bug number here, please.
Attachment #8990151 -
Flags: review+
Updated•6 years ago
|
Attachment #8990151 -
Flags: review?(core-build-config-reviews)
Assignee | ||
Comment 4•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8990151 [details]
Bug 1469091 - Build the clang plugin as a host shared library.
https://reviewboard.mozilla.org/r/255156/#review262178
> Can these get moved to moz.build now?
We can't *replace* HOST_LDFLAGS in moz.build land.
> File a followup on `HOST_UNIFIED_SOURCES`? Or maybe we could just try making all `HOST_SOURCES` build as unified and see if anything breaks? There aren't that many of them.
Or maybe, as suggested in bug 1409276, get rid of the separate notion for host things.
> Considering that you only implemented enough of this for the clang-plugin, maybe it's not worthwhile to even have this MSVC block?
We have static analysis jobs on windows, using the plugin.
Comment hidden (mozreview-request) |
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/531b966781e6
Build the clang plugin as a host shared library. r=ted
Comment 7•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Updated•6 years ago
|
Version: Version 3 → 3 Branch
You need to log in
before you can comment on or make changes to this bug.
Description
•