Closed Bug 1469091 Opened 2 years ago Closed Last year

--enable-lto is not compatible with --enable-clang-plugin

Categories

(Firefox Build System :: General, defect)

3 Branch
defect
Not set

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
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)
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
https://hg.mozilla.org/mozilla-central/rev/531b966781e6
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Duplicate of this bug: 1429549
Version: Version 3 → 3 Branch
Blocks: 1556662
You need to log in before you can comment on or make changes to this bug.