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)

3 Branch
defect
Not set
normal

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.
I feel like I keep saying it, but maybe *this* time we should just make host shared libraries work?
Assignee: nobody → mh+mozilla
Blocks: macOS-LTO
No longer blocks: macOS-PGO
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+
Attachment #8990151 - Flags: review?(core-build-config-reviews)
Blocks: 1474022
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.
Pushed by mh@glandium.org: https://hg.mozilla.org/integration/autoland/rev/531b966781e6 Build the clang plugin as a host shared library. r=ted
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Version: Version 3 → 3 Branch
Blocks: 1556662
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: